Skip to contentSkip to conversation

riscv: sophgo: add clock support for sg2042

PATCH Superseded Chen Wang 6 participants 28 messages Jan 8, 2024 – Jan 13, 2024

Subsystems (4)

v7 of 16 Superseded Latest is v16 →

lore
Share & tools
lore

Chen Wang

author
#0 lore raw

v7 cover riscv: sophgo: add clock support for sg2042

From: Chen Wang <[email protected]>

This series adds clock controller support for sophgo sg2042.

Thanks,
Chen

---

Changes in v7:
The patch series is based on v6.7. You can simply review or test the patches at the link [8].
- fixed initval issue.
- fixed pll clk crash issue.
- fixed warning reported by <[email protected]>
- code optimization as per review comments.
- code cleanup and style improvements as per review comments and checkpatch

    with "--strict"

Changes in v6:
The patch series is based on v6.7-rc1. You can simply review or test the patches at the link [7].
- fixed some warnings/errors reported by kernel test robot <[email protected]>.

Changes in v5:
The patch series is based on v6.7-rc1. You can simply review or test the patches at the link [6].
- dt-bindings: improved yaml, such as:
- add vendor prefix for system-ctrl property for clock generator.
- Add explanation for system-ctrl property.
- move sophgo,sg2042-clkgen.yaml to directly under clock folder.
- fixed bugs for driver Makefile/Kconfig
- continue cleaning-up debug print for driver code.

Changes in v4:
The patch series is based on v6.7-rc1. You can simply review or test the patches at the link [5].
- dt-bindings: fixed a dt_binding_check error.

Changes in v3:
The patch series is based on v6.7-rc1. You can simply review or test the patches at the link [3].
- DTS: don't use syscon but define sg2042 specific system control node. More background info can read [4].
- Updating minor issues in dt-bindings as per input from reviews.

Changes in v2:
The patch series is based on v6.7-rc1. You can simply review or test the patches at the link [2].
- Squashed the patch adding clock definitions with the patch adding the binding for the clock controller.
- Updating dt-binding for syscon, remove oneOf for property compatible; define clock controller as child of syscon.
- DTS changes: merge sg2042-clock.dtsi into sg2042.dtsi; move clock-frequency property of osc to board devicethree due to the oscillator is outside the SoC.
- Fixed some bugs in driver code during testing, including removing warnings for rv32_defconfig.
- Updated MAINTAINERS info.

Changes in v1:
The patch series is based on v6.7-rc1. You can simply review or test the patches at the link [1].

---

Chen Wang (4):
dt-bindings: soc: sophgo: Add Sophgo system control module
dt-bindings: clock: sophgo: support SG2042
clk: sophgo: Add SG2042 clock generator driver
riscv: dts: add clock generator for Sophgo SG2042 SoC

 .../bindings/clock/sophgo,sg2042-clkgen.yaml  |   53 +
 .../soc/sophgo/sophgo,sg2042-sysctrl.yaml     |   34 +
 MAINTAINERS                                   |    7 +
 .../boot/dts/sophgo/sg2042-milkv-pioneer.dts  |    4 +
 arch/riscv/boot/dts/sophgo/sg2042.dtsi        |   23 +
 drivers/clk/Kconfig                           |    1 +
 drivers/clk/Makefile                          |    1 +
 drivers/clk/sophgo/Kconfig                    |    8 +
 drivers/clk/sophgo/Makefile                   |    2 +
 drivers/clk/sophgo/clk-sophgo-sg2042.c        | 1316 +++++++++++++++++
 drivers/clk/sophgo/clk-sophgo-sg2042.h        |  236 +++
 .../dt-bindings/clock/sophgo,sg2042-clkgen.h  |  169 +++
 12 files changed, 1854 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml
 create mode 100644 Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml
 create mode 100644 drivers/clk/sophgo/Kconfig
 create mode 100644 drivers/clk/sophgo/Makefile
 create mode 100644 drivers/clk/sophgo/clk-sophgo-sg2042.c
 create mode 100644 drivers/clk/sophgo/clk-sophgo-sg2042.h
 create mode 100644 include/dt-bindings/clock/sophgo,sg2042-clkgen.h

base-commit: 0dd3ee31125508cd67f7e7172247f05b7fd1753a

Signature · 1 line
2.25.1
author
#1 lore raw

PATCH v7 1/4 dt-bindings: soc: sophgo: Add Sophgo system control module

From: Chen Wang <[email protected]>

Add documentation to describe Sophgo System Controller for SG2042.

---
 .../soc/sophgo/sophgo,sg2042-sysctrl.yaml     | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml
Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml new file +34 -0
new file mode 100644
index 000000000000..1ec1eaa55598
@@ -0,0 +1,34 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/sophgo/sophgo,sg2042-sysctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo SG2042 SoC system controller
+
+maintainers:
+  - Chen Wang <[email protected]>
+
+description:
+  The Sophgo SG2042 SoC system controller provides register information such
+  as offset, mask and shift that can be used by other modules, such as clocks.
+
+properties:
+  compatible:
+    const: sophgo,sg2042-sysctrl
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    system-controller@30010000 {
+        compatible = "sophgo,sg2042-sysctrl";
+        reg = <0x30010000 0x1000>;
+    };
Signature · 1 line
2.25.1
#2 lore raw

Re: [PATCH v7 1/4] dt-bindings: soc: sophgo: Add Sophgo system control module

Quoted 21 lines · Chen Wang

From: Chen Wang <[email protected]>

Add documentation to describe Sophgo System Controller for SG2042.

---
 .../soc/sophgo/sophgo,sg2042-sysctrl.yaml     | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml
Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml new file +4 -0
new file mode 100644
index 000000000000..1ec1eaa55598
@@ -0,0 +1,34 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/sophgo/sophgo,sg2042-sysctrl.yaml#

Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml line 14

Quoted 10 lines
Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml new file +10 -0
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo SG2042 SoC system controller
+
+maintainers:
+  - Chen Wang <[email protected]>
+
+description:
+  The Sophgo SG2042 SoC system controller provides register information such
+  as offset, mask and shift that can be used by other modules, such as clocks.

"offset, mask and shift" is not a register information stored in syscons. Are you really sure, that your system controller hardware stores offsets of some other registers?

Show as some example of such offsets, masks and shifts provided by this hardware.

Best regards,
Krzysztof

#3 lore raw

Re: [PATCH v7 1/4] dt-bindings: soc: sophgo: Add Sophgo system control module

Quoted 36 lines · Krzysztof Kozlowski
Quoted 21 lines · Chen Wang

From: Chen Wang <[email protected]>

Add documentation to describe Sophgo System Controller for SG2042.

---
  .../soc/sophgo/sophgo,sg2042-sysctrl.yaml     | 34 +++++++++++++++++++
  1 file changed, 34 insertions(+)

create mode 100644 Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml

Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml new file +4 -0
new file mode 100644
index 000000000000..1ec1eaa55598
@@ -0,0 +1,34 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/sophgo/sophgo,sg2042-sysctrl.yaml#

Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml line 14

Quoted 10 lines
Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml new file +10 -0
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo SG2042 SoC system controller
+
+maintainers:
+  - Chen Wang <[email protected]>
+
+description:
+  The Sophgo SG2042 SoC system controller provides register information such
+  as offset, mask and shift that can be used by other modules, such as clocks.

"offset, mask and shift" is not a register information stored in syscons. Are you really sure, that your system controller hardware stores offsets of some other registers?

Quoted 2 lines

Show as some example of such offsets, masks and shifts provided by this hardware.

The system control module is defined here: https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/system-control.rst. It contains some registers related to pll and gates.

Some other clocks registars are defined in https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/clock.rst.

memory-map is defined in https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/mmap.rst

Quoted 5 lines

Best regards,
Krzysztof

#4 lore raw

Re: [PATCH v7 1/4] dt-bindings: soc: sophgo: Add Sophgo system control module

Quoted 38 lines · Chen Wang
Quoted 36 lines · Krzysztof Kozlowski
Quoted 21 lines · Chen Wang

From: Chen Wang <[email protected]>

Add documentation to describe Sophgo System Controller for SG2042.

---
  .../soc/sophgo/sophgo,sg2042-sysctrl.yaml     | 34 +++++++++++++++++++
  1 file changed, 34 insertions(+)

create mode 100644 Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml

Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml new file +4 -0
new file mode 100644
index 000000000000..1ec1eaa55598
@@ -0,0 +1,34 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/sophgo/sophgo,sg2042-sysctrl.yaml#

Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml line 14

