[RFC 2/2] ARM:Tegra: Device Tree Support: Initialize audio card gpio's from the device tree.

[Stephen Warren]

Mitch Bradley wrote at Wednesday, June 01, 2011 3:33 PM:
> On 6/1/2011 5:59 AM, Stephen Warren wrote:
> > ...
> > I have to say, I don't like that aspect; i.e. having to repeat
> > #mode-cells in every gpio definition that's inside/underneath the
> > controller definition; in my mind, "passing on" the requirement to
> > define the mode would be the default state, so forcing the namer of
> > GPIOs (i.e. whoever writes the "gpio1: gpio at 12,0 {" definitions) to
> > do this seems almost like busy work. Is there a way in *.dts to mark
> > the #mode-cells field as inherited by children unless overridden?
> 
> That is a very good point.  Addressing it led me to a revised scheme
> that I like much better.  (Notice that in the notes below I address your
> reasonable desire for an immutable SoC core definition.)
> 
> The example:
> 
>      gpio-controller1: gpio-controller {
>          #address-cells = <2>;
>          #mode-cells = <2>;
>          unbound_gpio1: gpio {
>              /* No reg property */
>              /* No mode property */
>          }
>          fully_bound_gpio1: audio-chipsel at 12,0 {
>              reg = <12 0>;
>              mode = <55 66>;
>              usage = "Audio Codec chip select";  /* Optional */
>          }
>          address_bound_gpio1: gpio at 13,0 {
>              reg = <13 0>;
>              /* No mode property */
>          }
>          mode_bound_gpio1: open-drain-gpio {
>              /* No reg property */
>              mode = <77 88>;
>          }
>      };
>      gpio-controller2: gpio-controller {
>           #address-cells = <1>;
>           #mode-cells = <1>;
>           unbound_gpio2: gpio {
>               /* No reg property */
>               /* No mode property */
>           }
>           address_bound_gpio2: gpio at 4 {
>               reg = <4>;
>               /* No mode property */
>           }
>      };
>      [...]
>      chipsel-gpio =
>          <&fully_bound_gpio1>,
>          <&unbound_gpio1 13 0 55 77>,
>          <&mode_bound_gpio1 14 0>,
>          <&address_bound_gpio2 88>,
>          <&unbound_gpio2 5 1>;

Sorry for the slow response. Some brief comments:

a) I like this better than the previous proposal; the handling of 
   #address-cells and #mode-cells is more consistent here.

b) I like that the GPIO controller (or some overlay file on top of it)
   can provide names for GPIOs and names for modes.

c) I suspect that something like the following won't work:

   chipsel-gpio =
       <&address_bound_gpio2 &mode_bound_gpio2>;

   It's an attempt to symbolically reference both a GPIO name/address
   and mode. I feel this kind of reference would be the most common
   case; a device wanting to use symbolic names for a particular GPIO
   location and mode. Only being able to specify name/address *or* mode
   symbolically, but not both, seems like a rather serious hole, and
   makes the scheme a lot less interesting to me.

d) Doing this all with DT node references seems complex and overkill. I'd
   personally far rather see the device-tree compiler grow something like
   the C pre-processor, so you could just do:

   gpioc: gpio-controller {
       #address-cells = <2>;
       #mode-cells = <2>;
   };
   #define TEGRA_GPIO_PA1 0 0
   #define TEGRA_GPIO_PX2 23 1
   #define TERGA_GPIO_PULLUP 0
   #define TERGA_GPIO_PULLDOWN 1
   #define TEGRA_GPIO_FAST 0
   #define TEGRA_GPIO_SLOW 1

   And then reference them like:

   chipsel-gpio =
       <&gpioc TEGRA_GPIO_PX2 TEGRA_GPIO_PULLUP TEGRA_GPIO_FAST>;

   Related to this, I'm having difficulty understanding why editing a
   GPIO reference in the style immediately above is any harder than
   editing a GPIO node within the GPIO controller of the style you most
   recently proposed.

-- 
nvpublic