Message-ID: <20110817061703.GB10037@S2100-06.ap.freescale.net>
Date:	Wed, 17 Aug 2011 14:17:04 +0800
From:	Shawn Guo <shawn.guo@...escale.com>
To:	Stephen Warren <swarren@...dia.com>
CC:	Arnd Bergmann <arnd@...db.de>,
	Linus Walleij <linus.walleij@...aro.org>,
	Jamie Iles <jamie@...ieiles.com>,
	Grant Likely <grant.likely@...retlab.ca>,
	Colin Cross <ccross@...roid.com>,
	Erik Gilling <konkers@...roid.com>,
	Olof Johansson <olof@...om.net>,
	Russell King <linux@....linux.org.uk>,
	"devicetree-discuss@...ts.ozlabs.org" 
	<devicetree-discuss@...ts.ozlabs.org>,
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Belisko Marek <marek.belisko@...il.com>,
	Sergei Shtylyov <sshtylyov@...sta.com>
Subject: Re: [RFC PATCH v2 06/13] docs/dt: Document nvidia,tegra20-pinmux
 binding

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@...dia.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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists