RE: Pinmux bindings proposal
From: Stephen Warren
Date: Tue Jan 17 2012 - 14:21:53 EST
Shawn Guo wrote at Tuesday, January 17, 2012 1:24 AM:
> On Mon, Jan 16, 2012 at 12:50:02PM +0000, Dong Aisheng-B29396 wrote:
...
> > > * 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.
I responded directly to Dong's email re: how to map the DT values into
something for pinctrl. The mapping being described here isn't quite
what I had in mind....
> 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:
I don't think there's a need for another level of indirection. The 1:n
model I was talking about already handles this, I believe. See below.
> 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>;
> };
Those 3 nodes make sense to me.
> 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>;
> };
I think we can avoid those 2 nodes.
> sdhci@c8000200 {
> ...
> pinctrl = <&pinctrl_sdhci_active> <&pinctrl_sdhci_suspend>;
> pinctrl-names = "active", "suspend";
> };
And rewrite that node as:
sdhci@c8000200 {
...
pinctrl = <&pinmux_sdhci> <&pinconf_sdhci_active>
<&pinmux_sdhci> <&pinconf_sdhci_suspend>;
pinctrl-names = "active", "active", "suspend", "suspend";
};
The only slight disadvantage here is that the person constructing the
pinctrl/pinctrl-names properties needs to know to explicitly list both
the separate mux/config phandles for each value in pinctrl-names. Still,
this seems a reasonable compromise; the user is still picking from a
bunch of pre-defined/canned nodes, they simply need to list 2 (or n in
general) of them for each state.
> 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.
Yes, I definitely agree there's a need for this.
As an aside, I wonder if the following would be any better:
sdhci@c8000200 {
...
pinctrl = <&pinmux_sdhci> <&pinconf_sdhci_active>
<&pinmux_sdhci> <&pinconf_sdhci_suspend>;
/* Number of entries in pinctrl for each in pinctrl-names */
pinctrl-entries = <2 2>;
pinctrl-names = "active", "suspend";
};
That seems more complex though.
--
nvpublic
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/