On 27/09/2022 14:49, Geert Uytterhoeven wrote:
> Hi Ulf,
>
> On Tue, Sep 27, 2022 at 11:49 AM Ulf Hansson <[email protected]> wrote:
>>>>>>> The main concern that was raised on this topic was that we have to
>>>>>>> somehow link the power-domain to the specific peripherals (the power
>>>>>>> domain consumer) in the device tree.
>>>>>>
>>>>>> Yes, that is needed. Although, I don't see how that is a concern?
>>>>>>
>>>>>> We already have the valid bindings to use for this, see more below.
>>>>>>
>>>>>>>
>>>>>>> Adding the power-domain property there will trigger validation errors
>>>>>>> unless we do explicitly add the power-domains to the schema for each
>>>>>>> peripheral we need this. To me this does not really work, but maybe I'm
>>>>>>> not understanding something.
>>>>>>>
>>>>>>> This is what Rob wrote on the topic [1]:
>>>>>>> > No. For 'power-domains' bindings have to define how many there are and
>>>>>>> > what each one is.
>>>>>>>
>>>>>>> Just as an example from patch [2]:
>>>>>>>
>>>>>>> can1: can@0 {
>>>>>>> compatible = "microchip,mcp251xfd";
>>>>>>> power-domains = <&pd_sleep_moci>;
>>>>>>> };
>>>>>>>
>>>>>>> leads to:
>>>>>>>
>>>>>>> imx8mm-verdin-nonwifi-dahlia.dtb: can@0: 'power-domains' does not match any of the regexes: 'pinctrl-[0-9]+'
>>>>>>> From schema: .../bindings/net/can/microchip,mcp251xfd.yaml
>>>>>>
>>>>>> I think it should be fine to just add the below line to the DT
>>>>>> bindings, for each peripheral device to fix the above problem.
>>>>>>
>>>>>> power-domains: true
>>>>>
>>>>> Again, as Rob said, no, because it must be strictly defined. So for
>>>>> example: "maxItems: 1" for simple cases. But what if device is then part
>>>>> of two power domains?
>>>>>
>>>>>>
>>>>>> That should be okay, right?
>>>>>
>>>>> Adding it to each peripheral scales poorly. Especially that literally
>>>>> any device can be part of such power domain.
>>>>
>>>> Right.
>>>>
>>>>>
>>>>> If we are going with power domain approach, then it should be applicable
>>>>> basically to every device or to every device of some class (e.g. I2C,
>>>>> SPI). This means it should be added to respective core schema in
>>>>> dtschema repo, in a way it does not interfere with other power-domains
>>>>> properties (existing ones).
>>>>
>>>> Isn't that already taken care of [1]?
>>>
>>> No, because it does not define the items (what are the power domains and
>>> how many). This binding expects that any device has maxItems restricting it.
>>
>> Right, apologize for my ignorance.
>>
>>>
>>>>
>>>> If there is more than one power domain per device, perhaps we may need
>>>> to extend it with a more strict binding? But, that doesn't seem to be
>>>> the case here - and if it turns out to be needed later on, we can
>>>> always extend the bindings, no?
>>>>
>>>> Note also that we already have DT bindings specifying "power-domains:
>>>> true" to deal with the above. Isn't that what we want?
>>>
>>> You mentioned it before and both me and Rob already responded - no,
>>> because it does not restrict the number of items.
>>
>> Okay, so maxItems need to be specified for each peripheral. It's not a
>> big deal, right?

It's a bit of effort to add it manually to each device binding. It just
does not scale well.

>>
>> Of course, it would be even easier if the core schema would use a
>> default "maxItems: 1" for power domain consumers, which of course must
>> be possible to be overridden for those consumers that need something
>> else. But perhaps it's not that simple. :-)

I think this would be the way to do it properly.

>
> It's not that simple: being part of a PM Domain is not a property of the
> device being described, but a property of the integration into the SoC.

I agree.

This concept of power domains for every device does not look like really
describing the hardware. The hardware itself, e.g. some camera sensor or
I2C device, might have power supply and reset pin. It does not have
something like power-domain.

Although one could also argue that it is the same case with SoC blocks -
being part of power domain is a property of a SoC and its power domain
controller.

> All synchronous hardware needs power (single/multiple), clock(s), and
> reset(s). But the granularity of control over power(s), clocks, and resets
> depends on the integration. So the related properties can appear
> anywhere.

Best regards,
Krzysztof

2022-09-28 08:34:04

by Ulf Hansson

[permalink] [raw]

Subject: Re: [PATCH v1 0/5] power: domain: Add driver for a PM domain provider which controls

