Re: [linux-sunxi] Re: [PATCH 0/4] Add AXP209 GPIO driver

From: Hans de Goede <hdegoede@redhat.com>
To: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Alexandre Courbot <gnurou@gmail.com>,
	Lee Jones <lee.jones@linaro.org>, Chen-Yu Tsai <wens@csie.org>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-sunxi@googlegroups.com
Subject: Re: [linux-sunxi] Re: [PATCH 0/4] Add AXP209 GPIO driver
Date: Wed, 9 Mar 2016 16:44:01 +0100	[thread overview]
Message-ID: <56E044C1.7080300@redhat.com> (raw)
In-Reply-To: <20160309152822.GQ8418@lukather>

Hi,

On 09-03-16 16:28, Maxime Ripard wrote:
> Hi,
>
> On Wed, Mar 09, 2016 at 01:17:50PM +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 09-03-16 11:50, Maxime Ripard wrote:
>>> Hi,
>>>
>>> The axp209 PMIC used in combination to some Allwinner SoCs has a bunch
>>> of GPIOs accessible. Some boards use these to control their backlight
>>> or a few LEDs.
>>
>> Thanks for working on this, but IMHO this cannot go upstream like this,
>> the gpio pins on the axp pmics need a pinctrl driver, not a gpio
>> driver. I.E. on the axp209 gpio0 and gpio1 can also be used to output
>> an additional low-noise ldo (so as a regulator), or as an adc input.
>
> Eventually, yes, it needs both. But they don't even have to be the
> same driver, since they provide two different features. The only
> reason we have that construct in the pio case is because they share
> the same address space, but in the AXP case, the regmap and our mfd
> take care of that already.

Hmm, so your suggesting to have mfd instantiate 2 platform devices
for this, a gpio and a pinctrl device, each with their own
driver. Yes that would work, but I'm a bit worried about the 2
racing or some such since they both will end up touching
bit 0-2 of register 0x90 / 0x92, more-over since they are both
touching the exact same bits I've the feeling that this really
should be one driver.

I guess that in a proper written dts we either use pinctrl to enable
a special function, or gpio, but still.

>> I've been working on gsl1680 touchscreen support lately and on at least
>> a few a23 tablets, the low-noise ldo is used as AVCC for the touchscreen
>> controller.
>
> Yeah, the AXP209 also has an ADC connected to these pins.
>
>> Now these use an axp223 pmic, but nothing is stopping someone from
>> doing something similar with an axp209 and I think it would be best
>> to support this from day one, rather then hope we can retro-fit this
>> later without breaking dts.
>
> I considered that, but I don't see how it would break the DT later. If
> someone wants to enable say the ADC, he will of course have to add the
> pinctrl driver, and the pinctrl handles, but the old DT will only
> reference the gpio driver directly, which would still be something
> that would work.

I was assuming we would use one mfd-child(-platform)-device for this,
not two. I guess that with 2 devices you're right and there should
not be any problem, still as said it feels wrong-ish to have 2 drivers
poking bits 0-2 of reg 0x90 / reg 0x92.

Regards,

Hans