Pinmux bindings proposal
Stephen Warren
that I like well enough, and is pretty similar to the binding that Dong
posted recently. I think it'll work for both Tegra's and IMX's needs.
Please take a look!
Note: I've used named constants below just to make this easier to read.
We still don't have a solution to actually use named constants in dtc yet.
tegra20.dtsi:
/ {
tegra_pmx: pinmux@70000000 {
compatible = "nvidia,tegra20-pinmux";
reg = <0x70000014 0x10 /* Tri-state registers */
0x70000080 0x20 /* Mux registers */
0x700000a0 0x14 /* Pull-up/down registers */
0x70000868 0xa8>; /* Pad control registers */
};
sdhci@c8000200 {
compatible = "nvidia,tegra20-sdhci";
reg = <0xc8000200 0x200>;
interrupts = <0 15 0x04>;
};
};
tegra-harmony.dts:
/{
sdhci@c8000200 {
cd-gpios = <&gpio 69 0>; /* gpio PI5 */
wp-gpios = <&gpio 57 0>; /* gpio PH1 */
power-gpios = <&gpio 155 0>; /* gpio PT3 */
/*
* A list of named configurations that this device needs.
* Format is a list of <"name" &phandle_of_pmx_configuration>
*
* Multiple "name"s are needed e.g. to support active/suspend,
* or whatever device-defined states are appropriate. The
* device defines which names are needed, just like a device
* defines which regulators, clocks, GPIOs, interrupts, ...
* it needs.
*
* This example shows a 1:1 relation between name and phandle.
* We might want a 1:n relation, so that we can blend multiple
* pre-defined sets of data together, e.g. one pre-defined set
* for the pin mux configuration, another for the pin config
* settings, both being put into the single "default" setting
* for this one device.
*
* A pinmux controller can contain this property too, to
* define "hogged" or "system" pin mux configuration.
*
* Note: Mixing strings and integers in a property seems
* unusual. However, I have seen other bindings floating
* around that are starting to do this...
*/
pinmux =
<"default" &pmx_sdhci_active>
<"suspend" &pmx_sdhci_suspend>;
/* 1:n example: */
pinmux =
<"default" &pmx_sdhci_mux_a>
<"default" &pmx_sdhci_pincfg_a>
<"suspend" &pmx_sdhci_mux_a>
<"suspend" &pmx_sdhci_pincfg_a_suspend>;
/*
* Alternative: One property for each required state. But,
* how does pinctrl core know which properties to parse?
* Every property named "pinctrl*" seems a little too far-
* reaching. Perhaps if we used vendor-name "pinmux", that'd
* be OK, i.e. pinmux,default and pinmux,suspend?
*/
pinmux = <&pmx_sdhci_active>;
pinmux-suspend <&pmx_sdhci_suspend>;
/* 1:n example: */
pinmux = <&pmx_sdhci_mux_a &pmx_sdhci_pincfg_a>
pinmux-suspend = <&pmx_sdhci_mux_a &pmx_sdhci_pincfg_a_suspend>;
/*
* 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?
* 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>;
/*
* 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:
TEGRA_PMX_PG_DTA
TEGRA_PMX_PG_DTD
Each individual pinmux driver's bindings needs to define what each integer
ID represents.
Integer IDs for the "mux functions". Note that these are the raw values
written into hardware, not any driver-defined abstraction, and not any
kind of "virtual group" that's been invented to make OEMs life easier:
TEGRA_PMX_MUX_0
TEGRA_PMX_MUX_1
...
TEGRA_PMX_MUX_3 (for Tegra, 7 for IMX)
Since these are the raw IDs that go into HW, there's no need to specify
each ID's meanings in the binding.
Integer IDs for "pin config parameters":
TEGRA_PMX_CONF_TRISTATE
TEGRA_PMX_CONF_DRIVE_STRENGTH
TEGRA_PMX_CONF_SLEW_RATE
Each individual pinmux driver's bindings needs to define what each integer
ID represents, and what the legal "value"s are for each one.
Advantages:
* Provides for both mux settings and "pin configuration".
* Allows the "pinmux configuration" nodes to be part of the SoC .dtsi
file if desired to provide pre-defined pinmux configurations to
choose from.
* Allows the "pinmux configuration" nodes to be part of the per-device
node if you don't want to use pre-defined configurations.
* When pre-defined configurations are present, if you need something
custom, you can do it easily.
* Can augment pre-defined configurations by listing n nodes for each
"name"d pinmux configuration, e.g. to add one extra pin config
value.
* Parsing is still quite simple:
1) Parse "pinmux" property in device node to get phandle.
2) Parse "mux" property in the node reference by the phandle,
splitting into a list of pmx phandle, entity, mux func.
3) For each entry, pass entity, function to the appropriate mux
driver. (For U-Boot, this might mean check that the phandle
points at the expected place, and ignore the entry if not?)
4) Mux driver simply converts "muxable entity" to the register
address, write the "function" value straight to the register.
Disadvantages:
* If you're not using pre-defined configurations, you still have to dump
all the pinmux configuration into a sub-node of the device node, and
have a property point at it using a phandle. This is slightly more
complex than simply putting the mux/config properties right into the
device node. However, it additionally allows one to have multiple
"name"d configurations (e.g. for suspend) very easily, and isn't overly
complex, so I can live with this.
Changes to pinctrl subsystem:
Very little, I think:
* Need to pass raw function IDs through to the driver instead of the driver
defining some logical table. Actually, this is just a change to individual
drivers, since they can define the functions "func0", "func1", ... "funcn"
as I mentioned before.
* Need to add the code to actually parse this of course.
One additional thought: If dtc does grow named constants, we can provide
HW-function-based names for the mux functions, e.g.:
TEGRA_PMX_DTA_RSVD0 0
TEGRA_PMX_DTA_SDIO2 1
TEGRA_PMX_DTA_VI 2
TEGRA_PMX_DTA_RSVD3 3
TEGRA_PMX_DTF_I2C3 0
TEGRA_PMX_DTF_RSVD1 1
TEGRA_PMX_DTF_VI 2
TEGRA_PMX_DTF_RSVD3 3
...
That'd allow the .dts to include functionality-based named for the mux
function selection, but the .dtb to still include the raw HW mux field
values. And this is something completely within the control of the SoC
.dtsi file; if you don't like it, you don't have to do it.
--
nvpublic
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Shawn Guo
> I thought a bit more about pinmux DT bindings. I came up with something
> that I like well enough, and is pretty similar to the binding that Dong
> posted recently. I think it'll work for both Tegra's and IMX's needs.
> Please take a look!
>
agreement on the binding. I have a few comments below. Other than
those, this looks like a pretty sensible pinctrl DT binding to me.
I prefer to have the property named 'pinctrl' than 'pinmux'.
> <"default" &pmx_sdhci_active>
> <"suspend" &pmx_sdhci_suspend>;
>
I would rather do something like what clock DT binding proposal is
doing.
pinctrl = <&pmx_sdhci_active>, <&pmx_sdhci_suspend>;
pinctrl-names = "default", "suspend";
> /* 1:n example: */
> pinmux =
> <"default" &pmx_sdhci_mux_a>
> <"default" &pmx_sdhci_pincfg_a>
> <"suspend" &pmx_sdhci_mux_a>
> <"suspend" &pmx_sdhci_pincfg_a_suspend>;
>
I personally do not like the 1:n binding. To me, any particular pinctrl
configuration, e.g. pmx_sdhci_active, should consist of a pair of pinmux
and pinconf, which should be given by the pmx_sdhci_active node.
I prefer to just put such nodes inside pinctrl controller itself and
drop those phandles.
> * 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>;
> /*
> * Pin configuration settings. Optional.
> *
I guess pinconf can be optional because some pin/group that have pinmux
setting do not necessarily have pinconf setting? If that's case,
yes, agreed.
Agreed on these. But we really need to push named constants support
for dtc, otherwise the binding is so difficult for engineering. (We
have a lot of pinconfig parameters on imx)
Regards,
Shawn
Dong Aisheng-B29396
> From: Stephen Warren [mailto:swa...@nvidia.com]
> Sent: Saturday, January 14, 2012 4:40 AM
> To: Dong Aisheng-B29396; linus....@stericsson.com; s.h...@pengutronix.de;
> rob.h...@calxeda.com; ker...@pengutronix.de; c...@laptop.org; Simon Glass
> (s...@chromium.org); Dong Aisheng
> Cc: linux-...@vger.kernel.org; linux-ar...@lists.infradead.org;
> devicetre...@lists.ozlabs.org
> Subject: Pinmux bindings proposal
> Importance: High
>
> I thought a bit more about pinmux DT bindings. I came up with something that I
> like well enough, and is pretty similar to the binding that Dong posted recently.
> I think it'll work for both Tegra's and IMX's needs.
> Please take a look!
>
A few comments:
It we support whatever device-defined states, it's meaningless to use one property
For each required state since pinctrl core does not know the property name the
customer defined.
> */
> pinmux = <&pmx_sdhci_active>;
> pinmux-suspend <&pmx_sdhci_suspend>;
>
> /* 1:n example: */
> pinmux = <&pmx_sdhci_mux_a &pmx_sdhci_pincfg_a>
This looks ok to me since the mux and pincfg usually is 1 to 1.
And if we do not have a pincfg for a pinmux we can find a way to tell the pinctl
core, maybe just set pincfg phandle to 0.
> pinmux-suspend = <&pmx_sdhci_mux_a &pmx_sdhci_pincfg_a_suspend>;
>
> /*
> * 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?
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?
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.
> 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?
I don't think the sub-node of the device node is a good place to define
Custom configurations.
Putting those things into device node will make its size big and also not look good.
Since it uses phandle, why not put it under pinctrl device node in board dts file?
It may also work.
Grant Likely
Yeah, don't do this. Mixing phandle, string and cell values in a
property gets messy and could become troublesome to parse. I've
backed away from it in the clk binding.
pinumx-* is better, but I'm not thrilled with it and I avoided that
pattern too for the latest iteration of the clk binding. I prefer
using a "pinmux" + "pinmux-names" pair of properties when dealing with
an array of like objects (ie. reg, interrupts, clks, etc), but that
might not fit well since each setting has multiple state nodes.
>
> /*
> * Alternative: One property for each required state. But,
> * how does pinctrl core know which properties to parse?
> * Every property named "pinctrl*" seems a little too far-
> * reaching. Perhaps if we used vendor-name "pinmux", that'd
> * be OK, i.e. pinmux,default and pinmux,suspend?
It isn't actually a vendor name, so don't use a ','. "pinmux-" prefix
is fine.
g.
Shawn Guo
...
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@c8000200 {
...
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.
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@02190000 { /* 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
Dong Aisheng-B29396
> From: Shawn Guo [mailto:shaw...@linaro.org]
> Sent: Tuesday, January 17, 2012 4:24 PM
> To: Dong Aisheng-B29396
> Cc: Stephen Warren; linus....@stericsson.com; s.h...@pengutronix.de;
> rob.h...@calxeda.com; ker...@pengutronix.de; c...@laptop.org; Simon Glass
> (s...@chromium.org); Dong Aisheng; linux-...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; devicetre...@lists.ozlabs.org
> Subject: Re: Pinmux bindings proposal
> Importance: High
>
From the pinctrl driver since Tagre prefers to define those things in driver
Rather than in board file or soc.dts file.
But it does not fit for IMX since we define it in soc.dts.
> For above example, the function name can be picked from sdhci device node
> pinctr-names property I proposed,
If I understand correctly, the pinctrl-names property you proposed represents
The pin group state.
> and the group name can just be
> 'pmx_sdhci_active', which is not a very nice name here and reminds me the
> following point.
>
No, I don't think it's suitable for group name since pmx_sdhci_active is not a
group node (actually it includes many groups).
> Considering the different pinctrl configurations for the same client device
> usually share the same pinmux and only pinconf varies.
I have the same doubts before.
Is there a real case that device has the different pinmux in different state?
Stephen?
> 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>;
> };
>
The config makes sense to me.
The only question is how to get group name to match with the predefined groups.
Besides per pin group configuration support, we may also want per pin configuration
Support as the latest patch sent by Linus.
http://www.spinics.net/lists/arm-kernel/msg155712.html
Yes, I agree.
And in this way we're still using virtual groups.
It has no big difference as we did before like:
pinmux-groups {
uart4grp: group@0 {
grp-name = "uart4grp";
grp-pins = <107 108>;
grp-mux = <4 4>;
};
sd4grp: group@1 {
grp-name = "sd4grp";
grp-pins = <170 171 180 181 182 183 184 185 186 187>;
grp-mux = <0 0 1 1 1 1 1 1 1 1>;
};
};
The real problem is do we need to support individual pin mux
Or still using virtual pin group?
For the way Stephen proposed, we can only support individual pin mux
Since IMX pins are not grouped together in HW.
Yes, It's still under development by Linus.
The last patch Linus sent still does not support state change for specific device.
But the method is ok to me.
> > > 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.
>
I'm also care about the potential consistent issue.
Regards
Dong Aisheng
Shawn Guo
...
> From the pinctrl driver since Tagre prefers to define those things in driver
> Rather than in board file or soc.dts file.
> But it does not fit for IMX since we define it in soc.dts.
>
Tegra, I assume he plans to move those from Tegra pinctrl driver into
device tree. Stephen?
...
I still incline to say there should not be any predefined groups for
DT case, if we choose to find the group in use only when pinmux_get()
gets called. Even if you insist to have a global scanning on device
tree for all groups to create the pinmux mapping table, those predefined
groups can only be created by your scanning code, so you should know
how the name comes from device tree, and you should be able to recreate
the name when you are trying to matching the name in the table you
created before.
> Besides per pin group configuration support, we may also want per pin configuration
> Support as the latest patch sent by Linus.
> http://www.spinics.net/lists/arm-kernel/msg155712.html
>
This binding proposal has covered both pin and group configuration,
as the second parameter of 'config' property could be either a pin
or group which depends the on whether the configurable entity at HW
level is a pin or group.
...
> > > 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>
> > ...
> > };
> >
> Yes, I agree.
> And in this way we're still using virtual groups.
> It has no big difference as we did before like:
> pinmux-groups {
> uart4grp: group@0 {
> grp-name = "uart4grp";
> grp-pins = <107 108>;
> grp-mux = <4 4>;
> };
>
> sd4grp: group@1 {
> grp-name = "sd4grp";
> grp-pins = <170 171 180 181 182 183 184 185 186 187>;
> grp-mux = <0 0 1 1 1 1 1 1 1 1>;
> };
> };
>
You are right. They are fundamentally same and just in different
format.
> The real problem is do we need to support individual pin mux
> Or still using virtual pin group?
> For the way Stephen proposed, we can only support individual pin mux
> Since IMX pins are not grouped together in HW.
>
I do not see any problem here. If you look at the first column of
'mux' property of node pinmux-usdhc1, it is a group of pins for usdhc1.
Isn't it one virtual pin group for usdhc1?
Yes, we need per device state switch.
--
Regards,
Shawn
Stephen Warren
> On Fri, Jan 13, 2012 at 12:39:42PM -0800, Stephen Warren wrote:
> > I thought a bit more about pinmux DT bindings. I came up with something
> > that I like well enough, and is pretty similar to the binding that Dong
> > posted recently. I think it'll work for both Tegra's and IMX's needs.
> > Please take a look!
Yes. I think I worried that "pinctrl" was a little too tied to the Linux
driver so didn't name it that, but you're right that "pinmux" doesn't
encompass everything this binding covers, so "pinctrl" does seem to be
the right choice.
> > <"default" &pmx_sdhci_active>
> > <"suspend" &pmx_sdhci_suspend>;
>
> I would rather do something like what clock DT binding proposal is
> doing.
>
> pinctrl = <&pmx_sdhci_active>, <&pmx_sdhci_suspend>;
> pinctrl-names = "default", "suspend";
Yes, that's a good idea.
I'm afraid I haven't been following the clock binding discussions at
all, but just started looking at them right after I wrote the email you
were responding to.
> > /* 1:n example: */
> > pinmux =
> > <"default" &pmx_sdhci_mux_a>
> > <"default" &pmx_sdhci_pincfg_a>
> > <"suspend" &pmx_sdhci_mux_a>
> > <"suspend" &pmx_sdhci_pincfg_a_suspend>;
>
> I personally do not like the 1:n binding. To me, any particular pinctrl
> configuration, e.g. pmx_sdhci_active, should consist of a pair of pinmux
> and pinconf, which should be given by the pmx_sdhci_active node.
Just a further explanation on my original code: I always intended that
each entry in that list was a full pinmux configuration that could include
mux and pin configuration settings. Thus, the above is more like:
pinctrl =
<"default" &pmx_sdhci_a>
<"default" &pmx_sdhci_overrides>
(overrides rather than explicit separate mux/config; the separate mux
And config were just an example use case).
My thoughts here:
With this binding, we can certainly define a lot of pre-defined/canned
configurations to a set of pins. However, there are so many pin config
options (at least on Tegra) for different aspects of drive strength, slew
rate, ... that I sincerely doubt every single board is going to be able
to use one of those pre-defined/canned *exactly* without changes. The ways
to cope with these small board-specific differences are:
a) Cut/paste the entire pre-defined/canned configuration and tweak it
as necessary. You can do this with the 1:1 model.
b) Use the pre-defined/canned as a base, then modify it to add extra
configuration options, or change existing configuration options, as
appropriate. I think the 1:n model works best for this. Given previous
comments, I'd now propose the following syntax for a 1:n model:
pinctrl = <&pmx_sdhci_a>, <&pmx_sdhci_overrides>, <&pmx_sdhci_suspend>;
pinctrl-names = "default", "default", "suspend";
This should be easy to implement; instead of roughly:
prop = get_prop(np, "pinctrl-names");
index = find_index(prop, "default");
handle_pinctrl_prop(np, "pinctrl", index);
something more like:
prop = get_prop(np, "pinctrl-names");
prev = NULL;
while (find_index(prop, "default", &prev))
handle_pinctrl_prop(np, "pinctrl", index);
...
My rationale here:
Forcing those nodes to be inside the controller node forces us to store
any board-specific custom configurations or overrides in the controller
node too; I'd simply prefer the flexibility to put them anywhere.
Equally, I want SoC vendors to be able to choose whether to use these
pre-defined/canned configuration nodes at all. If you do use them, putting
them into the controller node makes perfect sense. If you don't use them,
putting the pin configuration nodes into the individual device nodes (in
the board.dts file) makes much more sense.
Having each property start with the phandle of the relevant controller
is also far more consistent with the way all the GPIO, IRQ, ... bindings
work.
> > * 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>;
> > /*
> > * Pin configuration settings. Optional.
> > *
>
> I guess pinconf can be optional because some pin/group that have pinmux
> setting do not necessarily have pinconf setting? If that's case,
> yes, agreed.
Yes, that's true.
Also, if you're using the 1:n model I mentioned above, your "base" node
may well have both, but your "override" node may well only need to add
additional pinmux settings, or tweak just a single pin control setting,
so forcing someone to specify both wouldn't work.
Still, I suppose we could just force both properties to be present, but
just write an empty list if one type of setting wasn't needed; at least
this would allow for some amount of error-checking of the node content.
...
> > Integer IDs for "muxable entities": Pins on IMX, pin groups on Tegra:
> >
> > TEGRA_PMX_PG_DTA
> > TEGRA_PMX_PG_DTD
> >
> > Each individual pinmux driver's bindings needs to define what each integer
> > ID represents.
...
>
> Agreed on these. But we really need to push named constants support
> for dtc, otherwise the binding is so difficult for engineering. (We
> have a lot of pinconfig parameters on imx)
Yes, there is a discussion about this going on.
(Well, it started a good few years back and had a hiatus. There are a lot
of long-term syntax issues to plan out before we can get the basic support
in though, to make sure we don't screw our future selves.)
--
nvpublic
Stephen Warren
> Stephen Warren wrote at Saturday, January 14, 2012 4:40 AM:
> > I thought a bit more about pinmux DT bindings. I came up with something that I
> > like well enough, and is pretty similar to the binding that Dong posted recently.
> > I think it'll work for both Tegra's and IMX's needs.
> > Please take a look!
> > pinmux =
> > <"default" &pmx_sdhci_active>
> > <"suspend" &pmx_sdhci_suspend>;
> > /*
> > * Alternative: One property for each required state. But,
> > * how does pinctrl core know which properties to parse?
> > * Every property named "pinctrl*" seems a little too far-
> > * reaching. Perhaps if we used vendor-name "pinmux", that'd
> > * be OK, i.e. pinmux,default and pinmux,suspend?
>
> It we support whatever device-defined states, it's meaningless to use one property
> For each required state since pinctrl core does not know the property name the
> customer defined.
Yes, that's the problem. Shawn proposed copying the clock bindings which
I think solves all the issues; see my previous email.
> > */
> > pinmux = <&pmx_sdhci_active>;
> > pinmux-suspend <&pmx_sdhci_suspend>;
> >
> > /* 1:n example: */
> > pinmux = <&pmx_sdhci_mux_a &pmx_sdhci_pincfg_a>
>
> This looks ok to me since the mux and pincfg usually is 1 to 1.
> And if we do not have a pincfg for a pinmux we can find a way to tell the pinctl
> core, maybe just set pincfg phandle to 0.
Just to be clear, I'll repeat part of my previous response to Shawn:
* Just a further explanation on my original code: I always intended that
* each entry in that list was a full pinmux configuration that could include
* mux and pin configuration settings. Thus, the above is more like:
*
* pinctrl =
* <"default" &pmx_sdhci_a>
* <"default" &pmx_sdhci_overrides>
*
* (overrides rather than explicit separate mux/config; the separate mux
* And config were just an example use case).
So that was a list where the /examples/ had two entries, one for mux and
one for pin config. In general, there could be 1, 2, 3, ... entries and
each could define whatever mux and config entries they wanted.
> > pinmux-suspend = <&pmx_sdhci_mux_a &pmx_sdhci_pincfg_a_suspend>;
> >
> > /*
> > * 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?
I was thinking that each pin mux configuration node MUST specify at
least some mux settings. However, that may not make sense; it may be
reasonable to specify just pin config settings and no mux settings
(in a 1:n model in the "override" node).
a) pinctrl device: We can extract this from pmx_controller_phandle;
simply search for a device with the same OF node, and retrieve that
registered device from the pinctrl subsystem. This is how GPIO and IRQ
work.
b) function name: The pinmux_ops for the driver of pmx_controller_phandle
needs a function to convert integer IDs such as TEGRA_PMX_MUX_1 to whatever
function IDs are used by the pinctrl subsystem.
c) group name: This should be handled just like (b).
Also you'll need to know:
struct pinmux_map's .name field. This is the value in the pinctrl-names
property, assuming we're switching to the following syntax:
pinctrl = <&pmx_sdhci_a_active>, <&pmx_sdhci_a_suspend>;
pinctrl-names = "default", "suspend";
> > /*
By "predefined", do you mean:
a) Does the DT need to list all the functions and groups.
The answer here is no in general. However, if you want to parameterize
your pinctrl driver's list of known functions and groups, then you can
certainly do this; the pinctrl binding simple doesn't force you to do
this.
b) Does the pinctrl driver need to list all functions and groups to the
pinctrl core?
As the pinctrl subsystem APIs stand right now, the answer here is yes.
I don't think there's really any way around this, although depending on
HW, you can do things like:
1) Instead of defining logical functions (SDHCI1, I2C1, ...) you could
simply define a function for each mux register value (func0, func1, ...)
2) On IMX, you'd define a pin group for each individual pin, since the
HW muxing is at a pin not a group level.
> > 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. As Shawn mentions later, perhaps we can find some way to define
these values in one place, and generate both a .dtsi file and a .h file
from that single list, or generate a .h file from the .dtsi file or vice-
versa.
Both you and Shawn have said you don't like allowing the pin configurations
to be sub-nodes of the devices. Can you expand a little more on that? I
mentioned some of my reasons for allowing this in my previous email, so
see that for reference. I guess I have a couple more points to make:
1) It's optional. If everything you need can be represented in pre-
defined/canned pin configuration nodes under the pin controller's node,
you can certainly do that.
2) Putting this in the device's node seems more consistent with existing
bindings, which put all configuration inside the device node: The IRQ
binding places interrupt IDs and configuration (level/edge, high/low)
in the device node. The GPIO binding places GPIO IDs and possible flags
(e.g. inverted/not) in the device node. Etc.
3) I don't think there will be any .dtb size difference at all between
putting the pin configuration in the pin controller node or the device
node; the content of the sub-node will be entirely identical. The same
holds true for the .dts files (when you consider soc.dtsi and board.dts
together).
Stephen Warren
> On Mon, Jan 16, 2012 at 12:50:02PM +0000, Dong Aisheng-B29396 wrote:
...
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
Stephen Warren
> Shawn Guo wrote at Tuesday, January 17, 2012 4:24 PM:
> > On Mon, Jan 16, 2012 at 12:50:02PM +0000, Dong Aisheng-B29396 wrote:
> > ...
> > > > 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.
>
> I guess Stephen's idea is to retrieving the function name and group name
> From the pinctrl driver since Tagre prefers to define those things in driver
> Rather than in board file or soc.dts file.
> But it does not fit for IMX since we define it in soc.dts.
You can still get the data from the driver, even if the driver got the
data from the DT instead of static tables.
> > For above example, the function name can be picked from sdhci device node
> > pinctr-names property I proposed,
>
> If I understand correctly, the pinctrl-names property you proposed represents
> The pin group state.
Yes, that's my understanding.
...
> And in this way we're still using virtual groups.
> It has no big difference as we did before like:
> pinmux-groups {
> uart4grp: group@0 {
> grp-name = "uart4grp";
> grp-pins = <107 108>;
> grp-mux = <4 4>;
> };
>
> sd4grp: group@1 {
> grp-name = "sd4grp";
> grp-pins = <170 171 180 181 182 183 184 185 186 187>;
> grp-mux = <0 0 1 1 1 1 1 1 1 1>;
> };
> };
I'd prefer not to call these virtual groups, since "group" already has
a meaning to the pinctrl subsystem that is somewhat different. As you've
probably noticed, I've been using the term "pre-defined/canned pin
configuration"!
> The real problem is do we need to support individual pin mux
> Or still using virtual pin group?
>
> For the way Stephen proposed, we can only support individual pin mux
> Since IMX pins are not grouped together in HW.
I think the "mux" and "config" properties I had in my proposal should
deal purely with raw HW entities; nothing virtual/pre-defined/canned/...
You can get what you're calling "virtual groups" simply by defining a
bunch of pinmux configuration nodes in the pin controller's device node
and exclusively referencing those in the per-device pinctrl properties.
--
nvpublic
Stephen Warren
> On Tue, Jan 17, 2012 at 09:46:42AM +0000, Dong Aisheng-B29396 wrote:
> ...
> > I guess Stephen's idea is to retrieving the function name and group name
> > From the pinctrl driver since Tagre prefers to define those things in driver
> > Rather than in board file or soc.dts file.
> > But it does not fit for IMX since we define it in soc.dts.
>
> Hmm, when he came up with this proposal and said it should work for
> Tegra, I assume he plans to move those from Tegra pinctrl driver into
> device tree. Stephen?
I plan to have the following:
tegra20.dtsi:
A regular device node for the pin controller, listing just the basic
device tree properties: "compatible" and "regs".
${board}.dts:
Each device node lists its pin mux/config setup using the bindings we
decide upon.
drivers/pinctrl/pinctrl-tegra20.c:
Static tables listing all pins, pin groups, mux functions, and pin
configuration options that Tegra supports.
--
nvpublic
Shawn Guo
> Shawn Guo wrote at Saturday, January 14, 2012 12:09 AM:
> > On Fri, Jan 13, 2012 at 12:39:42PM -0800, Stephen Warren wrote:
Hmm, this type of flexibility does not make too much point to me. On
the contrary, it's good to have a centralized place for these nodes,
so that they can be well organized and people can find them easily.
> Equally, I want SoC vendors to be able to choose whether to use these
> pre-defined/canned configuration nodes at all. If you do use them, putting
> them into the controller node makes perfect sense. If you don't use them,
> putting the pin configuration nodes into the individual device nodes (in
> the board.dts file) makes much more sense.
>
Having 'pinctrl' phandle point to a configuration node which is in the
same device node looks odd to me. I'd rather define these configuration
nodes in <board>.dts still under controller node.
> Having each property start with the phandle of the relevant controller
> is also far more consistent with the way all the GPIO, IRQ, ... bindings
> work.
>
The GPIO and IRQ gets only one level phandle reference from device node
to the destination, while you are proposing two levels of phandle
reference for pinctrl. Having 'pinctrl' phandle point to the
configuration node which sits under controller node seems well aligned
with GPIO and IRQ etc to me.
--
Regards,
Shawn
Dong Aisheng-B29396
> > > 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>;
> > > };
> > >
> > The config makes sense to me.
> > The only question is how to get group name to match with the predefined groups.
> >
> I still incline to say there should not be any predefined groups for DT case, if
> we choose to find the group in use only when pinmux_get() gets called.
1) Inconsistent issue
sysfs pinmux map entry has different meaning
2) need to change the pinmux_maps from arrays to list for better to dynamically
Insert the new created pinmux map detected.
But this does affect the non-dt case a lot.
> Even if
> you insist to have a global scanning on device tree for all groups to create the
> pinmux mapping table, those predefined groups can only be created by your
> scanning code,
The predefined groups and functions are in soc.dts file and will be parsed
Independently with the pinmux map table creation.
> so you should know how the name comes from device tree, and you
> should be able to recreate the name when you are trying to matching the name in
> the table you created before.
>
> > Besides per pin group configuration support, we may also want per pin
> > configuration Support as the latest patch sent by Linus.
> > http://www.spinics.net/lists/arm-kernel/msg155712.html
> >
> This binding proposal has covered both pin and group configuration, as the
> second parameter of 'config' property could be either a pin or group which
> depends the on whether the configurable entity at HW level is a pin or group.
>
The pinctrl driver does not know if it's group or pin.
We may find a way to identify it.
If we treat the whole pins in 'mux' property as a group,
There may be potential inconsistent issue since Tegra will treat each
One in the list of 'mux' as a group.
And it would be a problem for pinctrl core to parse it in a standard way.
Regards
Dong Aisheng
Shawn Guo
> Shawn Guo wrote at Tuesday, January 17, 2012 1:24 AM:
...
> > 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.
>
...
> > 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:
>
It does look better to me.
Regards,
Shawn
> 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.
>
--
Shawn Guo
...
> > > using virtual pin group?
> > > For the way Stephen proposed, we can only support individual pin mux
> > > Since IMX pins are not grouped together in HW.
> > >
> > I do not see any problem here. If you look at the first column of 'mux'
> > property of node pinmux-usdhc1, it is a group of pins for usdhc1.
> > Isn't it one virtual pin group for usdhc1?
> >
> If we treat the whole pins in 'mux' property as a group,
> There may be potential inconsistent issue since Tegra will treat each
> One in the list of 'mux' as a group.
> And it would be a problem for pinctrl core to parse it in a standard way.
>
a group of pin/group defined by hardware, or mixing of the two.
--
Regards,
Shawn