linux-kernel - Re: [PATCH 2/2] pinctrl: pinctrl-single: Add pinctrl-single,bits type of mux

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]

Date:	Mon, 10 Sep 2012 10:10:03 -0700
From:	Tony Lindgren <tony@...mide.com>
To:	Peter Ujfalusi <peter.ujfalusi@...com>
Cc:	Linus Walleij <linus.walleij@...aro.org>,
	linux-kernel@...r.kernel.org, linux-omap@...r.kernel.org
Subject: Re: [PATCH 2/2] pinctrl: pinctrl-single: Add pinctrl-single,bits
 type of mux

* Peter Ujfalusi <peter.ujfalusi@...com> [120910 04:55]:
> On 09/07/2012 07:55 PM, Tony Lindgren wrote:
> > 
> > I'd like to have something that specifies the controller type so
> > we don't need to mix two types of controllers and test for
> > non-existing properties when parsing the pins.
> > 
> > How about we require something specified for the pinctrl driver
> > in the "one-bit-per-mux" case like pinctrl-single,bit-per-mux?
> > 
> > And then in the pinctrl-single probe we set params = 3 if
> > pinctrl-single,bit-per-mux is specified. And if no
> > pinctrl-single,bit-per-mux is specified then set params = 2.
> > 
> > That way pcs_parse_one_pinctrl_entry() can just test for params.
> > 
> > Sorry I don't have a better name in mind than bit-per-mux..
> 
> I'm not sure if this would make things more prone to error or make the DTS or
> the code more clean in any ways.
> 
> Both pinctrl-single,pins and pinctrl-single,bits works on top of the same
> pinctrl-single area.
> In most cases we are going to use pinctrl-single,pins for the mux
> configuration and only in few places we need to fall back to pinctrl-single,bits.

What about hardware that has tons of one-bit-per-mux type of registers?
Then we end up trying to find the non-existing property potentially
hundreds of times as the pinctrl-single,pins is never specified.
 
> With this patch we will have maximum of two query to find out which type is
> used, while if we add the 'bit-per-mux' property we will always have need to
> have two of_get_property() call to figure out what is used.
> IMHO in this way we could also have copy-paste errors coming at us when adding
> the mux configurations for the pins and at the end we also need to do same
> safety/sanity checks if the user really provided the correct type with
> correlation to the 'bit-per-mux'.

Well I think we should specify the binding for the type of the controller,
not mix different types of bindings for the same controller. So we should
probably have something just like address-cells, gpio-cells and
interrupt-cells, although in this case it would be specific to pinctrl-single
only and not be called cells.

> This would just complicate the code.
> The existence of pinctrl-single,pins or pinctrl-single,bits property alone
> gives enough information for the number of parameters.

Yes but we don't want to allow mixing different kind of registers within
the same controller, they should be specified as separate controllers.

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/