[PATCH] dt-bindings: power: Add SiFive Domain Management controllers

[PATCH] dt-bindings: power: Add SiFive Domain Management controllers

[Nick Hu]

SiFive Domain Management controller includes the following components
- SiFive Tile Management Controller
- SiFive Cluster Management Controller
- SiFive Core Complex Management Controller

These controllers control the clock and power domain of the
corresponding domain.

Signed-off-by: Nick Hu <nick.hu@sifive.com>
Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
---
 .../devicetree/bindings/power/sifive,tmc.yaml | 89 +++++++++++++++++++
 1 file changed, 89 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/sifive,tmc.yaml

diff --git a/Documentation/devicetree/bindings/power/sifive,tmc.yaml b/Documentation/devicetree/bindings/power/sifive,tmc.yaml
new file mode 100644
index 000000000000..7ed4f290b94b
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/sifive,tmc.yaml
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/sifive,tmc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SiFive Domain Management Controller
+
+maintainers:
+  - Cyan Yang <cyan.yang@sifive.com>
+  - Nick Hu <nick.hu@sifive.com>
+  - Samuel Holland <samuel.holland@sifive.com>
+
+description: |
+  This is the device tree binding for the following SiFive Domain Management Controllers.
+  - Tile Management Controller
+      - TMC0
+      - TMC1
+      - TMC2
+      - TMC3
+  - Subsystem Management Controller
+      - SMC0
+      - SMC1
+      - SMC2
+      - SMC3
+  - Cluster Management Controller
+      - CMC2
+      - CMC3
+  SiFive Domain Management Controllers support the SiFive Quiet Interface
+  Protocol (SQIP) starting from the Version 1. The control method is
+  different from the Version 0, making them incompatible.
+
+allOf:
+  - $ref: power-domain.yaml#
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - {}
+          - pattern: "^sifive,[ts]mc0$"
+      - items:
+          - {}
+          - pattern: "^sifive,[ts]mc3$"
+          - pattern: "^sifive,[ts]mc2$"
+          - pattern: "^sifive,[ts]mc1$"
+      - items:
+          - {}
+          - pattern: "^sifive,[ts]mc2$"
+          - pattern: "^sifive,[ts]mc1$"
+      - items:
+          - {}
+          - pattern: "^sifive,[ts]mc1$"
+      - items:
+          - {}
+          - const: sifive,cmc3
+          - const: sifive,cmc2
+      - items:
+          - {}
+          - const: sifive,cmc2
+
+  reg:
+    maxItems: 1
+
+  sifive,feature-level:
+    description: |
+      Supported power features. This property is absent if the full set of features
+      is supported
+    $ref: /schemas/types.yaml#/definitions/string
+    enum: ["nopg", "ceasepg", "runonlypg"]
+
+  "#power-domain-cells":
+    const: 0
+
+if:
+  not:
+    properties:
+      compatible:
+        contains:
+          pattern: "^sifive,[tsc]mc3$"
+then:
+  properties:
+    sifive,feature-level: false
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
-- 
2.17.1

Re: [PATCH] dt-bindings: power: Add SiFive Domain Management controllers

[Krzysztof Kozlowski]

On Fri, May 09, 2025 at 10:16:04AM GMT, Nick Hu wrote:
> SiFive Domain Management controller includes the following components
> - SiFive Tile Management Controller
> - SiFive Cluster Management Controller
> - SiFive Core Complex Management Controller
> 
> These controllers control the clock and power domain of the
> corresponding domain.
> 
> Signed-off-by: Nick Hu <nick.hu@sifive.com>
> Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
> ---
>  .../devicetree/bindings/power/sifive,tmc.yaml | 89 +++++++++++++++++++

Where is a patch with the driver (user of the binding)?

