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: <20120117082334.GA31295@S2101-09.ap.freescale.net>
Date:	Tue, 17 Jan 2012 16:23:37 +0800
From:	Shawn Guo <shawn.guo@...aro.org>
To:	Dong Aisheng-B29396 <B29396@...escale.com>
Cc:	Stephen Warren <swarren@...dia.com>,
	"linus.walleij@...ricsson.com" <linus.walleij@...ricsson.com>,
	"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>,
	"Simon Glass (sjg@...omium.org)" <sjg@...omium.org>,
	Dong Aisheng <dongas86@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"devicetree-discuss@...ts.ozlabs.org" 
	<devicetree-discuss@...ts.ozlabs.org>
Subject: Re: Pinmux bindings proposal

On Mon, Jan 16, 2012 at 12:50:02PM +0000, Dong Aisheng-B29396 wrote:
...
> >                 /*
> >                  * The actual definition of the complete state of the
> >                  * pinmux as required by some driver.
> >                  *
> >                  * These can be either directly in the device node, or
> >                  * somewhere in tegra20.dtsi in order to provide pre-
> >                  * selected/common configurations. Hence, they're referred
> >                  * to by phandle above.
> >                  */
> >                 pmx_sdhci_active: {
> >                         /*
> >                          * Pin mux settings. Mandatory?
> Mandatory for what?
> 
> >                          * Not mandatory if the 1:1 mentioned above is
> >                          * extended to 1:n.
> >                          *
> >                          * Format is <&pmx_controller_phandle muxable_entity_id
> >                          * selected_function>.
> >                          *
> >                          * The pmx phandle is required since there may be more
> >                          * than one pinmux controller in the system. Even if
> >                          * this node is inside the pinmux controller itself, I
> >                          * think it's simpler to just always have this field
> >                          * present in the binding for consistency.
> >                          *
> >                          * Alternative: Format is <&pmx_controller_phandle
> >                          * pmx_controller_specific_data>. In this case, the
> >                          * pmx controller needs to define #pinmux-mux-cells,
> >                          * and provide the pinctrl core with a mapping
> >                          * function to handle the rest of the data in the
> >                          * property. This is how GPIOs and interrupts work.
> >                          * However, this will probably interact badly with
> >                          * wanting to parse the entire pinmux map early in
> >                          * boot, when perhaps the pinctrl core is initialized,
> >                          * but the pinctrl driver itself is not.
> >                          */
> >                         mux =
> >                                 <&tegra_pmx TEGRA_PMX_PG_DTA TEGRA_PMX_MUX_1>
> >                                 <&tegra_pmx TEGRA_PMX_PG_DTD TEGRA_PMX_MUX_1>
> >                                 /* Syntax example */
> >                                 <&foo_pmx FOO_PMX_PG_X FOO_PMX_MUX_0>;
> I'm still think how do we construct the pinmux map for such binding.
> The format you're using is:
> <&pmx_controller_phandle muxable_entity_id selected_function>
> For contruct pinmux map, we need to know at least 3 things for a device:
> a) pinctrl device b) function name c) group name.
> For a, we can get it from this binding.
> But for b and c, since they are constants, how to convert to name string?
> 
I guess, for function name, it should be retrieved from the client
device node, and for the group name, it should be retrieved from the
node here.

For above example, the function name can be picked from sdhci device
node pinctr-names property I proposed, and the group name can just be
'pmx_sdhci_active', which is not a very nice name here and reminds me
the following point.