Quoted 10 lines
Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml new file +10 -0
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo SG2042 SoC system controller
+
+maintainers:
+  - Chen Wang <[email protected]>
+
+description:
+  The Sophgo SG2042 SoC system controller provides register information such
+  as offset, mask and shift that can be used by other modules, such as clocks.

"offset, mask and shift" is not a register information stored in syscons. Are you really sure, that your system controller hardware stores offsets of some other registers?

Quoted 6 lines
Quoted 2 lines

Show as some example of such offsets, masks and shifts provided by this hardware.

The system control module is defined here:
https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/system-control.rst.
It contains some registers related to pll and gates.

I do not see there registers providing shifts and offsets... just values.

Quoted 6 lines

Some other clocks registars are defined in
https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/clock.rst.

memory-map is defined in
https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/mmap.rst

Please fix the wording because it does not make sense. System controller does not provide register information. Your datasheet provides register information.

Best regards,
Krzysztof

#5 lore raw

Re: [PATCH v7 1/4] dt-bindings: soc: sophgo: Add Sophgo system control module

Quoted 38 lines · Krzysztof Kozlowski
Quoted 37 lines · Chen Wang
Quoted 36 lines · Krzysztof Kozlowski
Quoted 21 lines · Chen Wang

From: Chen Wang <[email protected]>

Add documentation to describe Sophgo System Controller for SG2042.

---
   .../soc/sophgo/sophgo,sg2042-sysctrl.yaml     | 34 +++++++++++++++++++
   1 file changed, 34 insertions(+)

create mode 100644 Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml

Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml new file +4 -0
new file mode 100644
index 000000000000..1ec1eaa55598
@@ -0,0 +1,34 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/sophgo/sophgo,sg2042-sysctrl.yaml#

Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml line 14

Quoted 10 lines
Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml new file +10 -0
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo SG2042 SoC system controller
+
+maintainers:
+  - Chen Wang <[email protected]>
+
+description:
+  The Sophgo SG2042 SoC system controller provides register information such
+  as offset, mask and shift that can be used by other modules, such as clocks.

"offset, mask and shift" is not a register information stored in syscons. Are you really sure, that your system controller hardware stores offsets of some other registers?

Quoted 6 lines
Quoted 5 lines
Quoted 2 lines

Show as some example of such offsets, masks and shifts provided by this hardware.

The system control module is defined here:
https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/system-control.rst.
It contains some registers related to pll and gates.

I do not see there registers providing shifts and offsets... just values.

Let me first clarify more what the "offset"/"shift"/"mask" I meant,

Use https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/system-control.rst#mpll_control-offset-0x0e8 as example, this register is used to control Main PLL:

- Offset: 0x0E8, to my understand, it is the offest between this MPLL_CONTROL register and the start of system-control base

- Shift: the conlumn "LSB", for example, to locate the field MPLL_CONTROL.MPLL_FBDIV, we can first use system-control base + offset to get the address of MPLL_CONTROL, then use LSB(16) as shift to get the start position of this field.

- Mask:  still use MPLL_CONTROL.MPLL_FBDIV as example, use MSB(27) and LSB(16), this means the width of this field is 12 and with this we can get bit-mask for this field.

For SG2042, IC define clock related registers in two parts, one is in https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/clock.rst, and another in https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/system-control.rst. I define the system control node in DTS and just treat it as a block of registers array and after regmap I can get some registers address such as MPLL_CONTROL to access it from my driver code.

Quoted 9 lines
Quoted 5 lines

Some other clocks registars are defined in
https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/clock.rst.

memory-map is defined in
https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/mmap.rst

Please fix the wording because it does not make sense. System controller does not provide register information. Your datasheet provides register information.

Sorry, I don't understand why you say "System controller does not provide register information."? As I explained above, I did see the information about these clock-related registers from the system control module, such as the offset/shift/mask I mentioned above. That's why I wrote "that can be used by other modules, such as clocks".  How should I express, please enlighten me.

Thanks,

Chen

Quoted 3 lines

Best regards,
Krzysztof

#6 lore raw

Re: [PATCH v7 1/4] dt-bindings: soc: sophgo: Add Sophgo system control module

In reply to Krzysztof Kozlowski
Quoted 48 lines · Krzysztof Kozlowski
Quoted 37 lines · Chen Wang
Quoted 36 lines · Krzysztof Kozlowski
Quoted 21 lines · Chen Wang

From: Chen Wang <[email protected]>

Add documentation to describe Sophgo System Controller for SG2042.

---
   .../soc/sophgo/sophgo,sg2042-sysctrl.yaml     | 34 +++++++++++++++++++
   1 file changed, 34 insertions(+)

create mode 100644 Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml

Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml new file +4 -0
new file mode 100644
index 000000000000..1ec1eaa55598
@@ -0,0 +1,34 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/sophgo/sophgo,sg2042-sysctrl.yaml#

Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml line 14

Quoted 10 lines
Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml new file +10 -0
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo SG2042 SoC system controller
+
+maintainers:
+  - Chen Wang <[email protected]>
+
+description:
+  The Sophgo SG2042 SoC system controller provides register information such
+  as offset, mask and shift that can be used by other modules, such as clocks.

"offset, mask and shift" is not a register information stored in syscons. Are you really sure, that your system controller hardware stores offsets of some other registers?

Quoted 5 lines
Quoted 2 lines

Show as some example of such offsets, masks and shifts provided by this hardware.

The system control module is defined here:
https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/system-control.rst.
It contains some registers related to pll and gates.

I do not see there registers providing shifts and offsets... just values.

Quoted 3 lines

Some other clocks registars are defined in
https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/clock.rst.

Quoted 6 lines
Quoted 2 lines

memory-map is defined in
https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/mmap.rst

Please fix the wording because it does not make sense. System controller does not provide register information. Your datasheet provides register information.

Could it be that what I said "that can be used by other modules, such as clocks." may cause misunderstanding. I plan to change it to "The Sophgo SG2042 SoC system controller provides register information such as offset, mask and shift to configure related modules such as clock." Is this better?

Quoted 4 lines

Best regards,
Krzysztof

#7 lore raw

Re: [PATCH v7 1/4] dt-bindings: soc: sophgo: Add Sophgo system control module

Quoted 50 lines · Chen Wang
Quoted 48 lines · Krzysztof Kozlowski
Quoted 37 lines · Chen Wang
Quoted 36 lines · Krzysztof Kozlowski
Quoted 21 lines · Chen Wang

From: Chen Wang <[email protected]>

Add documentation to describe Sophgo System Controller for SG2042.

---
   .../soc/sophgo/sophgo,sg2042-sysctrl.yaml     | 34 +++++++++++++++++++
   1 file changed, 34 insertions(+)

create mode 100644 Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml

Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml new file +4 -0
new file mode 100644
index 000000000000..1ec1eaa55598
@@ -0,0 +1,34 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/sophgo/sophgo,sg2042-sysctrl.yaml#

Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml line 14

Quoted 10 lines
Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml new file +10 -0
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo SG2042 SoC system controller
+
+maintainers:
+  - Chen Wang <[email protected]>
+
+description:
+  The Sophgo SG2042 SoC system controller provides register information such
+  as offset, mask and shift that can be used by other modules, such as clocks.

"offset, mask and shift" is not a register information stored in syscons. Are you really sure, that your system controller hardware stores offsets of some other registers?

Quoted 5 lines
Quoted 2 lines

Show as some example of such offsets, masks and shifts provided by this hardware.

The system control module is defined here:
https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/system-control.rst.
It contains some registers related to pll and gates.

I do not see there registers providing shifts and offsets... just values.

Quoted 3 lines

Some other clocks registars are defined in
https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/clock.rst.

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

memory-map is defined in
https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/mmap.rst

Please fix the wording because it does not make sense. System controller does not provide register information. Your datasheet provides register information.

Could it be that what I said "that can be used by other modules, such as

modules as Linux modules should not be involved in this description.

Quoted 5 lines

clocks." may cause misunderstanding. I plan to change it to "The Sophgo SG2042 SoC system controller provides register information such as offset, mask and shift to configure related modules such as clock." Is this better?

Still does not make sense. To provide "offset" means that some other hardware reads sophgo module to get the value of offset. That's not the case here.

Best regards,
Krzysztof

#8 lore raw

Re: [PATCH v7 1/4] dt-bindings: soc: sophgo: Add Sophgo system control module

