Tegra pinmux Device Tree support

Tegra pinmux Device Tree support

[Stephen Warren]

Grant, Linus, & all,

I was thinking of adding some Device Tree support to Tegra's existing
pinmux driver. This would remove the dependence of mach-tegra/board-dt.c
on board-{harmony,seaboard}-pinmux.c, and make the DT board stand on its
own.

My thoughts were:

a) Every Tegra board currently has a board-$name-pinmux.c which contains
a table like:

static struct tegra_pingroup_config harmony_pinmux[] = {
	{TEGRA_PINGROUP_ATA,   TEGRA_MUX_IDE,           TEGRA_PUPD_NORMAL,    TEGRA_TRI_NORMAL},

I'd just move that table into the Device Tree, keeping logically the same
structure, but just changing the syntax as appropriate for DT.

(I'm slightly worried that DT wants me to encode all that using an
unformatted table consistent of integers without any pre-processor macros
or named constants though...)

b) I'd probably need to modify the pinmux driver to be a platform device
in order to simplify integration with DT parsing etc.

c) I think the DT binding would not be affected by Linus W's work on a
pinctrl API; I see this table as platform data that sets up the initial
state of the SoC at boot-time, whereas the pinmux driver is more about
drivers dynamically altering the pinmux at run-time. Is this a reasonable
assertion? The alternative would be that once the new pinctrl API is
available, it would have an associated DT binding that represented this
data in a completely cross-SoC manner. Hence, any work on Tegra's pinmux
right now would be throwaway. I'm interested in people's thoughts here.

Finally, I was thinking that a similar thing could be done for the Tegra
GPIO driver, to replace gpio_table[] also in board-*-pinmux.c.

If this isn't useful right now, is there anything else that would help
out on Tegra?

Thanks.

-- 
nvpublic

Tegra pinmux Device Tree support

[Stephen Warren]

Grant, Linus, & all,

I was thinking of adding some Device Tree support to Tegra's existing
pinmux driver. This would remove the dependence of mach-tegra/board-dt.c
on board-{harmony,seaboard}-pinmux.c, and make the DT board stand on its
own.

My thoughts were:

a) Every Tegra board currently has a board-$name-pinmux.c which contains
a table like:

static struct tegra_pingroup_config harmony_pinmux[] = {
	{TEGRA_PINGROUP_ATA,   TEGRA_MUX_IDE,           TEGRA_PUPD_NORMAL,    TEGRA_TRI_NORMAL},

I'd just move that table into the Device Tree, keeping logically the same
structure, but just changing the syntax as appropriate for DT.

(I'm slightly worried that DT wants me to encode all that using an
unformatted table consistent of integers without any pre-processor macros
or named constants though...)

b) I'd probably need to modify the pinmux driver to be a platform device
in order to simplify integration with DT parsing etc.

c) I think the DT binding would not be affected by Linus W's work on a
pinctrl API; I see this table as platform data that sets up the initial
state of the SoC at boot-time, whereas the pinmux driver is more about
drivers dynamically altering the pinmux at run-time. Is this a reasonable
assertion? The alternative would be that once the new pinctrl API is
available, it would have an associated DT binding that represented this
data in a completely cross-SoC manner. Hence, any work on Tegra's pinmux
right now would be throwaway. I'm interested in people's thoughts here.

Finally, I was thinking that a similar thing could be done for the Tegra
GPIO driver, to replace gpio_table[] also in board-*-pinmux.c.

If this isn't useful right now, is there anything else that would help
out on Tegra?

Thanks.

-- 
nvpublic

Re: Tegra pinmux Device Tree support

[Grant Likely]

On Mon, Aug 08, 2011 at 02:42:56PM -0700, Stephen Warren wrote:
> Grant, Linus, & all,
> 
> I was thinking of adding some Device Tree support to Tegra's existing
> pinmux driver. This would remove the dependence of mach-tegra/board-dt.c
> on board-{harmony,seaboard}-pinmux.c, and make the DT board stand on its
> own.
> 
> My thoughts were:
> 
> a) Every Tegra board currently has a board-$name-pinmux.c which contains
> a table like:
> 
> static struct tegra_pingroup_config harmony_pinmux[] = {
> 	{TEGRA_PINGROUP_ATA,   TEGRA_MUX_IDE,           TEGRA_PUPD_NORMAL,    TEGRA_TRI_NORMAL},
> 
> I'd just move that table into the Device Tree, keeping logically the same
> structure, but just changing the syntax as appropriate for DT.
> 
> (I'm slightly worried that DT wants me to encode all that using an
> unformatted table consistent of integers without any pre-processor macros
> or named constants though...)
> 
> b) I'd probably need to modify the pinmux driver to be a platform device
> in order to simplify integration with DT parsing etc.
> 
> c) I think the DT binding would not be affected by Linus W's work on a
> pinctrl API; I see this table as platform data that sets up the initial
> state of the SoC at boot-time, whereas the pinmux driver is more about
> drivers dynamically altering the pinmux at run-time. Is this a reasonable
> assertion? The alternative would be that once the new pinctrl API is
> available, it would have an associated DT binding that represented this
> data in a completely cross-SoC manner. Hence, any work on Tegra's pinmux
> right now would be throwaway. I'm interested in people's thoughts here.
> 
> Finally, I was thinking that a similar thing could be done for the Tegra
> GPIO driver, to replace gpio_table[] also in board-*-pinmux.c.
> 
> If this isn't useful right now, is there anything else that would help
> out on Tegra?

We talked about this a bit at Linaro connect.  Outside of the more
complex runtime-reconfiguration of pin mux, there is a general need
for arbitrary initialization sequences to registers.  Also, pretty
much exactly what you need for tegra is needed for imx, omap and many
others.  The though was, rather than trying to come up with some kind
of pinmux-specific binding for pin states, which could end up being
very verbose if everything was split out into properties, we could
instead have a binding for register initialization sequences.
Something like this:

	pinmux-initseq = <reg1 value1> <reg1 value2> ...;

