diff mbox series

[v5,2/7] dt-bindings: soc: starfive: Add StarFive syscon module

Message ID 20230613125852.211636-3-xingyu.wu@starfivetech.com
State Changes Requested, archived
Headers show
Series Add PLL clocks driver and syscon for StarFive JH7110 SoC | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Xingyu Wu June 13, 2023, 12:58 p.m. UTC
From: William Qiu <william.qiu@starfivetech.com>

Add documentation to describe StarFive System Controller Registers.

Co-developed-by: Xingyu Wu <xingyu.wu@starfivetech.com>
Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
Signed-off-by: William Qiu <william.qiu@starfivetech.com>
---
 .../soc/starfive/starfive,jh7110-syscon.yaml  | 62 +++++++++++++++++++
 MAINTAINERS                                   |  7 +++
 2 files changed, 69 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml

Comments

Krzysztof Kozlowski June 13, 2023, 6:31 p.m. UTC | #1
On 13/06/2023 14:58, Xingyu Wu wrote:
> From: William Qiu <william.qiu@starfivetech.com>
> 
> Add documentation to describe StarFive System Controller Registers.
> 
> Co-developed-by: Xingyu Wu <xingyu.wu@starfivetech.com>
> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> ---
>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 62 +++++++++++++++++++
>  MAINTAINERS                                   |  7 +++
>  2 files changed, 69 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> 
> diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> new file mode 100644
> index 000000000000..a81190f8a54d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/soc/starfive/starfive,jh7110-syscon.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive JH7110 SoC system controller
> +
> +maintainers:
> +  - William Qiu <william.qiu@starfivetech.com>
> +
> +description: |
> +  The StarFive JH7110 SoC system controller provides register information such
> +  as offset, mask and shift to configure related modules such as MMC and PCIe.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - const: starfive,jh7110-sys-syscon
> +          - const: syscon
> +          - const: simple-mfd
> +      - items:
> +          - enum:
> +              - starfive,jh7110-aon-syscon
> +              - starfive,jh7110-stg-syscon
> +          - const: syscon
> +
> +  reg:
> +    maxItems: 1
> +
> +  clock-controller:
> +    $ref: /schemas/clock/starfive,jh7110-pll.yaml#
> +    type: object
> +
> +  "#power-domain-cells":
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: starfive,jh7110-aon-syscon
> +    then:
> +      required:
> +        - "#power-domain-cells"

Where did you implement the results of the discussion that only some
devices can have power and clock controller?

According to your code all of above - sys, aon and stg - have clock and
power controllers. If not, then the code is not correct, so please do
not respond with what is where (like you did last time) but actually
implement what you say.

Best regards,
Krzysztof
Krzysztof Kozlowski June 13, 2023, 6:32 p.m. UTC | #2
On 13/06/2023 14:58, Xingyu Wu wrote:
> From: William Qiu <william.qiu@starfivetech.com>
> 
> Add documentation to describe StarFive System Controller Registers.
> 
> Co-developed-by: Xingyu Wu <xingyu.wu@starfivetech.com>
> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> ---
>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 62 +++++++++++++++++++
>  MAINTAINERS                                   |  7 +++
>  2 files changed, 69 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> 
> diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> new file mode 100644
> index 000000000000..a81190f8a54d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/soc/starfive/starfive,jh7110-syscon.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive JH7110 SoC system controller
> +
> +maintainers:
> +  - William Qiu <william.qiu@starfivetech.com>
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  The StarFive JH7110 SoC system controller provides register information such
> +  as offset, mask and shift to configure related modules such as MMC and PCIe.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - const: starfive,jh7110-sys-syscon
> +          - const: syscon
> +          - const: simple-mfd
> +      - items:
> +          - enum:
> +              - starfive,jh7110-aon-syscon
> +              - starfive,jh7110-stg-syscon
> +          - const: syscon
> +
> +  reg:
> +    maxItems: 1
> +
> +  clock-controller:
> +    $ref: /schemas/clock/starfive,jh7110-pll.yaml#
> +    type: object
> +
> +  "#power-domain-cells":
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: starfive,jh7110-aon-syscon
> +    then:
> +      required:
> +        - "#power-domain-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    syscon@10240000 {
> +        compatible = "starfive,jh7110-stg-syscon", "syscon";
> +        reg = <0x10240000 0x1000>;

Extend example - add clock controller, add power-domains to STG (since
it has them, as you claim in the binding).

Make this one example complete.

Best regards,
Krzysztof
Xingyu Wu June 28, 2023, 6:44 a.m. UTC | #3
On 2023/6/14 2:31, Krzysztof Kozlowski wrote:
> On 13/06/2023 14:58, Xingyu Wu wrote:
>> From: William Qiu <william.qiu@starfivetech.com>
>> 
>> Add documentation to describe StarFive System Controller Registers.
>> 
>> Co-developed-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
>> ---
>>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 62 +++++++++++++++++++
>>  MAINTAINERS                                   |  7 +++
>>  2 files changed, 69 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>> new file mode 100644
>> index 000000000000..a81190f8a54d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>> @@ -0,0 +1,62 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/soc/starfive/starfive,jh7110-syscon.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: StarFive JH7110 SoC system controller
>> +
>> +maintainers:
>> +  - William Qiu <william.qiu@starfivetech.com>
>> +
>> +description: |
>> +  The StarFive JH7110 SoC system controller provides register information such
>> +  as offset, mask and shift to configure related modules such as MMC and PCIe.
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - items:
>> +          - const: starfive,jh7110-sys-syscon
>> +          - const: syscon
>> +          - const: simple-mfd
>> +      - items:
>> +          - enum:
>> +              - starfive,jh7110-aon-syscon
>> +              - starfive,jh7110-stg-syscon
>> +          - const: syscon
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clock-controller:
>> +    $ref: /schemas/clock/starfive,jh7110-pll.yaml#
>> +    type: object
>> +
>> +  "#power-domain-cells":
>> +    const: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: starfive,jh7110-aon-syscon
>> +    then:
>> +      required:
>> +        - "#power-domain-cells"
> 
> Where did you implement the results of the discussion that only some
> devices can have power and clock controller?
> 
> According to your code all of above - sys, aon and stg - have clock and
> power controllers. If not, then the code is not correct, so please do
> not respond with what is where (like you did last time) but actually
> implement what you say.
> 

Hi Krzysztof, I need to modify the code to implement it.
If I drop the 'clock-controller' and '"#power-domain-cells"' in properites, and change to this:

--- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
+++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
@@ -29,28 +29,33 @@ properties:
   reg:
     maxItems: 1
 
-  clock-controller:
-    $ref: /schemas/clock/starfive,jh7110-pll.yaml#
-    type: object
-
-  "#power-domain-cells":
-    const: 1
-
 required:
   - compatible
   - reg
 
 allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: starfive,jh7110-sys-syscon
+    then:
+      properties:
+        clock-controller:
+          $ref: /schemas/clock/starfive,jh7110-pll.yaml#
+          type: object
+
   - if:
       properties:
         compatible:
           contains:
             const: starfive,jh7110-aon-syscon
     then:
-      required:
-        - "#power-domain-cells"
+      properties:
+        "#power-domain-cells":
+          const: 1
 
-additionalProperties: false
+additionalProperties: true


Would it be better to show that sys-syscon only has clock-controller and aon-syscon is power controller?

Best regards,
Xingyu Wu
Conor Dooley June 28, 2023, 5:34 p.m. UTC | #4
On Wed, Jun 28, 2023 at 02:44:10PM +0800, Xingyu Wu wrote:
> On 2023/6/14 2:31, Krzysztof Kozlowski wrote:
> > On 13/06/2023 14:58, Xingyu Wu wrote:
> >> From: William Qiu <william.qiu@starfivetech.com>
> >> 
> >> Add documentation to describe StarFive System Controller Registers.
> >> 
> >> Co-developed-by: Xingyu Wu <xingyu.wu@starfivetech.com>
> >> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
> >> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> >> ---
> >>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 62 +++++++++++++++++++
> >>  MAINTAINERS                                   |  7 +++
> >>  2 files changed, 69 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> >> 
> >> diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> >> new file mode 100644
> >> index 000000000000..a81190f8a54d
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> >> @@ -0,0 +1,62 @@
> >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/soc/starfive/starfive,jh7110-syscon.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: StarFive JH7110 SoC system controller
> >> +
> >> +maintainers:
> >> +  - William Qiu <william.qiu@starfivetech.com>
> >> +
> >> +description: |
> >> +  The StarFive JH7110 SoC system controller provides register information such
> >> +  as offset, mask and shift to configure related modules such as MMC and PCIe.
> >> +
> >> +properties:
> >> +  compatible:
> >> +    oneOf:
> >> +      - items:
> >> +          - const: starfive,jh7110-sys-syscon
> >> +          - const: syscon
> >> +          - const: simple-mfd
> >> +      - items:
> >> +          - enum:
> >> +              - starfive,jh7110-aon-syscon
> >> +              - starfive,jh7110-stg-syscon
> >> +          - const: syscon
> >> +
> >> +  reg:
> >> +    maxItems: 1
> >> +
> >> +  clock-controller:
> >> +    $ref: /schemas/clock/starfive,jh7110-pll.yaml#
> >> +    type: object
> >> +
> >> +  "#power-domain-cells":
> >> +    const: 1
> >> +
> >> +required:
> >> +  - compatible
> >> +  - reg
> >> +
> >> +allOf:
> >> +  - if:
> >> +      properties:
> >> +        compatible:
> >> +          contains:
> >> +            const: starfive,jh7110-aon-syscon
> >> +    then:
> >> +      required:
> >> +        - "#power-domain-cells"
> > 
> > Where did you implement the results of the discussion that only some
> > devices can have power and clock controller?
> > 
> > According to your code all of above - sys, aon and stg - have clock and
> > power controllers. If not, then the code is not correct, so please do
> > not respond with what is where (like you did last time) but actually
> > implement what you say.
> > 
> 
> Hi Krzysztof, I need to modify the code to implement it.
> If I drop the 'clock-controller' and '"#power-domain-cells"' in properites, and change to this:
> 
> --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> @@ -29,28 +29,33 @@ properties:
>    reg:
>      maxItems: 1
>  
> -  clock-controller:
> -    $ref: /schemas/clock/starfive,jh7110-pll.yaml#
> -    type: object
> -
> -  "#power-domain-cells":
> -    const: 1
> -
>  required:
>    - compatible
>    - reg
>  
>  allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: starfive,jh7110-sys-syscon
> +    then:
> +      properties:
> +        clock-controller:
> +          $ref: /schemas/clock/starfive,jh7110-pll.yaml#
> +          type: object

Why do this?
Why not define the property has you have been doing, but only allow it
on the syscons that support it?
See the section starting at L205 of example-schema.yaml.

> +
>    - if:
>        properties:
>          compatible:
>            contains:
>              const: starfive,jh7110-aon-syscon
>      then:
> -      required:
> -        - "#power-domain-cells"
> +      properties:
> +        "#power-domain-cells":
> +          const: 1
>  

> -additionalProperties: false
> +additionalProperties: true

Why do you need this?
Allowing "additionalProperties: true" sounds like you've got some prblem
that you are trying to hide...

> Would it be better to show that sys-syscon only has clock-controller and aon-syscon is power controller?

You should only permit the properties where they are valid, yes.

Cheers,
Conor.
Xingyu Wu June 29, 2023, 6:42 a.m. UTC | #5
On 2023/6/29 1:34, Conor Dooley wrote:
> On Wed, Jun 28, 2023 at 02:44:10PM +0800, Xingyu Wu wrote:
>> On 2023/6/14 2:31, Krzysztof Kozlowski wrote:
>> > On 13/06/2023 14:58, Xingyu Wu wrote:
>> >> From: William Qiu <william.qiu@starfivetech.com>
>> >> 
>> >> Add documentation to describe StarFive System Controller Registers.
>> >> 
>> >> Co-developed-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>> >> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>> >> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
>> >> ---
>> >>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 62 +++++++++++++++++++
>> >>  MAINTAINERS                                   |  7 +++
>> >>  2 files changed, 69 insertions(+)
>> >>  create mode 100644 Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>> >> 
>> >> diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>> >> new file mode 100644
>> >> index 000000000000..a81190f8a54d
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>> >> @@ -0,0 +1,62 @@
>> >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> >> +%YAML 1.2
>> >> +---
>> >> +$id: http://devicetree.org/schemas/soc/starfive/starfive,jh7110-syscon.yaml#
>> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> >> +
>> >> +title: StarFive JH7110 SoC system controller
>> >> +
>> >> +maintainers:
>> >> +  - William Qiu <william.qiu@starfivetech.com>
>> >> +
>> >> +description: |
>> >> +  The StarFive JH7110 SoC system controller provides register information such
>> >> +  as offset, mask and shift to configure related modules such as MMC and PCIe.
>> >> +
>> >> +properties:
>> >> +  compatible:
>> >> +    oneOf:
>> >> +      - items:
>> >> +          - const: starfive,jh7110-sys-syscon
>> >> +          - const: syscon
>> >> +          - const: simple-mfd
>> >> +      - items:
>> >> +          - enum:
>> >> +              - starfive,jh7110-aon-syscon
>> >> +              - starfive,jh7110-stg-syscon
>> >> +          - const: syscon
>> >> +
>> >> +  reg:
>> >> +    maxItems: 1
>> >> +
>> >> +  clock-controller:
>> >> +    $ref: /schemas/clock/starfive,jh7110-pll.yaml#
>> >> +    type: object
>> >> +
>> >> +  "#power-domain-cells":
>> >> +    const: 1
>> >> +
>> >> +required:
>> >> +  - compatible
>> >> +  - reg
>> >> +
>> >> +allOf:
>> >> +  - if:
>> >> +      properties:
>> >> +        compatible:
>> >> +          contains:
>> >> +            const: starfive,jh7110-aon-syscon
>> >> +    then:
>> >> +      required:
>> >> +        - "#power-domain-cells"
>> > 
>> > Where did you implement the results of the discussion that only some
>> > devices can have power and clock controller?
>> > 
>> > According to your code all of above - sys, aon and stg - have clock and
>> > power controllers. If not, then the code is not correct, so please do
>> > not respond with what is where (like you did last time) but actually
>> > implement what you say.
>> > 
>> 
>> Hi Krzysztof, I need to modify the code to implement it.
>> If I drop the 'clock-controller' and '"#power-domain-cells"' in properites, and change to this:
>> 
>> --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>> @@ -29,28 +29,33 @@ properties:
>>    reg:
>>      maxItems: 1
>>  
>> -  clock-controller:
>> -    $ref: /schemas/clock/starfive,jh7110-pll.yaml#
>> -    type: object
>> -
>> -  "#power-domain-cells":
>> -    const: 1
>> -
>>  required:
>>    - compatible
>>    - reg
>>  
>>  allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: starfive,jh7110-sys-syscon
>> +    then:
>> +      properties:
>> +        clock-controller:
>> +          $ref: /schemas/clock/starfive,jh7110-pll.yaml#
>> +          type: object
> 
> Why do this?
> Why not define the property has you have been doing, but only allow it
> on the syscons that support it?
> See the section starting at L205 of example-schema.yaml.
> 
>> +
>>    - if:
>>        properties:
>>          compatible:
>>            contains:
>>              const: starfive,jh7110-aon-syscon
>>      then:
>> -      required:
>> -        - "#power-domain-cells"
>> +      properties:
>> +        "#power-domain-cells":
>> +          const: 1
>>  
> 
>> -additionalProperties: false
>> +additionalProperties: true
> 
> Why do you need this?
> Allowing "additionalProperties: true" sounds like you've got some prblem
> that you are trying to hide...
> 
>> Would it be better to show that sys-syscon only has clock-controller and aon-syscon is power controller?
> 
> You should only permit the properties where they are valid, yes.
> 

Yeah, following your advice, I modified the codes and there are two options:

--- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
+++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
@@ -41,6 +41,16 @@ required:
   - reg
 
 allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: starfive,jh7110-sys-syscon
+    then:
+      required:
+        - clock-controller
+      properties:
+        "#power-domain-cells": false
   - if:
       properties:
         compatible:
           contains:
             const: starfive,jh7110-aon-syscon
     then:
       required:
         - "#power-domain-cells"
+      properties:
+        clock-controller: false
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: starfive,jh7110-stg-syscon
+    then:
+      properties:
+        clock-controller: false
+        "#power-domain-cells": false
 
 additionalProperties: false

Or :

--- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
+++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
@@ -41,6 +41,17 @@ required:
   - reg
 
 allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: starfive,jh7110-sys-syscon
+    then:
+      required:
+        - clock-controller
+    else:
+      properties:
+        clock-controller: false
   - if:
       properties:
         compatible:
           contains:
             const: starfive,jh7110-aon-syscon
     then:
       required:
         - "#power-domain-cells"
+    else:
+      properties:
+        "#power-domain-cells": false
 
 additionalProperties: false

Which one is better? Thanks.

Best regards,
Xingyu Wu
Conor Dooley June 29, 2023, 9:01 a.m. UTC | #6
On Thu, Jun 29, 2023 at 02:42:39PM +0800, Xingyu Wu wrote:
> On 2023/6/29 1:34, Conor Dooley wrote:
> > On Wed, Jun 28, 2023 at 02:44:10PM +0800, Xingyu Wu wrote:
> >> On 2023/6/14 2:31, Krzysztof Kozlowski wrote:
> >> > On 13/06/2023 14:58, Xingyu Wu wrote:
> >> >> From: William Qiu <william.qiu@starfivetech.com>

