[PATCH] dt-bindings: mfd: Explain lack of child dependency in simple-mfd

Krzysztof Kozlowski posted 1 patch 1 year, 6 months ago
There is a newer version of this series
Documentation/devicetree/bindings/mfd/mfd.txt | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
[PATCH] dt-bindings: mfd: Explain lack of child dependency in simple-mfd
Posted by Krzysztof Kozlowski 1 year, 6 months ago
Common mistake of usage of 'simple-mfd' compatible is a dependency of
children on resources acquired and managed by the parent, e.g. clocks.
Extend the simple-mfd documentation to cover this case.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 Documentation/devicetree/bindings/mfd/mfd.txt | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/mfd.txt b/Documentation/devicetree/bindings/mfd/mfd.txt
index 336c0495c8a3..98b4340b65f3 100644
--- a/Documentation/devicetree/bindings/mfd/mfd.txt
+++ b/Documentation/devicetree/bindings/mfd/mfd.txt
@@ -18,12 +18,13 @@ A typical MFD can be:
 Optional properties:
 
 - compatible : "simple-mfd" - this signifies that the operating system should
-  consider all subnodes of the MFD device as separate devices akin to how
-  "simple-bus" indicates when to see subnodes as children for a simple
-  memory-mapped bus. For more complex devices, when the nexus driver has to
-  probe registers to figure out what child devices exist etc, this should not
-  be used. In the latter case the child devices will be determined by the
-  operating system.
+  consider all subnodes of the MFD device as separate and independent devices
+  akin to how "simple-bus" indicates when to see subnodes as children for a
+  simple memory-mapped bus. "Independent devices" means that children do not
+  need any resources to be provided by the parent device.
+  For more complex devices, when the nexus driver has to probe registers to
+  figure out what child devices exist etc, this should not be used. In the
+  latter case the child devices will be determined by the operating system.
 
 - ranges: Describes the address mapping relationship to the parent. Should set
   the child's base address to 0, the physical address within parent's address
-- 
2.43.0
Re: [PATCH] dt-bindings: mfd: Explain lack of child dependency in simple-mfd
Posted by Lee Jones 1 year, 5 months ago
On Sun, 16 Jun 2024, Krzysztof Kozlowski wrote:

> Common mistake of usage of 'simple-mfd' compatible is a dependency of
> children on resources acquired and managed by the parent, e.g. clocks.
> Extend the simple-mfd documentation to cover this case.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  Documentation/devicetree/bindings/mfd/mfd.txt | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/mfd.txt b/Documentation/devicetree/bindings/mfd/mfd.txt
> index 336c0495c8a3..98b4340b65f3 100644
> --- a/Documentation/devicetree/bindings/mfd/mfd.txt
> +++ b/Documentation/devicetree/bindings/mfd/mfd.txt
> @@ -18,12 +18,13 @@ A typical MFD can be:
>  Optional properties:
>  
>  - compatible : "simple-mfd" - this signifies that the operating system should
> -  consider all subnodes of the MFD device as separate devices akin to how
> -  "simple-bus" indicates when to see subnodes as children for a simple
> -  memory-mapped bus. For more complex devices, when the nexus driver has to
> -  probe registers to figure out what child devices exist etc, this should not
> -  be used. In the latter case the child devices will be determined by the
> -  operating system.
> +  consider all subnodes of the MFD device as separate and independent devices
> +  akin to how "simple-bus" indicates when to see subnodes as children for a
> +  simple memory-mapped bus. "Independent devices" means that children do not

I'm not against the change, but I think it can be phased better.

Quoting the new part and going on to explain what you mean by it doesn't
flow very well.  Are you able to massage it so it reads a little more
nicely please?

> +  need any resources to be provided by the parent device.
> +  For more complex devices, when the nexus driver has to probe registers to
> +  figure out what child devices exist etc, this should not be used. In the
> +  latter case the child devices will be determined by the operating system.
>  
>  - ranges: Describes the address mapping relationship to the parent. Should set
>    the child's base address to 0, the physical address within parent's address
> -- 
> 2.43.0
> 