>  1 file changed, 89 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/sifive,tmc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/power/sifive,tmc.yaml b/Documentation/devicetree/bindings/power/sifive,tmc.yaml
> new file mode 100644
> index 000000000000..7ed4f290b94b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/sifive,tmc.yaml
> @@ -0,0 +1,89 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/sifive,tmc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SiFive Domain Management Controller
> +
> +maintainers:
> +  - Cyan Yang <cyan.yang@sifive.com>
> +  - Nick Hu <nick.hu@sifive.com>
> +  - Samuel Holland <samuel.holland@sifive.com>
> +
> +description: |
> +  This is the device tree binding for the following SiFive Domain Management Controllers.

Explain the hardware, not that "binding is a binding for ...".

Also, wrap according to Linux coding style.


> +  - Tile Management Controller
> +      - TMC0
> +      - TMC1
> +      - TMC2
> +      - TMC3
> +  - Subsystem Management Controller
> +      - SMC0
> +      - SMC1
> +      - SMC2
> +      - SMC3
> +  - Cluster Management Controller
> +      - CMC2
> +      - CMC3
> +  SiFive Domain Management Controllers support the SiFive Quiet Interface
> +  Protocol (SQIP) starting from the Version 1. The control method is
> +  different from the Version 0, making them incompatible.
> +
> +allOf:
> +  - $ref: power-domain.yaml#
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - {}
> +          - pattern: "^sifive,[ts]mc0$"
> +      - items:
> +          - {}
> +          - pattern: "^sifive,[ts]mc3$"
> +          - pattern: "^sifive,[ts]mc2$"
> +          - pattern: "^sifive,[ts]mc1$"
> +      - items:
> +          - {}
> +          - pattern: "^sifive,[ts]mc2$"
> +          - pattern: "^sifive,[ts]mc1$"
> +      - items:
> +          - {}
> +          - pattern: "^sifive,[ts]mc1$"
> +      - items:
> +          - {}
> +          - const: sifive,cmc3
> +          - const: sifive,cmc2
> +      - items:
> +          - {}
> +          - const: sifive,cmc2

All of this is just unexpected. Why any compatible should come with
these? You need to use SoC specific compatibles.


> +
> +  reg:
> +    maxItems: 1
> +
> +  sifive,feature-level:
> +    description: |
> +      Supported power features. This property is absent if the full set of features
> +      is supported

Compatible defines this. Drop.


> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum: ["nopg", "ceasepg", "runonlypg"]
> +
> +  "#power-domain-cells":
> +    const: 0
> +
> +if:
> +  not:
> +    properties:
> +      compatible:
> +        contains:
> +          pattern: "^sifive,[tsc]mc3$"
> +then:
> +  properties:
> +    sifive,feature-level: false
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false

Missing example.

Best regards,
Krzysztof

Re: [PATCH] dt-bindings: power: Add SiFive Domain Management controllers

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4921 bytes --]

