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@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@12,0 {
> reg = <12 0>;
> mode = <55 66>;
> usage = "Audio Codec chip select"; /* Optional */
> }
> address_bound_gpio1: gpio@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@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
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