And then add some macros for DTC to make it easier to define things
like pinmux setup tables.  Although the binding above is probably too
simple and naive.

g.

Tegra pinmux Device Tree support

[Grant Likely]

On Mon, Aug 08, 2011 at 02:42:56PM -0700, Stephen Warren wrote:
> Grant, Linus, & all,
> 
> I was thinking of adding some Device Tree support to Tegra's existing
> pinmux driver. This would remove the dependence of mach-tegra/board-dt.c
> on board-{harmony,seaboard}-pinmux.c, and make the DT board stand on its
> own.
> 
> My thoughts were:
> 
> a) Every Tegra board currently has a board-$name-pinmux.c which contains
> a table like:
> 
> static struct tegra_pingroup_config harmony_pinmux[] = {
> 	{TEGRA_PINGROUP_ATA,   TEGRA_MUX_IDE,           TEGRA_PUPD_NORMAL,    TEGRA_TRI_NORMAL},
> 
> I'd just move that table into the Device Tree, keeping logically the same
> structure, but just changing the syntax as appropriate for DT.
> 
> (I'm slightly worried that DT wants me to encode all that using an
> unformatted table consistent of integers without any pre-processor macros
> or named constants though...)
> 
> b) I'd probably need to modify the pinmux driver to be a platform device
> in order to simplify integration with DT parsing etc.
> 
> c) I think the DT binding would not be affected by Linus W's work on a
> pinctrl API; I see this table as platform data that sets up the initial
> state of the SoC at boot-time, whereas the pinmux driver is more about
> drivers dynamically altering the pinmux at run-time. Is this a reasonable
> assertion? The alternative would be that once the new pinctrl API is
> available, it would have an associated DT binding that represented this
> data in a completely cross-SoC manner. Hence, any work on Tegra's pinmux
> right now would be throwaway. I'm interested in people's thoughts here.
> 
> Finally, I was thinking that a similar thing could be done for the Tegra
> GPIO driver, to replace gpio_table[] also in board-*-pinmux.c.
> 
> If this isn't useful right now, is there anything else that would help
> out on Tegra?

We talked about this a bit at Linaro connect.  Outside of the more
complex runtime-reconfiguration of pin mux, there is a general need
for arbitrary initialization sequences to registers.  Also, pretty
much exactly what you need for tegra is needed for imx, omap and many
others.  The though was, rather than trying to come up with some kind
of pinmux-specific binding for pin states, which could end up being
very verbose if everything was split out into properties, we could
instead have a binding for register initialization sequences.
Something like this:

	pinmux-initseq = <reg1 value1> <reg1 value2> ...;

And then add some macros for DTC to make it easier to define things
like pinmux setup tables.  Although the binding above is probably too
simple and naive.

g.

Re: Tegra pinmux Device Tree support

[Olof Johansson]

On Mon, Aug 8, 2011 at 2:42 PM, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> Grant, Linus, & all,
>
> I was thinking of adding some Device Tree support to Tegra's existing
> pinmux driver. This would remove the dependence of mach-tegra/board-dt.c
> on board-{harmony,seaboard}-pinmux.c, and make the DT board stand on its
> own.
>
> My thoughts were:
>
> a) Every Tegra board currently has a board-$name-pinmux.c which contains
> a table like:
>
> static struct tegra_pingroup_config harmony_pinmux[] = {
>        {TEGRA_PINGROUP_ATA,   TEGRA_MUX_IDE,           TEGRA_PUPD_NORMAL,    TEGRA_TRI_NORMAL},
>
> I'd just move that table into the Device Tree, keeping logically the same
> structure, but just changing the syntax as appropriate for DT.
>
> (I'm slightly worried that DT wants me to encode all that using an
> unformatted table consistent of integers without any pre-processor macros
> or named constants though...)
>
> b) I'd probably need to modify the pinmux driver to be a platform device
> in order to simplify integration with DT parsing etc.
>
> c) I think the DT binding would not be affected by Linus W's work on a
> pinctrl API; I see this table as platform data that sets up the initial
> state of the SoC at boot-time, whereas the pinmux driver is more about
> drivers dynamically altering the pinmux at run-time. Is this a reasonable
> assertion? The alternative would be that once the new pinctrl API is
> available, it would have an associated DT binding that represented this
> data in a completely cross-SoC manner. Hence, any work on Tegra's pinmux
> right now would be throwaway. I'm interested in people's thoughts here.
>
> Finally, I was thinking that a similar thing could be done for the Tegra
> GPIO driver, to replace gpio_table[] also in board-*-pinmux.c.
>
> If this isn't useful right now, is there anything else that would help
> out on Tegra?


This sounds similar to what I had in mind to look at when I got back
from vacation (not enough rainy days here to deal with it during :).

Some devices/drivers might need to make references to a pinmux (what
comes to mind is the i2c driver that has two virtual busses on two
separate multiplexed outputs on some boards). So having a tegra-pinmux
node with a sub-node for every pin group might get verbose but
definitely most flexible.

Each sub-node could encode the mux type with separate properties, for
example for the ata case above:

.../pinmux/ata:
    nvidia,pingroup-number = <>
    nvidia,tristate-normal
    nvidia,mux-mode = <>   (this should probably be an integer, seems
appropriate since the TRM uses integers as well)
    nvidia,pull-up-down-normal

(It could be argued that the 'normal' properties could just be omitted
and assumed)

'reg' could be used to specify the pingroup-number, I suppose -- but
omitting that means that you can have two entries for the same
pingroup, and mark one as inactive, so it can also be referenced with
a phandle.

This way the i2c virtual busses could just refer to the pingroup by
phandle for the mux flipping, with one of them being enabled by
default and the others inactive.


Thoughts?

-Olof

Tegra pinmux Device Tree support

[Olof Johansson]

