Re: [RFC PATCH v2 06/13] docs/dt: Document nvidia,tegra20-pinmux binding

Shawn Guo
2011-08-17T06:09:02Z

On Wed, Aug 17, 2011 at 02:02:43PM +0800, Shawn Guo wrote:
> On Tue, Aug 16, 2011 at 10:32:05AM -0700, Stephen Warren wrote:
> > Arnd Bergmann wrote at Tuesday, August 16, 2011 7:52 AM:
> > > On Monday 15 August 2011, Stephen Warren wrote:
> > > > Signed-off-by: Stephen Warren <swarren@nvidia.com>
> > > > ---
> > > > .../devicetree/bindings/pinmux/pinmux_nvidia.txt | 294 ++++++++++++++++++++
> > > > 1 files changed, 294 insertions(+), 0 deletions(-)
> > > > create mode 100644 Documentation/devicetree/bindings/pinmux/pinmux_nvidia.txt
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/pinmux/pinmux_nvidia.txt b/Documentation/devicetree/bindings/pinmux/pinmux_nvidia.txt
> > > > new file mode 100644
> > > > index 0000000..744e1b7
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/pinmux/pinmux_nvidia.txt
> > > > @@ -0,0 +1,294 @@
> > > > +NVIDIA Tegra 2 pinmux controller
> > > > +
> > > > +Required properties:
> > > > +- compatible : "nvidia,tegra20-pinmux"
> > >
> > > Hmm, I think it would be much better in general if we could define
> > > pinmux bindings in a way that is less specific to just one soc.
> >
> > > The contents of this file seem to be specific to even just one
> > > version of the tegra chip, and might be completely different for
> > > tegra30 and later, right?
> >
> > The general concepts are the same between tegra20 and tegra30.
> >
> > Tegra30 has a different set of mux and drive pingroups.
> >
> > Tegra30 has more functions that can be assigned to pins.
> >
> > Tegra30 has three more options per mux pingroup, although I haven't
> > Investigated whether any of those would need to be represented in DT;
> > I suspect at least one will, possibly all.
> >
> > > Maybe Linus W can comment on this and say whether he thinks it can
> > > be generalized enough to apply to other pinmux drivers.
> > >
> > > > +Optional sub-nodes:
> > > > +- nvidia,mux-groups : Mux group settings; see below.
> > > > +- nvidia,drive-groups : Drive group settings; see below.
> > > > +
> > > > +nvidia,mux-groups sub-node:
> > >
> > > These concepts seem general enough to me that they can apply to
> > > other chips, and I would consequently drop the nvidia, prefix.
> >
> > Two things to note about Tegra that might not apply to all SoCs:
> >
> > * There are separate groups of pins for Muxing (plus some config) vs.
> > drive-strength (and related config). Some SoCs might use the same set
> > of groups for both. Perhaps some SoC might even have three or more types
> > of groups! I expect this to have some impact on the binding; I created
> > explicit mux-groups and drive-groups sub-nodes to represent this.
> >
> > * Tegra's pinmux apply settings to groups of pins instead of individual
> > pins. Some SoCs might allow each setting to be configurable per-pin.
> > I don't expect this to have any impact as far as the bindings go though;
> > it'll simply impact the names of the available pin "groups"; some SoCs
> > will name groups, others pins.
> >
> +1
>
> imx's pinmux applies settings to individual pins.
>
> > > > +Each mux pin group is represented as a sub-node of the nvidia,mux-groups node.
> > > > +The name of the sub-node should be the name of the mux pingroup. The following
> > > > +names are valid:
> > > > +
> > > > + ata
> > > > + atb
> > > > + atc
> > > > + atd
> > > > + ate
> > > > + cdev1
> > > > + cdev2
> > > > ...
> > >
> > > I noticed that each board you define has a complete list of these. Would
> > > it be possible to move a generic list into a tegra20-pinmux.dtsi file and
> > > just override the pins in the per-board .dts file that require some special
> > > setup?
> >
>
> It sounds sane for imx though. I'm going to take this suggestion to
> have imx53-pinmux.dtsi holding register offset for each pin, which is
> common across all i.mx53 boards.
>
> > I'm not sure how much commonality there will really be.
> >
> > Certainly, many boards are based off our reference designs and so will
> > have many similarities that could be shared.
> >
> > That said, comparing even tegra-harmony.dts and tegra-seaboard.dts shows
> > a lot of differences. It seems a lot less error-prone to just completely
> > describe the entire pinmux state in each board's .dts file, rather than
> > trying to represent half the information as default in the SoC .dtsi file,
> > and just the diff in the board .dts file. I suppose perhaps if we put the
> > hardware's own default settings in tegra20.dtsi, that'd make sense, since
> > people are presumably reasonably aware of the delta relative to that.
> >
> > We'd need to add new properties to override defaults, like:
> >
> > nvidia,tristate --> nvidia,drive
> > nvidia,pull-* --> nvidia,no-pull
> >
> > > > +
> > > > +optional subnode-properties:
> > > > +- nvidia,pull-up : Boolean, apply Tegra's internal pull-up to the pin.
> > > > +- nvidia,pull-down : Boolean, apply Tegra's internal pull-down to the pin.
> > > > +- nvidia,tristate : Boolean, tristate the pin. Otherwise, drive it.
> > > > +
> > > > +If both nvidia,pull-up and nvidia,pull-down are specified, nvidia,pull-up
> > > > +takes precedence.
> > >
> > > These again seem generic enough to go into a general pinmux binding, without
> > > the nvidia, prefix.
> >
> > Yes, I believe Jamie Iles was going to try cooking up a generic core binding
> > that could be shared across different SoCs, and extended with custom stuff.
> >
> I would image that whatever common binding for pin/pad configuration
> comes can hardly work for imx out of box. For each pin/pad, imx have
> 4 configurations for pull:
>
> 00: 100KOhm Pull Down
> 01: 47KOhm Pull Up
> 10: 100KOhm Pull Up
> 11: 22KOhm Pull Up
>
> 4 configurations for drive:
>
> 00: 100KOhm Pull Down
> 01: 47KOhm Pull Up
> 10: 100KOhm Pull Up
> 11: 22KOhm Pull Up
>
Correction:

00 Low Drive Strength
01 Medium Drive Strength
10 High Drive Strength
11 Max Drive Strength

> some other 1 bit configuration fields:
>
> low/high output voltage Field
> Hyst. Enable Field
> Pull / Keep Enable Field
> Pull / Keep Select Field
> Slew Rate Field
>
> All these are all in one register <pad_name>_PAD_CTL (mux configuration
> is in another register <pad_name>_PAD_MUX). The best binding for
> PAD_CTL I can think of so far is to encode the register value in DT
> directly.
>

--
Regards,
Shawn