On Fri, May 09, 2025 at 08:40:28AM +0200, Krzysztof Kozlowski wrote:
> On Fri, May 09, 2025 at 10:16:04AM GMT, Nick Hu wrote:
> > SiFive Domain Management controller includes the following components
> > - SiFive Tile Management Controller
> > - SiFive Cluster Management Controller
> > - SiFive Core Complex Management Controller
> > 
> > These controllers control the clock and power domain of the
> > corresponding domain.
> > 
> > Signed-off-by: Nick Hu <nick.hu@sifive.com>
> > Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
> > ---
> >  .../devicetree/bindings/power/sifive,tmc.yaml | 89 +++++++++++++++++++
> 
> Where is a patch with the driver (user of the binding)?
> 
> >  1 file changed, 89 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/power/sifive,tmc.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/power/sifive,tmc.yaml b/Documentation/devicetree/bindings/power/sifive,tmc.yaml
> > new file mode 100644
> > index 000000000000..7ed4f290b94b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/sifive,tmc.yaml
> > @@ -0,0 +1,89 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/power/sifive,tmc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: SiFive Domain Management Controller
> > +
> > +maintainers:
> > +  - Cyan Yang <cyan.yang@sifive.com>
> > +  - Nick Hu <nick.hu@sifive.com>
> > +  - Samuel Holland <samuel.holland@sifive.com>
> > +
> > +description: |
> > +  This is the device tree binding for the following SiFive Domain Management Controllers.
> 
> Explain the hardware, not that "binding is a binding for ...".
> 
> Also, wrap according to Linux coding style.
> 
> 
> > +  - Tile Management Controller
> > +      - TMC0
> > +      - TMC1
> > +      - TMC2
> > +      - TMC3
> > +  - Subsystem Management Controller
> > +      - SMC0
> > +      - SMC1
> > +      - SMC2
> > +      - SMC3
> > +  - Cluster Management Controller
> > +      - CMC2
> > +      - CMC3
> > +  SiFive Domain Management Controllers support the SiFive Quiet Interface
> > +  Protocol (SQIP) starting from the Version 1. The control method is
> > +  different from the Version 0, making them incompatible.
> > +
> > +allOf:
> > +  - $ref: power-domain.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - items:
> > +          - {}
> > +          - pattern: "^sifive,[ts]mc0$"
> > +      - items:
> > +          - {}
> > +          - pattern: "^sifive,[ts]mc3$"
> > +          - pattern: "^sifive,[ts]mc2$"
> > +          - pattern: "^sifive,[ts]mc1$"
> > +      - items:
> > +          - {}
> > +          - pattern: "^sifive,[ts]mc2$"
> > +          - pattern: "^sifive,[ts]mc1$"
> > +      - items:
> > +          - {}
> > +          - pattern: "^sifive,[ts]mc1$"
> > +      - items:
> > +          - {}
> > +          - const: sifive,cmc3
> > +          - const: sifive,cmc2
> > +      - items:
> > +          - {}
> > +          - const: sifive,cmc2
> 
> All of this is just unexpected. Why any compatible should come with
> these?

It's also not quite correct either, right? Or may not be correct at
least. It permits "xxx", "tmc2", "smc1" and "xxx", "smc2", "tmc1"
which mean that the smc and tmc must be identical in terms of
programming model.

> You need to use SoC specific compatibles.

I think there's some slack to provide here, sifive are upstreaming it in
advance of there being customers (or customers ready to upstream) and this
format allows us to accept bindings/drivers and the customer will have
to add a soc-specific compatible in order to actually use these in a
dts. I think it's better to accept something along these lines than
stall out until a customer decides to upstream their user. That said, I
would expect this to come (as you mentioned above) with the driver.

> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  sifive,feature-level:
> > +    description: |
> > +      Supported power features. This property is absent if the full set of features
> > +      is supported
> 
> Compatible defines this. Drop.
> 
> 
> > +    $ref: /schemas/types.yaml#/definitions/string
> > +    enum: ["nopg", "ceasepg", "runonlypg"]
> > +
> > +  "#power-domain-cells":
> > +    const: 0
> > +
> > +if:
> > +  not:
> > +    properties:
> > +      compatible:
> > +        contains:
> > +          pattern: "^sifive,[tsc]mc3$"
> > +then:
> > +  properties:
> > +    sifive,feature-level: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> 
> Missing example.

You can't actually make an example that passes validation when the
soc-specific compatibles are not added, so this would require adding
some.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] dt-bindings: power: Add SiFive Domain Management controllers

[Krzysztof Kozlowski]

On 09/05/2025 17:57, Conor Dooley wrote:
>>> +          - pattern: "^sifive,[ts]mc1$"
>>> +      - items:
>>> +          - {}
>>> +          - const: sifive,cmc3
>>> +          - const: sifive,cmc2
>>> +      - items:
>>> +          - {}
>>> +          - const: sifive,cmc2
>>
>> All of this is just unexpected. Why any compatible should come with
>> these?
> 
> It's also not quite correct either, right? Or may not be correct at
> least. It permits "xxx", "tmc2", "smc1" and "xxx", "smc2", "tmc1"
> which mean that the smc and tmc must be identical in terms of
> programming model.