Quoted 58 lines · Krzysztof Kozlowski
Quoted 49 lines · Chen Wang
Quoted 48 lines · Krzysztof Kozlowski
Quoted 37 lines · Chen Wang
Quoted 36 lines · Krzysztof Kozlowski
Quoted 21 lines · Chen Wang

From: Chen Wang <[email protected]>

Add documentation to describe Sophgo System Controller for SG2042.

---
    .../soc/sophgo/sophgo,sg2042-sysctrl.yaml     | 34 +++++++++++++++++++
    1 file changed, 34 insertions(+)

create mode 100644 Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml

Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml new file +4 -0
new file mode 100644
index 000000000000..1ec1eaa55598
@@ -0,0 +1,34 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/sophgo/sophgo,sg2042-sysctrl.yaml#

Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml line 14

Quoted 10 lines
Documentation/devicetree/bindings/soc/sophgo/sophgo,sg2042-sysctrl.yaml new file +10 -0
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo SG2042 SoC system controller
+
+maintainers:
+  - Chen Wang <[email protected]>
+
+description:
+  The Sophgo SG2042 SoC system controller provides register information such
+  as offset, mask and shift that can be used by other modules, such as clocks.

"offset, mask and shift" is not a register information stored in syscons. Are you really sure, that your system controller hardware stores offsets of some other registers?

Quoted 5 lines
Quoted 2 lines

Show as some example of such offsets, masks and shifts provided by this hardware.

The system control module is defined here:
https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/system-control.rst.
It contains some registers related to pll and gates.

I do not see there registers providing shifts and offsets... just values.

Quoted 3 lines

Some other clocks registars are defined in
https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/clock.rst.

Quoted 6 lines
Quoted 5 lines
Quoted 2 lines

memory-map is defined in
https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/mmap.rst

Please fix the wording because it does not make sense. System controller does not provide register information. Your datasheet provides register information.

Could it be that what I said "that can be used by other modules, such as

modules as Linux modules should not be involved in this description.

Quoted 8 lines
Quoted 5 lines

clocks." may cause misunderstanding. I plan to change it to "The Sophgo SG2042 SoC system controller provides register information such as offset, mask and shift to configure related modules such as clock." Is this better?

Still does not make sense. To provide "offset" means that some other hardware reads sophgo module to get the value of offset. That's not the case here.

I'm probably starting to understand what you mean. How about changing it to the following?

The Sophgo system controller is a registers block, providing multiple low level platform functions like chip configuration, clock control, etc.

Quoted 4 lines

Best regards,
Krzysztof

#9 lore raw

Re: [PATCH v7 1/4] dt-bindings: soc: sophgo: Add Sophgo system control module

Quoted 9 lines · Chen Wang
Quoted 8 lines
Quoted 5 lines

clocks." may cause misunderstanding. I plan to change it to "The Sophgo SG2042 SoC system controller provides register information such as offset, mask and shift to configure related modules such as clock." Is this better?

Still does not make sense. To provide "offset" means that some other hardware reads sophgo module to get the value of offset. That's not the case here.

Quoted 5 lines

I'm probably starting to understand what you mean. How about changing it to the following?

The Sophgo system controller is a registers block, providing multiple low level platform functions like chip configuration, clock control, etc.

Yes, that sounds good.

Best regards,
Krzysztof

author
#10 lore raw

PATCH v7 2/4 dt-bindings: clock: sophgo: support SG2042

In reply to Chen Wang

From: Chen Wang <[email protected]>

Add bindings for the clock generator on the SG2042 RISC-V SoC.

---
 .../bindings/clock/sophgo,sg2042-clkgen.yaml  |  53 ++++++
 .../dt-bindings/clock/sophgo,sg2042-clkgen.h  | 169 ++++++++++++++++++
 2 files changed, 222 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml
 create mode 100644 include/dt-bindings/clock/sophgo,sg2042-clkgen.h
Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml new file +53 -0
new file mode 100644
index 000000000000..f9935e66fc95
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/sophgo,sg2042-clkgen.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo SG2042 Clock Generator
+
+maintainers:
+  - Chen Wang <[email protected]>
+
+properties:
+  compatible:
+    const: sophgo,sg2042-clkgen
+
+  reg:
+    maxItems: 1
+
+  sophgo,system-ctrl:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Phandle to SG2042 System Controller node. On SG2042, part of control
+      registers of Clock Controller are defined in System controller. Clock
+      driver will use this phandle to get the register map base to plus the
+      offset of the registers to access them.
+
+  clocks:
+    items:
+      - description: Clock Generation IC (25 MHz)
+
+  '#clock-cells':
+    const: 1
+    description:
+      See <dt-bindings/clock/sophgo,sg2042-clkgen.h> for valid indices.
+
+required:
+  - compatible
+  - reg
+  - sophgo,system-ctrl
+  - clocks
+  - '#clock-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    clock-controller@30012000 {
+        compatible = "sophgo,sg2042-clkgen";
+        reg = <0x30012000 0x1000>;
+        sophgo,system-ctrl = <&sys_ctrl>;
+        clocks = <&cgi>;
+        #clock-cells = <1>;
+    };
include/dt-bindings/clock/sophgo,sg2042-clkgen.h new file +169 -0

169 lines not on this page · raw message

Signature · 1 line
2.25.1
#11 lore raw

Re: [PATCH v7 2/4] dt-bindings: clock: sophgo: support SG2042

Quoted 34 lines · Chen Wang

From: Chen Wang <[email protected]>

Add bindings for the clock generator on the SG2042 RISC-V SoC.

---
 .../bindings/clock/sophgo,sg2042-clkgen.yaml  |  53 ++++++
 .../dt-bindings/clock/sophgo,sg2042-clkgen.h  | 169 ++++++++++++++++++
 2 files changed, 222 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml
 create mode 100644 include/dt-bindings/clock/sophgo,sg2042-clkgen.h
Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml new file +15 -0
new file mode 100644
index 000000000000..f9935e66fc95
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/sophgo,sg2042-clkgen.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo SG2042 Clock Generator
+
+maintainers:
+  - Chen Wang <[email protected]>
+
+properties:
+  compatible:
+    const: sophgo,sg2042-clkgen
+

Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml line 25

Quoted 10 lines
Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml new file +10 -0
+  reg:
+    maxItems: 1
+
+  sophgo,system-ctrl:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Phandle to SG2042 System Controller node. On SG2042, part of control
+      registers of Clock Controller are defined in System controller. Clock
+      driver will use this phandle to get the register map base to plus the
+      offset of the registers to access them.

Do not describe the driver, but hardware. What registers are in system-ctrl? What are their purpose? Why this hardware needs them?

Best regards,
Krzysztof

#12 lore raw

Re: [PATCH v7 2/4] dt-bindings: clock: sophgo: support SG2042

Quoted 39 lines · Krzysztof Kozlowski
Quoted 38 lines · Chen Wang

From: Chen Wang <[email protected]>

Add bindings for the clock generator on the SG2042 RISC-V SoC.

---
  .../bindings/clock/sophgo,sg2042-clkgen.yaml  |  53 ++++++
  .../dt-bindings/clock/sophgo,sg2042-clkgen.h  | 169 ++++++++++++++++++
  2 files changed, 222 insertions(+)

create mode 100644 Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml create mode 100644 include/dt-bindings/clock/sophgo,sg2042-clkgen.h

Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml new file +19 -0
new file mode 100644
index 000000000000..f9935e66fc95
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/sophgo,sg2042-clkgen.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo SG2042 Clock Generator
+
+maintainers:
+  - Chen Wang <[email protected]>
+
+properties:
+  compatible:
+    const: sophgo,sg2042-clkgen
+
+  reg:
+    maxItems: 1
+
+  sophgo,system-ctrl:
Quoted 8 lines

Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml line 25

Quoted 6 lines
Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml new file +6 -0
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Phandle to SG2042 System Controller node. On SG2042, part of control
+      registers of Clock Controller are defined in System controller. Clock
+      driver will use this phandle to get the register map base to plus the
+      offset of the registers to access them.

Do not describe the driver, but hardware. What registers are in system-ctrl? What are their purpose? Why this hardware needs them?

Understood, will fix the words in revision, thanks.

Quoted 6 lines

Best regards,
Krzysztof

#13 lore raw

Re: [PATCH v7 2/4] dt-bindings: clock: sophgo: support SG2042

Hey,

Quoted 48 lines · Chen Wang
Quoted 39 lines · Krzysztof Kozlowski
Quoted 38 lines · Chen Wang

From: Chen Wang <[email protected]>

Add bindings for the clock generator on the SG2042 RISC-V SoC.

