Re: [PATCH v7 5/7] ARM: dts: add pbias dt node

[Balaji T K <balajitk@ti.com>]

On Friday 10 January 2014 05:02 PM, Mark Rutland wrote:
> On Wed, Jan 08, 2014 at 02:51:46PM +0000, Balaji T K wrote:
>> On Tuesday 07 January 2014 05:53 PM, Balaji T K wrote:
>>> On Tuesday 07 January 2014 04:27 PM, Mark Rutland wrote:
>>>> On Tue, Jan 07, 2014 at 10:18:15AM +0000, Balaji T K wrote:
>>>>> On Monday 06 January 2014 11:49 PM, Mark Rutland wrote:
>>>>>> On Fri, Dec 20, 2013 at 05:35:53PM +0000, Balaji T K wrote:
>>>>>>> Add pbias regulator node as a child of system control
>>>>>>> module - syscon.
>>>>>>>
>>>>>>> Signed-off-by: Balaji T K <balajitk@ti.com>
>>>>>>> ---
>>>>>>>     arch/arm/boot/dts/dra7.dtsi     |   18 ++++++++++++++++++
>>>>>>>     arch/arm/boot/dts/omap2430.dtsi |   18 ++++++++++++++++++
>>>>>>>     arch/arm/boot/dts/omap3.dtsi    |   18 ++++++++++++++++++
>>>>>>>     arch/arm/boot/dts/omap4.dtsi    |   18 ++++++++++++++++++
>>>>>>>     arch/arm/boot/dts/omap5.dtsi    |   18 ++++++++++++++++++
>>>>>>>     5 files changed, 90 insertions(+), 0 deletions(-)
>>>>>>>
>>>>>>> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
>>>>>>> index d0df4c4..4e68df1 100644
>>>>>>> --- a/arch/arm/boot/dts/dra7.dtsi
>>>>>>> +++ b/arch/arm/boot/dts/dra7.dtsi
>>>>>>> @@ -110,6 +110,23 @@
>>>>>>>                 ti,hwmods = "counter_32k";
>>>>>>>             };
>>>>>>>
>>>>>>> +        dra7_ctrl_general: tisyscon@4a002e00 {
>>>>>>> +            compatible = "ti,control-syscon", "syscon", "simple-bus";
>>>>>>
>>>>>> Please, don't use simple-bus like that. The components below this node
>>>>>> depend on it. It is _NOT_ a simple bus. Make the ti,control-syscon
>>>>>> driver probe it's children.
>>>>>
>>>>> Hi Mark,
>>>>>
>>>>> Actually ti,control-syscon driver does not exist, so I can remove it,
>>>>> and simple-bus is needed for child creation.
>>>>
>>>> This still shows up as a syscon node, with a reg property, and syscon is
>>>> not an extension of simple-bus.
>>>>
>>>> There are properties in the parent node that children depend on, and
>>>> that makes me wary of describing it as a simple bus. I'd expect to be
>>>> able to move child nodes out of a simple-bus if ranges provided an
>>>> idmap, and I can't do that here.
>>>>
>>> Hi Mark,
>>>
>>> Not sure if I am understanding here, can you please add more info.
>
> A node's compatible string list describes the set of devices (or rather
> descriptions of devices) with which it is compatible. The list goes from
> most specific to least specific. The idea is that an OS reads through
> the list until it finds a string it knows how to handle, then treats the
> node as that.
>
> Ignoring "ti,control-syscon", the node has both "syscon" and
> "simple-bus". Given you expect the OS to recognise "syscon", you should
> _not_ expect it to also treat the node as "simple-bus". Doing so is an
> abuse of the property and current Linux implementation details.
>
> The "syscon" binding is not a more specific version of "simple-bus".
> They imply completely different things.
>
> Either make a ti,control-syscon driver that probes the child nodes, or
> move the child nodes out and give them a phandle to the
> dra7_ctrl_general node that they can parse. I'd prefer the latter as
> relies on fewer topology details, is as easy to implement as your
> current parent node parsing, and is easier to extend in future.
>

Thanks Mark,
phandle to syscon is fine for me.