-- 
Lee Jones [李琼斯]
Re: [PATCH] dt-bindings: mfd: Explain lack of child dependency in simple-mfd
Posted by Krzysztof Kozlowski 1 year, 5 months ago
On 20/06/2024 19:17, Lee Jones wrote:
> On Sun, 16 Jun 2024, Krzysztof Kozlowski wrote:
> 
>> Common mistake of usage of 'simple-mfd' compatible is a dependency of
>> children on resources acquired and managed by the parent, e.g. clocks.
>> Extend the simple-mfd documentation to cover this case.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>  Documentation/devicetree/bindings/mfd/mfd.txt | 13 +++++++------
>>  1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/mfd.txt b/Documentation/devicetree/bindings/mfd/mfd.txt
>> index 336c0495c8a3..98b4340b65f3 100644
>> --- a/Documentation/devicetree/bindings/mfd/mfd.txt
>> +++ b/Documentation/devicetree/bindings/mfd/mfd.txt
>> @@ -18,12 +18,13 @@ A typical MFD can be:
>>  Optional properties:
>>  
>>  - compatible : "simple-mfd" - this signifies that the operating system should
>> -  consider all subnodes of the MFD device as separate devices akin to how
>> -  "simple-bus" indicates when to see subnodes as children for a simple
>> -  memory-mapped bus. For more complex devices, when the nexus driver has to
>> -  probe registers to figure out what child devices exist etc, this should not
>> -  be used. In the latter case the child devices will be determined by the
>> -  operating system.
>> +  consider all subnodes of the MFD device as separate and independent devices
>> +  akin to how "simple-bus" indicates when to see subnodes as children for a
>> +  simple memory-mapped bus. "Independent devices" means that children do not
> 
> I'm not against the change, but I think it can be phased better.
> 
> Quoting the new part and going on to explain what you mean by it doesn't
> flow very well.  Are you able to massage it so it reads a little more
> nicely please?

Does this feels better?

compatible : "simple-mfd" - this signifies that the operating system
should consider all subnodes of the MFD device as separate and
independent devices, so not needing any resources to be provided by the
parent device. Similarly to how "simple-bus" indicates when to see
subnodes as children for a simple memory-mapped bus.

For more complex devices, when the nexus driver has to probe registers
to figure out what child devices exist etc, this should not be used. In
the latter case the child devices will be determined by the operating
system.


Best regards,
Krzysztof
Re: [PATCH] dt-bindings: mfd: Explain lack of child dependency in simple-mfd
Posted by Lee Jones 1 year, 5 months ago
On Thu, 20 Jun 2024, Krzysztof Kozlowski wrote:

> On 20/06/2024 19:17, Lee Jones wrote:
> > On Sun, 16 Jun 2024, Krzysztof Kozlowski wrote:
> > 
> >> Common mistake of usage of 'simple-mfd' compatible is a dependency of
> >> children on resources acquired and managed by the parent, e.g. clocks.
> >> Extend the simple-mfd documentation to cover this case.
> >>
> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> ---
> >>  Documentation/devicetree/bindings/mfd/mfd.txt | 13 +++++++------
> >>  1 file changed, 7 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/mfd/mfd.txt b/Documentation/devicetree/bindings/mfd/mfd.txt
> >> index 336c0495c8a3..98b4340b65f3 100644
> >> --- a/Documentation/devicetree/bindings/mfd/mfd.txt
> >> +++ b/Documentation/devicetree/bindings/mfd/mfd.txt
> >> @@ -18,12 +18,13 @@ A typical MFD can be:
> >>  Optional properties:
> >>  
> >>  - compatible : "simple-mfd" - this signifies that the operating system should
> >> -  consider all subnodes of the MFD device as separate devices akin to how
> >> -  "simple-bus" indicates when to see subnodes as children for a simple
> >> -  memory-mapped bus. For more complex devices, when the nexus driver has to
> >> -  probe registers to figure out what child devices exist etc, this should not
> >> -  be used. In the latter case the child devices will be determined by the
> >> -  operating system.
> >> +  consider all subnodes of the MFD device as separate and independent devices
> >> +  akin to how "simple-bus" indicates when to see subnodes as children for a
> >> +  simple memory-mapped bus. "Independent devices" means that children do not
> > 
> > I'm not against the change, but I think it can be phased better.
> > 
> > Quoting the new part and going on to explain what you mean by it doesn't
> > flow very well.  Are you able to massage it so it reads a little more
> > nicely please?
> 
> Does this feels better?
> 
> compatible : "simple-mfd" - this signifies that the operating system
> should consider all subnodes of the MFD device as separate and
> independent devices, so not needing any resources to be provided by the
> parent device. Similarly to how "simple-bus" indicates when to see
> subnodes as children for a simple memory-mapped bus.
> 
> For more complex devices, when the nexus driver has to probe registers
> to figure out what child devices exist etc, this should not be used. In
> the latter case the child devices will be determined by the operating
> system.