> >> >> +allOf:
> >> >> +  - if:
> >> >> +      properties:
> >> >> +        compatible:
> >> >> +          contains:
> >> >> +            const: starfive,jh7110-aon-syscon
> >> >> +    then:
> >> >> +      required:
> >> >> +        - "#power-domain-cells"
> >> > 
> >> > Where did you implement the results of the discussion that only some
> >> > devices can have power and clock controller?
> >> > 
> >> > According to your code all of above - sys, aon and stg - have clock and
> >> > power controllers. If not, then the code is not correct, so please do
> >> > not respond with what is where (like you did last time) but actually
> >> > implement what you say.

> Yeah, following your advice, I modified the codes and there are two options:
> 
> --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> @@ -41,6 +41,16 @@ required:
>    - reg
>  
>  allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: starfive,jh7110-sys-syscon
> +    then:
> +      required:
> +        - clock-controller
> +      properties:
> +        "#power-domain-cells": false
>    - if:
>        properties:
>          compatible:
>            contains:
>              const: starfive,jh7110-aon-syscon
>      then:
>        required:
>          - "#power-domain-cells"
> +      properties:
> +        clock-controller: false
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: starfive,jh7110-stg-syscon
> +    then:
> +      properties:
> +        clock-controller: false
> +        "#power-domain-cells": false
>  
>  additionalProperties: false
> 
> Or :
> 
> --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> @@ -41,6 +41,17 @@ required:
>    - reg
>  
>  allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: starfive,jh7110-sys-syscon
> +    then:
> +      required:
> +        - clock-controller
> +    else:
> +      properties:
> +        clock-controller: false
>    - if:
>        properties:
>          compatible:
>            contains:
>              const: starfive,jh7110-aon-syscon
>      then:
>        required:
>          - "#power-domain-cells"
> +    else:
> +      properties:
> +        "#power-domain-cells": false
>  
>  additionalProperties: false
> 
> Which one is better? Thanks.

This second one looks better to me, as it achieves the same thing in a
simpler way.

Cheers,
Conor.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
new file mode 100644
index 000000000000..a81190f8a54d
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
@@ -0,0 +1,62 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/starfive/starfive,jh7110-syscon.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive JH7110 SoC system controller
+
+maintainers:
+  - William Qiu <william.qiu@starfivetech.com>
+
+description: |
+  The StarFive JH7110 SoC system controller provides register information such
+  as offset, mask and shift to configure related modules such as MMC and PCIe.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - const: starfive,jh7110-sys-syscon
+          - const: syscon
+          - const: simple-mfd
+      - items:
+          - enum:
+              - starfive,jh7110-aon-syscon
+              - starfive,jh7110-stg-syscon
+          - const: syscon
+
+  reg:
+    maxItems: 1
+
+  clock-controller:
+    $ref: /schemas/clock/starfive,jh7110-pll.yaml#
+    type: object
+
+  "#power-domain-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: starfive,jh7110-aon-syscon
+    then:
+      required:
+        - "#power-domain-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    syscon@10240000 {
+        compatible = "starfive,jh7110-stg-syscon", "syscon";
+        reg = <0x10240000 0x1000>;
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index f794002a192e..ae037ba7fc47 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20122,6 +20122,12 @@  S:	Supported
 F:	Documentation/devicetree/bindings/mmc/starfive*
 F:	drivers/mmc/host/dw_mmc-starfive.c
 
+STARFIVE JH7110 SYSCON
+M:	William Qiu <william.qiu@starfivetech.com>
+M:	Xingyu Wu <xingyu.wu@starfivetech.com>
+S:	Supported
+F:	Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
+
 STARFIVE JH71X0 CLOCK DRIVERS
 M:	Emil Renner Berthing <kernel@esmil.dk>
 M:	Hal Feng <hal.feng@starfivetech.com>
@@ -20159,6 +20165,7 @@  STARFIVE SOC DRIVERS
 M:	Conor Dooley <conor@kernel.org>
 S:	Maintained
 T:	git https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/
+F:	Documentation/devicetree/bindings/soc/starfive/
 F:	drivers/soc/starfive/
 
 STARFIVE TRNG DRIVER