On Mon, Aug 8, 2011 at 2:42 PM, Stephen Warren <swarren@nvidia.com> wrote:
> Grant, Linus, & all,
>
> I was thinking of adding some Device Tree support to Tegra's existing
> pinmux driver. This would remove the dependence of mach-tegra/board-dt.c
> on board-{harmony,seaboard}-pinmux.c, and make the DT board stand on its
> own.
>
> My thoughts were:
>
> a) Every Tegra board currently has a board-$name-pinmux.c which contains
> a table like:
>
> static struct tegra_pingroup_config harmony_pinmux[] = {
> ? ? ? ?{TEGRA_PINGROUP_ATA, ? TEGRA_MUX_IDE, ? ? ? ? ? TEGRA_PUPD_NORMAL, ? ?TEGRA_TRI_NORMAL},
>
> I'd just move that table into the Device Tree, keeping logically the same
> structure, but just changing the syntax as appropriate for DT.
>
> (I'm slightly worried that DT wants me to encode all that using an
> unformatted table consistent of integers without any pre-processor macros
> or named constants though...)
>
> b) I'd probably need to modify the pinmux driver to be a platform device
> in order to simplify integration with DT parsing etc.
>
> c) I think the DT binding would not be affected by Linus W's work on a
> pinctrl API; I see this table as platform data that sets up the initial
> state of the SoC at boot-time, whereas the pinmux driver is more about
> drivers dynamically altering the pinmux at run-time. Is this a reasonable
> assertion? The alternative would be that once the new pinctrl API is
> available, it would have an associated DT binding that represented this
> data in a completely cross-SoC manner. Hence, any work on Tegra's pinmux
> right now would be throwaway. I'm interested in people's thoughts here.
>
> Finally, I was thinking that a similar thing could be done for the Tegra
> GPIO driver, to replace gpio_table[] also in board-*-pinmux.c.
>
> If this isn't useful right now, is there anything else that would help
> out on Tegra?


This sounds similar to what I had in mind to look at when I got back
from vacation (not enough rainy days here to deal with it during :).

Some devices/drivers might need to make references to a pinmux (what
comes to mind is the i2c driver that has two virtual busses on two
separate multiplexed outputs on some boards). So having a tegra-pinmux
node with a sub-node for every pin group might get verbose but
definitely most flexible.

Each sub-node could encode the mux type with separate properties, for
example for the ata case above:

.../pinmux/ata:
    nvidia,pingroup-number = <>
    nvidia,tristate-normal
    nvidia,mux-mode = <>   (this should probably be an integer, seems
appropriate since the TRM uses integers as well)
    nvidia,pull-up-down-normal

(It could be argued that the 'normal' properties could just be omitted
and assumed)

'reg' could be used to specify the pingroup-number, I suppose -- but
omitting that means that you can have two entries for the same
pingroup, and mark one as inactive, so it can also be referenced with
a phandle.

This way the i2c virtual busses could just refer to the pingroup by
phandle for the mux flipping, with one of them being enabled by
default and the others inactive.


Thoughts?

-Olof

Re: Tegra pinmux Device Tree support

[Olof Johansson]

On Tue, Aug 9, 2011 at 10:34 AM, Grant Likely <grant.likely@secretlab.ca> wrote:

> We talked about this a bit at Linaro connect.  Outside of the more
> complex runtime-reconfiguration of pin mux, there is a general need
> for arbitrary initialization sequences to registers.  Also, pretty
> much exactly what you need for tegra is needed for imx, omap and many
> others.  The though was, rather than trying to come up with some kind
> of pinmux-specific binding for pin states, which could end up being
> very verbose if everything was split out into properties, we could
> instead have a binding for register initialization sequences.
> Something like this:
>
>        pinmux-initseq = <reg1 value1> <reg1 value2> ...;
>
> And then add some macros for DTC to make it easier to define things
> like pinmux setup tables.  Although the binding above is probably too
> simple and naive.

This seems to break the philosophy of what the device tree should do
-- it should describe the hardware so that the driver will know how to
program it, not include the code itself?

I know it's a hairy problem to deal with one compact pinmux
representation that fits all needs. Maybe making it overly generic is
the wrong way to go about it, and let each implementation define what
it needs? That way the simpler implementations can do something
reasonably simple without having to deal with the overly complex
options of the (over?) generalized platforms out there.


-Olof

Tegra pinmux Device Tree support

[Olof Johansson]

On Tue, Aug 9, 2011 at 10:34 AM, Grant Likely <grant.likely@secretlab.ca> wrote:

> We talked about this a bit at Linaro connect. ?Outside of the more
> complex runtime-reconfiguration of pin mux, there is a general need
> for arbitrary initialization sequences to registers. ?Also, pretty
> much exactly what you need for tegra is needed for imx, omap and many
> others. ?The though was, rather than trying to come up with some kind
> of pinmux-specific binding for pin states, which could end up being
> very verbose if everything was split out into properties, we could
> instead have a binding for register initialization sequences.
> Something like this:
>
> ? ? ? ?pinmux-initseq = <reg1 value1> <reg1 value2> ...;
>
> And then add some macros for DTC to make it easier to define things
> like pinmux setup tables. ?Although the binding above is probably too
> simple and naive.

This seems to break the philosophy of what the device tree should do
-- it should describe the hardware so that the driver will know how to
program it, not include the code itself?

I know it's a hairy problem to deal with one compact pinmux
representation that fits all needs. Maybe making it overly generic is
the wrong way to go about it, and let each implementation define what
it needs? That way the simpler implementations can do something
reasonably simple without having to deal with the overly complex
options of the (over?) generalized platforms out there.


-Olof

RE: Tegra pinmux Device Tree support

[Stephen Warren]