Considering the different pinctrl configurations for the same client
device usually share the same pinmux and only pinconf varies.  It may
worth introducing another level phandle reference.  Something like
the following:

	pinmux_sdhci: pinmux-sdhci {
		mux =
			<&tegra_pmx TEGRA_PMX_PG_DTA TEGRA_PMX_MUX_1>
			<&tegra_pmx TEGRA_PMX_PG_DTD TEGRA_PMX_MUX_1>;
	};

	pinconf_sdhci_active: pinconf-sdhci-active {
		config =
			<&tegra_pmx TEGRA_PMX_PG_DTA TEGRA_PMX_CONF_DRIVE_STRENGTH 5>
			<&tegra_pmx TEGRA_PMX_PG_DTD TEGRA_PMX_CONF_DRIVE_STRENGTH 5>
			<&tegra_pmx TEGRA_PMX_PG_DTA TEGRA_PMX_CONF_SLEW_RATE 4>
			<&tegra_pmx TEGRA_PMX_PG_DTD TEGRA_PMX_CONF_SLEW_RATE 8>;
	};

	pinconf_sdhci_suspend: pinconf-sdhci-suspend {
		config =
			<&tegra_pmx TEGRA_PMX_PG_DTA TEGRA_PMX_CONF_TRISTATE 1>
			<&tegra_pmx TEGRA_PMX_PG_DTD TEGRA_PMX_CONF_TRISTATE 1>
			<&tegra_pmx TEGRA_PMX_PG_DTA TEGRA_PMX_CONF_DRIVE_STRENGTH 5>
			<&tegra_pmx TEGRA_PMX_PG_DTD TEGRA_PMX_CONF_DRIVE_STRENGTH 5>
			<&tegra_pmx TEGRA_PMX_PG_DTA TEGRA_PMX_CONF_SLEW_RATE 4>
			<&tegra_pmx TEGRA_PMX_PG_DTD TEGRA_PMX_CONF_SLEW_RATE 8>;
	};

	pinctrl_sdhci_active: pinctrl-sdhci-active {
		pinmux = <&pinmux_sdhci>;
		pinconf = <&pinconf_sdhci_active>;
	};

	pinctrl_sdhci_suspend: pinctrl-sdhci-suspend {
		pinmux = <&pinmux_sdhci>;
		pinconf = <&pinconf_sdhci_suspend>;
	};

	sdhci@...00200 {
		...
		pinctrl = <&pinctrl_sdhci_active> <&pinctrl_sdhci_suspend>;
		pinctrl-names = "active", "suspend";
	};

This will be pretty useful for imx6 usdhc case, which will have 3
pinctrl configuration for each usdhc device (imx6 has 4 usdhc devices),
pinctrl-50mhz, pinctrl-100mhz and pinctrl-200mhz.  All these 3 states
have the exactly same pinmux settings, and only varies on pinconf.

