[2/5] dt-bindings: soc: sophgo: Add Sophgo syscon module

Commit Message

Chen Wang Nov. 13, 2023, 1:19 p.m. UTC

From: Chen Wang <[e-mail]>

Add documentation to describe Sophgo System Controller Registers for
SG2042.

Signed-off-by: Chen Wang <[e-mail]>
---
 .../soc/sophgo/sophgo,sg2042-syscon.yaml      | 38 +++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-syscon.yaml

Comments

Conor Dooley Nov. 14, 2023, 5:40 p.m. UTC | #1

On Mon, Nov 13, 2023 at 09:19:02PM +0800, Chen Wang wrote:
> From: Chen Wang <[e-mail]>
> 
> Add documentation to describe Sophgo System Controller Registers for
> SG2042.
> 
> Signed-off-by: Chen Wang <[e-mail]>
> ---
>  .../soc/sophgo/sophgo,sg2042-syscon.yaml      | 38 +++++++++++++++++++
>  1 file changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-syscon.yaml
> 
> diff --git a/Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-syscon.yaml b/Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-syscon.yaml
> new file mode 100644
> index 000000000000..829abede4fd5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-syscon.yaml
> @@ -0,0 +1,38 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/soc/sophgo/sophgo,sg2042-syscon.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sophgo SG2042 SoC system controller
> +
> +maintainers:
> +  - Chen Wang <[e-mail]>
> +
> +description:
> +  The Sophgo SG2042 SoC system controller provides register information such
> +  as offset, mask and shift to configure related modules.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - sophgo,sg2042-syscon
> +          - const: syscon

THere's only one option here, so the oneOf should be removed. Similarly,
since there's only one SoC, and it sounds like the next large sophgo
system is going to be using an entirely different core provider, I think
should just simplify this to a pair of "const:" entries.

> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    syscon@30010000 {
> +        compatible = "sophgo,sg2042-syscon", "syscon";
> +        reg = <0x30010000 0x1000>;
> +    };

Per my comments elsewhere, I think the clock controller should be a
child of this node, rather than an unrelated node, linked by a phandle.

Cheers,
Conor.

Chen Wang Nov. 15, 2023, 1:27 a.m. UTC | #2

On 2023/11/15 1:40, Conor Dooley wrote:
> On Mon, Nov 13, 2023 at 09:19:02PM +0800, Chen Wang wrote:
>> From: Chen Wang <[e-mail]>
>>
>> Add documentation to describe Sophgo System Controller Registers for
>> SG2042.
>>
>> Signed-off-by: Chen Wang <[e-mail]>
>> ---
>>   .../soc/sophgo/sophgo,sg2042-syscon.yaml      | 38 +++++++++++++++++++
>>   1 file changed, 38 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-syscon.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-syscon.yaml b/Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-syscon.yaml
>> new file mode 100644
>> index 000000000000..829abede4fd5
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-syscon.yaml
>> @@ -0,0 +1,38 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/soc/sophgo/sophgo,sg2042-syscon.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Sophgo SG2042 SoC system controller
>> +
>> +maintainers:
>> +  - Chen Wang <[e-mail]>
>> +
>> +description:
>> +  The Sophgo SG2042 SoC system controller provides register information such
>> +  as offset, mask and shift to configure related modules.
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - items:
>> +          - enum:
>> +              - sophgo,sg2042-syscon
>> +          - const: syscon
> THere's only one option here, so the oneOf should be removed. Similarly,
> since there's only one SoC, and it sounds like the next large sophgo
> system is going to be using an entirely different core provider, I think
> should just simplify this to a pair of "const:" entries.

Yes, "oneOf" should be removed.

Regarding the "enum", I heard from Sophgo people, that they will 
announce a new SG2044 chip next year and it will share some modules from 
SG2042, including syscon, clock ...

So it is better keeping the "enum" here and we may add another 
"sophgo,sg2044-syscon" later.

Regarding the next large sophgo system you mentioned, yes, there is 
anther chip, I think you are talking about SG2380, it will use 
core(P670) from SiFive. Maybe we can see it in next year also.

More news, I find a report from web: 
https://www.hpcwire.com/2023/11/13/chinese-company-developing-64-core-risc-v-chip-with-tech-from-u-s/, 
FYI.


>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    syscon@30010000 {
>> +        compatible = "sophgo,sg2042-syscon", "syscon";
>> +        reg = <0x30010000 0x1000>;
>> +    };
> Per my comments elsewhere, I think the clock controller should be a
> child of this node, rather than an unrelated node, linked by a phandle.

Agree, I will correct this in next revision.


>
> Cheers,
> Conor.

Rob Herring (Arm) Nov. 16, 2023, 6:13 p.m. UTC | #3

On Tue, Nov 14, 2023 at 05:40:20PM +0000, Conor Dooley wrote:
> On Mon, Nov 13, 2023 at 09:19:02PM +0800, Chen Wang wrote:
> > From: Chen Wang <[e-mail]>
> > 
> > Add documentation to describe Sophgo System Controller Registers for
> > SG2042.
> > 
> > Signed-off-by: Chen Wang <[e-mail]>
> > ---
> >  .../soc/sophgo/sophgo,sg2042-syscon.yaml      | 38 +++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-syscon.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-syscon.yaml b/Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-syscon.yaml
> > new file mode 100644
> > index 000000000000..829abede4fd5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-syscon.yaml
> > @@ -0,0 +1,38 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/soc/sophgo/sophgo,sg2042-syscon.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Sophgo SG2042 SoC system controller
> > +
> > +maintainers:
> > +  - Chen Wang <[e-mail]>
> > +
> > +description:
> > +  The Sophgo SG2042 SoC system controller provides register information such
> > +  as offset, mask and shift to configure related modules.
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - items:
> > +          - enum:
> > +              - sophgo,sg2042-syscon
> > +          - const: syscon
> 
> THere's only one option here, so the oneOf should be removed. Similarly,
> since there's only one SoC, and it sounds like the next large sophgo
> system is going to be using an entirely different core provider, I think
> should just simplify this to a pair of "const:" entries.

For a simple syscon binding like this, you can just add the compatible 
to syscon.yaml.

Rob

Patch

diff --git a/Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-syscon.yaml b/Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-syscon.yaml
new file mode 100644
index 000000000000..829abede4fd5
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-syscon.yaml
@@ -0,0 +1,38 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/sophgo/sophgo,sg2042-syscon.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo SG2042 SoC system controller
+
+maintainers:
+  - Chen Wang <[e-mail]>
+
+description:
+  The Sophgo SG2042 SoC system controller provides register information such
+  as offset, mask and shift to configure related modules.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - sophgo,sg2042-syscon
+          - const: syscon
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    syscon@30010000 {
+        compatible = "sophgo,sg2042-syscon", "syscon";
+        reg = <0x30010000 0x1000>;
+    };