Olof Johansson wrote at Tuesday, August 09, 2011 11:35 AM:
> On Mon, Aug 8, 2011 at 2:42 PM, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> > Grant, Linus, & all,
> >
> > I was thinking of adding some Device Tree support to Tegra's existing
> > pinmux driver. This would remove the dependence of mach-tegra/board-dt.c
> > on board-{harmony,seaboard}-pinmux.c, and make the DT board stand on its
> > own.
> >
> > My thoughts were:
> >
> > a) Every Tegra board currently has a board-$name-pinmux.c which contains
> > a table like:
> >
> > static struct tegra_pingroup_config harmony_pinmux[] = {
> >        {TEGRA_PINGROUP_ATA,   TEGRA_MUX_IDE,           TEGRA_PUPD_NORMAL,    TEGRA_TRI_NORMAL},
> >
> > I'd just move that table into the Device Tree, keeping logically the same
> > structure, but just changing the syntax as appropriate for DT.
> >
> > (I'm slightly worried that DT wants me to encode all that using an
> > unformatted table consistent of integers without any pre-processor macros
> > or named constants though...)
...
> > c) I think the DT binding would not be affected by Linus W's work on a
> > pinctrl API; I see this table as platform data that sets up the initial
> > state of the SoC at boot-time, whereas the pinmux driver is more about
> > drivers dynamically altering the pinmux at run-time. Is this a reasonable
> > assertion? The alternative would be that once the new pinctrl API is
> > available, it would have an associated DT binding that represented this
> > data in a completely cross-SoC manner. Hence, any work on Tegra's pinmux
> > right now would be throwaway. I'm interested in people's thoughts here.
> >
> > Finally, I was thinking that a similar thing could be done for the Tegra
> > GPIO driver, to replace gpio_table[] also in board-*-pinmux.c.
> >
> > If this isn't useful right now, is there anything else that would help
> > out on Tegra?
...
> Some devices/drivers might need to make references to a pinmux (what
> comes to mind is the i2c driver that has two virtual busses on two
> separate multiplexed outputs on some boards). So having a tegra-pinmux
> node with a sub-node for every pin group might get verbose but
> definitely most flexible.

My previous thinking was that:

* We'd invent a new I2C bus mux driver for this purpose, rather than
  building the muxing functionality solely into the Tegra I2C driver
  like it is now.

* The new mux driver would use Linus W's pinctrl API to perform the
  muxing, and hence be generic across SoCs.

* The mux driver would need to be configured based on the pinctrl
  API's (well, the SoC's pinctrl driver's) names for pin groups and
  functions, rather than with something from Device Tree. This would
  allow the mux driver to work on systems that used the pinctrl API,
  but not Device Tree.

However, hooking Device Tree phandles and the pinctrl API together
sounds like it'd be a good idea; allowing the above points to still be
valid, and encompassing your idea. I'm not totally sure how that'd end
up working though; the original pinctrl idea was that the drivers
themselves would drive the names/numbers of the exposed pin groups,
whereas we're talking about driving that all from Device Tree data. That
would require each pingroup to contain e.g. a register address and bit
for each group. And Tegra's pinctrl driver would have all the drive
strength groups too, which are different groups in different registers,
and since all that data is encoded in the SoC HW rather than board-
specific, it seems like there's little benefit putting it into Device
Tree.

> Each sub-node could encode the mux type with separate properties, for
> example for the ata case above:
> 
> .../pinmux/ata:
>     nvidia,pingroup-number = <>
>     nvidia,tristate-normal
>     nvidia,mux-mode = <>   (this should probably be an integer, seems
> appropriate since the TRM uses integers as well)
>     nvidia,pull-up-down-normal

That's pretty verbose. However, it certainly addresses the problems with
a huge table of apparently meaningless numbers. It's probably workable.

> (It could be argued that the 'normal' properties could just be omitted
> and assumed)
> 
> 'reg' could be used to specify the pingroup-number, I suppose -- but
> omitting that means that you can have two entries for the same
> pingroup, and mark one as inactive, so it can also be referenced with
> a phandle.
> 
> This way the i2c virtual busses could just refer to the pingroup by
> phandle for the mux flipping, with one of them being enabled by
> default and the others inactive.
> 
> 
> Thoughts?
> 
> -Olof

-- 
nvpublic

Tegra pinmux Device Tree support

[Stephen Warren]

Olof Johansson wrote at Tuesday, August 09, 2011 11:35 AM:
> On Mon, Aug 8, 2011 at 2:42 PM, Stephen Warren <swarren@nvidia.com> wrote:
> > Grant, Linus, & all,
> >
> > I was thinking of adding some Device Tree support to Tegra's existing
> > pinmux driver. This would remove the dependence of mach-tegra/board-dt.c
> > on board-{harmony,seaboard}-pinmux.c, and make the DT board stand on its
> > own.
> >
> > My thoughts were:
> >
> > a) Every Tegra board currently has a board-$name-pinmux.c which contains
> > a table like:
> >
> > static struct tegra_pingroup_config harmony_pinmux[] = {
> > ? ? ? ?{TEGRA_PINGROUP_ATA, ? TEGRA_MUX_IDE, ? ? ? ? ? TEGRA_PUPD_NORMAL, ? ?TEGRA_TRI_NORMAL},
> >
> > I'd just move that table into the Device Tree, keeping logically the same
> > structure, but just changing the syntax as appropriate for DT.
> >
> > (I'm slightly worried that DT wants me to encode all that using an
> > unformatted table consistent of integers without any pre-processor macros
> > or named constants though...)
...
> > c) I think the DT binding would not be affected by Linus W's work on a
> > pinctrl API; I see this table as platform data that sets up the initial
> > state of the SoC at boot-time, whereas the pinmux driver is more about
> > drivers dynamically altering the pinmux at run-time. Is this a reasonable
> > assertion? The alternative would be that once the new pinctrl API is
> > available, it would have an associated DT binding that represented this
> > data in a completely cross-SoC manner. Hence, any work on Tegra's pinmux
> > right now would be throwaway. I'm interested in people's thoughts here.
> >
> > Finally, I was thinking that a similar thing could be done for the Tegra
> > GPIO driver, to replace gpio_table[] also in board-*-pinmux.c.
> >
> > If this isn't useful right now, is there anything else that would help
> > out on Tegra?
...
> Some devices/drivers might need to make references to a pinmux (what
> comes to mind is the i2c driver that has two virtual busses on two
> separate multiplexed outputs on some boards). So having a tegra-pinmux
> node with a sub-node for every pin group might get verbose but
> definitely most flexible.

My previous thinking was that:

* We'd invent a new I2C bus mux driver for this purpose, rather than
  building the muxing functionality solely into the Tegra I2C driver
  like it is now.

* The new mux driver would use Linus W's pinctrl API to perform the
  muxing, and hence be generic across SoCs.

* The mux driver would need to be configured based on the pinctrl
  API's (well, the SoC's pinctrl driver's) names for pin groups and
  functions, rather than with something from Device Tree. This would
  allow the mux driver to work on systems that used the pinctrl API,
  but not Device Tree.

However, hooking Device Tree phandles and the pinctrl API together
sounds like it'd be a good idea; allowing the above points to still be
valid, and encompassing your idea. I'm not totally sure how that'd end
up working though; the original pinctrl idea was that the drivers
themselves would drive the names/numbers of the exposed pin groups,
whereas we're talking about driving that all from Device Tree data. That
would require each pingroup to contain e.g. a register address and bit
for each group. And Tegra's pinctrl driver would have all the drive
strength groups too, which are different groups in different registers,
and since all that data is encoded in the SoC HW rather than board-
specific, it seems like there's little benefit putting it into Device
Tree.

> Each sub-node could encode the mux type with separate properties, for
> example for the ata case above:
> 
> .../pinmux/ata:
>     nvidia,pingroup-number = <>
>     nvidia,tristate-normal
>     nvidia,mux-mode = <>   (this should probably be an integer, seems
> appropriate since the TRM uses integers as well)
>     nvidia,pull-up-down-normal

That's pretty verbose. However, it certainly addresses the problems with
a huge table of apparently meaningless numbers. It's probably workable.

> (It could be argued that the 'normal' properties could just be omitted
> and assumed)
> 
> 'reg' could be used to specify the pingroup-number, I suppose -- but
> omitting that means that you can have two entries for the same
> pingroup, and mark one as inactive, so it can also be referenced with
> a phandle.
> 
> This way the i2c virtual busses could just refer to the pingroup by
> phandle for the mux flipping, with one of them being enabled by
> default and the others inactive.
> 
> 
> Thoughts?
> 
> -Olof

-- 
nvpublic

Re: Tegra pinmux Device Tree support

[Jamie Iles]

Hi Grant,

On Tue, Aug 09, 2011 at 11:34:28AM -0600, Grant Likely wrote:
> We talked about this a bit at Linaro connect.  Outside of the more
> complex runtime-reconfiguration of pin mux, there is a general need
> for arbitrary initialization sequences to registers.  Also, pretty
> much exactly what you need for tegra is needed for imx, omap and many
> others.  The though was, rather than trying to come up with some kind
> of pinmux-specific binding for pin states, which could end up being
> very verbose if everything was split out into properties, we could
> instead have a binding for register initialization sequences.
> Something like this:
> 
> 	pinmux-initseq = <reg1 value1> <reg1 value2> ...;
> 
> And then add some macros for DTC to make it easier to define things
> like pinmux setup tables.  Although the binding above is probably too
> simple and naive.

