On 11/05/2013 07:02 PM, Sherman Yin wrote:
> On 13-11-04 04:04 PM, Stephen Warren wrote:
>> On 11/04/2013 04:26 PM, Heiko Stübner wrote:
>>
>>> I remember we had a discussion about how things like bias-disable
>>> explicitly
>>> shouldn't have a value, when they are represented in the list-format:
>>>
>>> pcfg_pull_none: pcfg_pull_none {
>>> bias-disable;
>>> };
>>>
>>> so a bias-disable = <1> was explicitly "forbidden" [for a lack of a
>>> better
>>> word]. And it was similar for other options, the parameter not meant
>>> to be
>>> indicating if they are active but really only setting the "strength"
>>> or so.
>>
>> Pure Boolean values should be represented as a valueless property. If
>> the property is present, the value is true, otherwise false.
>>
>> However, pinctrl bindings often don't represent Boolean values, but
>> rather tri-states, with the following values:
>>
>> * Don't touch this configuration option at all (missing)
>> * Enable the option (<1>)
>> * Disable the option (<0>)
>>
>> The reason for using tri-states being that you might want to write:
>>
>> xxx1 {
>> pins = <PINA>, <PINB>, <PINC>;
>> function = <...>;
>> // this node doesn't affect pullup
>> }
>> xxx2 {
>> pins = <PINA>, <PINB>;
>> // this node doesn't affect function
>> pull-up = <1>; // change, and enable
>> }
>> xxx3 {
>> pins = <PINAC>;
>> // this node doesn't affect function
>> pull-up = <0>; // change, and disable
>> }
>
> If I understand correctly, in Stephen's example, if a certain driver
> wants to configure PINA PINB and PINC, the pin configuration nodes
> "xxx1", "xxx2", and "xxx3" will all have to be selected for the
> particular pin state.
You probably don't want to reference the individual xxx1/2/3 nodes in
the client pinctrl properties, but instead wrap them in a higher-level
node that represents the whole pinctrl state. Then, the client pinctrl
properties can reference just that single parent node, instead of many
small nodes. In other words:
pinctrl@... {
...
sx: state_xxx {
xxx1 { ... };
xxx2 { ... };
xxx3 { ... };
};
sy: state_yyy {
yyy1 { ... };
yyy2 { ... };
};
}
some_client@... {
...
pinctrl-names = "default";
pinctrl-0 = <&sx>;
};
other_client@... {
...
pinctrl-names = "default";
pinctrl-0 = <&sy>;
};
rather than:
pinctrl@... {
...
sx1: xxx1 { ... };
sx2: xxx2 { ... };
sx3: xxx3 { ... };
sy1: yyy1 { ... };
sy2: yyy2 { ... };
}
some_client@... {
...
pinctrl-names = "default";
pinctrl-0 = <&sx1 &sx2 &sx3>;
};
other_client@... {
...
pinctrl-names = "default";
pinctrl-0 = <&sy1 &sy2>;
};
This is exactly how the Tegra pinctrl bindings work for example.
> This works fine. However, I'm just thinking that
> it would have been easier if we could specify just one node:
>
> xxx {
> pins = <PINA>, <PINB>, <PINC>;
> function = <...>;
> pull-up = <1 1 0>;
> }
>
> This "feature" seems a bit more concise to me and is what I did for my
> original pinctrl driver. The only downside is that with this method,
> one cannot specify "don't touch this option for this pin" if the same
> property must provide values for other pins.
The other downside is that if the lists get even slightly long, it get
really hard to match up the entries in the t properties.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