> >                          /*
> >                          * Pin configuration settings. Optional.
> >                          *
> >                          * Format is <&pmx_controller_phandle muxable_entity_id
> >                          * configuration_option configuration_value>.
> >                          */
> >                         config =
> >                                 <&tegra_pmx TEGRA_PMX_PG_DTA
> > TEGRA_PMX_CONF_DRIVE_STRENGTH 5>
> >                                 <&tegra_pmx TEGRA_PMX_PG_DTD
> > TEGRA_PMX_CONF_DRIVE_STRENGTH 5>
> >                                 <&tegra_pmx TEGRA_PMX_PG_DTA
> > TEGRA_PMX_CONF_SLEW_RATE 4>
> >                                 <&tegra_pmx TEGRA_PMX_PG_DTD
> > TEGRA_PMX_CONF_SLEW_RATE 8>;
> >                         /*
> >                          * Perhaps allow additional custom properties here to
> >                          * express things we haven't thought of. The pinctrl
> >                          * drivers would be responsible for parsing them.
> >                          */
> >                 };
> >                 pmx_sdhci_standby: {
> >                         mux =
> >                                 <&tegra_pmx TEGRA_PMX_PG_DTA TEGRA_PMX_MUX_1>
> >                                 <&tegra_pmx TEGRA_PMX_PG_DTD TEGRA_PMX_MUX_1>;
> >                         config =
> >                                 <&tegra_pmx TEGRA_PMX_PG_DTA
> > TEGRA_PMX_CONF_TRISTATE 1>
> >                                 <&tegra_pmx TEGRA_PMX_PG_DTD
> > TEGRA_PMX_CONF_TRISTATE 1>
> >                                 <&tegra_pmx TEGRA_PMX_PG_DTA
> > TEGRA_PMX_CONF_DRIVE_STRENGTH 5>
> >                                 <&tegra_pmx TEGRA_PMX_PG_DTD
> > TEGRA_PMX_CONF_DRIVE_STRENGTH 5>
> >                                 <&tegra_pmx TEGRA_PMX_PG_DTA
> > TEGRA_PMX_CONF_SLEW_RATE 4>
> >                                 <&tegra_pmx TEGRA_PMX_PG_DTD
> > TEGRA_PMX_CONF_SLEW_RATE 8>;
> >                 };
> >         };
> > };
> > 
> > Integer IDs for "muxable entities": Pins on IMX, pin groups on Tegra:
> > 
> If "muxable entities" is pins on IMX, I'm wondering how we define the predefined
> Functions and groups or if we still need to do that.
> 
Let's put this in the example below.

	pinmux_usdhc1: pinmux-usdhc1 {
		mux = <IMX6Q_PAD_SD1_DAT1 0>
		      <IMX6Q_PAD_SD1_DAT2 0>
		      ...
	};

	pinconf_usdhc1_50mhz: pinconf-usdhc1-50mhz {
		config = <IMX6Q_PAD_SD1_DAT1 IMX6Q_PAD_CONF_ALL 0x834>
			 <IMX6Q_PAD_SD1_DAT2 IMX6Q_PAD_CONF_ALL 0x834>
			 ...
	};

	pinconf_usdhc1_100mhz: pinconf-usdhc1-100mhz {
		config = <IMX6Q_PAD_SD1_DAT1 IMX6Q_PAD_CONF_ALL 0x330>
			 <IMX6Q_PAD_SD1_DAT2 IMX6Q_PAD_CONF_ALL 0x330>
			 ...
	};

	pinconf_usdhc1_200mhz: pinconf-usdhc1-200mhz {
		config = <IMX6Q_PAD_SD1_DAT1 IMX6Q_PAD_CONF_ALL 0x334>
			 <IMX6Q_PAD_SD1_DAT2 IMX6Q_PAD_CONF_ALL 0x334>
			 ...
	};

	pinctrl_usdhc1_50mhz: pinctrl-usdhc1-50mhz {
		pinmux = <&pinmux_usdhc1>;
		pinconf = <&pinconf_usdhc1_50mhz>;
	};

	pinctrl_usdhc1_100mhz: pinctrl-usdhc1-100mhz {
		pinmux = <&pinmux_usdhc1>;
		pinconf = <&pinconf_usdhc1_100mhz>;
	};

	pinctrl_usdhc1_200mhz: pinctrl-usdhc1-200mhz {
		pinmux = <&pinmux_usdhc1>;
		pinconf = <&pinconf_usdhc1_200mhz>;
	};

	usdhc@...90000 { /* uSDHC1 */
		...
		pinctrl = <&usdhc1_50mhz>, <&usdhc1_100mhz>, <&usdhc1_200mhz>;
		pinctrl-names = "usdhc1-50mhz", "usdhc1-100mhz", "usdhc1-200mhz";
	};

In this example, we have 3 functions/states for client device usdhc1,
"usdhc1-50mhz", "usdhc1-100mhz", and "usdhc1-200mhz".  The group is
being defined by enumerating the pins in property 'mux' of node
pinmux_usdhc1.

> >     TEGRA_PMX_PG_DTA
> >     TEGRA_PMX_PG_DTD
> > 
> > Each individual pinmux driver's bindings needs to define what each integer ID
> > represents.
> > 
> Does it mean both pinmux driver and soc.dtsi file need define those macros if dtc
> Supports constants?
> 
Yes, I think it does.  But we should try to work out some way letting
dts and linux driver include the same header file to avoid maintaining
two copies of the same data.

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

Powered by Openwall GNU/*/Linux Powered by OpenVZ