Unfortunately we have devices where the pinmux registers aren't directly 
memory mapped so I'm not sure that this would work for our devices 
(which admittedly don't have a friendly muxing configuration...).  We 
don't have a single way of muxing pins so at the moment we have a fairly 
hideous set of structures and macros to allow us to create smallish 
definitions of pins, but I can't see a nice way to express this in the 
DT.

At the moment I have a simple pinmux API that takes the pin name and 
setting name as strings, so I was thinking for a simple binding (for 
configuration) I could have something like:

	pinmux {
		pinmux-decode0 = "decode";
		pinmux-decode1 = "gpio";
	};

I remember in another thread you mentioned the possibility of building 
some tables and embedding the binary data in the DT, but that has 
potential problems with versioning of the structures.

Jamie

Tegra pinmux Device Tree support

[Jamie Iles]

Hi Grant,

On Tue, Aug 09, 2011 at 11:34:28AM -0600, Grant Likely wrote:
> We talked about this a bit at Linaro connect.  Outside of the more
> complex runtime-reconfiguration of pin mux, there is a general need
> for arbitrary initialization sequences to registers.  Also, pretty
> much exactly what you need for tegra is needed for imx, omap and many
> others.  The though was, rather than trying to come up with some kind
> of pinmux-specific binding for pin states, which could end up being
> very verbose if everything was split out into properties, we could
> instead have a binding for register initialization sequences.
> Something like this:
> 
> 	pinmux-initseq = <reg1 value1> <reg1 value2> ...;
> 
> And then add some macros for DTC to make it easier to define things
> like pinmux setup tables.  Although the binding above is probably too
> simple and naive.

Unfortunately we have devices where the pinmux registers aren't directly 
memory mapped so I'm not sure that this would work for our devices 
(which admittedly don't have a friendly muxing configuration...).  We 
don't have a single way of muxing pins so at the moment we have a fairly 
hideous set of structures and macros to allow us to create smallish 
definitions of pins, but I can't see a nice way to express this in the 
DT.

At the moment I have a simple pinmux API that takes the pin name and 
setting name as strings, so I was thinking for a simple binding (for 
configuration) I could have something like:

	pinmux {
		pinmux-decode0 = "decode";
		pinmux-decode1 = "gpio";
	};

I remember in another thread you mentioned the possibility of building 
some tables and embedding the binary data in the DT, but that has 
potential problems with versioning of the structures.

Jamie

Re: Tegra pinmux Device Tree support

[Grant Likely]

On Tue, Aug 09, 2011 at 09:18:15PM +0100, Jamie Iles wrote:
> Hi Grant,
> 
> On Tue, Aug 09, 2011 at 11:34:28AM -0600, Grant Likely wrote:
> > We talked about this a bit at Linaro connect.  Outside of the more
> > complex runtime-reconfiguration of pin mux, there is a general need
> > for arbitrary initialization sequences to registers.  Also, pretty
> > much exactly what you need for tegra is needed for imx, omap and many
> > others.  The though was, rather than trying to come up with some kind
> > of pinmux-specific binding for pin states, which could end up being
> > very verbose if everything was split out into properties, we could
> > instead have a binding for register initialization sequences.
> > Something like this:
> > 
> > 	pinmux-initseq = <reg1 value1> <reg1 value2> ...;
> > 
> > And then add some macros for DTC to make it easier to define things
> > like pinmux setup tables.  Although the binding above is probably too
> > simple and naive.
> 
> Unfortunately we have devices where the pinmux registers aren't directly 
> memory mapped so I'm not sure that this would work for our devices 
> (which admittedly don't have a friendly muxing configuration...).  We 
> don't have a single way of muxing pins so at the moment we have a fairly 
> hideous set of structures and macros to allow us to create smallish 
> definitions of pins, but I can't see a nice way to express this in the 
> DT.
> 
> At the moment I have a simple pinmux API that takes the pin name and 
> setting name as strings, so I was thinking for a simple binding (for 
> configuration) I could have something like:
> 
> 	pinmux {
> 		pinmux-decode0 = "decode";
> 		pinmux-decode1 = "gpio";
> 	};
> 
> I remember in another thread you mentioned the possibility of building 
> some tables and embedding the binary data in the DT, but that has 
> potential problems with versioning of the structures.

Structure/binding versioning is easy.  If it really is necessary to
change the binding in an incompatible way, then change the compatible
value too.

g.

Tegra pinmux Device Tree support

[Grant Likely]

On Tue, Aug 09, 2011 at 09:18:15PM +0100, Jamie Iles wrote:
> Hi Grant,
> 
> On Tue, Aug 09, 2011 at 11:34:28AM -0600, Grant Likely wrote:
> > We talked about this a bit at Linaro connect.  Outside of the more
> > complex runtime-reconfiguration of pin mux, there is a general need
> > for arbitrary initialization sequences to registers.  Also, pretty
> > much exactly what you need for tegra is needed for imx, omap and many
> > others.  The though was, rather than trying to come up with some kind
> > of pinmux-specific binding for pin states, which could end up being
> > very verbose if everything was split out into properties, we could
> > instead have a binding for register initialization sequences.
> > Something like this:
> > 
> > 	pinmux-initseq = <reg1 value1> <reg1 value2> ...;
> > 
> > And then add some macros for DTC to make it easier to define things
> > like pinmux setup tables.  Although the binding above is probably too
> > simple and naive.
> 
> Unfortunately we have devices where the pinmux registers aren't directly 
> memory mapped so I'm not sure that this would work for our devices 
> (which admittedly don't have a friendly muxing configuration...).  We 
> don't have a single way of muxing pins so at the moment we have a fairly 
> hideous set of structures and macros to allow us to create smallish 
> definitions of pins, but I can't see a nice way to express this in the 
> DT.
> 
> At the moment I have a simple pinmux API that takes the pin name and 
> setting name as strings, so I was thinking for a simple binding (for 
> configuration) I could have something like:
> 
> 	pinmux {
> 		pinmux-decode0 = "decode";
> 		pinmux-decode1 = "gpio";
> 	};
> 
> I remember in another thread you mentioned the possibility of building 
> some tables and embedding the binary data in the DT, but that has 
> potential problems with versioning of the structures.

Structure/binding versioning is easy.  If it really is necessary to
change the binding in an incompatible way, then change the compatible
value too.

g.

Re: Tegra pinmux Device Tree support

[David Gibson]

On Tue, Aug 09, 2011 at 10:44:29AM -0700, Olof Johansson wrote:
> On Tue, Aug 9, 2011 at 10:34 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> 
> > We talked about this a bit at Linaro connect.  Outside of the more
> > complex runtime-reconfiguration of pin mux, there is a general need
> > for arbitrary initialization sequences to registers.  Also, pretty
> > much exactly what you need for tegra is needed for imx, omap and many
> > others.  The though was, rather than trying to come up with some kind
> > of pinmux-specific binding for pin states, which could end up being
> > very verbose if everything was split out into properties, we could
> > instead have a binding for register initialization sequences.
> > Something like this:
> >
> >        pinmux-initseq = <reg1 value1> <reg1 value2> ...;
> >
> > And then add some macros for DTC to make it easier to define things
> > like pinmux setup tables.  Although the binding above is probably too
> > simple and naive.
> 
> This seems to break the philosophy of what the device tree should do
> -- it should describe the hardware so that the driver will know how to
> program it, not include the code itself?

Arguably, yes, although it's a barrier that's been frequently broken
by Apple already.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Tegra pinmux Device Tree support

[David Gibson]

On Tue, Aug 09, 2011 at 10:44:29AM -0700, Olof Johansson wrote:
> On Tue, Aug 9, 2011 at 10:34 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> 
> > We talked about this a bit at Linaro connect. ?Outside of the more
> > complex runtime-reconfiguration of pin mux, there is a general need
> > for arbitrary initialization sequences to registers. ?Also, pretty
> > much exactly what you need for tegra is needed for imx, omap and many
> > others. ?The though was, rather than trying to come up with some kind
> > of pinmux-specific binding for pin states, which could end up being
> > very verbose if everything was split out into properties, we could
> > instead have a binding for register initialization sequences.
> > Something like this:
> >
> > ? ? ? ?pinmux-initseq = <reg1 value1> <reg1 value2> ...;
> >
> > And then add some macros for DTC to make it easier to define things
> > like pinmux setup tables. ?Although the binding above is probably too
> > simple and naive.
> 
> This seems to break the philosophy of what the device tree should do
> -- it should describe the hardware so that the driver will know how to
> program it, not include the code itself?

Arguably, yes, although it's a barrier that's been frequently broken
by Apple already.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

RE: Tegra pinmux Device Tree support

[Stephen Warren]

Olof Johansson wrote at Tuesday, August 09, 2011 11:35 AM:
> On Mon, Aug 8, 2011 at 2:42 PM, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> > Grant, Linus, & all,
> >
> > I was thinking of adding some Device Tree support to Tegra's existing
> > pinmux driver. This would remove the dependence of mach-tegra/board-dt.c
> > on board-{harmony,seaboard}-pinmux.c, and make the DT board stand on its
> > own.
...
> This sounds similar to what I had in mind to look at when I got back
> from vacation (not enough rainy days here to deal with it during :).
> 
> Some devices/drivers might need to make references to a pinmux (what
> comes to mind is the i2c driver that has two virtual busses on two
> separate multiplexed outputs on some boards). So having a tegra-pinmux
> node with a sub-node for every pin group might get verbose but
> definitely most flexible.
> 
> Each sub-node could encode the mux type with separate properties, for
> example for the ata case above:
> 
> .../pinmux/ata:
>     nvidia,pingroup-number = <>
>     nvidia,tristate-normal
>     nvidia,mux-mode = <>   (this should probably be an integer, seems
> appropriate since the TRM uses integers as well)
>     nvidia,pull-up-down-normal
... 
> This way the i2c virtual busses could just refer to the pingroup by
> phandle for the mux flipping, with one of them being enabled by
> default and the others inactive.

I now have both the Tegra GPIO and pinmux drivers working as platform
devices, and performing initial setup from device tree, in Grant's
devicetree/test branch. I implemented a syntax similar to Olof's above,
but with a few differences I'll explain below.

For GPIOs, the DT node gains a list of GPIOs replaces gpio_table[] in
board-harmony-pinmux.c. For each entry, tegra_gpio_enable() is called so
that the GPIO driver gains control over the pin.

	gpio: gpio@6000d000 {
		nvidia,gpios = <
			69	// TEGRA_GPIO_PI5 SD2_CD
			57	// TEGRA_GPIO_PH1 SD2_WP
		>;
	};

For pinmux, I allowed a sub-node for each group, which configures the
properties of that group. "nvidia,pull" is optional, and defaults to
none. "nvidia,tristate" is optional, and defaults to driven. For a given
pinmux group, the whole node is optional; if not present, the driver will
simply leave that group alone at boot time.

	pinmux: pinmux@70000000 {
		CDEV1 {
			nvidia,function = "PLLA_OUT";
		};
		CDEV2 {
			nvidia,function = "PLLP_OUT4";
			nvidia,pull = "down";
			nvidia,tristate;
		};
	};

Some differences from Olof's proposal:

1) I chose not to support disabling the pingroup (child) nodes. That
would only be required if other devices want to reference these pingroup
nodes using phandles when defining any dynamic pinmux configuration they
will use. I'm not convinced that we want to tie that configuration into
device tree, since drivers using the dynamic pinmux feature would use the
future pinmux/pinctrl API to implement the muxing, and I don't think we
want to restrict that API to systems that use device tree.

2) Given (1), having multiple entries for the same pingroup isn't
required, so storing the pingroup name/number in a property of the node
isn't required. Instead, I name the child nodes based on the pingroup
they apply to.

3) I made the value of "nvidia,function" be the textual name of the
function. This is a value from the Tegra pinmux API's global list of all
functions, rather than the pin-specific mux selector 0..3 used by the
Tegra registers and manual. I did this because:

3a) It was easier to convert the existing board files' pinmux tables; I
didn't need to map each name back to the mux select value using the
manual.