Yep

> 
>> You need to use SoC specific compatibles.
> 
> I think there's some slack to provide here, sifive are upstreaming it in
> advance of there being customers (or customers ready to upstream) and this
> format allows us to accept bindings/drivers and the customer will have
> to add a soc-specific compatible in order to actually use these in a
> dts. I think it's better to accept something along these lines than

Sure, commit msg should explain that and probably these {} wildcards
should have comment as well.

> stall out until a customer decides to upstream their user. That said, I
> would expect this to come (as you mentioned above) with the driver.
> 

Best regards,
Krzysztof

Re: [PATCH] dt-bindings: power: Add SiFive Domain Management controllers

[Nick Hu]

On Sat, May 10, 2025 at 10:57 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 09/05/2025 17:57, Conor Dooley wrote:
> >>> +          - pattern: "^sifive,[ts]mc1$"
> >>> +      - items:
> >>> +          - {}
> >>> +          - const: sifive,cmc3
> >>> +          - const: sifive,cmc2
> >>> +      - items:
> >>> +          - {}
> >>> +          - const: sifive,cmc2
> >>
> >> All of this is just unexpected. Why any compatible should come with
> >> these?
> >
> > It's also not quite correct either, right? Or may not be correct at
> > least. It permits "xxx", "tmc2", "smc1" and "xxx", "smc2", "tmc1"
> > which mean that the smc and tmc must be identical in terms of
> > programming model.
>
> Yep
>
> >
> >> You need to use SoC specific compatibles.
> >
> > I think there's some slack to provide here, sifive are upstreaming it in
> > advance of there being customers (or customers ready to upstream) and this
> > format allows us to accept bindings/drivers and the customer will have
> > to add a soc-specific compatible in order to actually use these in a
> > dts. I think it's better to accept something along these lines than
>
> Sure, commit msg should explain that and probably these {} wildcards
> should have comment as well.
>
I'll update it in the next version. Thanks.

> > stall out until a customer decides to upstream their user. That said, I
> > would expect this to come (as you mentioned above) with the driver.
> >
>
> Best regards,
> Krzysztof

Re: [PATCH] dt-bindings: power: Add SiFive Domain Management controllers

[Nick Hu]

