Message-ID: <20120127181004.GA9339@atomide.com>
Date:	Fri, 27 Jan 2012 10:10:04 -0800
From:	Tony Lindgren <tony@...mide.com>
To:	Stephen Warren <swarren@...dia.com>
Cc:	Simon Glass <sjg@...omium.org>,
	Dong Aisheng-B29396 <B29396@...escale.com>,
	"Linus Walleij (linus.walleij@...aro.org)" <linus.walleij@...aro.org>,
	"Sascha Hauer (s.hauer@...gutronix.de)" <s.hauer@...gutronix.de>,
	"rob.herring@...xeda.com" <rob.herring@...xeda.com>,
	"kernel@...gutronix.de" <kernel@...gutronix.de>,
	"cjb@...top.org" <cjb@...top.org>,
	Dong Aisheng <dongas86@...il.com>,
	"Shawn Guo (shawn.guo@...aro.org)" <shawn.guo@...aro.org>,
	Thomas Abraham <thomas.abraham@...aro.org>,
	"Grant Likely (grant.likely@...retlab.ca)" 
	<grant.likely@...retlab.ca>,
	"devicetree-discuss@...ts.ozlabs.org" 
	<devicetree-discuss@...ts.ozlabs.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: Pinmux bindings proposal V2

* Stephen Warren <swarren@...dia.com> [120127 09:21]:
> Tony Lindgren wrote at Friday, January 27, 2012 10:38 AM:
> ...
> > So how about let's do separate static and dynamic bindings,
> > something like this:
> > 
> > 	/*
> > 	 * Static init time only mux where
> > 	 * we only specify phandle to driver
> > 	 * and, offset of the mux, and the value.
> > 	 * These pins are discarded after init.
> > 	 *
> > 	 * Format:	  mux_ctrl      offset value
> > 	 */
> > 	pinctrl-static = <&pmx_driver1  0x0020 0x1245
> > 			   &pmx_driver2 0x0022 0x6578>;
> 
> So those are direct register writes? That sounds pretty scary, and a
> royal pain to author the device tree.

Driver specific, either registers or enumeration. It could also
it could be also generic defines, something along the lines of
PMX_MUX_FUNCTION_1 | PMX_MUX_GPIO_INPUT.
 
> If we do go with this, I think we'd need a mask for each register write
> too, so you can leave certain bits unaffected; there's no reason to
> believe in general that each pin has a dedicated register only for that
> pin, or even that only pinmux/config data is in the register.

I'm currently using "pinmux-simple,function-mask", but yeah sounds like
it should be a generic mask name.
 
> This also makes it difficult to extract semantic information from the
> DT. How can the pinctrl subsystem know which pins are in use and which
> aren't here? This is relevant if some module loads later and attempts
> to claim some pins - are they already in use by another driver or not?

We could just have one spinlock for all the discarded pins instead of
having a single spinlock for each discarded pin?
 
> Now individual pinctrl drivers could interpret those register values
> and know that this means pin/group "x" is programmed to mux value "y",
> but does that mean pin "x" is actually /used/, or just that the init
> table had to program value "y" because the default for that pin is "z"
> which conflicted in HW with some other mux setting that the board
> needed (e.g. muxing signal "y" to some other pin).
> 
> (Put another way, this binding completely bypasses the pinctrl subsystem;
> is that OK?)

Well I was thinking we should still register the pins, and have pinctrl
fwk set those values, then discard those pins but still keep them as
locked.
 
> > 	/*
> > 	 * Dynamic mux where the mux is kept around after
> > 	 * init and multiple states can be defined for
> > 	 * a mux as a subnode of the pinmux controller.
> > 	 *
> > 	 * Format:	   mux_phandle   initial state
> > 	 */
> > 	pinctrl-dynamic = <&pmx_sdhci    PMX_STATE_ENABLED
> > 			   &pmx_ehci_xcv PMX_STATE_ENABLED>;
> > 
> > This would make pinctrl-static binding follow the same
> > standard as GPIO binding and can be parsed easily with
> > of_parse_phandle_with_args.
> > 
> > Then for pinctrl-dynamic we can make a custom parser,
> > and the binding can follow the more readable format as
> > Simon posted.
> 
> I don't think there's any point in having 2 separate bindings; it's been
> hard enough to come up with /one/ binding! If we do go for raw register
> writes for the static stuff, we should just do the same for the dynamic
> stuff too.

But then we need to waste a register for the static/dynamic flag for
each mux.. Or make the tree deeper.
 
> In fact, given this would all bypass the pinctrl subsystem entirely,
> perhaps lets not even define a standard format for pinctrl-static or
> pinctrl-dynamic, and just have each pin controller driver parse tables
> inside its own node, in a format specific to that pin controller's
> binding. I already had that working for the static case back in last
> August and would love to just apply those patches and be done with this.

Hmm I don't think it would by pass it, we just need to let pinctrl
fwk deal with the init time only pins too.

For the dynamic pins, note that PMX_STATE_* defines would be one of the
standard states supported by the pinctrl fwk. Then of course where
pmx_sdhci binding is implemented, you could have either hardware specific
register values, enumeration or generic defines depending on how the
pinctrl driver is implemented.

Regards,

Tony 

--
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