---
  .../bindings/clock/sophgo,sg2042-clkgen.yaml  |  53 ++++++
  .../dt-bindings/clock/sophgo,sg2042-clkgen.h  | 169 ++++++++++++++++++
  2 files changed, 222 insertions(+)

create mode 100644 Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml create mode 100644 include/dt-bindings/clock/sophgo,sg2042-clkgen.h

Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml new file +19 -0
new file mode 100644
index 000000000000..f9935e66fc95
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/sophgo,sg2042-clkgen.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo SG2042 Clock Generator
+
+maintainers:
+  - Chen Wang <[email protected]>
+
+properties:
+  compatible:
+    const: sophgo,sg2042-clkgen
+
+  reg:
+    maxItems: 1
+
+  sophgo,system-ctrl:
Quoted 8 lines

Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml line 25

Quoted 6 lines
Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml new file +6 -0
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Phandle to SG2042 System Controller node. On SG2042, part of control
+      registers of Clock Controller are defined in System controller. Clock
+      driver will use this phandle to get the register map base to plus the
+      offset of the registers to access them.

Do not describe the driver, but hardware. What registers are in system-ctrl? What are their purpose? Why this hardware needs them?

Quoted 1 line

Understood, will fix the words in revision, thanks.

I hope that I am not misunderstanding things, but I got a bit suspicious of this binding and look at the driver, and saw that there are clocks registered like:

| static int sg2042_clk_register_gates(struct sg2042_clk_data *clk_data,
| 				     const struct sg2042_gate_clock gate_clks[],
| 				     int num_gate_clks)
| {
| 	struct clk_hw *hw;
| 	const struct sg2042_gate_clock *gate;
| 	int i, ret = 0;
| 	void __iomem *reg;
| 
| 	for (i = 0; i < num_gate_clks; i++) {
| 		gate = &gate_clks[i];
| 		if (gate->flag_sysctrl)
| 			reg = clk_data->iobase_syscon + gate->offset_enable;
| 		else
| 			reg = clk_data->iobase + gate->offset_enable;

iobase_syscon is the base address of the system controller that this property points at & iobase is the base address of the clock controller itself.

| 		hw = clk_hw_register_gate(NULL,
| 					  gate->name,
| 					  gate->parent_name,
| 					  gate->flags,
| 					  reg,
| 					  gate->bit_idx,
| 					  0,
| 					  &sg2042_clk_lock);

As far as I can tell, in this particular case, for any gate clock that flag_sysctrl is set, none of the registers actually lie inside the clkgen region, but instead are entirely contained in the sysctrl region.

I think that this is because your devicetree does not correctly define the relationship between clocks, and these clocks are actually provided by the system controller block and are inputs to the clkgen block.

| 		if (IS_ERR(hw)) {
| 			pr_err("failed to register clock %s\n", gate->name);
| 			ret = PTR_ERR(hw);
| 			break;
| 		}
| 
| 		clk_data->onecell_data.hws[gate->id] = hw;
| 	}
| 
| 	/* leave unregister to outside if failed */
| 	return ret;
| }

I had a much briefer look at the sg2042_pll_clocks that make use of the regmap, and it doesn't seem like they "mix and match" registers between both blocks, and instead only have registers in the system controller? If so, it doesn't seem like this clkgen block should be providing the PLL clocks either, but instead be taking them as inputs.

Reading stuff like
https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/system-control.rst#pll_stat-offset-0x0c0
(and onwards) makes it seem like those PLLs are fully contained within the system controller register space.

It seems like
https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/clock-reg.rst
is the register map for the clkgen region? It seems like that region only contains gates and divider clocks, but no PLLs.

Am I missing something, or is this description of the clock controllers on the soc incomplete?

Cheers,
Conor.

#14 lore raw

Re: [PATCH v7 2/4] dt-bindings: clock: sophgo: support SG2042

Quoted 117 lines · Conor Dooley

Hey,

Quoted 48 lines · Chen Wang
Quoted 39 lines · Krzysztof Kozlowski
Quoted 38 lines · Chen Wang

From: Chen Wang <[email protected]>

Add bindings for the clock generator on the SG2042 RISC-V SoC.

---
   .../bindings/clock/sophgo,sg2042-clkgen.yaml  |  53 ++++++
   .../dt-bindings/clock/sophgo,sg2042-clkgen.h  | 169 ++++++++++++++++++
   2 files changed, 222 insertions(+)

create mode 100644 Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml create mode 100644 include/dt-bindings/clock/sophgo,sg2042-clkgen.h

Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml new file +19 -0
new file mode 100644
index 000000000000..f9935e66fc95
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/sophgo,sg2042-clkgen.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo SG2042 Clock Generator
+
+maintainers:
+  - Chen Wang <[email protected]>
+
+properties:
+  compatible:
+    const: sophgo,sg2042-clkgen
+
+  reg:
+    maxItems: 1
+
+  sophgo,system-ctrl:
Quoted 8 lines

Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml line 25

Quoted 6 lines
Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml new file +6 -0
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Phandle to SG2042 System Controller node. On SG2042, part of control
+      registers of Clock Controller are defined in System controller. Clock
+      driver will use this phandle to get the register map base to plus the
+      offset of the registers to access them.

Do not describe the driver, but hardware. What registers are in system-ctrl? What are their purpose? Why this hardware needs them?

Quoted 1 line

Understood, will fix the words in revision, thanks.

I hope that I am not misunderstanding things, but I got a bit suspicious of this binding and look at the driver, and saw that there are clocks registered like:

| static int sg2042_clk_register_gates(struct sg2042_clk_data *clk_data,
| 				     const struct sg2042_gate_clock gate_clks[],
| 				     int num_gate_clks)
| {
| 	struct clk_hw *hw;
| 	const struct sg2042_gate_clock *gate;
| 	int i, ret = 0;
| 	void __iomem *reg;
|
| 	for (i = 0; i < num_gate_clks; i++) {
| 		gate = &gate_clks[i];
| 		if (gate->flag_sysctrl)
| 			reg = clk_data->iobase_syscon + gate->offset_enable;
| 		else
| 			reg = clk_data->iobase + gate->offset_enable;

iobase_syscon is the base address of the system controller that this property points at & iobase is the base address of the clock controller itself.

| 		hw = clk_hw_register_gate(NULL,
| 					  gate->name,
| 					  gate->parent_name,
| 					  gate->flags,
| 					  reg,
| 					  gate->bit_idx,
| 					  0,
| 					  &sg2042_clk_lock);

As far as I can tell, in this particular case, for any gate clock that flag_sysctrl is set, none of the registers actually lie inside the clkgen region, but instead are entirely contained in the sysctrl region.

I think that this is because your devicetree does not correctly define the relationship between clocks, and these clocks are actually provided by the system controller block and are inputs to the clkgen block.

| 		if (IS_ERR(hw)) {
| 			pr_err("failed to register clock %s\n", gate->name);
| 			ret = PTR_ERR(hw);
| 			break;
| 		}
|
| 		clk_data->onecell_data.hws[gate->id] = hw;
| 	}
|
| 	/* leave unregister to outside if failed */
| 	return ret;
| }

I had a much briefer look at the sg2042_pll_clocks that make use of the regmap, and it doesn't seem like they "mix and match" registers between both blocks, and instead only have registers in the system controller? If so, it doesn't seem like this clkgen block should be providing the PLL clocks either, but instead be taking them as inputs.

Reading stuff like
https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/system-control.rst#pll_stat-offset-0x0c0
(and onwards) makes it seem like those PLLs are fully contained within the system controller register space.

Quoted 7 lines

It seems like
https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/clock-reg.rst
is the register map for the clkgen region? It seems like that region only contains gates and divider clocks, but no PLLs.

Am I missing something, or is this description of the clock controllers on the soc incomplete?

hi,Conor,

There are four types of clocks for SG2042 and following are where their control registers are defined in:

PLL:all in SYS_CTRL
DIV: all in CLOCK
GATE: some are in SYS_CTRL, some others are in CLOCK
MUX: all in CLOCK

For PLLs, yes, they are all controlled by registers defined in SYS_CTRL. About what you said "it doesn't seem like this clkgen block should be providing the PLL clocks either, but instead be taking them as inputs.", I am not very sure what your meaning of "inputs". I try to write DTS with my undrstadning, please help me see if it fits what you mean.