Flows a lot better, yes.

Submit it and please include the original author this time.

-- 
Lee Jones [李琼斯]
Re: [PATCH] dt-bindings: mfd: Explain lack of child dependency in simple-mfd
Posted by Krzysztof Kozlowski 1 year, 5 months ago
On 21/06/2024 13:10, Lee Jones wrote:
> On Thu, 20 Jun 2024, Krzysztof Kozlowski wrote:
> 
>> On 20/06/2024 19:17, Lee Jones wrote:
>>> On Sun, 16 Jun 2024, Krzysztof Kozlowski wrote:
>>>
>>>> Common mistake of usage of 'simple-mfd' compatible is a dependency of
>>>> children on resources acquired and managed by the parent, e.g. clocks.
>>>> Extend the simple-mfd documentation to cover this case.
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> ---
>>>>  Documentation/devicetree/bindings/mfd/mfd.txt | 13 +++++++------
>>>>  1 file changed, 7 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mfd/mfd.txt b/Documentation/devicetree/bindings/mfd/mfd.txt
>>>> index 336c0495c8a3..98b4340b65f3 100644
>>>> --- a/Documentation/devicetree/bindings/mfd/mfd.txt
>>>> +++ b/Documentation/devicetree/bindings/mfd/mfd.txt
>>>> @@ -18,12 +18,13 @@ A typical MFD can be:
>>>>  Optional properties:
>>>>  
>>>>  - compatible : "simple-mfd" - this signifies that the operating system should
>>>> -  consider all subnodes of the MFD device as separate devices akin to how
>>>> -  "simple-bus" indicates when to see subnodes as children for a simple
>>>> -  memory-mapped bus. For more complex devices, when the nexus driver has to
>>>> -  probe registers to figure out what child devices exist etc, this should not
>>>> -  be used. In the latter case the child devices will be determined by the
>>>> -  operating system.
>>>> +  consider all subnodes of the MFD device as separate and independent devices
>>>> +  akin to how "simple-bus" indicates when to see subnodes as children for a
>>>> +  simple memory-mapped bus. "Independent devices" means that children do not
>>>
>>> I'm not against the change, but I think it can be phased better.
>>>
>>> Quoting the new part and going on to explain what you mean by it doesn't
>>> flow very well.  Are you able to massage it so it reads a little more
>>> nicely please?
>>
>> Does this feels better?
>>
>> compatible : "simple-mfd" - this signifies that the operating system
>> should consider all subnodes of the MFD device as separate and
>> independent devices, so not needing any resources to be provided by the
>> parent device. Similarly to how "simple-bus" indicates when to see
>> subnodes as children for a simple memory-mapped bus.
>>
>> For more complex devices, when the nexus driver has to probe registers
>> to figure out what child devices exist etc, this should not be used. In
>> the latter case the child devices will be determined by the operating
>> system.
> 
> Flows a lot better, yes.

Sure.

> 
> Submit it and please include the original author this time.

Everything is scripted, so you ask me for additional, manual steps just
to find the author and then Cc-them. I'll do it but it would be much
easier if the interested party added themself as reviewer or maintainer
of the binding.

> 

Best regards,
Krzysztof
Re: [PATCH] dt-bindings: mfd: Explain lack of child dependency in simple-mfd
Posted by Lee Jones 1 year, 5 months ago
On Sun, 23 Jun 2024, Krzysztof Kozlowski wrote:

> On 21/06/2024 13:10, Lee Jones wrote:
> > On Thu, 20 Jun 2024, Krzysztof Kozlowski wrote:
> > 
> >> On 20/06/2024 19:17, Lee Jones wrote:
> >>> On Sun, 16 Jun 2024, Krzysztof Kozlowski wrote:
> >>>
> >>>> Common mistake of usage of 'simple-mfd' compatible is a dependency of
> >>>> children on resources acquired and managed by the parent, e.g. clocks.
> >>>> Extend the simple-mfd documentation to cover this case.
> >>>>
> >>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >>>> ---
> >>>>  Documentation/devicetree/bindings/mfd/mfd.txt | 13 +++++++------
> >>>>  1 file changed, 7 insertions(+), 6 deletions(-)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/mfd/mfd.txt b/Documentation/devicetree/bindings/mfd/mfd.txt
> >>>> index 336c0495c8a3..98b4340b65f3 100644
> >>>> --- a/Documentation/devicetree/bindings/mfd/mfd.txt
> >>>> +++ b/Documentation/devicetree/bindings/mfd/mfd.txt
> >>>> @@ -18,12 +18,13 @@ A typical MFD can be:
> >>>>  Optional properties:
> >>>>  
> >>>>  - compatible : "simple-mfd" - this signifies that the operating system should
> >>>> -  consider all subnodes of the MFD device as separate devices akin to how
> >>>> -  "simple-bus" indicates when to see subnodes as children for a simple
> >>>> -  memory-mapped bus. For more complex devices, when the nexus driver has to
> >>>> -  probe registers to figure out what child devices exist etc, this should not
> >>>> -  be used. In the latter case the child devices will be determined by the
> >>>> -  operating system.
> >>>> +  consider all subnodes of the MFD device as separate and independent devices
> >>>> +  akin to how "simple-bus" indicates when to see subnodes as children for a
> >>>> +  simple memory-mapped bus. "Independent devices" means that children do not
> >>>
> >>> I'm not against the change, but I think it can be phased better.
> >>>
> >>> Quoting the new part and going on to explain what you mean by it doesn't
> >>> flow very well.  Are you able to massage it so it reads a little more
> >>> nicely please?
> >>
> >> Does this feels better?
> >>
> >> compatible : "simple-mfd" - this signifies that the operating system
> >> should consider all subnodes of the MFD device as separate and
> >> independent devices, so not needing any resources to be provided by the
> >> parent device. Similarly to how "simple-bus" indicates when to see
> >> subnodes as children for a simple memory-mapped bus.
> >>
> >> For more complex devices, when the nexus driver has to probe registers
> >> to figure out what child devices exist etc, this should not be used. In
> >> the latter case the child devices will be determined by the operating
> >> system.
> > 
> > Flows a lot better, yes.
> 
> Sure.
> 
> > 
> > Submit it and please include the original author this time.
> 
> Everything is scripted, so you ask me for additional, manual steps just
> to find the author and then Cc-them. I'll do it but it would be much
> easier if the interested party added themself as reviewer or maintainer
> of the binding.

How you set-up your tooling is your business.  =;)

Who to Cc is often situation dependent, some of the semantics you'd find
hard to script.  `scripts/get_maintainer.pl` provides some helpful
arguments (e.g. --git-min-percent 75) which seem to work a lot of the
time.

In this particular case, you're making heavy changes to a passage of text
which someone has taken the time to craft.  Adding them to the
conversation should be seen as a common courtesy.

-- 
Lee Jones [李琼斯]
Re: [PATCH] dt-bindings: mfd: Explain lack of child dependency in simple-mfd
Posted by Conor Dooley 1 year, 6 months ago
On Sun, Jun 16, 2024 at 10:06:59AM +0200, Krzysztof Kozlowski wrote:
> Common mistake of usage of 'simple-mfd' compatible is a dependency of
> children on resources acquired and managed by the parent, e.g. clocks.
> Extend the simple-mfd documentation to cover this case.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>