> -----Original Message-----
> From: Stephen Warren [mailto:[email protected]]
....
> > 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.
>
This makes sense to me.
Regards
Dong Aisheng
_______________________________________________
devicetree-discuss mailing list
[email protected]
https://lists.ozlabs.org/listinfo/devicetree-discuss