```dts

sys_ctrl: system-controller@7030010000 { compatible = "sophgo,sg2042-sysctrl"; reg = <0x70 0x30010000 0x0 0x1000>; pllclk: clock-controller { compatible = "sophgo,sg2042-pll"; #clock-cells = <1>; clocks = <&cgi>; }; }; clkgen: clock-controller@7030012000 { compatible = "sophgo,sg2042-clkgen"; reg = <0x70 0x30012000 0x0 0x1000>; #clock-cells = <1>; clocks = <&pllclk MPLL_CLK>, <&pllclk FPLL_CLK>, <&pllclk DPLL0_CLK>, <&pllclk DPLL1_CLK>,; clock-names = "cgi", "mpll", "fpll", "dpll0", "dpll1"; };

```

With this change, we describe the plls defined in system control as pllclk, as a child node of system controller. clkgen will use pllclk as "input" because pll clocks are parent of div clocks .

But there is another remaining question about the gate clock. For those gate clocks controlled by CLOCK, no problem we will provide then in clkgen, but  for those gate clocks controlled by registers in SYS_CTRL, they are child gate of the "clk_gate_rp_cpu_normal", which is a gate clock provided by clkgen. If I extracted those SYS_CTRL gate clocks and define them in system controller dts node, I may have to use "clk_gate_rp_cpu_normal" as their input, it looks a bit wierd becasue there are cases where each other serves as input. I try to draft below DTS to explan what I meant. I'm not sure if it can work and I'd love to hear your guidance.

```dts

sys_ctrl: system-controller@7030010000 {
    compatible = "sophgo,sg2042-sysctrl";
    reg = <0x70 0x30010000 0x0 0x1000>;

    pllclk: clock-controller {
        compatible = "sophgo,sg2042-pll";
        #clock-cells = <1>;
        clocks = <&cgi>;
    };

    somegateclk: clock-controller2 {
        compatible = "sophgo,sg2042-somegate";
        #clock-cells = <1>;
        clocks = <&clkgen GATE_CLK_RP_CPU_NORMAL>;
        clock-names = "clk_gate_rp_cpu_normal";
    };
};

clkgen: clock-controller@7030012000 {
    compatible = "sophgo,sg2042-clkgen";
    reg = <0x70 0x30012000 0x0 0x1000>;
    #clock-cells = <1>;
    clocks =      <&pllclk MPLL_CLK>,
             <&pllclk FPLL_CLK>,
             <&pllclk DPLL0_CLK>,
             <&pllclk DPLL1_CLK>,;
    clock-names = "cgi", "mpll", "fpll", "dpll0", "dpll1";
};

```

So, can we put all gate clocks in clkgen to simplify this?

Thanks

Chen

Quoted 2 lines

Cheers,
Conor.

#15 lore raw

Re: [PATCH v7 2/4] dt-bindings: clock: sophgo: support SG2042

In reply to Conor Dooley

Resent and fixed some format issue in last email.

Quoted 117 lines · Conor Dooley

Hey,

Quoted 48 lines · Chen Wang
Quoted 39 lines · Krzysztof Kozlowski
Quoted 38 lines · Chen Wang

From: Chen Wang <[email protected]>

Add bindings for the clock generator on the SG2042 RISC-V SoC.

---
   .../bindings/clock/sophgo,sg2042-clkgen.yaml  |  53 ++++++
   .../dt-bindings/clock/sophgo,sg2042-clkgen.h  | 169 ++++++++++++++++++
   2 files changed, 222 insertions(+)

create mode 100644 Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml create mode 100644 include/dt-bindings/clock/sophgo,sg2042-clkgen.h

Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml new file +19 -0
new file mode 100644
index 000000000000..f9935e66fc95
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/sophgo,sg2042-clkgen.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo SG2042 Clock Generator
+
+maintainers:
+  - Chen Wang <[email protected]>
+
+properties:
+  compatible:
+    const: sophgo,sg2042-clkgen
+
+  reg:
+    maxItems: 1
+
+  sophgo,system-ctrl:
Quoted 8 lines

Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml line 25

Quoted 6 lines
Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml new file +6 -0
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Phandle to SG2042 System Controller node. On SG2042, part of control
+      registers of Clock Controller are defined in System controller. Clock
+      driver will use this phandle to get the register map base to plus the
+      offset of the registers to access them.

Do not describe the driver, but hardware. What registers are in system-ctrl? What are their purpose? Why this hardware needs them?

Quoted 1 line

Understood, will fix the words in revision, thanks.

I hope that I am not misunderstanding things, but I got a bit suspicious of this binding and look at the driver, and saw that there are clocks registered like:

| static int sg2042_clk_register_gates(struct sg2042_clk_data *clk_data,
| 				     const struct sg2042_gate_clock gate_clks[],
| 				     int num_gate_clks)
| {
| 	struct clk_hw *hw;
| 	const struct sg2042_gate_clock *gate;
| 	int i, ret = 0;
| 	void __iomem *reg;
|
| 	for (i = 0; i < num_gate_clks; i++) {
| 		gate = &gate_clks[i];
| 		if (gate->flag_sysctrl)
| 			reg = clk_data->iobase_syscon + gate->offset_enable;
| 		else
| 			reg = clk_data->iobase + gate->offset_enable;

iobase_syscon is the base address of the system controller that this property points at & iobase is the base address of the clock controller itself.

| 		hw = clk_hw_register_gate(NULL,
| 					  gate->name,
| 					  gate->parent_name,
| 					  gate->flags,
| 					  reg,
| 					  gate->bit_idx,
| 					  0,
| 					  &sg2042_clk_lock);

As far as I can tell, in this particular case, for any gate clock that flag_sysctrl is set, none of the registers actually lie inside the clkgen region, but instead are entirely contained in the sysctrl region.

I think that this is because your devicetree does not correctly define the relationship between clocks, and these clocks are actually provided by the system controller block and are inputs to the clkgen block.

| 		if (IS_ERR(hw)) {
| 			pr_err("failed to register clock %s\n", gate->name);
| 			ret = PTR_ERR(hw);
| 			break;
| 		}
|
| 		clk_data->onecell_data.hws[gate->id] = hw;
| 	}
|
| 	/* leave unregister to outside if failed */
| 	return ret;
| }

I had a much briefer look at the sg2042_pll_clocks that make use of the regmap, and it doesn't seem like they "mix and match" registers between both blocks, and instead only have registers in the system controller? If so, it doesn't seem like this clkgen block should be providing the PLL clocks either, but instead be taking them as inputs.

Reading stuff like
https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/system-control.rst#pll_stat-offset-0x0c0
(and onwards) makes it seem like those PLLs are fully contained within the system controller register space.

Quoted 7 lines

It seems like
https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/clock-reg.rst
is the register map for the clkgen region? It seems like that region only contains gates and divider clocks, but no PLLs.

Am I missing something, or is this description of the clock controllers on the soc incomplete?

hi,Conor,

There are four types of clocks for SG2042 and following are where their control registers are defined in:

PLL:all in SYS_CTRL
DIV: all in CLOCK
GATE: some are in SYS_CTRL, some others are in CLOCK
MUX: all in CLOCK

For PLLs, yes, they are all controlled by registers defined in SYS_CTRL. About what you said "it doesn't seem like this clkgen block should be providing the PLL clocks either, but instead be taking them as inputs.", I am not very sure what your meaning of "inputs". I try to write DTS with my undrstadning, please help me see if it fits what you mean.

```dts

sys_ctrl: system-controller@7030010000 {
    compatible = "sophgo,sg2042-sysctrl";
    reg = <0x70 0x30010000 0x0 0x1000>;

    pllclk: clock-controller {
        compatible = "sophgo,sg2042-pll";
        #clock-cells = <1>;
        clocks = <&cgi>;
    };
};

clkgen: clock-controller@7030012000 {
    compatible = "sophgo,sg2042-clkgen";
    reg = <0x70 0x30012000 0x0 0x1000>;
    #clock-cells = <1>;
    clocks = <&pllclk MPLL_CLK>,
        <&pllclk FPLL_CLK>,
        <&pllclk DPLL0_CLK>,
        <&pllclk DPLL1_CLK>;
    clock-names = "cgi", "mpll", "fpll", "dpll0", "dpll1";
};

```

With this change, we describe the plls defined in system control as pllclk, as a child node of system controller. clkgen will use pllclk as "input" because pll clocks are parent of div clocks .

But there is another remaining question about the gate clock. For those gate clocks controlled by CLOCK, no problem we will provide then in clkgen, but  for those gate clocks controlled by registers in SYS_CTRL, they are child gate of the "clk_gate_rp_cpu_normal", which is a gate clock provided by clkgen. If I extracted those SYS_CTRL gate clocks and define them in system controller dts node, I may have to use "clk_gate_rp_cpu_normal" as their input, it looks a bit wierd becasue there are cases where each other serves as input. I try to draft below DTS to explan what I meant. I'm not sure if it can work and I'd love to hear your guidance.

