Hi,

On Tue, Mar 22, 2016 at 3:22 PM, Maxime Ripard
[off-list ref] wrote:

Hi,

On Wed, Mar 09, 2016 at 04:44:01PM +0100, Hans de Goede wrote:

quoted

Hi,

On 09-03-16 16:28, Maxime Ripard wrote:

quoted

Hi,

On Wed, Mar 09, 2016 at 01:17:50PM +0100, Hans de Goede wrote:

quoted

Hi,

On 09-03-16 11:50, Maxime Ripard wrote:

quoted

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 don't think that's an issue. We basically have two cases here:
either we have a driver using the pin or not.

If we have a driver using it, then pinctrl will make sure we have an
exclusive use of the pin, before the gpio is requested and we start
changing that value. If not, then no one cares anyway.

AFAIK pinctrl binding happens before probe time, and is tied to the
DT. Each driver requesting pins would have to have a separate node.
Seems like there isn't one for the regulators.

Or we'd have to manually configure them for each regulator that needs
them, and list the relevant pinctrl properties under its own node.
One "on" and "off" setting pair for each LDO_IO regulator. We could
then tie the settings to the regulator device, not the underlying
mfd sub-device.

ChenYu

quoted

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

quoted

quoted

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.

quoted

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.

Note that the PIO is built on the same model and it works, while we
have 3 drivers in one. The only reason why we have everything linked
to the same device is because we have to share the address space, but
regmap takes care of that here.

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com