On Fri, May 9, 2025 at 11:57 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Fri, May 09, 2025 at 08:40:28AM +0200, Krzysztof Kozlowski wrote:
> > On Fri, May 09, 2025 at 10:16:04AM GMT, Nick Hu wrote:
> > > SiFive Domain Management controller includes the following components
> > > - SiFive Tile Management Controller
> > > - SiFive Cluster Management Controller
> > > - SiFive Core Complex Management Controller
> > >
> > > These controllers control the clock and power domain of the
> > > corresponding domain.
> > >
> > > Signed-off-by: Nick Hu <nick.hu@sifive.com>
> > > Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
> > > ---
> > >  .../devicetree/bindings/power/sifive,tmc.yaml | 89 +++++++++++++++++++
> >
> > Where is a patch with the driver (user of the binding)?
> >
> > >  1 file changed, 89 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/power/sifive,tmc.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/power/sifive,tmc.yaml b/Documentation/devicetree/bindings/power/sifive,tmc.yaml
> > > new file mode 100644
> > > index 000000000000..7ed4f290b94b
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/power/sifive,tmc.yaml
> > > @@ -0,0 +1,89 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/power/sifive,tmc.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: SiFive Domain Management Controller
> > > +
> > > +maintainers:
> > > +  - Cyan Yang <cyan.yang@sifive.com>
> > > +  - Nick Hu <nick.hu@sifive.com>
> > > +  - Samuel Holland <samuel.holland@sifive.com>
> > > +
> > > +description: |
> > > +  This is the device tree binding for the following SiFive Domain Management Controllers.
> >
> > Explain the hardware, not that "binding is a binding for ...".
> >
> > Also, wrap according to Linux coding style.
> >
> >
> > > +  - Tile Management Controller
> > > +      - TMC0
> > > +      - TMC1
> > > +      - TMC2
> > > +      - TMC3
> > > +  - Subsystem Management Controller
> > > +      - SMC0
> > > +      - SMC1
> > > +      - SMC2
> > > +      - SMC3
> > > +  - Cluster Management Controller
> > > +      - CMC2
> > > +      - CMC3
> > > +  SiFive Domain Management Controllers support the SiFive Quiet Interface
> > > +  Protocol (SQIP) starting from the Version 1. The control method is
> > > +  different from the Version 0, making them incompatible.
> > > +
> > > +allOf:
> > > +  - $ref: power-domain.yaml#
> > > +
> > > +properties:
> > > +  compatible:
> > > +    oneOf:
> > > +      - items:
> > > +          - {}
> > > +          - pattern: "^sifive,[ts]mc0$"
> > > +      - items:
> > > +          - {}
> > > +          - pattern: "^sifive,[ts]mc3$"
> > > +          - pattern: "^sifive,[ts]mc2$"
> > > +          - pattern: "^sifive,[ts]mc1$"
> > > +      - items:
> > > +          - {}
> > > +          - pattern: "^sifive,[ts]mc2$"
> > > +          - pattern: "^sifive,[ts]mc1$"
> > > +      - items:
> > > +          - {}
> > > +          - pattern: "^sifive,[ts]mc1$"
> > > +      - items:
> > > +          - {}
> > > +          - const: sifive,cmc3
> > > +          - const: sifive,cmc2
> > > +      - items:
> > > +          - {}
> > > +          - const: sifive,cmc2
> >
> > All of this is just unexpected. Why any compatible should come with
> > these?
>
> It's also not quite correct either, right? Or may not be correct at
> least. It permits "xxx", "tmc2", "smc1" and "xxx", "smc2", "tmc1"
> which mean that the smc and tmc must be identical in terms of
> programming model.
>
You are right, I hadn't considered that case. I'll address it in the
next version.

> > You need to use SoC specific compatibles.
>
> I think there's some slack to provide here, sifive are upstreaming it in
> advance of there being customers (or customers ready to upstream) and this
> format allows us to accept bindings/drivers and the customer will have
> to add a soc-specific compatible in order to actually use these in a
> dts. I think it's better to accept something along these lines than
> stall out until a customer decides to upstream their user. That said, I
> would expect this to come (as you mentioned above) with the driver.
>
Thanks for the understanding =)
I'm hoping the driver can be submitted at a later stage.
The driver that handles the MMIO is implemented in OpenSBI and depends
on some prerequisite patches [1], so it will follow afterward.

Links:
- [1] https://lore.kernel.org/opensbi/CAKddAkD00gLgpzOCXY9mXaebr2qZcU9mkUGOZ4278w0bmiLuBQ@mail.gmail.com/T/#t

> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  sifive,feature-level:
> > > +    description: |
> > > +      Supported power features. This property is absent if the full set of features
> > > +      is supported
> >
> > Compatible defines this. Drop.
> >
> >
> > > +    $ref: /schemas/types.yaml#/definitions/string
> > > +    enum: ["nopg", "ceasepg", "runonlypg"]
> > > +
> > > +  "#power-domain-cells":
> > > +    const: 0
> > > +
> > > +if:
> > > +  not:
> > > +    properties:
> > > +      compatible:
> > > +        contains:
> > > +          pattern: "^sifive,[tsc]mc3$"
> > > +then:
> > > +  properties:
> > > +    sifive,feature-level: false
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +
> > > +additionalProperties: false
> >
> > Missing example.
>
> You can't actually make an example that passes validation when the
> soc-specific compatibles are not added, so this would require adding
> some.
I can add a comment here to note that the example should be included
once a SoC-specific compatible string is available.