```dts

sys_ctrl: system-controller@7030010000 {
    compatible = "sophgo,sg2042-sysctrl";
    reg = <0x70 0x30010000 0x0 0x1000>;

    pllclk: clock-controller {
        compatible = "sophgo,sg2042-pll";
        #clock-cells = <1>;
        clocks = <&cgi>;
    };

    somegateclk: clock-controller2 {
        compatible = "sophgo,sg2042-somegate";
        #clock-cells = <1>;
        clocks = <&clkgen GATE_CLK_RP_CPU_NORMAL>;
        clock-names = "clk_gate_rp_cpu_normal";
    };
};

clkgen: clock-controller@7030012000 {
    compatible = "sophgo,sg2042-clkgen";
    reg = <0x70 0x30012000 0x0 0x1000>;
    #clock-cells = <1>;
    clocks =      <&pllclk MPLL_CLK>,
             <&pllclk FPLL_CLK>,
             <&pllclk DPLL0_CLK>,
             <&pllclk DPLL1_CLK>,;
    clock-names = "cgi", "mpll", "fpll", "dpll0", "dpll1";
};

```

So, can we put all gate clocks in clkgen to simplify this?

Thanks

Chen

Quoted 3 lines

Cheers,
Conor.

#16 lore raw

Re: [PATCH v7 2/4] dt-bindings: clock: sophgo: support SG2042

Quoted 127 lines · Chen Wang

Resent and fixed some format issue in last email.

Quoted 117 lines · Conor Dooley

Hey,

Quoted 48 lines · Chen Wang
Quoted 39 lines · Krzysztof Kozlowski
Quoted 38 lines · Chen Wang

From: Chen Wang <[email protected]>

Add bindings for the clock generator on the SG2042 RISC-V SoC.

---
   .../bindings/clock/sophgo,sg2042-clkgen.yaml  |  53 ++++++
   .../dt-bindings/clock/sophgo,sg2042-clkgen.h  | 169 ++++++++++++++++++
   2 files changed, 222 insertions(+)

create mode 100644 Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml create mode 100644 include/dt-bindings/clock/sophgo,sg2042-clkgen.h

Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml new file +19 -0
new file mode 100644
index 000000000000..f9935e66fc95
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/sophgo,sg2042-clkgen.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo SG2042 Clock Generator
+
+maintainers:
+  - Chen Wang <[email protected]>
+
+properties:
+  compatible:
+    const: sophgo,sg2042-clkgen
+
+  reg:
+    maxItems: 1
+
+  sophgo,system-ctrl:
Quoted 8 lines

Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml line 25

Quoted 6 lines
Documentation/devicetree/bindings/clock/sophgo,sg2042-clkgen.yaml new file +6 -0
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Phandle to SG2042 System Controller node. On SG2042, part of control
+      registers of Clock Controller are defined in System controller. Clock
+      driver will use this phandle to get the register map base to plus the
+      offset of the registers to access them.

Do not describe the driver, but hardware. What registers are in system-ctrl? What are their purpose? Why this hardware needs them?

Quoted 1 line

Understood, will fix the words in revision, thanks.

I hope that I am not misunderstanding things, but I got a bit suspicious of this binding and look at the driver, and saw that there are clocks registered like:

| static int sg2042_clk_register_gates(struct sg2042_clk_data *clk_data,
| 				     const struct sg2042_gate_clock gate_clks[],
| 				     int num_gate_clks)
| {
| 	struct clk_hw *hw;
| 	const struct sg2042_gate_clock *gate;
| 	int i, ret = 0;
| 	void __iomem *reg;
|
| 	for (i = 0; i < num_gate_clks; i++) {
| 		gate = &gate_clks[i];
| 		if (gate->flag_sysctrl)
| 			reg = clk_data->iobase_syscon + gate->offset_enable;
| 		else
| 			reg = clk_data->iobase + gate->offset_enable;

iobase_syscon is the base address of the system controller that this property points at & iobase is the base address of the clock controller itself.

| 		hw = clk_hw_register_gate(NULL,
| 					  gate->name,
| 					  gate->parent_name,
| 					  gate->flags,
| 					  reg,
| 					  gate->bit_idx,
| 					  0,
| 					  &sg2042_clk_lock);

As far as I can tell, in this particular case, for any gate clock that flag_sysctrl is set, none of the registers actually lie inside the clkgen region, but instead are entirely contained in the sysctrl region.

I think that this is because your devicetree does not correctly define the relationship between clocks, and these clocks are actually provided by the system controller block and are inputs to the clkgen block.

| 		if (IS_ERR(hw)) {
| 			pr_err("failed to register clock %s\n", gate->name);
| 			ret = PTR_ERR(hw);
| 			break;
| 		}
|
| 		clk_data->onecell_data.hws[gate->id] = hw;
| 	}
|
| 	/* leave unregister to outside if failed */
| 	return ret;
| }

I had a much briefer look at the sg2042_pll_clocks that make use of the regmap, and it doesn't seem like they "mix and match" registers between both blocks, and instead only have registers in the system controller? If so, it doesn't seem like this clkgen block should be providing the PLL clocks either, but instead be taking them as inputs.

Reading stuff like
https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/system-control.rst#pll_stat-offset-0x0c0
(and onwards) makes it seem like those PLLs are fully contained within the system controller register space.

Quoted 7 lines

It seems like
https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/clock-reg.rst
is the register map for the clkgen region? It seems like that region only contains gates and divider clocks, but no PLLs.

Am I missing something, or is this description of the clock controllers on the soc incomplete?

Quoted 8 lines

hi,Conor,

There are four types of clocks for SG2042 and following are where their control registers are defined in:

PLL:all in SYS_CTRL
DIV: all in CLOCK
GATE: some are in SYS_CTRL, some others are in CLOCK

When you say "some", do you meant some entire clocks are in SYS_CTRL and some entire clocks are in the CLOCKS? Or do you meant that for a given clock, some registers are in SYS_CTRL and some are in CLOCK? It's the first option, right?

Quoted 38 lines

MUX: all in CLOCK

For PLLs, yes, they are all controlled by registers defined in SYS_CTRL. About what you said "it doesn't seem like this clkgen block should be providing the PLL clocks either, but instead be taking them as inputs.", I am not very sure what your meaning of "inputs". I try to write DTS with my undrstadning, please help me see if it fits what you mean.

```dts

sys_ctrl: system-controller@7030010000 {
    compatible = "sophgo,sg2042-sysctrl";
    reg = <0x70 0x30010000 0x0 0x1000>;

    pllclk: clock-controller {
        compatible = "sophgo,sg2042-pll";
        #clock-cells = <1>;
        clocks = <&cgi>;
    };
};

clkgen: clock-controller@7030012000 {
    compatible = "sophgo,sg2042-clkgen";
    reg = <0x70 0x30012000 0x0 0x1000>;
    #clock-cells = <1>;
    clocks = <&pllclk MPLL_CLK>,
        <&pllclk FPLL_CLK>,
        <&pllclk DPLL0_CLK>,
        <&pllclk DPLL1_CLK>;
    clock-names = "cgi", "mpll", "fpll", "dpll0", "dpll1";
};

```

With this change, we describe the plls defined in system control as pllclk, as a child node of system controller. clkgen will use pllclk as "input" because pll clocks are parent of div clocks .

Quoted 9 lines

But there is another remaining question about the gate clock. For those gate clocks controlled by CLOCK, no problem we will provide then in clkgen, but for those gate clocks controlled by registers in SYS_CTRL, they are child gate of the "clk_gate_rp_cpu_normal", which is a gate clock provided by clkgen. If I extracted those SYS_CTRL gate clocks and define them in system controller dts node, I may have to use "clk_gate_rp_cpu_normal" as their input, it looks a bit wierd becasue there are cases where each other serves as input. I try to draft below DTS to explan what I meant. I'm not sure if it can work and I'd love to hear your guidance.

I'm not sure how this sort of circular relationship works for probing works either. Stephen etc would know more than me here.