On Tue, 27 Sept 2022 at 16:27, Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 27/09/2022 14:49, Geert Uytterhoeven wrote:
> > Hi Ulf,
> >
> > On Tue, Sep 27, 2022 at 11:49 AM Ulf Hansson <[email protected]> wrote:
> >>>>>>> The main concern that was raised on this topic was that we have to
> >>>>>>> somehow link the power-domain to the specific peripherals (the power
> >>>>>>> domain consumer) in the device tree.
> >>>>>>
> >>>>>> Yes, that is needed. Although, I don't see how that is a concern?
> >>>>>>
> >>>>>> We already have the valid bindings to use for this, see more below.
> >>>>>>
> >>>>>>>
> >>>>>>> Adding the power-domain property there will trigger validation errors
> >>>>>>> unless we do explicitly add the power-domains to the schema for each
> >>>>>>> peripheral we need this. To me this does not really work, but maybe I'm
> >>>>>>> not understanding something.
> >>>>>>>
> >>>>>>> This is what Rob wrote on the topic [1]:
> >>>>>>> > No. For 'power-domains' bindings have to define how many there are and
> >>>>>>> > what each one is.
> >>>>>>>
> >>>>>>> Just as an example from patch [2]:
> >>>>>>>
> >>>>>>> can1: can@0 {
> >>>>>>> compatible = "microchip,mcp251xfd";
> >>>>>>> power-domains = <&pd_sleep_moci>;
> >>>>>>> };
> >>>>>>>
> >>>>>>> leads to:
> >>>>>>>
> >>>>>>> imx8mm-verdin-nonwifi-dahlia.dtb: can@0: 'power-domains' does not match any of the regexes: 'pinctrl-[0-9]+'
> >>>>>>> From schema: .../bindings/net/can/microchip,mcp251xfd.yaml
> >>>>>>
> >>>>>> I think it should be fine to just add the below line to the DT
> >>>>>> bindings, for each peripheral device to fix the above problem.
> >>>>>>
> >>>>>> power-domains: true
> >>>>>
> >>>>> Again, as Rob said, no, because it must be strictly defined. So for
> >>>>> example: "maxItems: 1" for simple cases. But what if device is then part
> >>>>> of two power domains?
> >>>>>
> >>>>>>
> >>>>>> That should be okay, right?
> >>>>>
> >>>>> Adding it to each peripheral scales poorly. Especially that literally
> >>>>> any device can be part of such power domain.
> >>>>
> >>>> Right.
> >>>>
> >>>>>
> >>>>> If we are going with power domain approach, then it should be applicable
> >>>>> basically to every device or to every device of some class (e.g. I2C,
> >>>>> SPI). This means it should be added to respective core schema in
> >>>>> dtschema repo, in a way it does not interfere with other power-domains
> >>>>> properties (existing ones).
> >>>>
> >>>> Isn't that already taken care of [1]?
> >>>
> >>> No, because it does not define the items (what are the power domains and
> >>> how many). This binding expects that any device has maxItems restricting it.
> >>
> >> Right, apologize for my ignorance.
> >>
> >>>
> >>>>
> >>>> If there is more than one power domain per device, perhaps we may need
> >>>> to extend it with a more strict binding? But, that doesn't seem to be
> >>>> the case here - and if it turns out to be needed later on, we can
> >>>> always extend the bindings, no?
> >>>>
> >>>> Note also that we already have DT bindings specifying "power-domains:
> >>>> true" to deal with the above. Isn't that what we want?
> >>>
> >>> You mentioned it before and both me and Rob already responded - no,
> >>> because it does not restrict the number of items.
> >>
> >> Okay, so maxItems need to be specified for each peripheral. It's not a
> >> big deal, right?
>
> It's a bit of effort to add it manually to each device binding. It just
> does not scale well.

Whether it scales or not, that's how the power-domain bindings for
consumers look like. It's the similar situation for clocks, regulators
and other resources too.

My point is, for the discussion around $subject patch, it doesn't
really matter what option we pick. The DT docs for each peripheral
need to be updated anyway.

>
> >>
> >> Of course, it would be even easier if the core schema would use a
> >> default "maxItems: 1" for power domain consumers, which of course must
> >> be possible to be overridden for those consumers that need something
> >> else. But perhaps it's not that simple. :-)
>
> I think this would be the way to do it properly.
>
> >
> > It's not that simple: being part of a PM Domain is not a property of the
> > device being described, but a property of the integration into the SoC.
>
> I agree.
>
> This concept of power domains for every device does not look like really
> describing the hardware. The hardware itself, e.g. some camera sensor or
> I2C device, might have power supply and reset pin. It does not have
> something like power-domain.
>
> Although one could also argue that it is the same case with SoC blocks -
> being part of power domain is a property of a SoC and its power domain
> controller.

Yes. DT describes the platform too, not only the SoC and its IP blocks.

Moreover, as the power domain bindings were added back in kernel
v3.18, that's what we have to describe these kinds of platforms.

[...]

Kind regards
Uffe