Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1354003 > unrolled thread

[PATCH 0/4] Add AXP209 GPIO driver

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/4] Add AXP209 GPIO driver Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-03-09 12:10 +0100
    Re: [PATCH 0/4] Add AXP209 GPIO driver Hans de Goede <hdegoede@redhat.com> - 2016-03-09 13:20 +0100
      Re: [PATCH 0/4] Add AXP209 GPIO driver Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-03-09 16:30 +0100
        Re: [linux-sunxi] Re: [PATCH 0/4] Add AXP209 GPIO driver Hans de Goede <hdegoede@redhat.com> - 2016-03-09 16:50 +0100
          Re: [linux-sunxi] Re: [PATCH 0/4] Add AXP209 GPIO driver Linus Walleij <linus.walleij@linaro.org> - 2016-03-16 11:20 +0100
            Re: [linux-sunxi] Re: [PATCH 0/4] Add AXP209 GPIO driver Hans de Goede <hdegoede@redhat.com> - 2016-03-16 11:40 +0100

#1354003 — [PATCH 0/4] Add AXP209 GPIO driver

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.

There's supposed to be 4 of them, but the fourth one has a different
configuration register scheme, and I couldn't find any board that was
using this GPIO. It will be probably be supported eventually, but
until then, we support only the first 3 GPIOs.

Let me know what you think,
Maxime

Maxime Ripard (4):
  gpio: Add AXP209 GPIO driver
  mfd: axp20x: Add AXP209 GPIO driver to the mfd
  ARM: dt: axp209: Add AXP209 GPIO driver
  ARM: sun5i: chip: Add status LED

 .../devicetree/bindings/gpio/gpio-axp209.txt       |  30 ++++
 arch/arm/boot/dts/axp209.dtsi                      |   6 +
 arch/arm/boot/dts/sun5i-r8-chip.dts                |  10 ++
 drivers/gpio/Kconfig                               |   6 +
 drivers/gpio/Makefile                              |   1 +
 drivers/gpio/gpio-axp209.c                         | 166 +++++++++++++++++++++
 drivers/mfd/axp20x.c                               |   3 +
 7 files changed, 222 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-axp209.txt
 create mode 100644 drivers/gpio/gpio-axp209.c

-- 
2.7.2

[toc] | [next] | [standalone]

#1354065

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.

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. 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.

> There's supposed to be 4 of them, but the fourth one has a different
> configuration register scheme, and I couldn't find any board that was
> using this GPIO. It will be probably be supported eventually, but
> until then, we support only the first 3 GPIOs.

He he, this matches what I've done in u-boot, the 4th gpio indeed
is somewhat funky (and the docs are not quite clear on how it works
exactly IIRC), leaving it out for now indeed seems for the best.

Regards,

Hans

[toc] | [prev] | [next] | [standalone]

#1354212

[Multipart message — attachments visible in raw view] — view raw

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.

> 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.

Thanks!
Maxime

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

On Wed, Mar 9, 2016 at 4:44 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> On Wed, Mar 09, 2016 at 01:17:50PM +0100, Hans de Goede wrote:

>> 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.

We can put the driver in drivers/pinctrl/pinctrl-axp209.c from day
one, add in comments stating that it only implements GPIO
for now and that the GPIO portion must call
pinctrl_request_gpio() and the pin controller must implement
.gpio_request_enable() the day it is added so the GPIOs do
not conflict with other use of the pins.

Requireing a huge slew of upfront code is a bit hard on simple
drivers I think.

Also we have the solution in drivers/mfd/stmpe.c that just add
a simple mux code when pins on mixsigs or simple expanders
can just mux some two-three different functions, then I think
pin control may be a bit too thick overhead. (Not sure where
the limit is though.)

Yours,
Linus Walleij

[toc] | [prev] | [next] | [standalone]

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

Hi,

On 16-03-16 11:10, Linus Walleij wrote:
> On Wed, Mar 9, 2016 at 4:44 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>>> On Wed, Mar 09, 2016 at 01:17:50PM +0100, Hans de Goede wrote:
>
>>> 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.
>
> We can put the driver in drivers/pinctrl/pinctrl-axp209.c from day
> one, add in comments stating that it only implements GPIO
> for now and that the GPIO portion must call
> pinctrl_request_gpio() and the pin controller must implement
> .gpio_request_enable() the day it is added so the GPIOs do
> not conflict with other use of the pins.
>
> Requireing a huge slew of upfront code is a bit hard on simple
> drivers I think.
>
> Also we have the solution in drivers/mfd/stmpe.c that just add
> a simple mux code when pins on mixsigs or simple expanders
> can just mux some two-three different functions, then I think
> pin control may be a bit too thick overhead. (Not sure where
> the limit is though.)

I actually found out this weekend, that we already have
a problem here. Regulator on/off is implemented as
2 mux settings, just like gpio high/low/input are mux settings:

On the axp209 the gpio pin mux bit table looks like this:

000: low output
001: output (3.3V)
010: universal input function
011: low noise LDO5
100: ADC input
1XX: floating

And on the axp221 / axp223:

000: output low
001: output high (3.3V)
010: input
011: LDOIO0 off
100: LDOIO0 on
1XX: floating

Note the LDOIO0 off / LDOIO0 on being 2 different mux
settings, and then existing axp20x regulator driver for
LD05 / LDOIO0 actually already implements regulator on / off
by changing the mux for both axp209 and axp22x.

This results in the following potentially happening.

0) kernel boots

1) The gpio driver is in use for these pins, sets the pin
to say output low

2) kernel is ready to start init, calls the regulators
subsys late_init call, which disables any unused regulators,
this results in the mux getting set to 7 (axp209) resp
4 (axp22x), and the gpio is no longer driven low as
the intend of the driver using the pin was.

So we really need either some coordination here, or a way
to tell the regulator-driver to not register ldo5 / ldoio#
at all, so that the regulator-core won't try to turn them
off changing the mux.

I've submitted a patch series this weekend which allows
putting "status = disabled" in regulator child-dts-nodes, which
will then result in not registering a regulator at all,
which would fix that. But that series has not received any
feedback yet...

Regards,

Hans

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web