Quoted 33 lines
```dts

sys_ctrl: system-controller@7030010000 {
    compatible = "sophgo,sg2042-sysctrl";
    reg = <0x70 0x30010000 0x0 0x1000>;

    pllclk: clock-controller {
        compatible = "sophgo,sg2042-pll";
        #clock-cells = <1>;
        clocks = <&cgi>;
    };

    somegateclk: clock-controller2 {
        compatible = "sophgo,sg2042-somegate";
        #clock-cells = <1>;
        clocks = <&clkgen GATE_CLK_RP_CPU_NORMAL>;
        clock-names = "clk_gate_rp_cpu_normal";
    };
};

clkgen: clock-controller@7030012000 {
    compatible = "sophgo,sg2042-clkgen";
    reg = <0x70 0x30012000 0x0 0x1000>;
    #clock-cells = <1>;
    clocks =      <&pllclk MPLL_CLK>,
             <&pllclk FPLL_CLK>,
             <&pllclk DPLL0_CLK>,
             <&pllclk DPLL1_CLK>,;
    clock-names = "cgi", "mpll", "fpll", "dpll0", "dpll1";
};

```
Quoted 1 line

So, can we put all gate clocks in clkgen to simplify this?

The dts should describe how the hardware actually looks, even if that is not really convenient for the operating system.

#17 lore raw

Re: [PATCH v7 2/4] dt-bindings: clock: sophgo: support SG2042

Quoted 6 lines · Conor Dooley
Quoted 5 lines · Chen Wang

hi,Conor,

There are four types of clocks for SG2042 and following are where their control registers are defined in:

Quoted 7 lines
Quoted 3 lines

PLL:all in SYS_CTRL
DIV: all in CLOCK
GATE: some are in SYS_CTRL, some others are in CLOCK

When you say "some", do you meant some entire clocks are in SYS_CTRL and some entire clocks are in the CLOCKS? Or do you meant that for a given clock, some registers are in SYS_CTRL and some are in CLOCK? It's the first option, right?

It's the first option.

Quoted 1 line
Quoted 1 line

MUX: all in CLOCK

#18 lore raw

Re: [PATCH v7 2/4] dt-bindings: clock: sophgo: support SG2042

Quoted 9 lines · Chen Wang
Quoted 8 lines · Conor Dooley
Quoted 6 lines · Chen Wang

There are four types of clocks for SG2042 and following are where their control registers are defined in:

PLL:all in SYS_CTRL
DIV: all in CLOCK
GATE: some are in SYS_CTRL, some others are in CLOCK

Quoted 6 lines
Quoted 4 lines

When you say "some", do you meant some entire clocks are in SYS_CTRL and some entire clocks are in the CLOCKS? Or do you meant that for a given clock, some registers are in SYS_CTRL and some are in CLOCK? It's the first option, right?

It's the first option.

Then the gate clocks that are fully contained within SYS_CTRL are outputs of SYS_CTRL and gate clocks fully contained within CLOCK are outputs of CLOCK. You should not use a phandle to SYS_CTRL from the CLOCKS node so that you can pretend they are part of CLOCKS just because that makes writing your driver easier. That said, obviously you can share the routines for turning the gates on and off etc.

Cheers,
Conor.

#19 lore raw

Re: [PATCH v7 2/4] dt-bindings: clock: sophgo: support SG2042

Quoted 13 lines · Conor Dooley
Quoted 12 lines · Chen Wang
Quoted 11 lines · Conor Dooley
Quoted 6 lines · Chen Wang

There are four types of clocks for SG2042 and following are where their control registers are defined in:

PLL:all in SYS_CTRL
DIV: all in CLOCK
GATE: some are in SYS_CTRL, some others are in CLOCK

When you say "some", do you meant some entire clocks are in SYS_CTRL and some entire clocks are in the CLOCKS? Or do you meant that for a given clock, some registers are in SYS_CTRL and some are in CLOCK? It's the first option, right?

Quoted 7 lines
Quoted 1 line

It's the first option.

Then the gate clocks that are fully contained within SYS_CTRL are outputs of SYS_CTRL and gate clocks fully contained within CLOCK are outputs of CLOCK. You should not use a phandle to SYS_CTRL from the CLOCKS node so that you can pretend they are part of CLOCKS just because that makes writing your driver easier. That said, obviously you can share the routines for turning the gates on and off etc.

Um, seems that we need to define two clock-controllers to output their own clocks respectively. Thank you for your patient guidance, let me re-cook the code.

Regards,

Chen

Quoted 2 lines

Cheers,
Conor.

#20 lore raw

Re: [PATCH v7 2/4] dt-bindings: clock: sophgo: support SG2042

In reply to Conor Dooley

Conor and Krzysztof,

Just a quick question, due to I am planning to change the binding files you have reviewed,  should I remain your signature of “Reviewed-by" or remove it in next patchset?

Quoted 23 lines · Conor Dooley
Quoted 5 lines · Chen Wang
Quoted 4 lines · Conor Dooley
Quoted 3 lines · Chen Wang

There are four types of clocks for SG2042 and following are where their control registers are defined in:

Quoted 8 lines
Quoted 7 lines
Quoted 3 lines

PLL:all in SYS_CTRL
DIV: all in CLOCK
GATE: some are in SYS_CTRL, some others are in CLOCK

When you say "some", do you meant some entire clocks are in SYS_CTRL and some entire clocks are in the CLOCKS? Or do you meant that for a given clock, some registers are in SYS_CTRL and some are in CLOCK? It's the first option, right?

It's the first option.

Then the gate clocks that are fully contained within SYS_CTRL are outputs of SYS_CTRL and gate clocks fully contained within CLOCK are outputs of CLOCK. You should not use a phandle to SYS_CTRL from the CLOCKS node so that you can pretend they are part of CLOCKS just because that makes writing your driver easier. That said, obviously you can share the routines for turning the gates on and off etc.

Cheers,
Conor.

#21 lore raw

Re: [PATCH v7 2/4] dt-bindings: clock: sophgo: support SG2042

Quoted 5 lines · Chen Wang

Conor and Krzysztof,

Just a quick question, due to I am planning to change the binding files you have reviewed,  should I remain your signature of “Reviewed-by" or remove it in next patchset?

Depends on the amount of changes. If you remove or add properties, then please drop the tag. Whenever you drop a tag or skip it (so do not add after receiving), explain this in the changelog. changelog goes under ---.

Best regards,
Krzysztof

#22 lore raw

Re: [PATCH v7 2/4] dt-bindings: clock: sophgo: support SG2042

In reply to Conor Dooley

Hi Conor, Chen,

Quoted 15 lines · Conor Dooley
Quoted 4 lines · Chen Wang

With this change, we describe the plls defined in system control as pllclk, as a child node of system controller. clkgen will use pllclk as "input" because pll clocks are parent of div clocks .

Quoted 9 lines

But there is another remaining question about the gate clock. For those gate clocks controlled by CLOCK, no problem we will provide then in clkgen, but for those gate clocks controlled by registers in SYS_CTRL, they are child gate of the "clk_gate_rp_cpu_normal", which is a gate clock provided by clkgen. If I extracted those SYS_CTRL gate clocks and define them in system controller dts node, I may have to use "clk_gate_rp_cpu_normal" as their input, it looks a bit wierd becasue there are cases where each other serves as input. I try to draft below DTS to explan what I meant. I'm not sure if it can work and I'd love to hear your guidance.

Quoted 2 lines

I'm not sure how this sort of circular relationship works for probing works either. Stephen etc would know more than me here.

It generally works fine. The common clock framework can handle the child clock being registered before its parent, even when using a DT (fw_name) reference. See for example clk_core_fill_parent_index() and clk_core_reparent_orphans_nolock() in drivers/clk/clk.c

Regards,
Samuel

#23 lore raw

Re: [PATCH v7 2/4] dt-bindings: clock: sophgo: support SG2042

Quoted 17 lines · Samuel Holland

Hi Conor, Chen,

Quoted 14 lines · Conor Dooley
Quoted 13 lines · Chen Wang

With this change, we describe the plls defined in system control as pllclk, as a child node of system controller. clkgen will use pllclk as "input" because pll clocks are parent of div clocks .

But there is another remaining question about the gate clock. For those gate clocks controlled by CLOCK, no problem we will provide then in clkgen, but for those gate clocks controlled by registers in SYS_CTRL, they are child gate of the "clk_gate_rp_cpu_normal", which is a gate clock provided by clkgen. If I extracted those SYS_CTRL gate clocks and define them in system controller dts node, I may have to use "clk_gate_rp_cpu_normal" as their input, it looks a bit wierd becasue there are cases where each other serves as input. I try to draft below DTS to explan what I meant. I'm not sure if it can work and I'd love to hear your guidance.

Quoted 6 lines
Quoted 2 lines

I'm not sure how this sort of circular relationship works for probing works either. Stephen etc would know more than me here.