3b) When we do have a pinmux/pinctrl API, it looks like that API will
use a global (at least within a SoC) namespace for function names, and
one that's text-based. Using those same names here will keep everything
consistent.

Does that seem reasonable?

Finally note that I implemented handling of the pinmux node's children
as:

for each pingroup:
    find child node based on pingroup's name
    if it exists:
        process it

That required the following new function on drivers/of/base.c:

struct device_node *of_find_child_node_by_name(
	const struct device_node *parent, const char *name)
{
	struct device_node *child;

	read_lock(&devtree_lock);

	for_each_child_of_node(parent, child) {
		if (!strcmp(child->name, name))
			break;
	}

	read_unlock(&devtree_lock);

	return child;
}
EXPORT_SYMBOL(of_find_child_node_by_name);

Does that seem reasonable, or should I enumerate all the child nodes,
and map each node's name back to the pingroup value? That would remove
the need for the new function above, but the pinmux driver would need
to add a simple loop over the pingroup name table to map the name.

Thanks for any feedback.

-- 
nvpublic

Tegra pinmux Device Tree support

[Stephen Warren]

Olof Johansson wrote at Tuesday, August 09, 2011 11:35 AM:
> On Mon, Aug 8, 2011 at 2:42 PM, Stephen Warren <swarren@nvidia.com> wrote:
> > Grant, Linus, & all,
> >
> > I was thinking of adding some Device Tree support to Tegra's existing
> > pinmux driver. This would remove the dependence of mach-tegra/board-dt.c
> > on board-{harmony,seaboard}-pinmux.c, and make the DT board stand on its
> > own.
...
> This sounds similar to what I had in mind to look at when I got back
> from vacation (not enough rainy days here to deal with it during :).
> 
> Some devices/drivers might need to make references to a pinmux (what
> comes to mind is the i2c driver that has two virtual busses on two
> separate multiplexed outputs on some boards). So having a tegra-pinmux
> node with a sub-node for every pin group might get verbose but
> definitely most flexible.
> 
> Each sub-node could encode the mux type with separate properties, for
> example for the ata case above:
> 
> .../pinmux/ata:
>     nvidia,pingroup-number = <>
>     nvidia,tristate-normal
>     nvidia,mux-mode = <>   (this should probably be an integer, seems
> appropriate since the TRM uses integers as well)
>     nvidia,pull-up-down-normal
... 
> This way the i2c virtual busses could just refer to the pingroup by
> phandle for the mux flipping, with one of them being enabled by
> default and the others inactive.

I now have both the Tegra GPIO and pinmux drivers working as platform
devices, and performing initial setup from device tree, in Grant's
devicetree/test branch. I implemented a syntax similar to Olof's above,
but with a few differences I'll explain below.

