Trouble specifying dts gpio-leds with GPIO expander chip

Trouble specifying dts gpio-leds with GPIO expander chip

[Bill Gatliff]

Guys:


I'm trying to come up with the right stuff for my dts file to assign a
Linux "heartbeat" trigger to an I2C GPIO expansion device (MAX7314). 
I'm running on a platform that is very similar to the lite5200b, with
linux-2.6.32.  I'm a bit new to the device tree concept, having come
most recently from an ARM world, so please be gentle...  :)

I have this so far:

        soc5200@f0000000 {
            #address-cells = <1>;
            #size-cells = <1>;
            compatible = "fsl,mpc5200b-immr","simple-bus";
            ranges = <0 0xf0000000 0x0000c000>;
            reg = <0xf0000000 0x00000100>;
            bus-frequency = <0>;        // from bootloader
            system-frequency = <0>;        // from bootloader
    ...
            i2c@3d40 {
                #address-cells = <1>;
                #size-cells = <0>;
                compatible = "fsl,mpc5200b-i2c","fsl,mpc5200-i2c","fsl-i2c";
                reg = <0x3d40 0x40>;
                interrupts = <2 16 0>;
   
                lext20: max7314@20 {
                       #gpio-cells = <2>;
                       compatible = "maxim,max7314","phillips,pca953x";
                       reg = <0x20>;
                       linux,phandle = <0x20>;
                       gpio-controller;
                };
                ...
            };
        };
    ...
        system {
            compatible = "simple-bus";
   
            gpio-leds {
                compatible = "gpio-leds";
   
                heartbeat {
                    gpios = <&lext20 3 0>;
                    linux,default-trigger = "heartbeat";
                };
            };
        };
    ...


The MAX7314 chip is showing up under /debug/gpio:

    # cat /debug/gpio
    ...
    GPIOs 198-213, i2c/1-0020, max7314, can sleep:

And, indeed, I can twiddle the LED by hand:

    # echo 201 > /sys/class/gpio/export
    # while true; do for led in high low; \
        do echo $led > /sys/class/gpio/gpio201/direction; \
        sleep 1; done; done
       (observe blinking)

But I see this at boot:

    Skipping unavailable LED gpio -19 (heartbeat)

That error code is -ENODEV.  I double-checked that gpio-leds is showing
up on of_platform:

    # ls /sys/bus/of_platform/devices/
    ...
    gpio-leds.2
    ...


I'm stumped.  Could it be that the startup code is trying to register
the heartbeat device before it has registered the MAX7314, so it doesn't
know yet that the GPIO line exists?

Anyone have any suggestions?


b.g.

-- 
Bill Gatliff
bgat@billgatliff.com

Re: Trouble specifying dts gpio-leds with GPIO expander chip

[Anton Vorontsov]

On Mon, Dec 21, 2009 at 01:40:48PM -0600, Bill Gatliff wrote:
[...]
> I'm stumped.  Could it be that the startup code is trying to register
> the heartbeat device before it has registered the MAX7314, so it doesn't
> know yet that the GPIO line exists?
> 
> Anyone have any suggestions?

drivers/gpio/pca953x.c doesn't register itself with the
OF GPIO infrastructure (plus the driver has an absolutely
bogus linux,gpio-base stuff, heh).

As an example of how to do the OF GPIO properly, see
arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c:mcu_gpiochip_add().

Though, soon there will be an easier way to register I2C/SPI chips
with the OF GPIO infrastructure.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: Trouble specifying dts gpio-leds with GPIO expander chip

[Grant Likely]

On Mon, Dec 21, 2009 at 12:40 PM, Bill Gatliff <bgat@billgatliff.com> wrote=
:
> Guys:
>
>
> I'm trying to come up with the right stuff for my dts file to assign a
> Linux "heartbeat" trigger to an I2C GPIO expansion device (MAX7314).
> I'm running on a platform that is very similar to the lite5200b, with
> linux-2.6.32. =A0I'm a bit new to the device tree concept, having come
> most recently from an ARM world, so please be gentle... =A0:)
>
> I have this so far:
>
> =A0 =A0 =A0 =A0soc5200@f0000000 {
> =A0 =A0 =A0 =A0 =A0 =A0#address-cells =3D <1>;
> =A0 =A0 =A0 =A0 =A0 =A0#size-cells =3D <1>;
> =A0 =A0 =A0 =A0 =A0 =A0compatible =3D "fsl,mpc5200b-immr","simple-bus";
> =A0 =A0 =A0 =A0 =A0 =A0ranges =3D <0 0xf0000000 0x0000c000>;
> =A0 =A0 =A0 =A0 =A0 =A0reg =3D <0xf0000000 0x00000100>;
> =A0 =A0 =A0 =A0 =A0 =A0bus-frequency =3D <0>; =A0 =A0 =A0 =A0// from boot=
loader
> =A0 =A0 =A0 =A0 =A0 =A0system-frequency =3D <0>; =A0 =A0 =A0 =A0// from b=
ootloader
> =A0 =A0...
> =A0 =A0 =A0 =A0 =A0 =A0i2c@3d40 {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0#address-cells =3D <1>;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0#size-cells =3D <0>;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0compatible =3D "fsl,mpc5200b-i2c","fsl,mpc=
5200-i2c","fsl-i2c";
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0reg =3D <0x3d40 0x40>;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0interrupts =3D <2 16 0>;
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0lext20: max7314@20 {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 #gpio-cells =3D <2>;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 compatible =3D "maxim,max7314=
","phillips,pca953x";
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =3D <0x20>;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 linux,phandle =3D <0x20>;

Drop the linux,phandle.  The phandle property is generated
automatically by the device tree compiler.  This could mess things up.

As Anton says, the 7314 driver doesn't register the GPIO in a way that
the of_gpio layer understands, so cannot translate.  What the of_gpio
infrastructure *should* be doing is to fetch the device tree node out
of the struct device and use that path to translate device tree GPIO
references to Linux GPIO numbers.  ie.
dev_archdata_get_node(gc->dev->archdata).  Then as long as the device
tree node pointer is set in the struct device (which it always should
be), then OF translations for GPIO numbers should work without driver
changes.

In other words; given a device tree node pointer; loop over all the
registered GPIO devices and look for a matching node pointer in the
struct device.  When a matching one is found; do the translation.
However, on further thought, this may require an additional hook added
to the gpio_chip registration to override the translation function if
necessary.

Anton, what are you thinking about to simplify OF GPIO registrations?

Bill, in the mean time you could do as Anton suggests and modify the
GPIO driver to have an of_gpio_chip allocation.

Cheers,
g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: Trouble specifying dts gpio-leds with GPIO expander chip

[Bill Gatliff]

Anton Vorontsov wrote:
> Though, soon there will be an easier way to register I2C/SPI chips
> with the OF GPIO infrastructure.
>   

Should this make me hesitate to update the pca953x driver, or clone it?


b.g.

-- 
Bill Gatliff
Embedded systems training and consulting
http://billgatliff.com
bgat@billgatliff.com