It generally works fine. The common clock framework can handle the child clock being registered before its parent, even when using a DT (fw_name) reference. See for example clk_core_fill_parent_index() and clk_core_reparent_orphans_nolock() in drivers/clk/clk.c

Learned and thank you.

Quoted 4 lines

Regards,
Samuel

author
#24 lore raw

PATCH v7 3/4 clk: sophgo: Add SG2042 clock generator driver

In reply to Chen Wang

From: Chen Wang <[email protected]>

Add a driver for the SOPHGO SG2042 clock generator.

---
 MAINTAINERS                            |    7 +
 drivers/clk/Kconfig                    |    1 +
 drivers/clk/Makefile                   |    1 +
 drivers/clk/sophgo/Kconfig             |    8 +
 drivers/clk/sophgo/Makefile            |    2 +
 drivers/clk/sophgo/clk-sophgo-sg2042.c | 1316 ++++++++++++++++++++++++
 drivers/clk/sophgo/clk-sophgo-sg2042.h |  236 +++++
 7 files changed, 1571 insertions(+)
 create mode 100644 drivers/clk/sophgo/Kconfig
 create mode 100644 drivers/clk/sophgo/Makefile
 create mode 100644 drivers/clk/sophgo/clk-sophgo-sg2042.c
 create mode 100644 drivers/clk/sophgo/clk-sophgo-sg2042.h
MAINTAINERS +7 -0
index a7c4cf8201e0..6b5019ba0794 100644
@@ -20197,6 +20197,13 @@ S:	Maintained
 F:	arch/riscv/boot/dts/sophgo/
 F:	Documentation/devicetree/bindings/riscv/sophgo.yaml
 
+SOPHGO CLOCK DRIVER
+M:	Chen Wang <[email protected]>
+S:	Maintained
+F:	Documentation/devicetree/bindings/clock/sophgo/
+F:	drivers/clk/sophgo/
+F:	include/dt-bindings/clock/sophgo,sg2042-clkgen.h
+
 SOUND
 M:	Jaroslav Kysela <[email protected]>
 M:	Takashi Iwai <[email protected]>
drivers/clk/Kconfig +1 -0
index c30d0d396f7a..514343934fda 100644
@@ -499,6 +499,7 @@ source "drivers/clk/rockchip/Kconfig"
 source "drivers/clk/samsung/Kconfig"
 source "drivers/clk/sifive/Kconfig"
 source "drivers/clk/socfpga/Kconfig"
+source "drivers/clk/sophgo/Kconfig"
 source "drivers/clk/sprd/Kconfig"
 source "drivers/clk/starfive/Kconfig"
 source "drivers/clk/sunxi/Kconfig"
drivers/clk/Makefile +1 -0
index ed71f2e0ee36..eeae7ae93f89 100644
@@ -119,6 +119,7 @@ obj-$(CONFIG_ARCH_ROCKCHIP)		+= rockchip/
 obj-$(CONFIG_COMMON_CLK_SAMSUNG)	+= samsung/
 obj-$(CONFIG_CLK_SIFIVE)		+= sifive/
 obj-y					+= socfpga/
+obj-y					+= sophgo/
 obj-$(CONFIG_PLAT_SPEAR)		+= spear/
 obj-y					+= sprd/
 obj-$(CONFIG_ARCH_STI)			+= st/
drivers/clk/sophgo/Kconfig new file +8 -0
new file mode 100644
index 000000000000..feab0b805a29
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0
+
+config CLK_SOPHGO_SG2042
+	bool "Sophgo SG2042 clock support"
+	depends on ARCH_SOPHGO || COMPILE_TEST
+	default y
+	help
+	  Say yes here to support the clock controller on the Sophgo SG2042 SoC.
drivers/clk/sophgo/Makefile new file +2 -0
new file mode 100644
index 000000000000..13834cce260c
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_CLK_SOPHGO_SG2042)	+= clk-sophgo-sg2042.o
drivers/clk/sophgo/clk-sophgo-sg2042.c new file +1316 -0

1,316 lines not on this page · raw message

drivers/clk/sophgo/clk-sophgo-sg2042.h new file +236 -0

236 lines not on this page · raw message

Signature · 1 line
2.25.1
author
#25 lore raw

PATCH v7 4/4 riscv: dts: add clock generator for Sophgo SG2042 SoC

In reply to Chen Wang

From: Chen Wang <[email protected]>

Add clock generator node to device tree for SG2042, and enable clock for uart.

---
 .../boot/dts/sophgo/sg2042-milkv-pioneer.dts  |  4 ++++
 arch/riscv/boot/dts/sophgo/sg2042.dtsi        | 23 +++++++++++++++++++
 2 files changed, 27 insertions(+)
arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts +4 -0
index 49b4b9c2c101..0b3b3b2b0c64 100644
@@ -14,6 +14,10 @@ chosen {
 	};
 };
 
+&cgi {
+	clock-frequency = <25000000>;
+};
+
 &uart0 {
 	status = "okay";
 };
arch/riscv/boot/dts/sophgo/sg2042.dtsi +23 -0
index 93256540d078..c9616d111905 100644
@@ -5,6 +5,7 @@
 
 /dts-v1/;
 #include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/clock/sophgo,sg2042-clkgen.h>
 
 #include "sg2042-cpus.dtsi"
 
@@ -18,6 +19,12 @@ aliases {
 		serial0 = &uart0;
 	};
 
+	cgi: oscillator {
+		compatible = "fixed-clock";
+		clock-output-names = "cgi";
+		#clock-cells = <0>;
+	};
+
 	soc: soc {
 		compatible = "simple-bus";
 		#address-cells = <2>;
@@ -311,12 +318,28 @@ intc: interrupt-controller@7090000000 {
 			riscv,ndev = <224>;
 		};
 
+		sys_ctrl: system-controller@7030010000 {
+			compatible = "sophgo,sg2042-sysctrl";
+			reg = <0x70 0x30010000 0x0 0x1000>;
+		};
+
+		clkgen: clock-controller@7030012000 {
+			compatible = "sophgo,sg2042-clkgen";
+			reg = <0x70 0x30012000 0x0 0x1000>;
+			sophgo,system-ctrl = <&sys_ctrl>;
+			#clock-cells = <1>;
+			clocks = <&cgi>;
+		};
+
 		uart0: serial@7040000000 {
 			compatible = "snps,dw-apb-uart";
 			reg = <0x00000070 0x40000000 0x00000000 0x00001000>;
 			interrupt-parent = <&intc>;
 			interrupts = <112 IRQ_TYPE_LEVEL_HIGH>;
 			clock-frequency = <500000000>;
+			clocks = <&clkgen GATE_CLK_UART_500M>,
+				 <&clkgen GATE_CLK_APB_UART>;
+			clock-names = "baudclk", "apb_pclk";
 			reg-shift = <2>;
 			reg-io-width = <4>;
 			status = "disabled";
Signature · 1 line
2.25.1
#26 lore raw

Re: [PATCH v7 4/4] riscv: dts: add clock generator for Sophgo SG2042 SoC

arch/riscv/boot/dts/sophgo/sg2042.dtsi aliases { · line 26

Quoted 5 lines · Chen Wang
arch/riscv/boot/dts/sophgo/sg2042.dtsi +5 -0
+	cgi: oscillator {
+		compatible = "fixed-clock";
+		clock-output-names = "cgi";
+		#clock-cells = <0>;
+	};

Where does the name "cgi" come from and what does it mean? Clock Generator Input? Does the sg2042 documentation call it that?

Cheers,
Conor.

#27 lore raw

Re: [PATCH v7 4/4] riscv: dts: add clock generator for Sophgo SG2042 SoC

Quoted 9 lines · Conor Dooley

arch/riscv/boot/dts/sophgo/sg2042.dtsi aliases { · line 26

Quoted 5 lines · Chen Wang
arch/riscv/boot/dts/sophgo/sg2042.dtsi +5 -0
+	cgi: oscillator {
+		compatible = "fixed-clock";
+		clock-output-names = "cgi";
+		#clock-cells = <0>;
+	};

Where does the name "cgi" come from and what does it mean? Clock Generator Input? Does the sg2042 documentation call it that?

It's abbrevation of "Clock Gen IC", I found this in the clock tree diagram. https://github.com/sophgo/sophgo-doc/blob/main/SG2042/TRM/source/pic/clock-tree.png

BTW, There are three cgi on the diagram. Should I define 3 in DTS too? I just found they are the same so I just defined one.

Quoted 3 lines

Cheers,
Conor.