Best Regards,
Nick

Re: [PATCH] dt-bindings: power: Add SiFive Domain Management controllers

[Nick Hu]

On Fri, May 9, 2025 at 2:40 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Fri, May 09, 2025 at 10:16:04AM GMT, Nick Hu wrote:
> > SiFive Domain Management controller includes the following components
> > - SiFive Tile Management Controller
> > - SiFive Cluster Management Controller
> > - SiFive Core Complex Management Controller
> >
> > These controllers control the clock and power domain of the
> > corresponding domain.
> >
> > Signed-off-by: Nick Hu <nick.hu@sifive.com>
> > Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
> > ---
> >  .../devicetree/bindings/power/sifive,tmc.yaml | 89 +++++++++++++++++++
>
> Where is a patch with the driver (user of the binding)?
>
We are hoping the driver can be submitted at a later stage.
The driver that handles the MMIO is implemented in OpenSBI and depends
on some prerequisite patches [1], so it will follow afterward.

Links:
- [1] https://lore.kernel.org/opensbi/CAKddAkD00gLgpzOCXY9mXaebr2qZcU9mkUGOZ4278w0bmiLuBQ@mail.gmail.com/T/#t

> >  1 file changed, 89 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/power/sifive,tmc.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/power/sifive,tmc.yaml b/Documentation/devicetree/bindings/power/sifive,tmc.yaml
> > new file mode 100644
> > index 000000000000..7ed4f290b94b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/sifive,tmc.yaml
> > @@ -0,0 +1,89 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/power/sifive,tmc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: SiFive Domain Management Controller
> > +
> > +maintainers:
> > +  - Cyan Yang <cyan.yang@sifive.com>
> > +  - Nick Hu <nick.hu@sifive.com>
> > +  - Samuel Holland <samuel.holland@sifive.com>
> > +
> > +description: |
> > +  This is the device tree binding for the following SiFive Domain Management Controllers.
>
> Explain the hardware, not that "binding is a binding for ...".
>
> Also, wrap according to Linux coding style.
>
Thanks for the advice. I'll address it in the next version =)

> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  sifive,feature-level:
> > +    description: |
> > +      Supported power features. This property is absent if the full set of features
> > +      is supported
>
> Compatible defines this. Drop.
>
The property depends on how the IP is hooked up to the rest of the SoC.
Having this property simplifies the SW and allows us to use a single
fallback compatible string, so we prefer to keep it.

Best Regards,
Nick

Re: [PATCH] dt-bindings: power: Add SiFive Domain Management controllers

[Krzysztof Kozlowski]

On 12/05/2025 05:20, Nick Hu wrote:
> On Fri, May 9, 2025 at 2:40 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On Fri, May 09, 2025 at 10:16:04AM GMT, Nick Hu wrote:
>>> SiFive Domain Management controller includes the following components
>>> - SiFive Tile Management Controller
>>> - SiFive Cluster Management Controller
>>> - SiFive Core Complex Management Controller
>>>
>>> These controllers control the clock and power domain of the
>>> corresponding domain.
>>>
>>> Signed-off-by: Nick Hu <nick.hu@sifive.com>
>>> Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
>>> ---
>>>  .../devicetree/bindings/power/sifive,tmc.yaml | 89 +++++++++++++++++++
>>
>> Where is a patch with the driver (user of the binding)?
>>
> We are hoping the driver can be submitted at a later stage.
> The driver that handles the MMIO is implemented in OpenSBI and depends
> on some prerequisite patches [1], so it will follow afterward.

This patch alone makes little sense and brings little benefit. Post this
with user.

...

>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  sifive,feature-level:
>>> +    description: |
>>> +      Supported power features. This property is absent if the full set of features
>>> +      is supported
>>
>> Compatible defines this. Drop.
>>
> The property depends on how the IP is hooked up to the rest of the SoC.
> Having this property simplifies the SW and allows us to use a single
> fallback compatible string, so we prefer to keep it.