For GPIOs, the DT node gains a list of GPIOs replaces gpio_table[] in
board-harmony-pinmux.c. For each entry, tegra_gpio_enable() is called so
that the GPIO driver gains control over the pin.

	gpio: gpio at 6000d000 {
		nvidia,gpios = <
			69	// TEGRA_GPIO_PI5 SD2_CD
			57	// TEGRA_GPIO_PH1 SD2_WP
		>;
	};

For pinmux, I allowed a sub-node for each group, which configures the
properties of that group. "nvidia,pull" is optional, and defaults to
none. "nvidia,tristate" is optional, and defaults to driven. For a given
pinmux group, the whole node is optional; if not present, the driver will
simply leave that group alone at boot time.

	pinmux: pinmux at 70000000 {
		CDEV1 {
			nvidia,function = "PLLA_OUT";
		};
		CDEV2 {
			nvidia,function = "PLLP_OUT4";
			nvidia,pull = "down";
			nvidia,tristate;
		};
	};

Some differences from Olof's proposal:

1) I chose not to support disabling the pingroup (child) nodes. That
would only be required if other devices want to reference these pingroup
nodes using phandles when defining any dynamic pinmux configuration they
will use. I'm not convinced that we want to tie that configuration into
device tree, since drivers using the dynamic pinmux feature would use the
future pinmux/pinctrl API to implement the muxing, and I don't think we
want to restrict that API to systems that use device tree.

2) Given (1), having multiple entries for the same pingroup isn't
required, so storing the pingroup name/number in a property of the node
isn't required. Instead, I name the child nodes based on the pingroup
they apply to.

3) I made the value of "nvidia,function" be the textual name of the
function. This is a value from the Tegra pinmux API's global list of all
functions, rather than the pin-specific mux selector 0..3 used by the
Tegra registers and manual. I did this because:

3a) It was easier to convert the existing board files' pinmux tables; I
didn't need to map each name back to the mux select value using the
manual.

3b) When we do have a pinmux/pinctrl API, it looks like that API will
use a global (at least within a SoC) namespace for function names, and
one that's text-based. Using those same names here will keep everything
consistent.

Does that seem reasonable?

Finally note that I implemented handling of the pinmux node's children
as:

for each pingroup:
    find child node based on pingroup's name
    if it exists:
        process it

That required the following new function on drivers/of/base.c:

struct device_node *of_find_child_node_by_name(
	const struct device_node *parent, const char *name)
{
	struct device_node *child;

	read_lock(&devtree_lock);

	for_each_child_of_node(parent, child) {
		if (!strcmp(child->name, name))
			break;
	}

	read_unlock(&devtree_lock);

	return child;
}
EXPORT_SYMBOL(of_find_child_node_by_name);

Does that seem reasonable, or should I enumerate all the child nodes,
and map each node's name back to the pingroup value? That would remove
the need for the new function above, but the pinmux driver would need
to add a simple loop over the pingroup name table to map the name.

Thanks for any feedback.

-- 
nvpublic

Re: Tegra pinmux Device Tree support

[Shawn Guo]

On Tue, Aug 09, 2011 at 10:44:29AM -0700, Olof Johansson wrote:
> On Tue, Aug 9, 2011 at 10:34 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> 
> > We talked about this a bit at Linaro connect.  Outside of the more
> > complex runtime-reconfiguration of pin mux, there is a general need
> > for arbitrary initialization sequences to registers.  Also, pretty
> > much exactly what you need for tegra is needed for imx, omap and many
> > others.  The though was, rather than trying to come up with some kind
> > of pinmux-specific binding for pin states, which could end up being
> > very verbose if everything was split out into properties, we could
> > instead have a binding for register initialization sequences.
> > Something like this:
> >
> >        pinmux-initseq = <reg1 value1> <reg1 value2> ...;
> >
> > And then add some macros for DTC to make it easier to define things
> > like pinmux setup tables.  Although the binding above is probably too
> > simple and naive.
> 
> This seems to break the philosophy of what the device tree should do
> -- it should describe the hardware so that the driver will know how to
> program it, not include the code itself?
> 
> I know it's a hairy problem to deal with one compact pinmux
> representation that fits all needs. Maybe making it overly generic is
> the wrong way to go about it, and let each implementation define what
> it needs? That way the simpler implementations can do something
> reasonably simple without having to deal with the overly complex
> options of the (over?) generalized platforms out there.
> 
Though I was part of the talk on Linaro Connect about register-write
binding, I still like the soc specific binding a little bit more.
Let's see if we can eventually change Grant's mind with more people
on the other way :)

-- 
Regards,
Shawn

Tegra pinmux Device Tree support

[Shawn Guo]

On Tue, Aug 09, 2011 at 10:44:29AM -0700, Olof Johansson wrote:
> On Tue, Aug 9, 2011 at 10:34 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> 
> > We talked about this a bit at Linaro connect. ?Outside of the more
> > complex runtime-reconfiguration of pin mux, there is a general need
> > for arbitrary initialization sequences to registers. ?Also, pretty
> > much exactly what you need for tegra is needed for imx, omap and many
> > others. ?The though was, rather than trying to come up with some kind
> > of pinmux-specific binding for pin states, which could end up being
> > very verbose if everything was split out into properties, we could
> > instead have a binding for register initialization sequences.
> > Something like this:
> >
> > ? ? ? ?pinmux-initseq = <reg1 value1> <reg1 value2> ...;
> >
> > And then add some macros for DTC to make it easier to define things
> > like pinmux setup tables. ?Although the binding above is probably too
> > simple and naive.
> 
> This seems to break the philosophy of what the device tree should do
> -- it should describe the hardware so that the driver will know how to
> program it, not include the code itself?
> 
> I know it's a hairy problem to deal with one compact pinmux
> representation that fits all needs. Maybe making it overly generic is
> the wrong way to go about it, and let each implementation define what
> it needs? That way the simpler implementations can do something
> reasonably simple without having to deal with the overly complex
> options of the (over?) generalized platforms out there.
> 
Though I was part of the talk on Linaro Connect about register-write
binding, I still like the soc specific binding a little bit more.
Let's see if we can eventually change Grant's mind with more people
on the other way :)

-- 
Regards,
Shawn