linux-kernel - RE: Pinmux bindings proposal V2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <74CDBE0F657A3D45AFBB94109FB122FF178E123E62@HQMAIL01.nvidia.com>
Date:	Fri, 27 Jan 2012 09:51:58 -0800
From:	Stephen Warren <swarren@...dia.com>
To:	Tony Lindgren <tony@...mide.com>, Simon Glass <sjg@...omium.org>
CC:	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

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.

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.

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?

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

> 	/*
> 	 * 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.

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.

-- 
nvpublic

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