And we prefer you to follow standard DT rules, see writing bindings or
talks on conferences.


Best regards,
Krzysztof

Re: [PATCH] dt-bindings: power: Add SiFive Domain Management controllers

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1328 bytes --]

On Mon, May 12, 2025 at 12:39:37PM +0200, Krzysztof Kozlowski wrote:
> On 12/05/2025 05:20, Nick Hu wrote:
> > On Fri, May 9, 2025 at 2:40 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> On Fri, May 09, 2025 at 10:16:04AM GMT, Nick Hu wrote:
> >>> SiFive Domain Management controller includes the following components
> >>> - SiFive Tile Management Controller
> >>> - SiFive Cluster Management Controller
> >>> - SiFive Core Complex Management Controller
> >>>
> >>> These controllers control the clock and power domain of the
> >>> corresponding domain.
> >>>
> >>> Signed-off-by: Nick Hu <nick.hu@sifive.com>
> >>> Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
> >>> ---
> >>>  .../devicetree/bindings/power/sifive,tmc.yaml | 89 +++++++++++++++++++
> >>
> >> Where is a patch with the driver (user of the binding)?
> >>
> > We are hoping the driver can be submitted at a later stage.
> > The driver that handles the MMIO is implemented in OpenSBI and depends
> > on some prerequisite patches [1], so it will follow afterward.
> 
> This patch alone makes little sense and brings little benefit. Post this
> with user.

Unless the justification is that they're submitting a driver to OpenSBI
that uses this binding, but in that case link it in the commit message
and explain.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] dt-bindings: power: Add SiFive Domain Management controllers

[Nick Hu]

On Mon, May 12, 2025 at 6:39 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 12/05/2025 05:20, Nick Hu wrote:
> > On Fri, May 9, 2025 at 2:40 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> On Fri, May 09, 2025 at 10:16:04AM GMT, Nick Hu wrote:
> >>> SiFive Domain Management controller includes the following components
> >>> - SiFive Tile Management Controller
> >>> - SiFive Cluster Management Controller
> >>> - SiFive Core Complex Management Controller
> >>>
> >>> These controllers control the clock and power domain of the
> >>> corresponding domain.
> >>>
> >>> Signed-off-by: Nick Hu <nick.hu@sifive.com>
> >>> Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
> >>> ---
> >>>  .../devicetree/bindings/power/sifive,tmc.yaml | 89 +++++++++++++++++++
> >>
> >> Where is a patch with the driver (user of the binding)?
> >>
> > We are hoping the driver can be submitted at a later stage.
> > The driver that handles the MMIO is implemented in OpenSBI and depends
> > on some prerequisite patches [1], so it will follow afterward.
>
> This patch alone makes little sense and brings little benefit. Post this
> with user.
>
Thanks. Will update it with the user.

> ...
>
> >>> +  reg:
> >>> +    maxItems: 1
> >>> +
> >>> +  sifive,feature-level:
> >>> +    description: |
> >>> +      Supported power features. This property is absent if the full set of features
> >>> +      is supported
> >>
> >> Compatible defines this. Drop.
> >>
> > The property depends on how the IP is hooked up to the rest of the SoC.
> > Having this property simplifies the SW and allows us to use a single
> > fallback compatible string, so we prefer to keep it.
>
> And we prefer you to follow standard DT rules, see writing bindings or
> talks on conferences.
>
The CPU also exposes its D-cache size, which could technically be
inferred from the compatible string. However, it chose to specify it
explicitly using the `d-cache-size` property to allow reuse of a
common initialization function across different CPUs.
Similarly, while the `sifive,feature-level` could be inferred from the
compatible string, defining it as a property would enable all
`sifive,tmcX` users to share the same operation code, improving code
reuse and maintainability.

If you still disagree with this approach, we’ll drop the
`sifive,feature-level` property in the next revision.
Looking forward to hearing your input.

Best Regards,
Nick