Introduce Nuvoton Arbel NPCM8XX BMC SoC
Subsystems (3)
Tomer Maimon
maintainerv22 cover Introduce Nuvoton Arbel NPCM8XX BMC SoC
This patchset adds clock support for the Nuvoton Arbel NPCM8XX Board Management controller (BMC) SoC family.
This patchset cover letter is based from the initial support for NPCM8xx BMC to keep tracking the version history.
all the other initial support patches had been applied to Linux kernel 6.0.
This patchset was tested on the Arbel NPCM8XX evaluation board.
Addressed comments from:
- Stephen Boyd: https://www.spinics.net/lists/linux-clk/msg93838.html
Changes since version 21:
- Since using regmap instead of ioremap replace reg to syscon property in dt-bindings and dts.
- Add reference clock property to the dt-bindings and dts.
- Using .index instead of .name in clk_parent_data structures.
- Using string where any macros are used once.
Changes since version 20:
- Using regmap instead of ioremap.
the clock and reset modules are sharing the same memory region and cause failure when using devm_platform_ioremap_resource function, this version uses regmap to handle shared reset and clock memory region, in case it is approved I will modify the reset driver to use the regmap as well.
- Using clk_hw instead of clk_parent_data structre.
- Divider clock definition to one line
Changes since version 19:
- Remove unnecessary free command.
- Defining pr_fmt().
- Using dev_err_probe.
- Return zero in the end of the probe function.
Changes since version 18:
- NPCM8XX clock driver did not changed from version 18 only build and tested under kernel 6.6-rc1.
Changes since version 17:
- NPCM8XX clock driver did not changed from version 17 only build and tested under kernel 6.5-rc3.
Changes since version 16:
- NPCM8XX clock driver
- Using devm_kzalloc instead kzalloc.
- Remove unnecessary parenthesis.
- Modify incorrect spelling.
Changes since version 15:
- NPCM8XX clock driver
- Remove unused regs parameter from npcm8xx_pll_data structure.
- Using index and clk_hw parameters to set the clock parent in the clock structures.
Changes since version 14:
- NPCM8XX clock driver
- Remove unnecessary register definitions.
- Remove the internal reference clock, instead use the external DT reference clock.
- rearrange the driver.
- using .names parameter in DT to define clock (refclk).
Changes since version 13:
- NPCM8XX clock driver
- Remove unnecessary definitions and add module.h define
- Use in clk_parent_data struct.fw_name and .name.
- Add module_exit function.
- Add const to divider clock names.
- Add MODULE_DESCRIPTION and MODULE_LICENSE
Changes since version 12:
- NPCM8XX clock driver
- Use clk_parent_data in mux and div clock structure.
- Add const to mux tables.
- Using devm_clk_hw_register_fixed_rate function.
- use only .name clk_parent_data instead .name and .fw_name.
- Modify mask values in mux clocks.
Changes since version 11:
- NPCM8XX clock driver
- Modify Kconfig help.
- Modify loop variable to unsigned int.
Changes since version 11:
- NPCM8XX clock driver
- Modify Kconfig help.
- Modify loop variable to unsigned int.
Changes since version 10:
- NPCM8XX clock driver
- Fix const warning.
Changes since version 9:
- NPCM8XX clock driver
- Move configuration place.
- Using clk_parent_data instead of parent_name
- using devm_ioremap instead of ioremap. deeply sorry, I know we had a long discussion on what should the driver use, from other examples (also in other clock drivers) I see the combination of platform_get_resource and devm_ioremap are commonly used and it answer the reset and clock needs.
Changes since version 8:
- NPCM8XX clock driver
- Move configuration place.
- Add space before and aftre '{' '}'.
- Handle devm_of_clk_add_hw_provider function error.
Changes since version 7:
- NPCM8XX clock driver
- The clock and reset registers using the same memory region, due to it the clock driver should claim the ioremap directly without checking the memory region.
Changes since version 5:
- NPCM8XX clock driver
- Remove refclk if devm_of_clk_add_hw_provider function failed.
Changes since version 4:
- NPCM8XX clock driver
- Use the same quote in the dt-binding file.
Changes since version 3:
- NPCM8XX clock driver
- Rename NPCM8xx clock dt-binding header file.
- Remove unused structures.
- Improve Handling the clocks registration.
Changes since version 2:
- NPCM8XX clock driver
- Add debug new line.
- Add 25M fixed rate clock.
- Remove unused clocks and clock name from dt-binding.
Changes since version 1:
- NPCM8XX clock driver
- Modify dt-binding.
- Remove unsed definition and include.
- Include alphabetically.
- Use clock devm.
Tomer Maimon (8):
dt-bindings: clock: npcm845: Add reference 25m clock property
arm64: dts: nuvoton: npcm8xx: add refernace clock
arm: dts: nuvoton: npcm7xx: modify rst syscon node
dt-bindings: soc: nuvoton: add binding for clock and reset registers
arm64: dts: nuvoton: npcm8xx: add clock reset syscon node
dt-bindings: clock: npcm845: replace reg with syscon property
arm64: dts: nuvoton: npcm8xx: replace reg with syscon property
clk: npcm8xx: add clock controller
.../bindings/clock/nuvoton,npcm845-clk.yaml | 38 +- .../soc/nuvoton/nuvoton,npcm-clk-rst.yaml | 40 ++ .../dts/nuvoton/nuvoton-common-npcm7xx.dtsi | 4 +- .../dts/nuvoton/nuvoton-common-npcm8xx.dtsi | 19 +- .../boot/dts/nuvoton/nuvoton-npcm845-evb.dts | 6 + drivers/clk/clk-npcm8xx.c | 510 ++++++++++++++++++ 6 files changed, 597 insertions(+), 20 deletions(-) create mode 100644 Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml create mode 100644 drivers/clk/clk-npcm8xx.c
Signature · 1 line
2.34.1
Tomer Maimon
maintainerPATCH v22 1/8 dt-bindings: clock: npcm845: Add reference 25m clock property
The NPCM8XX clock driver uses 25Mhz external clock, therefor adding refclk property.
--- .../bindings/clock/nuvoton,npcm845-clk.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +16 -0
@@ -21,6 +21,14 @@ properties: reg: maxItems: 1 + clocks: + items: + - description: 25Mhz referance clock + + clock-names: + items: + - const: refclk + '#clock-cells': const: 1 description:
@@ -30,12 +38,20 @@ properties: required: - compatible - reg + - clocks + - clock-names - '#clock-cells' additionalProperties: false examples: - | + refclk: refclk-25mhz { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <25000000>; + }; + ahb { #address-cells = <2>; #size-cells = <2>;
Signature · 1 line
2.34.1
Rob Herring
maintainerRe: [PATCH v22 1/8] dt-bindings: clock: npcm845: Add reference 25m clock property
Quoted 8 lines · Tomer Maimon
The NPCM8XX clock driver uses 25Mhz external clock, therefor adding refclk property.
Signed-off-by Tomer Maimon <[email protected]>--- .../bindings/clock/nuvoton,npcm845-clk.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
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:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.example.dtb: clock-controller@f0801000: 'clocks' is a required property
from schema $id: http://devicetree.org/schemas/clock/nuvoton,npcm845-clk.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.example.dtb: clock-controller@f0801000: 'clock-names' is a required property
from schema $id: http://devicetree.org/schemas/clock/nuvoton,npcm845-clk.yaml#
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch.
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 after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Tomer Maimon
maintainerRe: [PATCH v22 1/8] dt-bindings: clock: npcm845: Add reference 25m clock property
Hi Rob,
Thanks for your comment.
Quoted 34 lines · Rob Herring
Quoted 8 lines · Tomer Maimon
The NPCM8XX clock driver uses 25Mhz external clock, therefor adding refclk property.
Signed-off-by Tomer Maimon <[email protected]>--- .../bindings/clock/nuvoton,npcm845-clk.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)My bot found errors running 'make DT_CHECKER_FLAGS=-m
dt_binding_check' on your patch (DT_CHECKER_FLAGSis new in v5.13):yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.example.dtb: clock-controller@f0801000: 'clocks' is a required property
from schema $id: http://devicetree.org/schemas/clock/nuvoton,npcm845-clk.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.example.dtb: clock-controller@f0801000: 'clock-names' is a required property
from schema $id: http://devicetree.org/schemas/clock/nuvoton,npcm845-clk.yaml#doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch.
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:
Quoted 6 lines
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note that
DT_SCHEMA_FILEScan be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
probably I missed adding the clock and clock-names to the example node, will be fixed next version
Rob Herring
maintainerRe: [PATCH v22 1/8] dt-bindings: clock: npcm845: Add reference 25m clock property
In reply to Tomer MaimonQuoted 1 line · Tomer Maimon
The NPCM8XX clock driver uses 25Mhz external clock, therefor adding
therefore
Quoted 1 line
refclk property.
'refclk' is not a property.
Quoted 11 lines
Signed-off-by Tomer Maimon <[email protected]>--- .../bindings/clock/nuvoton,npcm845-clk.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +0
-0
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml properties: · line 26
Quoted 6 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +3
-0@@ -21,6 +21,14 @@ properties: reg: maxItems: 1 + clocks: + items: + - description: 25Mhz referance clock
reference
Quoted 9 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +5
-0+ + clock-names: + items: + - const: refclk + '#clock-cells': const: 1 description:
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml properties: · line 42
Quoted 5 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +2
-0@@ -30,12 +38,20 @@ properties: required: - compatible - reg + - clocks + - clock-names
New required properties are an ABI break. That's fine if you explain why that's okay in the commit msg.
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml properties: · line 53
Quoted 11 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +5
-0- '#clock-cells' additionalProperties: false examples: - | + refclk: refclk-25mhz { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <25000000>; + };
Examples don't need to show providers.
Quoted 7 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +1
-0+ ahb { #address-cells = <2>; #size-cells = <2>;Signature · 1 line
2.34.1
Tomer Maimon
maintainerRe: [PATCH v22 1/8] dt-bindings: clock: npcm845: Add reference 25m clock property
Hi Rob,
Thanks for your comment.
Quoted 39 lines · Rob Herring
Quoted 1 line · Tomer Maimon
The NPCM8XX clock driver uses 25Mhz external clock, therefor adding
therefore
Quoted 1 line
refclk property.
'refclk' is not a property.
Quoted 11 lines
Signed-off-by Tomer Maimon <[email protected]>--- .../bindings/clock/nuvoton,npcm845-clk.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +0
-0Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml properties: · line 26
Quoted 6 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +3
-0@@ -21,6 +21,14 @@ properties: reg: maxItems: 1 + clocks: + items: + - description: 25Mhz referance clockreference
Quoted 9 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +5
-0+ + clock-names: + items: + - const: refclk + '#clock-cells': const: 1 description:@@ -30,12 +38,20 @@ properties:
Quoted 8 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml properties: · line 42
Quoted 5 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +2
-0required: - compatible - reg + - clocks + - clock-namesNew required properties are an ABI break. That's fine if you explain why that's okay in the commit msg.
What do you mean?
Could I add the new required properties to the required list?
Quoted 14 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml properties: · line 53
Quoted 13 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +5
-0- '#clock-cells' additionalProperties: false examples: - | + refclk: refclk-25mhz { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <25000000>; + };
Quoted 9 lines
Examples don't need to show providers.
Quoted 7 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +1
-1+ ahb { #address-cells = <2>; #size-cells = <2>; --2.34.1
Best regards,
Tomer
Re: [PATCH v22 1/8] dt-bindings: clock: npcm845: Add reference 25m clock property
Quoted 17 lines · Tomer Maimon
Quoted 9 lines
Quoted 9 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +5
-0+ + clock-names: + items: + - const: refclk + '#clock-cells': const: 1 description:@@ -30,12 +38,20 @@ properties:Quoted 8 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml properties: · line 42
Quoted 5 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +2
-0required: - compatible - reg + - clocks + - clock-namesNew required properties are an ABI break. That's fine if you explain why that's okay in the commit msg.
Quoted 1 line
What do you mean?
I think it was clear. Which part is not clear?
Quoted 1 line
Could I add the new required properties to the required list?
You just did, didn't you? And received feedback that you are breaking the ABI.
Best regards,
Krzysztof
Stephen Boyd
maintainerRe: [PATCH v22 1/8] dt-bindings: clock: npcm845: Add reference 25m clock property
Quoting Krzysztof Kozlowski (2024-01-10 12:54:14)
Quoted 18 lines · Tomer Maimon
Quoted 17 lines
Quoted 17 lines
Quoted 9 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +5
-0+ + clock-names: + items: + - const: refclk + '#clock-cells': const: 1 description:Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml properties: · line 42
Quoted 5 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +2
-0@@ -30,12 +38,20 @@ properties: required: - compatible - reg + - clocks + - clock-namesNew required properties are an ABI break. That's fine if you explain why that's okay in the commit msg.
Quoted 9 lines
Quoted 1 line
What do you mean?
I think it was clear. Which part is not clear?
Quoted 1 line
Could I add the new required properties to the required list?
You just did, didn't you? And received feedback that you are breaking the ABI.
It's fine to break the ABI as long as the commit message explains that the driver isn't merged yet.
Tomer Maimon
maintainerRe: [PATCH v22 1/8] dt-bindings: clock: npcm845: Add reference 25m clock property
Hi Stephen,
Quoted 24 lines · Stephen Boyd
Quoting Krzysztof Kozlowski (2024-01-10 12:54:14)
Quoted 22 lines · Tomer Maimon
Quoted 17 lines
Quoted 9 lines
Quoted 9 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +5
-0+ + clock-names: + items: + - const: refclk + '#clock-cells': const: 1 description:@@ -30,12 +38,20 @@ properties:Quoted 8 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml properties: · line 42
Quoted 5 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +2
-0required: - compatible - reg + - clocks + - clock-namesNew required properties are an ABI break. That's fine if you explain why that's okay in the commit msg.
Quoted 1 line
What do you mean?
I think it was clear. Which part is not clear?
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
Could I add the new required properties to the required list?
You just did, didn't you? And received feedback that you are breaking the ABI.
It's fine to break the ABI as long as the commit message explains that the driver isn't merged yet.
Thanks for your clarification.
Best regards,
Tomer
Tomer Maimon
maintainerPATCH v22 2/8 arm64: dts: nuvoton: npcm8xx: add refernace clock
In reply to Tomer MaimonAdd 25Mhz reference clock since the reference clock in not a part of Nuvoton BMC NPCM8XX SoC.
--- .../arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi | 10 ++++++---- arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts | 6 ++++++ 2 files changed, 12 insertions(+), 4 deletions(-)
arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +6 -4
@@ -58,6 +58,8 @@ clk: clock-controller@f0801000 { compatible = "nuvoton,npcm845-clk"; #clock-cells = <1>; reg = <0x0 0xf0801000 0x0 0x1000>; + clocks = <&refclk>; + clock-names = "refclk"; }; apb {
@@ -81,7 +83,7 @@ timer0: timer@8000 { compatible = "nuvoton,npcm845-timer"; interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>; reg = <0x8000 0x1C>; - clocks = <&clk NPCM8XX_CLK_REFCLK>; + clocks = <&refclk>; clock-names = "refclk"; };
@@ -153,7 +155,7 @@ watchdog0: watchdog@801c { interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>; reg = <0x801c 0x4>; status = "disabled"; - clocks = <&clk NPCM8XX_CLK_REFCLK>; + clocks = <&refclk>; syscon = <&gcr>; };
@@ -162,7 +164,7 @@ watchdog1: watchdog@901c { interrupts = <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>; reg = <0x901c 0x4>; status = "disabled"; - clocks = <&clk NPCM8XX_CLK_REFCLK>; + clocks = <&refclk>; syscon = <&gcr>; };
@@ -171,7 +173,7 @@ watchdog2: watchdog@a01c { interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>; reg = <0xa01c 0x4>; status = "disabled"; - clocks = <&clk NPCM8XX_CLK_REFCLK>; + clocks = <&refclk>; syscon = <&gcr>; }; };
arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts +6 -0
@@ -19,6 +19,12 @@ chosen { memory { reg = <0x0 0x0 0x0 0x40000000>; }; + + refclk: refclk-25mhz { + compatible = "fixed-clock"; + clock-frequency = <25000000>; + #clock-cells = <0>; + }; }; &serial0 {
Signature · 1 line
2.34.1
Tomer Maimon
maintainerPATCH v22 3/8 arm: dts: nuvoton: npcm7xx: modify rst syscon node
In reply to Tomer Maimonrst node name and compatible property modified since clock and reset are handled in the same memory region.
--- arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi +2 -2
@@ -93,8 +93,8 @@ gcr: gcr@800000 { reg = <0x800000 0x1000>; }; - rst: rst@801000 { - compatible = "nuvoton,npcm750-rst", "syscon", "simple-mfd"; + clk_rst: syscon@801000 { + compatible = "nuvoton,npcm750-clk-rst", "syscon", "simple-mfd"; reg = <0x801000 0x6C>; }; };
Signature · 1 line
2.34.1
Re: [PATCH v22 3/8] arm: dts: nuvoton: npcm7xx: modify rst syscon node
Quoted 13 lines · Tomer Maimon
rst node name and compatible property modified since clock and reset are handled in the same memory region.
Signed-off-by Tomer Maimon <[email protected]>--- arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi +0
-0
arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi gcr: gcr@800000 { · line 97
Quoted 7 lines
arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi +2
-2@@ -93,8 +93,8 @@ gcr: gcr@800000 { reg = <0x800000 0x1000>; }; - rst: rst@801000 { - compatible = "nuvoton,npcm750-rst", "syscon", "simple-mfd"; + clk_rst: syscon@801000 { + compatible = "nuvoton,npcm750-clk-rst", "syscon", "simple-mfd";
NAK. This breakes the users, is not justified, is not explained.
Best regards,
Krzysztof
Tomer Maimon
maintainerRe: [PATCH v22 3/8] arm: dts: nuvoton: npcm7xx: modify rst syscon node
Hi Krzysztof,
Thanks for your comment.
Quoted 18 lines · Krzysztof Kozlowski
Quoted 16 lines · Tomer Maimon
rst node name and compatible property modified since clock and reset are handled in the same memory region.
Signed-off-by Tomer Maimon <[email protected]>--- arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi +0
-0@@ -93,8 +93,8 @@ gcr: gcr@800000 { reg = <0x800000 0x1000>; };
Quoted 6 lines
arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi gcr: gcr@800000 { · line 97
Quoted 4 lines
arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi +2
-2- rst: rst@801000 { - compatible = "nuvoton,npcm750-rst", "syscon", "simple-mfd"; + clk_rst: syscon@801000 { + compatible = "nuvoton,npcm750-clk-rst", "syscon", "simple-mfd";NAK. This breakes the users, is not justified, is not explained.
Sorry, I didn't understand, which user it is breaking? there isn't a device tree node that uses the rst node.
Should I explain it better in the commit message?
Quoted 4 lines
Best regards,
Krzysztof
Best regards,
Tomer
Re: [PATCH v22 3/8] arm: dts: nuvoton: npcm7xx: modify rst syscon node
Quoted 24 lines · Tomer Maimon
Hi Krzysztof,
Thanks for your comment.
Quoted 18 lines · Krzysztof Kozlowski
Quoted 16 lines · Tomer Maimon
rst node name and compatible property modified since clock and reset are handled in the same memory region.
Signed-off-by Tomer Maimon <[email protected]>--- arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi +0
-0@@ -93,8 +93,8 @@ gcr: gcr@800000 { reg = <0x800000 0x1000>; };
Quoted 8 lines
Quoted 6 lines
arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi gcr: gcr@800000 { · line 97
Quoted 4 lines
arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi +2
-2- rst: rst@801000 { - compatible = "nuvoton,npcm750-rst", "syscon", "simple-mfd"; + clk_rst: syscon@801000 { + compatible = "nuvoton,npcm750-clk-rst", "syscon", "simple-mfd";NAK. This breakes the users, is not justified, is not explained.
Sorry, I didn't understand, which user it is breaking? there isn't a device tree node that uses the rst node.
Any user of this DTS.
Quoted 1 line
Should I explain it better in the commit message?
I doubt that you can find proper reason, because "I want different name" is not valid. It's just compatible, it cannot be changed just because you add new property.
Best regards,
Krzysztof
Tomer Maimon
maintainerPATCH v22 4/8 dt-bindings: soc: nuvoton: add binding for clock and reset registers
In reply to Tomer MaimonA nuvoton,*-clk-rst node is present in nuvoton-common-npcm7xx.dtsi and will be added to nuvoton-common-npcm8xx.dtsi. It is necessary for the NPCM7xx and NPCM8xx clock and reset drivers, and may later be used to retrieve SoC model and version information.
This patch adds a binding to describe this node.
--- .../soc/nuvoton/nuvoton,npcm-clk-rst.yaml | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +40 -0
@@ -0,0 +1,40 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/soc/nuvoton/nuvoton,npcm-clk-rst.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Clock and reset registers block in Nuvoton SoCs + +maintainers: + - Tomer Maimon <[email protected]> + +description: + The clock and reset registers are a registers block in Nuvoton SoCs that + handle both reset and clock functionality. + +properties: + compatible: + items: + - enum: + - nuvoton,npcm750-clk-rst + - nuvoton,npcm845-clk-rst + - const: syscon + - const: simple-mfd + + reg: + maxItems: 1 + +required: + - compatible + - reg + +additionalProperties: + type: object + +examples: + - | + clk_rst: syscon@801000 { + compatible = "nuvoton,npcm750-clk-rst", "syscon", "simple-mfd"; + reg = <0x801000 0x6C>; + };
Signature · 1 line
2.34.1
Re: [PATCH v22 4/8] dt-bindings: soc: nuvoton: add binding for clock and reset registers
Quoted 5 lines · Tomer Maimon
A nuvoton,*-clk-rst node is present in
nuvoton-common-npcm7xx.dtsiand will be added tonuvoton-common-npcm8xx.dtsi. It is necessary for the NPCM7xx and NPCM8xx clock and reset drivers, and may later be used to retrieve SoC model and version information.
A nit, subject: drop second/last, redundant "bindings". The "dt-bindings" prefix is already stating that these are bindings. See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
Quoted 1 line
This patch adds a binding to describe this node.
Please do not use "This commit/patch/change", but imperative mood. See longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
Quoted 3 lines
Signed-off-by Tomer Maimon <[email protected]>---
How possibly could it be v22 if there is:
1. No changelog
2. No previous submissions
?
NAK, it's something completely new without any explanation.
Limited review follows.
Quoted 10 lines
.../soc/nuvoton/nuvoton,npcm-clk-rst.yaml | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yamlDocumentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +0
-0
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml line 7
Quoted 7 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +7
-0@@ -0,0 +1,40 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/soc/nuvoton/nuvoton,npcm-clk-rst.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Clock and reset registers block in Nuvoton SoCs
This is vague. Any block? All blocks? Your SoC has only one block? I doubt, although possible.
Anyway, clocks go to clock directory, not to soc! We've been here and you already received that feedback.
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml line 14
Quoted 7 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +7
-0+ +maintainers: + - Tomer Maimon <[email protected]> + +description: + The clock and reset registers are a registers block in Nuvoton SoCs that + handle both reset and clock functionality.
That's still vague. Say something useful.
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml line 23
Quoted 9 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +9
-0+ +properties: + compatible: + items: + - enum: + - nuvoton,npcm750-clk-rst + - nuvoton,npcm845-clk-rst + - const: syscon + - const: simple-mfd
No, it's not a syscon and not a simple-mfd. You just said it is clock provider and reset controller. Thus missing clock cells and reset cells.
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml line 33
Quoted 10 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +10
-0+ + reg: + maxItems: 1 + +required: + - compatible + - reg + +additionalProperties: + type: object
No, instead:
additionalProperties: false
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml line 37
Quoted 4 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +4
-0+ +examples: + - | + clk_rst: syscon@801000 {
Suddenly a syscon?
Drop unused label.
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml line 39
Quoted 2 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +2
-0+ compatible = "nuvoton,npcm750-clk-rst", "syscon", "simple-mfd"; + reg = <0x801000 0x6C>;
Only lowercase hex.
You just sent some v22 of something new, making all the mistakes from the past submissions for which you received feedback.
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml line 40
Quoted 1 line
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +1
-0+ };
Best regards,
Krzysztof
Tomer Maimon
maintainerRe: [PATCH v22 4/8] dt-bindings: soc: nuvoton: add binding for clock and reset registers
Hi Krzysztof,
Thanks for your comments.
Sorry for the long explanation but I think it is necessary.
In the NPCM8XX SoC, the reset and the clock register modules are scrambled in the same memory register region.
The NPCM8XX Clock driver is still in the upstream process (for a long time) but the NPCM8XX reset driver is already upstreamed.
One of the main comments in the NPCM8XX Clock driver upstream process is that the clock register is mixed with the reset register and therefore we can't map (ioremap) the clock register
region because is already mapped by the reset module, therefore we decided to use an external syscon to handle the clock and the reset registers driver.
I highly appreciate your guidance on this topic.
Quoted 20 lines · Krzysztof Kozlowski
Quoted 5 lines · Tomer Maimon
A nuvoton,*-clk-rst node is present in
nuvoton-common-npcm7xx.dtsiand will be added tonuvoton-common-npcm8xx.dtsi. It is necessary for the NPCM7xx and NPCM8xx clock and reset drivers, and may later be used to retrieve SoC model and version information.A nit, subject: drop second/last, redundant "bindings". The "dt-bindings" prefix is already stating that these are bindings. See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18Quoted 1 line
This patch adds a binding to describe this node.
Please do not use "This commit/patch/change", but imperative mood. See longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95Quoted 1 line
Quoted 7 lines
Quoted 2 lines
Signed-off-by Tomer Maimon <[email protected]>---
How possibly could it be v22 if there is:
1. No changelog
2. No previous submissions
?
Should the dt-binding and dts patches be a part of the clock patch set (this is why it's V22) or should I open a new patch set?
Quoted 27 lines
NAK, it's something completely new without any explanation.
Limited review follows.
Quoted 10 lines
.../soc/nuvoton/nuvoton,npcm-clk-rst.yaml | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yamlDocumentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +0
-0Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml line 7
Quoted 7 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +7
-0@@ -0,0 +1,40 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/soc/nuvoton/nuvoton,npcm-clk-rst.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Clock and reset registers block in Nuvoton SoCsThis is vague. Any block? All blocks? Your SoC has only one block? I doubt, although possible.
Quoted 2 lines
Anyway, clocks go to clock directory, not to soc! We've been here and you already received that feedback.
Since one region handles the reset and the clock registers shouldn't I add the dt-binding to the SoC like the GCR and not to the clock directory?
https://elixir.bootlin.com/linux/v6.7/source/Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-gcr.yaml
Quoted 11 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml line 14
Quoted 9 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +7
-0+ +maintainers: + - Tomer Maimon <[email protected]> + +description: + The clock and reset registers are a registers block in Nuvoton SoCs that + handle both reset and clock functionality.That's still vague. Say something useful.
Will describe more
Quoted 12 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml line 23
Quoted 10 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +9
-0+ +properties: + compatible: + items: + - enum: + - nuvoton,npcm750-clk-rst + - nuvoton,npcm845-clk-rst + - const: syscon + - const: simple-mfdNo, it's not a syscon and not a simple-mfd. You just said it is clock
Yes, I understand the syscon node represents a register region containing a set of miscellaneous registers, but as explain above it is quite the case here.
I will remove the simple-mfd.
Quoted 1 line
provider and reset controller. Thus missing clock cells and reset cells.
The reset cell and clock cell found at the clock and reset dt-binding, is it enough?
Quoted 14 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml line 33
Quoted 11 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +10
-0+ + reg: + maxItems: 1 + +required: + - compatible + - reg + +additionalProperties: + type: objectNo, instead:
additionalProperties: false
O.K.
Quoted 5 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml line 37
Quoted 5 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +4
-0+ +examples: + - | + clk_rst: syscon@801000 {
I should use syscon no? if no what should I use?
Quoted 8 lines
Suddenly a syscon?
Drop unused label.
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml line 39
Quoted 2 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +2
-0+ compatible = "nuvoton,npcm750-clk-rst", "syscon", "simple-mfd"; + reg = <0x801000 0x6C>;
Quoted 9 lines
Only lowercase hex.
You just sent some v22 of something new, making all the mistakes from the past submissions for which you received feedback.
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml line 40
Quoted 1 line
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +1
-0+ };Best regards,
Krzysztof
Thanks a lot!
Tomer
Re: [PATCH v22 4/8] dt-bindings: soc: nuvoton: add binding for clock and reset registers
Quoted 11 lines · Tomer Maimon
Hi Krzysztof,
Thanks for your comments.
Sorry for the long explanation but I think it is necessary.
In the NPCM8XX SoC, the reset and the clock register modules are scrambled in the same memory register region.
The NPCM8XX Clock driver is still in the upstream process (for a long time) but the NPCM8XX reset driver is already upstreamed.
Quoted 8 lines
One of the main comments in the NPCM8XX Clock driver upstream process is that the clock register is mixed with the reset register and therefore we can't map (ioremap) the clock register
region because is already mapped by the reset module, therefore we decided to use an external syscon to handle the clock and the reset registers driver.I highly appreciate your guidance on this topic.
Linux deals with it easily, that's why we have regmaps. What's the problem exactly?
Quoted 26 lines
Quoted 23 lines · Krzysztof Kozlowski
Quoted 5 lines · Tomer Maimon
A nuvoton,*-clk-rst node is present in
nuvoton-common-npcm7xx.dtsiand will be added tonuvoton-common-npcm8xx.dtsi. It is necessary for the NPCM7xx and NPCM8xx clock and reset drivers, and may later be used to retrieve SoC model and version information.A nit, subject: drop second/last, redundant "bindings". The "dt-bindings" prefix is already stating that these are bindings. See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18Quoted 1 line
This patch adds a binding to describe this node.
Please do not use "This commit/patch/change", but imperative mood. See longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95Quoted 3 lines
Signed-off-by Tomer Maimon <[email protected]>---
Quoted 6 lines
Quoted 4 lines
How possibly could it be v22 if there is:
1. No changelog
2. No previous submissions
?Should the dt-binding and dts patches be a part of the clock patch set (this is why it's V22) or should I open a new patch set?
You should explain what is happening here. That's why you have changelog for.
Quoted 27 lines
Quoted 27 lines
NAK, it's something completely new without any explanation.
Limited review follows.
Quoted 10 lines
.../soc/nuvoton/nuvoton,npcm-clk-rst.yaml | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yamlDocumentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +0
-0Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml line 7
Quoted 7 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +7
-0@@ -0,0 +1,40 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/soc/nuvoton/nuvoton,npcm-clk-rst.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Clock and reset registers block in Nuvoton SoCsThis is vague. Any block? All blocks? Your SoC has only one block? I doubt, although possible.
Quoted 4 lines
Quoted 2 lines
Anyway, clocks go to clock directory, not to soc! We've been here and you already received that feedback.
Since one region handles the reset and the clock registers shouldn't I add the dt-binding to the SoC like the GCR and not to the clock
No, soc is not a dumping ground..
Quoted 2 lines
Choose the main feature of the block - either clock controller or reset controller - and put it there.
Quoted 24 lines
Quoted 11 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml line 14
Quoted 9 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +7
-0+ +maintainers: + - Tomer Maimon <[email protected]> + +description: + The clock and reset registers are a registers block in Nuvoton SoCs that + handle both reset and clock functionality.That's still vague. Say something useful.
Will describe more
Quoted 12 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml line 23
Quoted 10 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +9
-0+ +properties: + compatible: + items: + - enum: + - nuvoton,npcm750-clk-rst + - nuvoton,npcm845-clk-rst + - const: syscon + - const: simple-mfdNo, it's not a syscon and not a simple-mfd. You just said it is clock
Quoted 3 lines
Yes, I understand the syscon node represents a register region containing a set of miscellaneous registers, but as explain above it is quite the case here.
Nothing in this patch was telling this.
Quoted 4 lines
I will remove the simple-mfd.
Quoted 1 line
provider and reset controller. Thus missing clock cells and reset cells.
The reset cell and clock cell found at the clock and reset dt-binding, is it enough?
This is the reset and clock binding, isn't it? You called it (your title):
"Clock and reset registers block in Nuvoton SoCs"
Best regards,
Krzysztof
Tomer Maimon
maintainerRe: [PATCH v22 4/8] dt-bindings: soc: nuvoton: add binding for clock and reset registers
Hi Krzysztof,
Thanks for your comment
Quoted 20 lines · Krzysztof Kozlowski
Quoted 18 lines · Tomer Maimon
Hi Krzysztof,
Thanks for your comments.
Sorry for the long explanation but I think it is necessary.
In the NPCM8XX SoC, the reset and the clock register modules are scrambled in the same memory register region.
The NPCM8XX Clock driver is still in the upstream process (for a long time) but the NPCM8XX reset driver is already upstreamed.One of the main comments in the NPCM8XX Clock driver upstream process is that the clock register is mixed with the reset register and therefore we can't map (ioremap) the clock register
region because is already mapped by the reset module, therefore we decided to use an external syscon to handle the clock and the reset registers driver.
Quoted 4 lines
Quoted 1 line
I highly appreciate your guidance on this topic.
Linux deals with it easily, that's why we have regmaps. What's the problem exactly?
This is exactly what is done in the submitted clock driver.
Quoted 31 lines
Quoted 31 lines
Quoted 27 lines · Krzysztof Kozlowski
Quoted 5 lines · Tomer Maimon
A nuvoton,*-clk-rst node is present in
nuvoton-common-npcm7xx.dtsiand will be added tonuvoton-common-npcm8xx.dtsi. It is necessary for the NPCM7xx and NPCM8xx clock and reset drivers, and may later be used to retrieve SoC model and version information.A nit, subject: drop second/last, redundant "bindings". The "dt-bindings" prefix is already stating that these are bindings. See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18Quoted 1 line
This patch adds a binding to describe this node.
Please do not use "This commit/patch/change", but imperative mood. See longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95Quoted 3 lines
Signed-off-by Tomer Maimon <[email protected]>---
How possibly could it be v22 if there is:
1. No changelog
2. No previous submissions
?
Quoted 5 lines
Quoted 2 lines
Should the dt-binding and dts patches be a part of the clock patch set (this is why it's V22) or should I open a new patch set?
You should explain what is happening here. That's why you have changelog for.
Will explain it better in the cover letter in the change log.
Quoted 28 lines
Quoted 28 lines
Quoted 28 lines
NAK, it's something completely new without any explanation.
Limited review follows.
Quoted 10 lines
.../soc/nuvoton/nuvoton,npcm-clk-rst.yaml | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yamlDocumentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +0
-0Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml line 7
Quoted 7 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +7
-0@@ -0,0 +1,40 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/soc/nuvoton/nuvoton,npcm-clk-rst.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Clock and reset registers block in Nuvoton SoCsThis is vague. Any block? All blocks? Your SoC has only one block? I doubt, although possible.
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
Anyway, clocks go to clock directory, not to soc! We've been here and you already received that feedback.
Since one region handles the reset and the clock registers shouldn't I add the dt-binding to the SoC like the GCR and not to the clock
No, soc is not a dumping ground..
Maybe I do not need to add dt binding document for the clock and reset syscon but handle the registers as follows in the dtsi.
sysctrl: system-controller@f0801000 {
compatible = "syscon", "simple-mfd";
reg = <0x0 0xf0801000 0x0 0x1000>;
rstc: reset-controller {
compatible = "nuvoton,npcm845-reset";
#reset-cells = <2>;
nuvoton,sysgcr = <&gcr>;
};
clk: clock-controller {
compatible = "nuvoton,npcm845-clk";
#clock-cells = <1>;
clocks = <&refclk>;
clock-names = "refclk";
};
};is it acceptable?
Quoted 42 lines
Quoted 3 lines
Choose the main feature of the block - either clock controller or reset controller - and put it there.
Quoted 24 lines
Quoted 11 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml line 14
Quoted 9 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +7
-0+ +maintainers: + - Tomer Maimon <[email protected]> + +description: + The clock and reset registers are a registers block in Nuvoton SoCs that + handle both reset and clock functionality.That's still vague. Say something useful.
Will describe more
Quoted 12 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml line 23
Quoted 10 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +9
-0+ +properties: + compatible: + items: + - enum: + - nuvoton,npcm750-clk-rst + - nuvoton,npcm845-clk-rst + - const: syscon + - const: simple-mfdNo, it's not a syscon and not a simple-mfd. You just said it is clock
Quoted 3 lines
Yes, I understand the syscon node represents a register region containing a set of miscellaneous registers, but as explain above it is quite the case here.
Nothing in this patch was telling this.
Quoted 4 lines
I will remove the simple-mfd.
Quoted 1 line
provider and reset controller. Thus missing clock cells and reset cells.
The reset cell and clock cell found at the clock and reset dt-binding, is it enough?
Quoted 9 lines
This is the reset and clock binding, isn't it? You called it (your title):
"Clock and reset registers block in Nuvoton SoCs"Best regards,
Krzysztof
Best regards,
Tomer
Tomer Maimon
maintainerRe: [PATCH v22 4/8] dt-bindings: soc: nuvoton: add binding for clock and reset registers
Hi Krzysztof,
Quoted 124 lines · Tomer Maimon
Hi Krzysztof,
Thanks for your comment
Quoted 20 lines · Krzysztof Kozlowski
Quoted 18 lines · Tomer Maimon
Hi Krzysztof,
Thanks for your comments.
Sorry for the long explanation but I think it is necessary.
In the NPCM8XX SoC, the reset and the clock register modules are scrambled in the same memory register region.
The NPCM8XX Clock driver is still in the upstream process (for a long time) but the NPCM8XX reset driver is already upstreamed.One of the main comments in the NPCM8XX Clock driver upstream process is that the clock register is mixed with the reset register and therefore we can't map (ioremap) the clock register
region because is already mapped by the reset module, therefore we decided to use an external syscon to handle the clock and the reset registers driver.Quoted 4 lines
Quoted 1 line
I highly appreciate your guidance on this topic.
Linux deals with it easily, that's why we have regmaps. What's the problem exactly?
This is exactly what is done in the submitted clock driver.
Quoted 31 lines
Quoted 31 lines
Quoted 27 lines · Krzysztof Kozlowski
Quoted 5 lines · Tomer Maimon
A nuvoton,*-clk-rst node is present in
nuvoton-common-npcm7xx.dtsiand will be added tonuvoton-common-npcm8xx.dtsi. It is necessary for the NPCM7xx and NPCM8xx clock and reset drivers, and may later be used to retrieve SoC model and version information.A nit, subject: drop second/last, redundant "bindings". The "dt-bindings" prefix is already stating that these are bindings. See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18Quoted 1 line
This patch adds a binding to describe this node.
Please do not use "This commit/patch/change", but imperative mood. See longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95Quoted 3 lines
Signed-off-by Tomer Maimon <[email protected]>---
How possibly could it be v22 if there is:
1. No changelog
2. No previous submissions
?Quoted 5 lines
Quoted 2 lines
Should the dt-binding and dts patches be a part of the clock patch set (this is why it's V22) or should I open a new patch set?
You should explain what is happening here. That's why you have changelog for.
Will explain it better in the cover letter in the change log.
Quoted 28 lines
Quoted 28 lines
Quoted 28 lines
NAK, it's something completely new without any explanation.
Limited review follows.
Quoted 10 lines
.../soc/nuvoton/nuvoton,npcm-clk-rst.yaml | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yamlDocumentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +0
-0Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml line 7
Quoted 7 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +7
-0@@ -0,0 +1,40 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/soc/nuvoton/nuvoton,npcm-clk-rst.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Clock and reset registers block in Nuvoton SoCsThis is vague. Any block? All blocks? Your SoC has only one block? I doubt, although possible.
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
Anyway, clocks go to clock directory, not to soc! We've been here and you already received that feedback.
Since one region handles the reset and the clock registers shouldn't I add the dt-binding to the SoC like the GCR and not to the clock
No, soc is not a dumping ground..
Maybe I do not need to add dt binding document for the clock and reset syscon but handle the registers as follows in the dtsi.
sysctrl: system-controller@f0801000 { compatible = "syscon", "simple-mfd"; reg = <0x0 0xf0801000 0x0 0x1000>; rstc: reset-controller { compatible = "nuvoton,npcm845-reset"; #reset-cells = <2>; nuvoton,sysgcr = <&gcr>; }; clk: clock-controller { compatible = "nuvoton,npcm845-clk"; #clock-cells = <1>; clocks = <&refclk>; clock-names = "refclk"; }; };
Quoted 1 line
is it acceptable?
Appreciate for your advice on the question above.
Quoted 48 lines
Quoted 48 lines
Quoted 3 lines
Choose the main feature of the block - either clock controller or reset controller - and put it there.
Quoted 24 lines
Quoted 11 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml line 14
Quoted 9 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +7
-0+ +maintainers: + - Tomer Maimon <[email protected]> + +description: + The clock and reset registers are a registers block in Nuvoton SoCs that + handle both reset and clock functionality.That's still vague. Say something useful.
Will describe more
Quoted 12 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml line 23
Quoted 10 lines
Documentation/devicetree/bindings/soc/nuvoton/nuvoton,npcm-clk-rst.yaml new file +9
-0+ +properties: + compatible: + items: + - enum: + - nuvoton,npcm750-clk-rst + - nuvoton,npcm845-clk-rst + - const: syscon + - const: simple-mfdNo, it's not a syscon and not a simple-mfd. You just said it is clock
Quoted 3 lines
Yes, I understand the syscon node represents a register region containing a set of miscellaneous registers, but as explain above it is quite the case here.
Nothing in this patch was telling this.
Quoted 4 lines
I will remove the simple-mfd.
Quoted 1 line
provider and reset controller. Thus missing clock cells and reset cells.
The reset cell and clock cell found at the clock and reset dt-binding, is it enough?
This is the reset and clock binding, isn't it? You called it (your title):
"Clock and reset registers block in Nuvoton SoCs"
Quoted 7 lines
Quoted 3 lines
Best regards,
KrzysztofBest regards,
Tomer
Best regards,
Tomer
Tomer Maimon
maintainerPATCH v22 5/8 arm64: dts: nuvoton: npcm8xx: add clock reset syscon node
In reply to Tomer MaimonAdd clock reset syscon node to handle reset and clock registers controllers.
--- arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi | 5 +++++ 1 file changed, 5 insertions(+)
arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +5 -0
@@ -22,6 +22,11 @@ gcr: system-controller@f0800000 { reg = <0x0 0xf0800000 0x0 0x1000>; }; + clk_rst: syscon@f0801000 { + compatible = "nuvoton,npcm845-clk-rst", "syscon", "simple-mfd"; + reg = <0x0 0xf0801000 0x0 0xC4>; + }; + gic: interrupt-controller@dfff9000 { compatible = "arm,gic-400"; reg = <0x0 0xdfff9000 0x0 0x1000>,
Signature · 1 line
2.34.1
Re: [PATCH v22 5/8] arm64: dts: nuvoton: npcm8xx: add clock reset syscon node
Quoted 13 lines · Tomer Maimon
Add clock reset syscon node to handle reset and clock registers controllers.
Signed-off-by Tomer Maimon <[email protected]>--- arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi | 5 +++++ 1 file changed, 5 insertions(+)arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +0
-0
arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi gcr: system-controller@f0800000 { · line 26
Quoted 5 lines
arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +2
-0@@ -22,6 +22,11 @@ gcr: system-controller@f0800000 { reg = <0x0 0xf0800000 0x0 0x1000>; }; + clk_rst: syscon@f0801000 { + compatible = "nuvoton,npcm845-clk-rst", "syscon", "simple-mfd";
This is not a simple-mfd. No children,
arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi gcr: system-controller@f0800000 { · line 27
Quoted 1 line
arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +1
-0+ reg = <0x0 0xf0801000 0x0 0xC4>;
Use lowercase hex. Please store this feedback in your checklist and do not repeat the same mistakes in further submissions.
Best regards,
Krzysztof
Tomer Maimon
maintainerRe: [PATCH v22 5/8] arm64: dts: nuvoton: npcm8xx: add clock reset syscon node
Hi Krzysztof,
Thanks for your comments.
Quoted 23 lines · Krzysztof Kozlowski
Quoted 13 lines · Tomer Maimon
Add clock reset syscon node to handle reset and clock registers controllers.
Signed-off-by Tomer Maimon <[email protected]>--- arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi | 5 +++++ 1 file changed, 5 insertions(+)arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +0
-0arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi gcr: system-controller@f0800000 { · line 26
Quoted 5 lines
arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +2
-0@@ -22,6 +22,11 @@ gcr: system-controller@f0800000 { reg = <0x0 0xf0800000 0x0 0x1000>; }; + clk_rst: syscon@f0801000 { + compatible = "nuvoton,npcm845-clk-rst", "syscon", "simple-mfd";This is not a simple-mfd. No children,
Quoted 8 lines
arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi gcr: system-controller@f0800000 { · line 27
Quoted 1 line
arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +1
-0+ reg = <0x0 0xf0801000 0x0 0xC4>;Use lowercase hex. Please store this feedback in your checklist and do not repeat the same mistakes in further submissions.
Best regards,
Krzysztof
will be fixed in the next version.
Best regards,
Tomer
Tomer Maimon
maintainerPATCH v22 6/8 dt-bindings: clock: npcm845: replace reg with syscon property
In reply to Tomer MaimonReplace reg with syscon property since the clock registers handle the reset registers as well.
--- .../bindings/clock/nuvoton,npcm845-clk.yaml | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-)
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +10 -12
@@ -18,8 +18,9 @@ properties: enum: - nuvoton,npcm845-clk - reg: - maxItems: 1 + nuvoton,sysclk: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle to access clock registers. clocks: items:
@@ -37,7 +38,7 @@ properties: required: - compatible - - reg + - nuvoton,sysclk - clocks - clock-names - '#clock-cells'
@@ -52,14 +53,11 @@ examples: clock-frequency = <25000000>; }; - ahb { - #address-cells = <2>; - #size-cells = <2>; - - clock-controller@f0801000 { - compatible = "nuvoton,npcm845-clk"; - reg = <0x0 0xf0801000 0x0 0x1000>; - #clock-cells = <1>; - }; + clk: clock-controller { + compatible = "nuvoton,npcm845-clk"; + nuvoton,sysclk = <&clk_rst>; + #clock-cells = <1>; + clocks = <&refclk>; + clock-names = "refclk"; }; ...
Signature · 1 line
2.34.1
Re: [PATCH v22 6/8] dt-bindings: clock: npcm845: replace reg with syscon property
Quoted 13 lines · Tomer Maimon
Replace reg with syscon property since the clock registers handle the reset registers as well.
Signed-off-by Tomer Maimon <[email protected]>--- .../bindings/clock/nuvoton,npcm845-clk.yaml | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-)Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +0
-0
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml properties: · line 23
Quoted 8 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +3
-2@@ -18,8 +18,9 @@ properties: enum: - nuvoton,npcm845-clk - reg: - maxItems: 1 + nuvoton,sysclk: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle to access clock registers.
NAK. Not explained, not justified, not reasonable, breaking ABI.
Best regards,
Krzysztof
Tomer Maimon
maintainerRe: [PATCH v22 6/8] dt-bindings: clock: npcm845: replace reg with syscon property
Hi Krzysztof,
As explained in my [PATCH v22 4/8] dt-bindings: soc: nuvoton: add binding for clock and reset registers mail.
In the NPCM8XX SoC, the reset and the clock register modules are scrambled in the same memory register region.
The NPCM8XX Clock driver is still in the upstream process (for a long time) but the NPCM8XX reset driver is already upstreamed.
Quoted 18 lines · Krzysztof Kozlowski
Quoted 16 lines · Tomer Maimon
Replace reg with syscon property since the clock registers handle the reset registers as well.
Signed-off-by Tomer Maimon <[email protected]>--- .../bindings/clock/nuvoton,npcm845-clk.yaml | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-)Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +0
-0@@ -18,8 +18,9 @@ properties: enum: - nuvoton,npcm845-clk
Quoted 7 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml properties: · line 23
Quoted 5 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +3
-2- reg: - maxItems: 1 + nuvoton,sysclk: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle to access clock registers.NAK. Not explained, not justified, not reasonable, breaking ABI.
Should I explain more in the commit message or/and the nuvoton,sysclk property?
Quoted 4 lines
Best regards,
Krzysztof
Best regards,
Tomer
Re: [PATCH v22 6/8] dt-bindings: clock: npcm845: replace reg with syscon property
Quoted 9 lines · Tomer Maimon
Hi Krzysztof,
As explained in my [PATCH v22 4/8] dt-bindings: soc: nuvoton: add binding for clock and reset registers mail.
In the NPCM8XX SoC, the reset and the clock register modules are scrambled in the same memory register region.
The NPCM8XX Clock driver is still in the upstream process (for a long time) but the NPCM8XX reset driver is already upstreamed.
First of all, the drivers itself don't matter here, we talk about bindings. I assume though they were going together, so that's why you mentioned driver... but just to clarify: we talk here only about drivers.
If reset bindings were accepted, then why they aren't referenced?
If clock bindings were not accepted, then what is this patch and this file about?
Quoted 21 lines
Quoted 18 lines · Krzysztof Kozlowski
Quoted 16 lines · Tomer Maimon
Replace reg with syscon property since the clock registers handle the reset registers as well.
Signed-off-by Tomer Maimon <[email protected]>--- .../bindings/clock/nuvoton,npcm845-clk.yaml | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-)Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +0
-0@@ -18,8 +18,9 @@ properties: enum: - nuvoton,npcm845-clk
Quoted 8 lines
Quoted 7 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml properties: · line 23
Quoted 5 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +3
-2- reg: - maxItems: 1 + nuvoton,sysclk: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle to access clock registers.NAK. Not explained, not justified, not reasonable, breaking ABI.
Should I explain more in the commit message or/and the nuvoton,sysclk property?
Let's try to explain here first. I really do not understand why do you change this binding. Your device did not change, so your binding should not. Now, if you say "but I changed drivers", then it does not matter. Bindings do not change because you did something in the drivers, in general. At least they should not.
Best regards,
Krzysztof
Tomer Maimon
maintainerRe: [PATCH v22 6/8] dt-bindings: clock: npcm845: replace reg with syscon property
Hi Krzysztof,
Thanks for your comments.
Quoted 49 lines · Krzysztof Kozlowski
Quoted 9 lines · Tomer Maimon
Hi Krzysztof,
As explained in my [PATCH v22 4/8] dt-bindings: soc: nuvoton: add binding for clock and reset registers mail.
In the NPCM8XX SoC, the reset and the clock register modules are scrambled in the same memory register region.
The NPCM8XX Clock driver is still in the upstream process (for a long time) but the NPCM8XX reset driver is already upstreamed.First of all, the drivers itself don't matter here, we talk about bindings. I assume though they were going together, so that's why you mentioned driver... but just to clarify: we talk here only about drivers.
If reset bindings were accepted, then why they aren't referenced?
If clock bindings were not accepted, then what is this patch and this file about?
Quoted 28 lines
Quoted 25 lines · Krzysztof Kozlowski
Quoted 13 lines · Tomer Maimon
Replace reg with syscon property since the clock registers handle the reset registers as well.
Signed-off-by Tomer Maimon <[email protected]>--- .../bindings/clock/nuvoton,npcm845-clk.yaml | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-)Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +0
-0Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml properties: · line 23
Quoted 8 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +3
-2@@ -18,8 +18,9 @@ properties: enum: - nuvoton,npcm845-clk - reg: - maxItems: 1 + nuvoton,sysclk: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle to access clock registers.NAK. Not explained, not justified, not reasonable, breaking ABI.
Quoted 7 lines
Quoted 1 line
Should I explain more in the commit message or/and the nuvoton,sysclk property?
Let's try to explain here first. I really do not understand why do you change this binding. Your device did not change, so your binding should not. Now, if you say "but I changed drivers", then it does not matter. Bindings do not change because you did something in the drivers, in general. At least they should not.
The confusion here is because the clock binding was upstreamed but the clock driver has not upstreamed yet.
The clock driver will use regmap and ioremap so reg property is not needed, should I remove it? or leave it?
Quoted 4 lines
Best regards,
Krzysztof
Best regards,
Tomer
Re: [PATCH v22 6/8] dt-bindings: clock: npcm845: replace reg with syscon property
Quoted 57 lines · Tomer Maimon
Hi Krzysztof,
Thanks for your comments.
Quoted 51 lines · Krzysztof Kozlowski
Quoted 9 lines · Tomer Maimon
Hi Krzysztof,
As explained in my [PATCH v22 4/8] dt-bindings: soc: nuvoton: add binding for clock and reset registers mail.
In the NPCM8XX SoC, the reset and the clock register modules are scrambled in the same memory register region.
The NPCM8XX Clock driver is still in the upstream process (for a long time) but the NPCM8XX reset driver is already upstreamed.First of all, the drivers itself don't matter here, we talk about bindings. I assume though they were going together, so that's why you mentioned driver... but just to clarify: we talk here only about drivers.
If reset bindings were accepted, then why they aren't referenced?
If clock bindings were not accepted, then what is this patch and this file about?
Quoted 29 lines
Quoted 18 lines · Krzysztof Kozlowski
Quoted 16 lines · Tomer Maimon
Replace reg with syscon property since the clock registers handle the reset registers as well.
Signed-off-by Tomer Maimon <[email protected]>--- .../bindings/clock/nuvoton,npcm845-clk.yaml | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-)Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +0
-0@@ -18,8 +18,9 @@ properties: enum: - nuvoton,npcm845-clkQuoted 7 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml properties: · line 23
Quoted 5 lines
Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml +3
-2- reg: - maxItems: 1 + nuvoton,sysclk: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle to access clock registers.NAK. Not explained, not justified, not reasonable, breaking ABI.
Should I explain more in the commit message or/and the nuvoton,sysclk property?
Quoted 7 lines
Quoted 5 lines
Let's try to explain here first. I really do not understand why do you change this binding. Your device did not change, so your binding should not. Now, if you say "but I changed drivers", then it does not matter. Bindings do not change because you did something in the drivers, in general. At least they should not.
The confusion here is because the clock binding was upstreamed but the clock driver has not upstreamed yet.
So what does it mean? Being upstreamed independently does not mean something is wrong, so I do not see any confusion. There is absolutely no confusion in upstreaming binding before upstreaming driver.
Quoted 2 lines
The clock driver will use regmap and ioremap so reg property is not needed, should I remove it? or leave it?
Just because you upstream drivers? You should not change bindings just because you figured out that you will implement something that or other way. Please describe the hardware in the binding, not the driver.
Best regards,
Krzysztof
Tomer Maimon
maintainerPATCH v22 7/8 arm64: dts: nuvoton: npcm8xx: replace reg with syscon property
In reply to Tomer MaimonReplace reg with syscon property since the clock registers handle the reset registers as well.
--- arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +2 -2
@@ -59,10 +59,10 @@ rstc: reset-controller@f0801000 { nuvoton,sysgcr = <&gcr>; }; - clk: clock-controller@f0801000 { + clk: clock-controller { compatible = "nuvoton,npcm845-clk"; #clock-cells = <1>; - reg = <0x0 0xf0801000 0x0 0x1000>; + nuvoton,sysclk = <&clk_rst>; clocks = <&refclk>; clock-names = "refclk"; };
Signature · 1 line
2.34.1
Re: [PATCH v22 7/8] arm64: dts: nuvoton: npcm8xx: replace reg with syscon property
Quoted 5 lines · Tomer Maimon
Replace reg with syscon property since the clock registers handle the reset registers as well.
Signed-off-by Tomer Maimon <[email protected]>---
NAK for the same reasons as previous patch.
Best regards,
Krzysztof
Tomer Maimon
maintainerRe: [PATCH v22 7/8] arm64: dts: nuvoton: npcm8xx: replace reg with syscon property
Hi Krzysztof,
Thanks for your comment.
Quoted 9 lines · Krzysztof Kozlowski
Quoted 5 lines · Tomer Maimon
Replace reg with syscon property since the clock registers handle the reset registers as well.
Signed-off-by Tomer Maimon <[email protected]>---
NAK for the same reasons as previous patch.
Will explain more in the commit message
Quoted 4 lines
Best regards,
Krzysztof
Best regards,
Tomer
Re: [PATCH v22 7/8] arm64: dts: nuvoton: npcm8xx: replace reg with syscon property
Quoted 8 lines · Tomer Maimon
Hi Krzysztof,
Thanks for your comment.
Quoted 2 lines · Krzysztof Kozlowski
On 08/01/2024 14:54, Tomer Maimon wrote:
Quoted 8 lines
Quoted 7 lines
Quoted 5 lines
Replace reg with syscon property since the clock registers handle the reset registers as well.
Signed-off-by Tomer Maimon <[email protected]>---
NAK for the same reasons as previous patch.
Will explain more in the commit message
No, this wasn't even tested. Build your code with W=1 and fix all warnings first. But anyway this is not the way to go.
Best regards,
Krzysztof
Tomer Maimon
maintainerPATCH v22 8/8 clk: npcm8xx: add clock controller
In reply to Tomer MaimonNuvoton Arbel BMC NPCM8XX contains an integrated clock controller which generates and supplies clocks to all modules within the BMC.
--- drivers/clk/clk-npcm8xx.c | 510 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 510 insertions(+) create mode 100644 drivers/clk/clk-npcm8xx.c
drivers/clk/clk-npcm8xx.c new file +510 -0
@@ -0,0 +1,510 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Nuvoton NPCM8xx Clock Generator +* All the clocks are initialized by the bootloader, so this driver allows only +* reading of current settings directly from the hardware. +* +* Copyright (C) 2020 Nuvoton Technologies +* Author: Tomer Maimon <[email protected]> +*/ + +#define pr_fmt (fmt) "npcm8xx_clk: " fmt + +#include <linux/bitfield.h> +#include <linux/clk-provider.h> +#include <linux/err.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/slab.h> +#include <linux/regmap.h> +#include <linux/mfd/syscon.h> + +#include <dt-bindings/clock/nuvoton,npcm845-clk.h> + +/* npcm8xx clock registers*/ +#define NPCM8XX_CLKSEL 0x04 +#define NPCM8XX_CLKDIV1 0x08 +#define NPCM8XX_CLKDIV2 0x2C +#define NPCM8XX_CLKDIV3 0x58 +#define NPCM8XX_CLKDIV4 0x7C +#define NPCM8XX_PLLCON0 0x0C +#define NPCM8XX_PLLCON1 0x10 +#define NPCM8XX_PLLCON2 0x54 +#define NPCM8XX_PLLCONG 0x60 +#define NPCM8XX_THRTL_CNT 0xC0 + +#define PLLCON_LOKI BIT(31) +#define PLLCON_LOKS BIT(30) +#define PLLCON_FBDV GENMASK(27, 16) +#define PLLCON_OTDV2 GENMASK(15, 13) +#define PLLCON_PWDEN BIT(12) +#define PLLCON_OTDV1 GENMASK(10, 8) +#define PLLCON_INDV GENMASK(5, 0) + +struct npcm8xx_clk { +struct regmap *clk_regmap; +unsigned int offset; +const char *name; +const u32 *table; +u32 mask; +u8 shift; +unsigned long width; +unsigned long flags; +struct clk_hw hw; +}; + +#define to_npcm8xx_clk (_hw) container_of(_hw, struct npcm8xx_clk, hw) + +struct npcm8xx_clk_pll_data { +const char *name; +struct clk_parent_data parent; +unsigned int reg; +unsigned long flags; +struct clk_hw hw; +}; + +struct npcm8xx_clk_div_data { +u32 reg; +u8 shift; +u8 width; +const char *name; +const struct clk_hw *parent_hw; +unsigned long clk_divider_flags; +unsigned long flags; +int onecell_idx; +struct clk_hw hw; +}; + +struct npcm8xx_clk_mux_data { +u8 shift; +u32 mask; +const u32 *table; +const char *name; +const struct clk_parent_data *parent_data; +u8 num_parents; +unsigned long flags; +struct clk_hw hw; +}; + +/* external clock definition */ +#define NPCM8XX_CLK_S_REFCLK "refclk" + +/* pll definition */ +#define NPCM8XX_CLK_S_PLL0 "pll0" +#define NPCM8XX_CLK_S_PLL1 "pll1" +#define NPCM8XX_CLK_S_PLL2 "pll2" +#define NPCM8XX_CLK_S_PLL_GFX "pll_gfx" + +/* early divider definition */ +#define NPCM8XX_CLK_S_PLL2_DIV2 "pll2_div2" +#define NPCM8XX_CLK_S_PLL_GFX_DIV2 "pll_gfx_div2" +#define NPCM8XX_CLK_S_PLL1_DIV2 "pll1_div2" + +/* mux definition */ +#define NPCM8XX_CLK_S_CPU_MUX "cpu_mux" + +/* div definition */ +#define NPCM8XX_CLK_S_TH "th" +#define NPCM8XX_CLK_S_AXI "axi" + +static struct clk_hw hw_pll1_div2, hw_pll2_div2, hw_gfx_div2, hw_pre_clk; +static struct npcm8xx_clk_pll_data npcm8xx_pll_clks[] = { + {NPCM8XX_CLK_S_PLL0 , { .index =0 },NPCM8XX_PLLCON0 ,0 }, + {NPCM8XX_CLK_S_PLL1 , { .index =0 },NPCM8XX_PLLCON1 ,0 }, + {NPCM8XX_CLK_S_PLL2 , { .index =0 },NPCM8XX_PLLCON2 ,0 }, + {NPCM8XX_CLK_S_PLL_GFX , { .index =0 },NPCM8XX_PLLCONG ,0 }, +}; + +static const u32 cpuck_mux_table[] = {0 ,1 ,2 ,7 }; +static const struct clk_parent_data cpuck_mux_parents[] = { + { .hw = &npcm8xx_pll_clks[0 ].hw }, + { .hw = &npcm8xx_pll_clks[1 ].hw }, + { .index =0 }, + { .hw = &npcm8xx_pll_clks[2 ].hw } +}; + +static const u32 pixcksel_mux_table[] = {0 ,2 }; +static const struct clk_parent_data pixcksel_mux_parents[] = { + { .hw = &npcm8xx_pll_clks[3 ].hw }, + { .index =0 } +}; + +static const u32 default_mux_table[] = {0 ,1 ,2 ,3 }; +static const struct clk_parent_data default_mux_parents[] = { + { .hw = &npcm8xx_pll_clks[0 ].hw }, + { .hw = &npcm8xx_pll_clks[1 ].hw }, + { .index =0 }, + { .hw = &hw_pll2_div2 } +}; + +static const u32 sucksel_mux_table[] = {2 ,3 }; +static const struct clk_parent_data sucksel_mux_parents[] = { + { .index =0 }, + { .hw = &hw_pll2_div2 } +}; + +static const u32 mccksel_mux_table[] = {0 ,2 }; +static const struct clk_parent_data mccksel_mux_parents[] = { + { .hw = &hw_pll1_div2 }, + { .index =0 } +}; + +static const u32 clkoutsel_mux_table[] = {0 ,1 ,2 ,3 ,4 }; +static const struct clk_parent_data clkoutsel_mux_parents[] = { + { .hw = &npcm8xx_pll_clks[0 ].hw }, + { .hw = &npcm8xx_pll_clks[1 ].hw }, + { .index =0 }, + { .hw = &hw_gfx_div2 }, + { .hw = &hw_pll2_div2 } +}; + +static const u32 gfxmsel_mux_table[] = {2 ,3 }; +static const struct clk_parent_data gfxmsel_mux_parents[] = { + { .index =0 }, + { .hw = &npcm8xx_pll_clks[2 ].hw } +}; + +static const u32 dvcssel_mux_table[] = {2 ,3 }; +static const struct clk_parent_data dvcssel_mux_parents[] = { + { .index =0 }, + { .hw = &npcm8xx_pll_clks[2 ].hw } +}; + +static const u32 default3_mux_table[] = {0 ,1 ,2 }; +static const struct clk_parent_data default3_mux_parents[] = { + { .hw = &npcm8xx_pll_clks[0 ].hw }, + { .hw = &npcm8xx_pll_clks[1 ].hw }, + { .index =0 } +}; + +static struct npcm8xx_clk_mux_data npcm8xx_muxes[] = { + {0 ,7 , cpuck_mux_table,NPCM8XX_CLK_S_CPU_MUX , cpuck_mux_parents, +ARRAY_SIZE (cpuck_mux_parents),CLK_IS_CRITICAL }, + {4 ,3 , pixcksel_mux_table,"gfx_pixel_mux" , pixcksel_mux_parents, +ARRAY_SIZE (pixcksel_mux_parents),0 }, + {6 ,3 , default_mux_table,"sd_mux" , default_mux_parents, +ARRAY_SIZE (default_mux_parents),0 }, + {8 ,3 , default_mux_table,"uart_mux" , default_mux_parents, +ARRAY_SIZE (default_mux_parents),0 }, + {10 ,3 , sucksel_mux_table,"serial_usb_mux" , sucksel_mux_parents, +ARRAY_SIZE (sucksel_mux_parents),0 }, + {12 ,3 , mccksel_mux_table,"mc_mux" , mccksel_mux_parents, +ARRAY_SIZE (mccksel_mux_parents),0 }, + {14 ,3 , default_mux_table,"adc_mux" , default_mux_parents, +ARRAY_SIZE (default_mux_parents),0 }, + {16 ,3 , default_mux_table,"gfx_mux" , default_mux_parents, +ARRAY_SIZE (default_mux_parents),0 }, + {18 ,7 , clkoutsel_mux_table,"clkout_mux" , clkoutsel_mux_parents, +ARRAY_SIZE (clkoutsel_mux_parents),0 }, + {21 ,3 , gfxmsel_mux_table,"gfxm_mux" , gfxmsel_mux_parents, +ARRAY_SIZE (gfxmsel_mux_parents),0 }, + {23 ,3 , dvcssel_mux_table,"dvc_mux" , dvcssel_mux_parents, +ARRAY_SIZE (dvcssel_mux_parents),0 }, + {25 ,3 , default3_mux_table,"rg_mux" , default3_mux_parents, +ARRAY_SIZE (default3_mux_parents),0 }, + {27 ,3 , default3_mux_table,"rcp_mux" , default3_mux_parents, +ARRAY_SIZE (default3_mux_parents),0 }, +}; + +static struct npcm8xx_clk_div_data npcm8xx_pre_divs[] = { + {NPCM8XX_CLKDIV1 ,21 ,5 ,"pre_adc" , &npcm8xx_muxes[6 ].hw,CLK_DIVIDER_READ_ONLY ,0 ,-1 }, + {NPCM8XX_CLKDIV1 ,26 ,2 ,"ahb" , &hw_pre_clk,CLK_DIVIDER_READ_ONLY ,CLK_IS_CRITICAL ,NPCM8XX_CLK_AHB }, +}; + +/* configurable dividers: */ +static struct npcm8xx_clk_div_data npcm8xx_divs[] = { + {NPCM8XX_CLKDIV1 ,28 ,3 ,"adc" , &npcm8xx_pre_divs[0 ].hw,CLK_DIVIDER_READ_ONLY |CLK_DIVIDER_POWER_OF_TWO ,0 ,NPCM8XX_CLK_ADC }, + {NPCM8XX_CLKDIV1 ,16 ,5 ,"uart" , &npcm8xx_muxes[3 ].hw,0 ,0 ,NPCM8XX_CLK_UART }, + {NPCM8XX_CLKDIV1 ,11 ,5 ,"mmc" , &npcm8xx_muxes[2 ].hw,CLK_DIVIDER_READ_ONLY ,0 ,NPCM8XX_CLK_MMC }, + {NPCM8XX_CLKDIV1 ,6 ,5 ,"spi3" , &npcm8xx_pre_divs[1 ].hw,0 ,0 ,NPCM8XX_CLK_SPI3 }, + {NPCM8XX_CLKDIV1 ,2 ,4 ,"pci" , &npcm8xx_muxes[7 ].hw,CLK_DIVIDER_READ_ONLY ,0 ,NPCM8XX_CLK_PCI }, + + {NPCM8XX_CLKDIV2 ,30 ,2 ,"apb4" , &npcm8xx_pre_divs[1 ].hw,CLK_DIVIDER_READ_ONLY |CLK_DIVIDER_POWER_OF_TWO ,0 ,NPCM8XX_CLK_APB4 }, + {NPCM8XX_CLKDIV2 ,28 ,2 ,"apb3" , &npcm8xx_pre_divs[1 ].hw,CLK_DIVIDER_READ_ONLY |CLK_DIVIDER_POWER_OF_TWO ,0 ,NPCM8XX_CLK_APB3 }, + {NPCM8XX_CLKDIV2 ,26 ,2 ,"apb2" , &npcm8xx_pre_divs[1 ].hw,CLK_DIVIDER_READ_ONLY |CLK_DIVIDER_POWER_OF_TWO ,0 ,NPCM8XX_CLK_APB2 }, + {NPCM8XX_CLKDIV2 ,24 ,2 ,"apb1" , &npcm8xx_pre_divs[1 ].hw,CLK_DIVIDER_READ_ONLY |CLK_DIVIDER_POWER_OF_TWO ,0 ,NPCM8XX_CLK_APB1 }, + {NPCM8XX_CLKDIV2 ,22 ,2 ,"apb5" , &npcm8xx_pre_divs[1 ].hw,CLK_DIVIDER_READ_ONLY |CLK_DIVIDER_POWER_OF_TWO ,0 ,NPCM8XX_CLK_APB5 }, + {NPCM8XX_CLKDIV2 ,16 ,5 ,"clkout" , &npcm8xx_muxes[8 ].hw,CLK_DIVIDER_READ_ONLY ,0 ,NPCM8XX_CLK_CLKOUT }, + {NPCM8XX_CLKDIV2 ,13 ,3 ,"gfx" , &npcm8xx_muxes[7 ].hw,CLK_DIVIDER_READ_ONLY ,0 ,NPCM8XX_CLK_GFX }, + {NPCM8XX_CLKDIV2 ,8 ,5 ,"usb_bridge" , &npcm8xx_muxes[4 ].hw,CLK_DIVIDER_READ_ONLY ,0 ,NPCM8XX_CLK_SU }, + {NPCM8XX_CLKDIV2 ,4 ,4 ,"usb_host" , &npcm8xx_muxes[4 ].hw,CLK_DIVIDER_READ_ONLY ,0 ,NPCM8XX_CLK_SU48 }, + {NPCM8XX_CLKDIV2 ,0 ,4 ,"sdhc" , &npcm8xx_muxes[2 ].hw,CLK_DIVIDER_READ_ONLY ,0 ,NPCM8XX_CLK_SDHC }, + + {NPCM8XX_CLKDIV3 ,16 ,8 ,"spi1" , &npcm8xx_pre_divs[1 ].hw,CLK_DIVIDER_READ_ONLY ,0 ,NPCM8XX_CLK_SPI1 }, + {NPCM8XX_CLKDIV3 ,11 ,5 ,"uart2" , &npcm8xx_muxes[3 ].hw,CLK_DIVIDER_READ_ONLY ,0 ,NPCM8XX_CLK_UART2 }, + {NPCM8XX_CLKDIV3 ,6 ,5 ,"spi0" , &npcm8xx_pre_divs[1 ].hw,CLK_DIVIDER_READ_ONLY ,0 ,NPCM8XX_CLK_SPI0 }, + {NPCM8XX_CLKDIV3 ,1 ,5 ,"spix" , &npcm8xx_pre_divs[1 ].hw,CLK_DIVIDER_READ_ONLY ,0 ,NPCM8XX_CLK_SPIX }, + + {NPCM8XX_CLKDIV4 ,28 ,4 ,"rg" , &npcm8xx_muxes[11 ].hw,CLK_DIVIDER_READ_ONLY ,0 ,NPCM8XX_CLK_RG }, + {NPCM8XX_CLKDIV4 ,12 ,4 ,"rcp" , &npcm8xx_muxes[12 ].hw,CLK_DIVIDER_READ_ONLY ,0 ,NPCM8XX_CLK_RCP }, + + {NPCM8XX_THRTL_CNT ,0 ,2 ,NPCM8XX_CLK_S_TH , &npcm8xx_muxes[0 ].hw,CLK_DIVIDER_READ_ONLY |CLK_DIVIDER_POWER_OF_TWO ,0 ,NPCM8XX_CLK_TH }, +}; + +static struct clk_hw * +npcm8xx_clk_register (struct device *dev,const char *name, +struct regmap *clk_regmap,unsigned int offset, +unsigned long flags,const struct clk_ops *npcm8xx_clk_ops, +const struct clk_parent_data *parent_data, +const struct clk_hw *parent_hw,u8 num_parents, +u8 shift,u32 mask,unsigned long width, +const u32 *table,unsigned long clk_flags) +{ +struct npcm8xx_clk *clk; +struct clk_init_data init = {}; +int ret; + + clk =devm_kzalloc (dev,sizeof (*clk),GFP_KERNEL ); +if (!clk) +return ERR_PTR (-ENOMEM ); + + init.name = name; + init.ops = npcm8xx_clk_ops; + init.parent_data = parent_data; + init.parent_hws = parent_hw ? &parent_hw :NULL ; + init.num_parents = num_parents; + init.flags = flags; + + clk->clk_regmap = clk_regmap; + clk->hw.init = &init; + clk->offset = offset; + clk->shift = shift; + clk->mask = mask; + clk->width = width; + clk->table = table; + clk->flags = clk_flags; + + ret =devm_clk_hw_register (dev, &clk->hw); +if (ret) +return ERR_PTR (ret); + +return &clk->hw; +} + +static unsigned long npcm8xx_clk_pll_recalc_rate (struct clk_hw *hw, +unsigned long parent_rate) +{ +struct npcm8xx_clk *pll =to_npcm8xx_clk (hw); +unsigned long fbdv, indv, otdv1, otdv2; +unsigned int val; +u64 ret; + +if (parent_rate ==0 ) { +pr_debug ("%s: parent rate is zero\n" , __func__); +return 0 ; + } + +regmap_read (pll->clk_regmap, pll->offset, &val); + + indv =FIELD_GET (PLLCON_INDV , val); + fbdv =FIELD_GET (PLLCON_FBDV , val); + otdv1 =FIELD_GET (PLLCON_OTDV1 , val); + otdv2 =FIELD_GET (PLLCON_OTDV2 , val); + + ret = (u64 )parent_rate * fbdv; +do_div (ret, indv * otdv1 * otdv2); + +return ret; +} + +static const struct clk_ops npcm8xx_clk_pll_ops = { + .recalc_rate = npcm8xx_clk_pll_recalc_rate, +}; + +static u8 npcm8xx_clk_mux_get_parent (struct clk_hw *hw) +{ +struct npcm8xx_clk *mux =to_npcm8xx_clk (hw); +u32 val; + +regmap_read (mux->clk_regmap, mux->offset, &val); + val = val >> mux->shift; + val &= mux->mask; + +return clk_mux_val_to_index (hw, mux->table, mux->flags, val); +} + +static const struct clk_ops npcm8xx_clk_mux_ops = { + .get_parent = npcm8xx_clk_mux_get_parent, +}; + +static unsigned long npcm8xx_clk_div_get_parent (struct clk_hw *hw, +unsigned long parent_rate) +{ +struct npcm8xx_clk *div =to_npcm8xx_clk (hw); +unsigned int val; + +regmap_read (div->clk_regmap, div->offset, &val); + val = val >> div->shift; + val &=clk_div_mask (div->width); + +return divider_recalc_rate (hw, parent_rate, val,NULL , div->flags, + div->width); +} + +static const struct clk_ops npcm8xx_clk_div_ops = { + .recalc_rate = npcm8xx_clk_div_get_parent, +}; + +static int npcm8xx_clk_probe (struct platform_device *pdev) +{ +struct clk_hw_onecell_data *npcm8xx_clk_data; +struct device_node *np = pdev->dev.of_node; +struct device *dev = &pdev->dev; +struct regmap *clk_regmap; +struct clk_hw *hw; +unsigned int i; + + npcm8xx_clk_data =devm_kzalloc (dev,struct_size (npcm8xx_clk_data, hws, +NPCM8XX_NUM_CLOCKS ), +GFP_KERNEL ); +if (!npcm8xx_clk_data) +return -ENOMEM ; + + clk_regmap =syscon_regmap_lookup_by_phandle (np,"nuvoton,sysclk" ); +if (IS_ERR (clk_regmap)) { +dev_err (&pdev->dev,"Failed to find nuvoton,sysclk\n" ); +return PTR_ERR (clk_regmap); + } + + npcm8xx_clk_data->num =NPCM8XX_NUM_CLOCKS ; + +for (i =0 ; i <NPCM8XX_NUM_CLOCKS ; i++) + npcm8xx_clk_data->hws[i] =ERR_PTR (-EPROBE_DEFER ); + +/* Register plls */ +for (i =0 ; i <ARRAY_SIZE (npcm8xx_pll_clks); i++) { +struct npcm8xx_clk_pll_data *pll_clk = &npcm8xx_pll_clks[i]; + + hw =npcm8xx_clk_register (dev, pll_clk->name, clk_regmap, + pll_clk->reg, pll_clk->flags, + &npcm8xx_clk_pll_ops, &pll_clk->parent, +NULL ,1 ,0 ,0 ,0 ,NULL ,0 ); +if (IS_ERR (hw)) +return dev_err_probe (dev,PTR_ERR (hw),"Can't register pll\n" ); + pll_clk->hw = *hw; + } + +/* Register fixed dividers */ + hw =devm_clk_hw_register_fixed_factor (dev,NPCM8XX_CLK_S_PLL1_DIV2 , +NPCM8XX_CLK_S_PLL1 ,0 ,1 ,2 ); +if (IS_ERR (hw)) +return dev_err_probe (dev,PTR_ERR (hw),"Can't register fixed div\n" ); + hw_pll1_div2 = *hw; + + hw =devm_clk_hw_register_fixed_factor (dev,NPCM8XX_CLK_S_PLL2_DIV2 , +NPCM8XX_CLK_S_PLL2 ,0 ,1 ,2 ); +if (IS_ERR (hw)) +return dev_err_probe (dev,PTR_ERR (hw),"Can't register pll2 div2\n" ); + hw_pll2_div2 = *hw; + + hw =devm_clk_hw_register_fixed_factor (dev,NPCM8XX_CLK_S_PLL_GFX_DIV2 , +NPCM8XX_CLK_S_PLL_GFX ,0 ,1 ,2 ); +if (IS_ERR (hw)) +return dev_err_probe (dev,PTR_ERR (hw),"Can't register gfx div2\n" ); + hw_gfx_div2 = *hw; + +/* Register muxes */ +for (i =0 ; i <ARRAY_SIZE (npcm8xx_muxes); i++) { +struct npcm8xx_clk_mux_data *mux_data = &npcm8xx_muxes[i]; + + hw =npcm8xx_clk_register (dev, mux_data->name, clk_regmap, +NPCM8XX_CLKSEL , mux_data->flags, + &npcm8xx_clk_mux_ops, + mux_data->parent_data,NULL , + mux_data->num_parents, + mux_data->shift, mux_data->mask,0 , + mux_data->table,0 ); +if (IS_ERR (hw)) +return dev_err_probe (dev,PTR_ERR (hw),"Can't register mux\n" ); + mux_data->hw = *hw; + } + + hw =devm_clk_hw_register_fixed_factor (dev,"pre_clk" , +NPCM8XX_CLK_S_CPU_MUX ,0 ,1 ,2 ); +if (IS_ERR (hw)) +return dev_err_probe (dev,PTR_ERR (hw),"Can't register pre clk div2\n" ); + hw_pre_clk = *hw; + + hw =devm_clk_hw_register_fixed_factor (dev,NPCM8XX_CLK_S_AXI , +NPCM8XX_CLK_S_TH ,0 ,1 ,2 ); +if (IS_ERR (hw)) +return dev_err_probe (dev,PTR_ERR (hw),"Can't register axi div2\n" ); + npcm8xx_clk_data->hws[NPCM8XX_CLK_AXI ] = hw; + + hw =devm_clk_hw_register_fixed_factor (dev,"atb" ,NPCM8XX_CLK_S_AXI ,0 , +1 ,2 ); +if (IS_ERR (hw)) +return dev_err_probe (dev,PTR_ERR (hw),"Can't register atb div2\n" ); + npcm8xx_clk_data->hws[NPCM8XX_CLK_ATB ] = hw; + +/* Register clock pre dividers specified in npcm8xx_divs */ +for (i =0 ; i <ARRAY_SIZE (npcm8xx_pre_divs); i++) { +struct npcm8xx_clk_div_data *div_data = &npcm8xx_pre_divs[i]; + + hw =npcm8xx_clk_register (dev, div_data->name, clk_regmap, + div_data->reg, div_data->flags, + &npcm8xx_clk_div_ops,NULL , + div_data->parent_hw,1 , + div_data->shift,0 , div_data->width, +NULL , div_data->clk_divider_flags); +if (IS_ERR (hw)) +return dev_err_probe (dev,PTR_ERR (hw),"Can't register pre div table\n" ); + div_data->hw = *hw; + +if (div_data->onecell_idx >=0 ) + npcm8xx_clk_data->hws[div_data->onecell_idx] = hw; + } + +/* Register clock dividers specified in npcm8xx_divs */ +for (i =0 ; i <ARRAY_SIZE (npcm8xx_divs); i++) { +struct npcm8xx_clk_div_data *div_data = &npcm8xx_divs[i]; + + hw =npcm8xx_clk_register (dev, div_data->name, clk_regmap, + div_data->reg, div_data->flags, + &npcm8xx_clk_div_ops,NULL , + div_data->parent_hw,1 , + div_data->shift,0 , div_data->width, +NULL , div_data->clk_divider_flags); +if (IS_ERR (hw)) +return dev_err_probe (dev,PTR_ERR (hw),"Can't register div table\n" ); + +if (div_data->onecell_idx >=0 ) + npcm8xx_clk_data->hws[div_data->onecell_idx] = hw; + } + +return devm_of_clk_add_hw_provider (dev, of_clk_hw_onecell_get, + npcm8xx_clk_data); +} + +static const struct of_device_id npcm8xx_clk_dt_ids[] = { + { .compatible ="nuvoton,npcm845-clk" , }, + { } +}; +MODULE_DEVICE_TABLE (of, npcm8xx_clk_dt_ids); + +static struct platform_driver npcm8xx_clk_driver = { + .probe = npcm8xx_clk_probe, + .driver = { + .name ="npcm8xx_clk" , + .of_match_table = npcm8xx_clk_dt_ids, + }, +}; + +static int __init npcm8xx_clk_driver_init (void ) +{ +return platform_driver_register (&npcm8xx_clk_driver); +} +arch_initcall (npcm8xx_clk_driver_init); + +static void __exit npcm8xx_clk_exit(void) +{ +platform_driver_unregister (&npcm8xx_clk_driver); +} +module_exit (npcm8xx_clk_exit); + +MODULE_DESCRIPTION ("Clock driver for Nuvoton NPCM8XX BMC SoC" ); +MODULE_AUTHOR ("Tomer Maimon <[email protected]>" ); +MODULE_LICENSE ("GPL v2" ); +
Signature · 1 line
2.34.1