diff mbox series

[v4,5/7] dt-bindings: soc: starfive: Add StarFive syscon module

Message ID 20230512022036.97987-6-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/dt-meta-schema fail build log

Commit Message

Xingyu Wu May 12, 2023, 2:20 a.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>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: William Qiu <william.qiu@starfivetech.com>
---
 .../soc/starfive/starfive,jh7110-syscon.yaml  | 67 +++++++++++++++++++
 MAINTAINERS                                   |  7 ++
 2 files changed, 74 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml

Comments

Krzysztof Kozlowski May 12, 2023, 6:35 a.m. UTC | #1 
On 12/05/2023 04:20, 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>
> Reviewed-by: Rob Herring <robh@kernel.org>

You made significant changes. Explain them in changelog here and drop
the tag.

> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> ---
>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 67 +++++++++++++++++++
>  MAINTAINERS                                   |  7 ++
>  2 files changed, 74 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..26dc99cb0c89
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> @@ -0,0 +1,67 @@
> +# 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

Add it to the existing examples.

This part confuses me... why aon appeared here?  Why power-controller
disappeared? I don't think that Rob or me proposed any of this.

> +
> +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>;
> +    };
> +
> +    syscon@13030000 {
> +        compatible = "starfive,jh7110-sys-syscon", "syscon", "simple-mfd";
> +        reg = <0x13030000 0x1000>;

Why simple-mfd? You do not have any children here.



Best regards,
Krzysztof
Conor Dooley May 12, 2023, 6:43 a.m. UTC | #2 
On Fri, May 12, 2023 at 08:35:43AM +0200, Krzysztof Kozlowski wrote:
> On 12/05/2023 04:20, Xingyu Wu wrote:
> > From: William Qiu <william.qiu@starfivetech.com>

> > +  "#power-domain-cells":
> > +    const: 1
> 
> Add it to the existing examples.
> 
> This part confuses me... why aon appeared here?  Why power-controller
> disappeared? I don't think that Rob or me proposed any of this.

Rob did actually suggest this, as the power-controller child node had no
properties other than #power-domain-cells.
Krzysztof Kozlowski May 12, 2023, 6:50 a.m. UTC | #3 
On 12/05/2023 08:43, Conor Dooley wrote:
> On Fri, May 12, 2023 at 08:35:43AM +0200, Krzysztof Kozlowski wrote:
>> On 12/05/2023 04:20, Xingyu Wu wrote:
>>> From: William Qiu <william.qiu@starfivetech.com>
> 
>>> +  "#power-domain-cells":
>>> +    const: 1
>>
>> Add it to the existing examples.
>>
>> This part confuses me... why aon appeared here?  Why power-controller
>> disappeared? I don't think that Rob or me proposed any of this.
> 
> Rob did actually suggest this, as the power-controller child node had no
> properties other than #power-domain-cells.

He suggested it for aon, but not for stg or sys... aon is not a child of
sys, is it? Then why power-controller disappeared from sys?

Best regards,
Krzysztof
Krzysztof Kozlowski May 12, 2023, 6:50 a.m. UTC | #4 
On Fri, 12 May 2023 10:20:34 +0800, 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>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> ---
>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 67 +++++++++++++++++++
>  MAINTAINERS                                   |  7 ++
>  2 files changed, 74 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/clock/starfive,jh7110-pll.yaml

See https://patchwork.ozlabs.org/patch/1780353

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
Xingyu Wu May 12, 2023, 7:24 a.m. UTC | #5 
On 2023/5/12 14:50, Krzysztof Kozlowski wrote:
> On 12/05/2023 08:43, Conor Dooley wrote:
>> On Fri, May 12, 2023 at 08:35:43AM +0200, Krzysztof Kozlowski wrote:
>>> On 12/05/2023 04:20, Xingyu Wu wrote:
>>>> From: William Qiu <william.qiu@starfivetech.com>
>> 
>>>> +  "#power-domain-cells":
>>>> +    const: 1
>>>
>>> Add it to the existing examples.
>>>
>>> This part confuses me... why aon appeared here?  Why power-controller
>>> disappeared? I don't think that Rob or me proposed any of this.
>> 
>> Rob did actually suggest this, as the power-controller child node had no
>> properties other than #power-domain-cells.
> 
> He suggested it for aon, but not for stg or sys... aon is not a child of
> sys, is it? Then why power-controller disappeared from sys?
> 

The power-controller is only for aon, but now just use power-domain-cells instead.
The sys only have the clock-controller child node not power-controller.
And stg has neither.

Best regards,
Xingyu Wu
Krzysztof Kozlowski May 12, 2023, 7:34 a.m. UTC | #6 
On 12/05/2023 09:24, Xingyu Wu wrote:
> On 2023/5/12 14:50, Krzysztof Kozlowski wrote:
>> On 12/05/2023 08:43, Conor Dooley wrote:
>>> On Fri, May 12, 2023 at 08:35:43AM +0200, Krzysztof Kozlowski wrote:
>>>> On 12/05/2023 04:20, Xingyu Wu wrote:
>>>>> From: William Qiu <william.qiu@starfivetech.com>
>>>
>>>>> +  "#power-domain-cells":
>>>>> +    const: 1
>>>>
>>>> Add it to the existing examples.
>>>>
>>>> This part confuses me... why aon appeared here?  Why power-controller
>>>> disappeared? I don't think that Rob or me proposed any of this.
>>>
>>> Rob did actually suggest this, as the power-controller child node had no
>>> properties other than #power-domain-cells.
>>
>> He suggested it for aon, but not for stg or sys... aon is not a child of
>> sys, is it? Then why power-controller disappeared from sys?
>>
> 
> The power-controller is only for aon, but now just use power-domain-cells instead.
> The sys only have the clock-controller child node not power-controller.
> And stg has neither.

OK, I see. Stuffing all of them in one binding suggests that anything
can be anything, but you actually have different devices with different
features/roles.

Best regards,
Krzysztof
Xingyu Wu May 12, 2023, 7:51 a.m. UTC | #7 
On 2023/5/12 14:50, Krzysztof Kozlowski wrote:
> On Fri, 12 May 2023 10:20:34 +0800, 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>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
>> ---
>>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 67 +++++++++++++++++++
>>  MAINTAINERS                                   |  7 ++
>>  2 files changed, 74 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>> 
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> ./Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/clock/starfive,jh7110-pll.yaml
> 
> See https://patchwork.ozlabs.org/patch/1780353
> 
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit.

This patch need patch 1 about pll clock driver binding.
Do I need to merge syscon binding and pll binding together?

Best regards,
Xingyu Wu
Krzysztof Kozlowski May 12, 2023, 4:15 p.m. UTC | #8 
On 12/05/2023 09:51, Xingyu Wu wrote:
> On 2023/5/12 14:50, Krzysztof Kozlowski wrote:
>> On Fri, 12 May 2023 10:20:34 +0800, 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>
>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
>>> ---
>>>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 67 +++++++++++++++++++
>>>  MAINTAINERS                                   |  7 ++
>>>  2 files changed, 74 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>
>>
>> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
>> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>>
>> yamllint warnings/errors:
>>
>> dtschema/dtc warnings/errors:
>> ./Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/clock/starfive,jh7110-pll.yaml
>>
>> See https://patchwork.ozlabs.org/patch/1780353
>>
>> This check can fail if there are any dependencies. The base for a patch
>> series is generally the most recent rc1.
>>
>> If you already ran 'make dt_binding_check' and didn't see the above
>> error(s), then make sure 'yamllint' is installed and dt-schema is up to
>> date:
>>
>> pip3 install dtschema --upgrade
>>
>> Please check and re-submit.
> 
> This patch need patch 1 about pll clock driver binding.
> Do I need to merge syscon binding and pll binding together?

No, bot has hickups due to failures in Linus' tree, thus false positives
are possible.

Best regards,
Krzysztof
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..26dc99cb0c89
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
@@ -0,0 +1,67 @@ 
+# 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>;
+    };
+
+    syscon@13030000 {
+        compatible = "starfive,jh7110-sys-syscon", "syscon", "simple-mfd";
+        reg = <0x13030000 0x1000>;
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 0fb4a703f66f..60bbc3a05d79 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20093,6 +20093,12 @@  S:	Supported
 F:	Documentation/devicetree/bindings/clock/starfive,jh7110-pll.yaml
 F:	drivers/clk/starfive/clk-starfive-jh7110-pll.*
 
+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>
@@ -20130,6 +20136,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