Hi Stephen,
I had a talk with Dong about this binding, and we think that it should
work well for imx if we have a couple of small pieces added.
On Fri, Jan 20, 2012 at 02:22:20PM -0800, Stephen Warren wrote:
...
> pmx_sdhci: pinconfig-sdhci {
> /*
> * The mux property is a list of muxable entities
> * and the mux function to select for it. The number
> * of cells in each entry is the pin controller's
> * #pinmux-cells property. The pin controller's
> * binding defines what the cells mean. The pinctrl
> * driver is responsible for mapping this data to
> * the (group, function) pair required to fill in
> * the pinctrl subsystem's pinmux mapping table.
> */
> mux =
> <TEGRA_PMX_PG_DTA TEGRA_PMX_MUX_SDIO1>
> <TEGRA_PMX_PG_DTD TEGRA_PMX_MUX_SDIO1>;
We need a property like 'mux-unit' whose value can be either 'pin' or
'pingroup' to reflect something you mentioned as muxable entity.
The reason behind this is the DT logic inside pinctrl core needs to
know how the pinmux_map should be constructed from device tree. In
tegra case, the 'mux-unit' is 'pingroup', the core should construct
pinmux_map entry for each row/element of 'mux'. In imx case, the
'mux-unit' will be 'pin', and we would expect core construct only
one pinmux_map entry there, with all the pins listed in 'mux' composing
the group that pinmux_map needs.
And in case of 'mux-unit' is 'pin', we would need one more property
'mux-name' to present the group name. Then we have all the pieces to
construct pinmux_map for cases like imx, where we do not define all
those functions, groups in pinctrl driver at all.
Regards,
Shawn
> /*
> * The config property is a list of muxable entities
> * and individual configuration setting. The number
> * of cells in each entry is the pin controller's
> * #pinconfig-cells property. The pin controller's
> * binding defines what the cells mean. The pinctrl
> * driver is responsible for mapping this data to
> * the (group, config) pair required to fill in
> * the pinctrl subsystem's pin configuration table.
> */
> config =
> <TEGRA_PMX_PG_DTA TEGRA_PMX_CONF_DRIVE_STRENGTH 5>
> <TEGRA_PMX_PG_DTD TEGRA_PMX_CONF_DRIVE_STRENGTH 5>
> <TEGRA_PMX_PG_DTA TEGRA_PMX_CONF_SLEW_RATE 4>
> <TEGRA_PMX_PG_DTD TEGRA_PMX_CONF_SLEW_RATE 8>;
> };
Shawn Guo wrote at Wednesday, February 01, 2012 7:36 AM:
...
> I had a talk with Dong about this binding, and we think that it should
> work well for imx if we have a couple of small pieces added.
>
> On Fri, Jan 20, 2012 at 02:22:20PM -0800, Stephen Warren wrote:
> ...
> > pmx_sdhci: pinconfig-sdhci {
> > /*
> > * The mux property is a list of muxable entities
> > * and the mux function to select for it. The number
> > * of cells in each entry is the pin controller's
> > * #pinmux-cells property. The pin controller's
> > * binding defines what the cells mean. The pinctrl
> > * driver is responsible for mapping this data to
> > * the (group, function) pair required to fill in
> > * the pinctrl subsystem's pinmux mapping table.
> > */
> > mux =
> > <TEGRA_PMX_PG_DTA TEGRA_PMX_MUX_SDIO1>
> > <TEGRA_PMX_PG_DTD TEGRA_PMX_MUX_SDIO1>;
>
> We need a property like 'mux-unit' whose value can be either 'pin' or
> 'pingroup' to reflect something you mentioned as muxable entity.
I'm not sure I agree; see below.
> The reason behind this is the DT logic inside pinctrl core needs to
> know how the pinmux_map should be constructed from device tree.
As a general statement, yes.
> In tegra case, the 'mux-unit' is 'pingroup', the core should construct
> pinmux_map entry for each row/element of 'mux'.
Yes.
> In imx case, the 'mux-unit' will be 'pin',
OK.
Just a note: Tegra30 also has per-pin muxability. Only Tegra20 muxes pins
in groups. (although Tegra30 does some if its pin configuration in groups)
> and we would expect core construct only
> one pinmux_map entry there, with all the pins listed in 'mux' composing
> the group that pinmux_map needs.
This is where I disagree.
If the pinmux_map should only contain a single entry, wouldn't the DT
mux property only contain a single entry?
The reason being that if there's a single entry in the pinmux_map, the
group name used in that entry must be a group that's supported directly
by the pinctrl driver (that's just the way pinctrl works). As such, why
not just write the device tree in terms of those groups?
The only way I can see this not being true is if your pinctrl driver is
also parsing these mux properties, and dynamically creating the groups
that it exposes based on the list of pins in the mux property. However,
that seems like the wrong approach; If you're dynamically defining groups
in DT, I'd expect separate explicit driver-specific properties/nodes to
define those groups, such that the pinctrl core's processing of the mux
property to be identical in all cases.
i.e. instead of what your "mux-unit" proposal:
pmx_sdhci: pinconfig-sdhci {
mux =
<TEGRA_PMX_PG_DTA TEGRA_PMX_MUX_SDIO1>
<TEGRA_PMX_PG_DTD TEGRA_PMX_MUX_SDIO1>;
mux-unit = "pingroup";
mux-name = "sdio-config-1";
};
I'd expect something more like:
/* Standardized pinctrl properties */
pmx_sdhci: pinconfig-sdhci {
mux = <IMX_PG_SDIO_CFG_1 IMX_MUX_SDIO1>;
};
/*
* Driver-specific properties which tell the driver which potentially
* board-specific pin-groups to implement.
*/
imx-pingroup-sdio-cfg-1 {
id = <IMX_PG_SDIO_CFG_1>;
pins = <IMX_PIN_SDIO1_CLK IMX_PIN_SDIO1_CMD IMX_PIN_SDIO1_DAT0...>;
};
Does that make sense?
> And in case of 'mux-unit' is 'pin', we would need one more property
> 'mux-name' to present the group name. Then we have all the pieces to
> construct pinmux_map for cases like imx, where we do not define all
> those functions, groups in pinctrl driver at all.
--
nvpublic
On Fri, Feb 3, 2012 at 2:36 AM, Stephen Warren <[email protected]> wrote:
> Shawn Guo wrote at Wednesday, February 01, 2012 7:36 AM:
> ...
>> I had a talk with Dong about this binding, and we think that it should
>> work well for imx if we have a couple of small pieces added.
>>
>> On Fri, Jan 20, 2012 at 02:22:20PM -0800, Stephen Warren wrote:
>> ...
>> > ? ? ? ? ? ? ? ? pmx_sdhci: pinconfig-sdhci {
>> > ? ? ? ? ? ? ? ? ? ? ? ? /*
>> > ? ? ? ? ? ? ? ? ? ? ? ? ?* The mux property is a list of muxable entities
>> > ? ? ? ? ? ? ? ? ? ? ? ? ?* and the mux function to select for it. The number
>> > ? ? ? ? ? ? ? ? ? ? ? ? ?* of cells in each entry is the pin controller's
>> > ? ? ? ? ? ? ? ? ? ? ? ? ?* #pinmux-cells property. The pin controller's
>> > ? ? ? ? ? ? ? ? ? ? ? ? ?* binding defines what the cells mean. The pinctrl
>> > ? ? ? ? ? ? ? ? ? ? ? ? ?* driver is responsible for mapping this data to
>> > ? ? ? ? ? ? ? ? ? ? ? ? ?* the (group, function) pair required to fill in
>> > ? ? ? ? ? ? ? ? ? ? ? ? ?* the pinctrl subsystem's pinmux mapping table.
>> > ? ? ? ? ? ? ? ? ? ? ? ? ?*/
>> > ? ? ? ? ? ? ? ? ? ? ? ? mux =
>> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? <TEGRA_PMX_PG_DTA TEGRA_PMX_MUX_SDIO1>
>> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? <TEGRA_PMX_PG_DTD TEGRA_PMX_MUX_SDIO1>;
>>
>> We need a property like 'mux-unit' whose value can be either 'pin' or
>> 'pingroup' to reflect something you mentioned as muxable entity.
>
First i'd explain more about this way for better discussion.
It could be:
pinctrl_usdhc4: pinconfig-usdhc4 {
/* 0: pin 1: group */
mux-entity = <0>;
func-name = "usdhc4func";
grp-name = "usdhc4grp";
mux =
<MX6Q_SD4_CMD 0>
<MX6Q_SD4_CLK 0>
<MX6Q_SD4_DAT0 1>
<MX6Q_SD4_DAT1 1>
<MX6Q_SD4_DAT2 1>
<MX6Q_SD4_DAT3 1>
<MX6Q_SD4_DAT4 1>
<MX6Q_SD4_DAT5 1>
<MX6Q_SD4_DAT6 1>
<MX6Q_SD4_DAT7 1>;
config =
<MX6Q_SD4_CMD MX6Q_USDHC_PAD_CTRL>
<MX6Q_SD4_CLK MX6Q_USDHC_PAD_CTRL>
<MX6Q_SD4_DAT0 MX6Q_USDHC_PAD_CTRL>
<MX6Q_SD4_DAT1 MX6Q_USDHC_PAD_CTRL>
<MX6Q_SD4_DAT2 MX6Q_USDHC_PAD_CTRL>
<MX6Q_SD4_DAT3 MX6Q_USDHC_PAD_CTRL>
<MX6Q_SD4_DAT4 MX6Q_USDHC_PAD_CTRL>
<MX6Q_SD4_DAT5 MX6Q_USDHC_PAD_CTRL>
<MX6Q_SD4_DAT6 MX6Q_USDHC_PAD_CTRL>
<MX6Q_SD4_DAT7 MX6Q_USDHC_PAD_CTRL>;
};
Actually i think i'd rather do not use config property, then i could
be more compact:
(anyway it's another issue and is flexible to be controlled by #pinmux-cells)
pinctrl_usdhc4: pinconfig-usdhc4 {
/* 0: pin 1: group */
mux-entity = <0>;
func-name = "usdhc4func";
grp-name = "usdhc4grp";
mux =
<MX6Q_SD4_CMD 0 MX6Q_USDHC_PAD_CTRL>
<MX6Q_SD4_CLK 0 MX6Q_USDHC_PAD_CTRL>
<MX6Q_SD4_DAT0 1 MX6Q_USDHC_PAD_CTRL>
<MX6Q_SD4_DAT1 1 MX6Q_USDHC_PAD_CTRL>
<MX6Q_SD4_DAT2 1 MX6Q_USDHC_PAD_CTRL>
<MX6Q_SD4_DAT3 1 MX6Q_USDHC_PAD_CTRL>
<MX6Q_SD4_DAT4 1 MX6Q_USDHC_PAD_CTRL>
<MX6Q_SD4_DAT5 1 MX6Q_USDHC_PAD_CTRL>
<MX6Q_SD4_DAT6 1 MX6Q_USDHC_PAD_CTRL>
<MX6Q_SD4_DAT7 1 MX6Q_USDHC_PAD_CTRL>;
};
The idea is that pinctrl-imx driver will parse all pinmux nodes like
pinctrl_usdhc4 to create the pinctrl functions and groups during
system boot up. For IMX, the mux entity is PIN
then we will treat all PINS in mux property is only on group. The
group name is grp-name##grp and the function name is mux-name##func.
Then we have predefined functions and groups.
When do pinmux map parsing during pinmux_get calling, the pinctrl core
will handle the two different cases for different muxable entity by
checking the mux-entity value.
If entity is PIN, all pins are a group and only one pinmux map
created, if entity is group,
each entry is a group and one map per group.
> I'm not sure I agree; see below.
>
>> The reason behind this is the DT logic inside pinctrl core needs to
>> know how the pinmux_map should be constructed from device tree.
>
> As a general statement, yes.
>
>> In tegra case, the 'mux-unit' is 'pingroup', the core should construct
>> pinmux_map entry for each row/element of 'mux'.
>
> Yes.
>
>> In imx case, the 'mux-unit' will be 'pin',
>
> OK.
>
> Just a note: Tegra30 also has per-pin muxability. Only Tegra20 muxes pins
> in groups. (although Tegra30 does some if its pin configuration in groups)
>
>> and we would expect core construct only
>> one pinmux_map entry there, with all the pins listed in 'mux' composing
>> the group that pinmux_map needs.
>
> This is where I disagree.
>
> If the pinmux_map should only contain a single entry, wouldn't the DT
> mux property only contain a single entry?
>
The reason is that the single entry of IMX is a PIN rather than a
group as tegra.
> The reason being that if there's a single entry in the pinmux_map, the
> group name used in that entry must be a group that's supported directly
> by the pinctrl driver (that's just the way pinctrl works). As such, why
> not just write the device tree in terms of those groups?
>
Because with the format:
<muxable_entity muxed_function>
The muxed_function applies for the whole muxable_entity.
If we use virtual group for muxable_entity here, for imx, we can not
use one muxed_function for whole pins in that group.
That means we need a more complicated data for muxed_function to represent
all function for each pin in that virtual group.
> The only way I can see this not being true is if your pinctrl driver is
> also parsing these mux properties, and dynamically creating the groups
> that it exposes based on the list of pins in the mux property.
Yes.
> However,
> that seems like the wrong approach; If you're dynamically defining groups
> in DT, I'd expect separate explicit driver-specific properties/nodes to
> define those groups, such that the pinctrl core's processing of the mux
> property to be identical in all cases.
>
> i.e. instead of what your "mux-unit" proposal:
>
> ? ?pmx_sdhci: pinconfig-sdhci {
> ? ? ? ?mux =
> ? ? ? ? ? ?<TEGRA_PMX_PG_DTA TEGRA_PMX_MUX_SDIO1>
> ? ? ? ? ? ?<TEGRA_PMX_PG_DTD TEGRA_PMX_MUX_SDIO1>;
> ? ? ? ?mux-unit = "pingroup";
> ? ? ? ?mux-name = "sdio-config-1";
> ? ?};
>
> I'd expect something more like:
>
> ? ?/* Standardized pinctrl properties */
> ? ?pmx_sdhci: pinconfig-sdhci {
> ? ? ? ?mux = <IMX_PG_SDIO_CFG_1 IMX_MUX_SDIO1>;
> ? ?};
>
> ? ?/*
> ? ? * Driver-specific properties which tell the driver which potentially
> ? ? * board-specific pin-groups to implement.
> ? ? */
> ? ?imx-pingroup-sdio-cfg-1 {
> ? ? ? ?id = <IMX_PG_SDIO_CFG_1>;
> ? ? ? ?pins = <IMX_PIN_SDIO1_CLK IMX_PIN_SDIO1_CMD IMX_PIN_SDIO1_DAT0...>;
> ? ?};
>
> Does that make sense?
>
As i explained above, we can not represent a function with one single
data IMX_MUX_SDIO1 for all pins in the group IMX_PG_SDIO_CFG_1.
A similar way i did before is:
imx-pingroup-sdio-cfg-1 {
? ? ?id = <IMX_PG_SDIO_CFG_1>;
? ?pins = <IMX_PIN_SDIO1_CLK mux_1>
<IMX_PIN_SDIO1_CMD mux_2...>;
};
pmx_sdhci: pinconfig-sdhci {
? ?mux = <IMX_PG_SDIO_CFG_1>;
};
And i prefered IMX_PG_SDIO_CFG_1 to be a phandle to imx-pingroup-sdio-cfg-1.
The real case i have done before is that:
soc.dtsi:
iomuxc@020e0000 {
compatible = "fsl,imx6q-iomuxc";
reg = <0x020e0000 0x4000>;
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>;
};
};
pinmux-functions {
uart4func: func@0 {
func-name = "uart4";
groups = <&uart4grp>;
};
sd4func: func@1 {
func-name = "sd4";
groups = <&sd4grp>;
};
};
};
board.dts:
uart3: uart@021f0000 { /* UART4 */
status = "okay";
pinmux = <&sd4grp>;
};
To be more like the way you proposed, it could be:
board.dts:
pmx_usdhc4: pinconfig-usdhc4 {
mux = <&sd4grp>
/* we can add a similar config node for each pin*/
config = <&sd4config>
};
uart3: uart@021f0000 { /* UART4 */
status = "okay";
pinmux = <&pmx_usdhc4>;
};
This is a working way and i have discussed it with Shawn before.
However, since these things are a little pinctrl core specific,
so more or less we're a little more like the data model you proposed
for Tegra since it represents hw better.
And the pinmux map parsing is done in pinctrl core, we also want to
align the binding as tegra.
That why we consider keep using your proposed data model and find a
new way(introduce mux_entity and create pinmux map differently) to use
virtual group for IMX.
However If IMX uses the data model i described above, the binding is
then a little different from tegra. that means we may need to change
to let each soc's pinctrl driver do real pinmux map parsing (maybe
add a callback in pinctrl.ops) based on their soc specific pinctrl
configuration node like pmx_usdhc4 above instread of let pinctrl core
do a standard pinmux map parsing which is our target we discussed so
long for.
Regards
Dong Aisheng
On Thu, Feb 02, 2012 at 10:36:54AM -0800, Stephen Warren wrote:
> Shawn Guo wrote at Wednesday, February 01, 2012 7:36 AM:
> ...
> > I had a talk with Dong about this binding, and we think that it should
> > work well for imx if we have a couple of small pieces added.
> >
> > On Fri, Jan 20, 2012 at 02:22:20PM -0800, Stephen Warren wrote:
> > ...
> > > pmx_sdhci: pinconfig-sdhci {
> > > /*
> > > * The mux property is a list of muxable entities
> > > * and the mux function to select for it. The number
> > > * of cells in each entry is the pin controller's
> > > * #pinmux-cells property. The pin controller's
> > > * binding defines what the cells mean. The pinctrl
> > > * driver is responsible for mapping this data to
> > > * the (group, function) pair required to fill in
> > > * the pinctrl subsystem's pinmux mapping table.
> > > */
> > > mux =
> > > <TEGRA_PMX_PG_DTA TEGRA_PMX_MUX_SDIO1>
> > > <TEGRA_PMX_PG_DTD TEGRA_PMX_MUX_SDIO1>;
> >
> > We need a property like 'mux-unit' whose value can be either 'pin' or
> > 'pingroup' to reflect something you mentioned as muxable entity.
>
> I'm not sure I agree; see below.
>
> > The reason behind this is the DT logic inside pinctrl core needs to
> > know how the pinmux_map should be constructed from device tree.
>
> As a general statement, yes.
>
> > In tegra case, the 'mux-unit' is 'pingroup', the core should construct
> > pinmux_map entry for each row/element of 'mux'.
>
> Yes.
>
> > In imx case, the 'mux-unit' will be 'pin',
>
> OK.
>
> Just a note: Tegra30 also has per-pin muxability. Only Tegra20 muxes pins
> in groups. (although Tegra30 does some if its pin configuration in groups)
>
> > and we would expect core construct only
> > one pinmux_map entry there, with all the pins listed in 'mux' composing
> > the group that pinmux_map needs.
>
> This is where I disagree.
>
So what I read is you disagree how pinctrl core uses property 'mux-unit'
not the property itself.
> If the pinmux_map should only contain a single entry, wouldn't the DT
> mux property only contain a single entry?
>
So you are saying we should have a pinmux_map for each entry in the
mux property? I disagree with that. For imx usdhc example, we have
10 entries in mux property representing 10 pins and their mux values.
What we need is one pinmux_map rather than 10 for just one client
device.
> The reason being that if there's a single entry in the pinmux_map, the
> group name used in that entry must be a group that's supported directly
> by the pinctrl driver (that's just the way pinctrl works). As such, why
> not just write the device tree in terms of those groups?
>
> The only way I can see this not being true is if your pinctrl driver is
> also parsing these mux properties, and dynamically creating the groups
> that it exposes based on the list of pins in the mux property.
Yes, that's exactly what we are trying to do for imx.
> However,
> that seems like the wrong approach;
Where does it go wrong exactly?
> If you're dynamically defining groups
> in DT, I'd expect separate explicit driver-specific properties/nodes to
> define those groups, such that the pinctrl core's processing of the mux
> property to be identical in all cases.
>
It's not imx specific. It's generally useful for any soc that has pin
as the muxable entity. I think it should be the common part of this
binding and implemented in pinctrl core DT support.
--
Regards,
Shawn
On Fri, Feb 03, 2012 at 04:07:23AM +0800, Dong Aisheng wrote:
...
> However If IMX uses the data model i described above, the binding is
> then a little different from tegra. that means we may need to change
> to let each soc's pinctrl driver do real pinmux map parsing (maybe
> add a callback in pinctrl.ops) based on their soc specific pinctrl
> configuration node like pmx_usdhc4 above instread of let pinctrl core
> do a standard pinmux map parsing which is our target we discussed so
> long for.
>
Yeah, this seems a reasonable alternative to me. Pushing the pinmux_map
construction down to individual pinctrl driver, who can interpret 'mux'
property best, will relieve the pinctrl core from understanding the
property for different cases, so that we can even save the property
'mux-unit' I asked for. And doing so somehow aligns with non-dt case,
where the pinmux_map is constructed by individual pinctrl driver too
(with help from PINMUX_MAP_* macros in board file).
--
Regards,
Shawn
On Fri, Feb 3, 2012 at 10:02 PM, Shawn Guo <[email protected]> wrote:
> On Fri, Feb 03, 2012 at 04:07:23AM +0800, Dong Aisheng wrote:
> ...
>> However If IMX uses the data model i described above, the binding is
>> then a little different from tegra. that means we may need to change
>> to ?let each soc's pinctrl driver do real pinmux map parsing (maybe
>> add a callback in pinctrl.ops) based on their soc specific pinctrl
>> configuration node like pmx_usdhc4 above instread of let pinctrl core
>> do a standard pinmux map parsing which is our target we discussed so
>> long for.
>>
> Yeah, this seems a reasonable alternative to me. ?Pushing the pinmux_map
> construction down to individual pinctrl driver, who can interpret 'mux'
> property best, will relieve the pinctrl core from understanding the
> property for different cases, so that we can even save the property
> 'mux-unit' I asked for. ?And doing so somehow aligns with non-dt case,
> where the pinmux_map is constructed by individual pinctrl driver too
> (with help from PINMUX_MAP_* macros in board file).
>
Yes, that's i think may be right way we should go now.
The reasons are as we met so many issues and discussed so long, we
feel like that it's hard to make a standard dt binding in pintrl core
based on one mux property format to cover all the differences for
different SOCs likeTegra(using HW group) and IMX(using virtual group).
We need to fix these issues on a higher level that each SOC pinctrl
driver implements their pinmux map parsing function, pinctrl core
calls these functions to get the pinmux maps the driver parsed instead
of pinctrl core parsing itself.
It could be something like:
in pinmux_get() {
struct pinmux_map *maps;
u32 num;
...
maps = pinctrl.ops.get_pinmux_map(pinmux_node, &num);
ret = pinmux_map_register(maps, num);
...
ret = pinmux_enable_maps(maps, num);
}
The basic binding idea for this way is:
1) pinctrl core is only responsible for the pinmux map parsing.
Each pinctrl driver is responsible for the function and group creation
no matter whether they're defined in drivers or parsed from device
tree.
2) The pinctrl driver is responsible for their pinmux node definitions
to cover their hw difference. However we prefer to reference the
format as Stephen proposed to align each SOC best. And each pinctrl
driver is responsible for the real pinmux map parsing by implementing
the pinmux_map_parse callback function.
3) when calling pinmux_get, we dynamically parse and create the pinmux
map table for each device.
4) the hog_on_boot map nodes are defined under SoC's pinctrl device node.
The pinctrl driver is responsible for parsing and register them.
Generally speaking, this way is very flexible and can cover most
differences between each SoCs.
It should work for both IMX and Tegra well. Tegra can still do on the
same binding as before as Stephen proposed. The only difference is
that the real pinmux map parsing is moved from pinctrl core to Tegra
pinctrl driver. And IMX can still use virtual group as we wished(i
think this is correct since pinctrl subsystem does not have a
restriction that user can only use pure hw group).
For other SOCs, i guess it should also work.
Regards
Dong Aisheng
* Dong Aisheng <[email protected]> [120202 11:36]:
>
> Actually i think i'd rather do not use config property, then i could
> be more compact:
> (anyway it's another issue and is flexible to be controlled by #pinmux-cells)
> pinctrl_usdhc4: pinconfig-usdhc4 {
> /* 0: pin 1: group */
> mux-entity = <0>;
> func-name = "usdhc4func";
> grp-name = "usdhc4grp";
The func-name and grp-name should be optional here.
This mux entry is already the group, and can be used as
the group name. And the function name can be generated
dynamically in most cases. I'm currently using np->full_name
of the driver claiming these pins as the function name.
> mux =
> <MX6Q_SD4_CMD 0 MX6Q_USDHC_PAD_CTRL>
> <MX6Q_SD4_CLK 0 MX6Q_USDHC_PAD_CTRL>
> <MX6Q_SD4_DAT0 1 MX6Q_USDHC_PAD_CTRL>
> <MX6Q_SD4_DAT1 1 MX6Q_USDHC_PAD_CTRL>
> <MX6Q_SD4_DAT2 1 MX6Q_USDHC_PAD_CTRL>
> <MX6Q_SD4_DAT3 1 MX6Q_USDHC_PAD_CTRL>
> <MX6Q_SD4_DAT4 1 MX6Q_USDHC_PAD_CTRL>
> <MX6Q_SD4_DAT5 1 MX6Q_USDHC_PAD_CTRL>
> <MX6Q_SD4_DAT6 1 MX6Q_USDHC_PAD_CTRL>
> <MX6Q_SD4_DAT7 1 MX6Q_USDHC_PAD_CTRL>;
> };
For listing basic pins this format works fine for me. It seems
to have low overhead for parsing. And the width of the array
can be driver specific.
Looks like it's the binding for altenative states that's still a
bit open..
So how about let's first standardize on the mux format above?
Then we can enhance it later for describing alternative states?
Regards,
Tony
On Sat, Feb 4, 2012 at 1:32 AM, Tony Lindgren <[email protected]> wrote:
> * Dong Aisheng <[email protected]> [120202 11:36]:
>>
>> Actually i think i'd rather do not use config property, then i could
>> be more compact:
>> (anyway it's another issue and is flexible to be controlled by #pinmux-cells)
>> pinctrl_usdhc4: pinconfig-usdhc4 {
>> ? ? ? ? /* 0: pin 1: group */
>> ? ? ? ? mux-entity = <0>;
>> ? ? ? ? func-name = "usdhc4func";
>> ? ? ? ? grp-name = "usdhc4grp";
>
> The func-name and grp-name should be optional here.
> This mux entry is already the group, and can be used as
> the group name.
For the case i discussed here, the mux entry is PIN.
(the mux-entity value is 0).
we introduce this value here for treating all pins is one group.
When do map parsing, only one pinmux map will be created.
So we need a grp-name.
And we also need a func-name here for construct pinmux map.
> And the function name can be generated
> dynamically in most cases. I'm currently using np->full_name
> of the driver claiming these pins as the function name.
>
Why i did not use np->name as function name is because the np->name
can be different
while actually these nodes may represent the same function but just
different pins, so the function name should be same.
>> ? ? ? ? mux =
>> ? ? ? ? ? ? ? ? <MX6Q_SD4_CMD ?0 MX6Q_USDHC_PAD_CTRL>
>> ? ? ? ? ? ? ? ? <MX6Q_SD4_CLK ?0 MX6Q_USDHC_PAD_CTRL>
>> ? ? ? ? ? ? ? ? <MX6Q_SD4_DAT0 1 MX6Q_USDHC_PAD_CTRL>
>> ? ? ? ? ? ? ? ? <MX6Q_SD4_DAT1 1 MX6Q_USDHC_PAD_CTRL>
>> ? ? ? ? ? ? ? ? <MX6Q_SD4_DAT2 1 MX6Q_USDHC_PAD_CTRL>
>> ? ? ? ? ? ? ? ? <MX6Q_SD4_DAT3 1 MX6Q_USDHC_PAD_CTRL>
>> ? ? ? ? ? ? ? ? <MX6Q_SD4_DAT4 1 MX6Q_USDHC_PAD_CTRL>
>> ? ? ? ? ? ? ? ? <MX6Q_SD4_DAT5 1 MX6Q_USDHC_PAD_CTRL>
>> ? ? ? ? ? ? ? ? <MX6Q_SD4_DAT6 1 MX6Q_USDHC_PAD_CTRL>
>> ? ? ? ? ? ? ? ? <MX6Q_SD4_DAT7 1 MX6Q_USDHC_PAD_CTRL>;
>> };
>
> For listing basic pins this format works fine for me. It seems
> to have low overhead for parsing. And the width of the array
> can be driver specific.
>
> Looks like it's the binding for altenative states that's still a
> bit open..
>
Yes, it does not have states support.
> So how about let's first standardize on the mux format above?
>
I'm afraid it may be hard for us to standardize the mux format for a
standard binding in pinctrl core due to hw difference.
I'm think the new way which i sent in this thread after the mail you replied.
You can refer to them to see if it's reasonable for you too.
Regards
Dong Aisheng
* Dong Aisheng <[email protected]> [120203 09:42]:
> On Sat, Feb 4, 2012 at 1:32 AM, Tony Lindgren <[email protected]> wrote:
> > * Dong Aisheng <[email protected]> [120202 11:36]:
> >>
> >> Actually i think i'd rather do not use config property, then i could
> >> be more compact:
> >> (anyway it's another issue and is flexible to be controlled by #pinmux-cells)
> >> pinctrl_usdhc4: pinconfig-usdhc4 {
> >> /* 0: pin 1: group */
> >> mux-entity = <0>;
> >> func-name = "usdhc4func";
> >> grp-name = "usdhc4grp";
> >
> > The func-name and grp-name should be optional here.
> > This mux entry is already the group, and can be used as
> > the group name.
> For the case i discussed here, the mux entry is PIN.
> (the mux-entity value is 0).
> we introduce this value here for treating all pins is one group.
> When do map parsing, only one pinmux map will be created.
> So we need a grp-name.
> And we also need a func-name here for construct pinmux map.
Sounds like a similar setup I have, I do not need any func-name
or grp-name though.. The group name is already unique with
pinconfig-usdhc4 in your example?
> > And the function name can be generated
> > dynamically in most cases. I'm currently using np->full_name
> > of the driver claiming these pins as the function name.
>
> Why i did not use np->name as function name is because the np->name
> can be different
> while actually these nodes may represent the same function but just
> different pins, so the function name should be same.
I used a function name based on the group name initially
(pinconfig-usdhc4 in your example), then renamed it to the
np->full_name of the device requesting the mux when the
mux was found ;)
> >> mux =
> >> <MX6Q_SD4_CMD 0 MX6Q_USDHC_PAD_CTRL>
> >> <MX6Q_SD4_CLK 0 MX6Q_USDHC_PAD_CTRL>
> >> <MX6Q_SD4_DAT0 1 MX6Q_USDHC_PAD_CTRL>
> >> <MX6Q_SD4_DAT1 1 MX6Q_USDHC_PAD_CTRL>
> >> <MX6Q_SD4_DAT2 1 MX6Q_USDHC_PAD_CTRL>
> >> <MX6Q_SD4_DAT3 1 MX6Q_USDHC_PAD_CTRL>
> >> <MX6Q_SD4_DAT4 1 MX6Q_USDHC_PAD_CTRL>
> >> <MX6Q_SD4_DAT5 1 MX6Q_USDHC_PAD_CTRL>
> >> <MX6Q_SD4_DAT6 1 MX6Q_USDHC_PAD_CTRL>
> >> <MX6Q_SD4_DAT7 1 MX6Q_USDHC_PAD_CTRL>;
> >> };
> >
> > For listing basic pins this format works fine for me. It seems
> > to have low overhead for parsing. And the width of the array
> > can be driver specific.
> >
> > Looks like it's the binding for altenative states that's still a
> > bit open..
> >
> Yes, it does not have states support.
>
> > So how about let's first standardize on the mux format above?
> >
> I'm afraid it may be hard for us to standardize the mux format for a
> standard binding in pinctrl core due to hw difference.
Yes the width would have to be hardare specific for the array.
> I'm think the new way which i sent in this thread after the mail you replied.
> You can refer to them to see if it's reasonable for you too.
Hmm, sorry now I'm confused. Got a link for that mail as so
many have been posted?
Regards,
Tony
On Sat, Feb 4, 2012 at 5:05 AM, Tony Lindgren <[email protected]> wrote:
....
>> I'm think the new way which i sent in this thread after the mail you replied.
>> You can refer to them to see if it's reasonable for you too.
>
> Hmm, sorry now I'm confused. Got a link for that mail as so
> many have been posted?
>
Pls see:
https://lkml.org/lkml/2012/2/3/276
For a simple example, you can refer to.
https://lkml.org/lkml/2012/2/2/405
Shawn and me will attend Linaro connect meeting on Feb 6th.
If you also attend, we may discuss it together with Linus and Stephen.
Regards
Dong Aisheng
* Dong Aisheng <[email protected]> [120204 08:24]:
> On Sat, Feb 4, 2012 at 5:05 AM, Tony Lindgren <[email protected]> wrote:
> ....
> >> I'm think the new way which i sent in this thread after the mail you replied.
> >> You can refer to them to see if it's reasonable for you too.
> >
> > Hmm, sorry now I'm confused. Got a link for that mail as so
> > many have been posted?
> >
> Pls see:
> https://lkml.org/lkml/2012/2/3/276
>
> For a simple example, you can refer to.
> https://lkml.org/lkml/2012/2/2/405
Thanks for clarifying. Yes I'm using something that's a subset
of that already, except I'm creating the functions and groups
automatically.
> Shawn and me will attend Linaro connect meeting on Feb 6th.
> If you also attend, we may discuss it together with Linus and Stephen.
Yes let's talk more there, I won't be there until on Tuesday
the 7th though.
Regards,
Tony