Skip to contentSkip to conversation

Nuvoton NPCM FIU DTS fixes and binding conversion

PATCH Superseded Tomer Maimon 4 participants 11 messages Jun 9 – Jun 11

Subsystems (3)

v4 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Tomer Maimon

author maintainer
#0 lore raw

v4 cover Nuvoton NPCM FIU DTS fixes and binding conversion

This series fixes the in-tree NPCM7xx FIU controller nodes so their resources match what the DTS actually describes, and converts the legacy Nuvoton NPCM FIU binding to YAML DT schema.

Patch 1 drops the bogus "memory" entry from reg-names on the NPCM7xx FIU nodes.

Patch 2 drops redundant clock-names from those single-clock FIU controllers.

Patch 3 renames the schema to nuvoton,npcm750-fiu.yaml, explains why the direct-mapped flash window is optional, keeps the requested example ordering, and simplifies reg/reg-names to the ordered minItems form.

Changes since version 3:
- Split the DTS cleanup so the reg-names fix stands alone as patch 1.
- Added review tag to Drop bogus FIU memory reg-names commit.

Changes since version 2:
- Drop redundant FIU clock-names from the NPCM7xx DTSI together with the bogus reg-names cleanup.
- Rename the schema to nuvoton,npcm750-fiu.yaml and keep reg/reg-names immediately after compatible in the example.
- Explain why the direct-mapped flash window is optional and model reg/reg-names as ordered minItems arrays.
- Drop clock-names from the schema to match the driver and updated DTS.

Changes since version 1:
- Drop interrupts property.
- Drop unused label.
- Keep reg as the second property, followed by reg-names.

Tomer Maimon (3):
arm: dts: nuvoton: npcm7xx: Drop bogus FIU memory reg-names
arm: dts: nuvoton: npcm7xx: Drop redundant FIU clock-names
spi: dt-bindings: nuvoton,npcm750-fiu: Convert to DT schema

Signature · 1 line
2.34.1
author maintainer
#1 lore raw

PATCH v4 1/3 arm: dts: nuvoton: npcm7xx: Drop bogus FIU memory reg-names

The NPCM7xx FIU controller nodes only describe the control register block, but they still advertise a second "memory" entry in reg-names. Drop the bogus name so the DTS matches the resources actually present in each node.

---
 arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi +3 -3
index ab3c3c5713ae..a16450abea0e 100644
@@ -191,7 +191,7 @@ fiu0: spi@fb000000 {
 			#address-cells = <1>;
 			#size-cells = <0>;
 			reg = <0xfb000000 0x1000>;
-			reg-names = "control", "memory";
+			reg-names = "control";
 			clocks = <&clk NPCM7XX_CLK_SPI0>;
 			clock-names = "clk_spi0";
 			status = "disabled";
@@ -202,7 +202,7 @@ fiu3: spi@c0000000 {
 			#address-cells = <1>;
 			#size-cells = <0>;
 			reg = <0xc0000000 0x1000>;
-			reg-names = "control", "memory";
+			reg-names = "control";
 			clocks = <&clk NPCM7XX_CLK_SPI3>;
 			clock-names = "clk_spi3";
 			pinctrl-names = "default";
@@ -215,7 +215,7 @@ fiux: spi@fb001000 {
 			#address-cells = <1>;
 			#size-cells = <0>;
 			reg = <0xfb001000 0x1000>;
-			reg-names = "control", "memory";
+			reg-names = "control";
 			clocks = <&clk NPCM7XX_CLK_SPIX>;
 			clock-names = "clk_spix";
 			status = "disabled";
Signature · 1 line
2.34.1
author maintainer
#2 lore raw

PATCH v4 2/3 arm: dts: nuvoton: npcm7xx: Drop redundant FIU clock-names

In reply to Tomer Maimon

The NPCM7xx FIU controller driver gets its single clock with devm_clk_get_enabled(dev, NULL) and does not perform a named clock lookup. Drop the redundant clock-names properties from the FIU controller nodes so the DTS describes only the resources the driver actually uses.

---
 arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi | 3 ---
 1 file changed, 3 deletions(-)
arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi +0 -3
index a16450abea0e..83cd10b47273 100644
@@ -193,7 +193,6 @@ fiu0: spi@fb000000 {
 			reg = <0xfb000000 0x1000>;
 			reg-names = "control";
 			clocks = <&clk NPCM7XX_CLK_SPI0>;
-			clock-names = "clk_spi0";
 			status = "disabled";
 		};
 
@@ -204,7 +203,6 @@ fiu3: spi@c0000000 {
 			reg = <0xc0000000 0x1000>;
 			reg-names = "control";
 			clocks = <&clk NPCM7XX_CLK_SPI3>;
-			clock-names = "clk_spi3";
 			pinctrl-names = "default";
 			pinctrl-0 = <&spi3_pins>;
 			status = "disabled";
@@ -217,7 +215,6 @@ fiux: spi@fb001000 {
 			reg = <0xfb001000 0x1000>;
 			reg-names = "control";
 			clocks = <&clk NPCM7XX_CLK_SPIX>;
-			clock-names = "clk_spix";
 			status = "disabled";
 		};
 
Signature · 1 line
2.34.1
Reviewed-by on v4 2/3
#3 lore raw

Re: [PATCH v4 2/3] arm: dts: nuvoton: npcm7xx: Drop redundant FIU clock-names

Quoted 10 lines · Tomer Maimon

The NPCM7xx FIU controller driver gets its single clock with devm_clk_get_enabled(dev, NULL) and does not perform a named clock lookup. Drop the redundant clock-names properties from the FIU controller nodes so the DTS describes only the resources the driver actually uses.

---
 arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi | 3 ---
 1 file changed, 3 deletions(-)

Best regards,
Krzysztof

maintainer
#4 lore raw

Re: [PATCH v4 2/3] arm: dts: nuvoton: npcm7xx: Drop redundant FIU clock-names

In reply to Tomer Maimon
Quoted 5 lines · Tomer Maimon

The NPCM7xx FIU controller driver gets its single clock with devm_clk_get_enabled(dev, NULL) and does not perform a named clock lookup. Drop the redundant clock-names properties from the FIU controller nodes so the DTS describes only the resources the driver actually uses.

The devicetree is a description of the hardware in the form documented by the bindings. Generally it's not right to discuss Linux drivers
here: they're only relevant in the context of Linux, but the devicetree binding governs devicetrees over multiple projects.

From a quick look it seems that these names are not described in the corresponding binding, therefore no drivers should be using them and as such they can (and should) be dropped. A driver would only be worth mentioning if it did use the undocumented names (as that would be a complication).

Can you please rework the description?

Andrew

Quoted 34 lines

---
 arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi | 3 ---

 1 file changed, 3 deletions(-)
arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi +0 -1
index a16450abea0e..83cd10b47273 100644
@@ -193,7 +193,6 @@ fiu0: spi@fb000000 {
  			reg = <0xfb000000 0x1000>;
  			reg-names = "control";
  			clocks = <&clk NPCM7XX_CLK_SPI0>;
-			clock-names = "clk_spi0";
  			status = "disabled";
  		};

arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi +0 -2
@@ -204,7 +203,6 @@ fiu3: spi@c0000000 {
  			reg = <0xc0000000 0x1000>;
  			reg-names = "control";
  			clocks = <&clk NPCM7XX_CLK_SPI3>;
-			clock-names = "clk_spi3";
  			pinctrl-names = "default";
  			pinctrl-0 = <&spi3_pins>;
  			status = "disabled";
@@ -217,7 +215,6 @@ fiux: spi@fb001000 {
  			reg = <0xfb001000 0x1000>;
  			reg-names = "control";
  			clocks = <&clk NPCM7XX_CLK_SPIX>;
-			clock-names = "clk_spix";
  			status = "disabled";
  		};

author maintainer
#5 lore raw

Re: [PATCH v4 2/3] arm: dts: nuvoton: npcm7xx: Drop redundant FIU clock-names

Quoted 13 lines · Andrew Jeffery
Quoted 5 lines · Tomer Maimon

The NPCM7xx FIU controller driver gets its single clock with devm_clk_get_enabled(dev, NULL) and does not perform a named clock lookup. Drop the redundant clock-names properties from the FIU controller nodes so the DTS describes only the resources the driver actually uses.

The devicetree is a description of the hardware in the form documented by the bindings. Generally it's not right to discuss Linux drivers
here: they're only relevant in the context of Linux, but the devicetree binding governs devicetrees over multiple projects.

Quoted 7 lines

From a quick look it seems that these names are not described in the corresponding binding, therefore no drivers should be using them and as such they can (and should) be dropped. A driver would only be worth mentioning if it did use the undocumented names (as that would be a complication).

Can you please rework the description?

do you mean change the description as follows
"
The NPCM7xx FIU controller nodes reference a single clock, but the FIU binding does not describe their clock-names properties. Drop the undocumented names so the DTS matches the binding. "

Quoted 21 lines

Andrew

Quoted 18 lines
---
 arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi | 3 ---
 1 file changed, 3 deletions(-)
arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi +0 -1
index a16450abea0e..83cd10b47273 100644
@@ -193,7 +193,6 @@ fiu0: spi@fb000000 {
                      reg = <0xfb000000 0x1000>;
                      reg-names = "control";
                      clocks = <&clk NPCM7XX_CLK_SPI0>;
-                     clock-names = "clk_spi0";
                      status = "disabled";
              };
 
Quoted 16 lines
Quoted 9 lines
arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi +0 -1
@@ -204,7 +203,6 @@ fiu3: spi@c0000000 {
                      reg = <0xc0000000 0x1000>;
                      reg-names = "control";
                      clocks = <&clk NPCM7XX_CLK_SPI3>;
-                     clock-names = "clk_spi3";
                      pinctrl-names = "default";
                      pinctrl-0 = <&spi3_pins>;
                      status = "disabled";

arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi fiux: spi@fb001000 { · line 220

Quoted 7 lines
arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi +0 -1
@@ -217,7 +215,6 @@ fiux: spi@fb001000 {
                      reg = <0xfb001000 0x1000>;
                      reg-names = "control";
                      clocks = <&clk NPCM7XX_CLK_SPIX>;
-                     clock-names = "clk_spix";
                      status = "disabled";
              };
 

Thanks,

Tomer

maintainer
#6 lore raw

Re: [PATCH v4 2/3] arm: dts: nuvoton: npcm7xx: Drop redundant FIU clock-names

Quoted 21 lines · Tomer Maimon
Quoted 19 lines · Andrew Jeffery
Quoted 5 lines · Tomer Maimon

The NPCM7xx FIU controller driver gets its single clock with devm_clk_get_enabled(dev, NULL) and does not perform a named clock lookup. Drop the redundant clock-names properties from the FIU controller nodes so the DTS describes only the resources the driver actually uses.

The devicetree is a description of the hardware in the form documented by the bindings. Generally it's not right to discuss Linux drivers
here: they're only relevant in the context of Linux, but the devicetree binding governs devicetrees over multiple projects.

From a quick look it seems that these names are not described in the corresponding binding, therefore no drivers should be using them and as such they can (and should) be dropped. A driver would only be worth mentioning if it did use the undocumented names (as that would be a complication).

Quoted 7 lines
Quoted 1 line

Can you please rework the description?

do you mean change the description as follows
"
The NPCM7xx FIU controller nodes reference a single clock, but the FIU binding does not describe their clock-names properties. Drop the undocumented names so the DTS matches the binding. "

That sounds reasonable to me.

Thanks,

Andrew

author maintainer
#7 lore raw

Re: [PATCH v4 2/3] arm: dts: nuvoton: npcm7xx: Drop redundant FIU clock-names

will send V5 soon with message modification

Quoted 36 lines · Andrew Jeffery
Quoted 21 lines · Tomer Maimon
Quoted 19 lines · Andrew Jeffery
Quoted 5 lines · Tomer Maimon

The NPCM7xx FIU controller driver gets its single clock with devm_clk_get_enabled(dev, NULL) and does not perform a named clock lookup. Drop the redundant clock-names properties from the FIU controller nodes so the DTS describes only the resources the driver actually uses.

The devicetree is a description of the hardware in the form documented by the bindings. Generally it's not right to discuss Linux drivers
here: they're only relevant in the context of Linux, but the devicetree binding governs devicetrees over multiple projects.

From a quick look it seems that these names are not described in the corresponding binding, therefore no drivers should be using them and as such they can (and should) be dropped. A driver would only be worth mentioning if it did use the undocumented names (as that would be a complication).

Quoted 7 lines
Quoted 1 line

Can you please rework the description?

do you mean change the description as follows
"
The NPCM7xx FIU controller nodes reference a single clock, but the FIU binding does not describe their clock-names properties. Drop the undocumented names so the DTS matches the binding. "

That sounds reasonable to me.

Thanks,

Andrew

author maintainer
#8 lore raw

PATCH v4 3/3 spi: dt-bindings: nuvoton,npcm750-fiu: Convert to DT schema

In reply to Tomer Maimon

Convert the Nuvoton NPCM FIU binding to DT schema format.

Document the required control registers and the optional direct- mapped flash window separately, matching the driver behavior when the direct mapping is not described.

---
 .../bindings/spi/nuvoton,npcm-fiu.txt         | 58 ------------
 .../bindings/spi/nuvoton,npcm750-fiu.yaml     | 93 +++++++++++++++++++
 2 files changed, 93 insertions(+), 58 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/spi/nuvoton,npcm-fiu.txt
 create mode 100644 Documentation/devicetree/bindings/spi/nuvoton,npcm750-fiu.yaml
Documentation/devicetree/bindings/spi/nuvoton,npcm-fiu.txt deleted +0 -58
deleted file mode 100644
index fb38e96d395f..000000000000
@@ -1,58 +0,0 @@
-* Nuvoton FLASH Interface Unit (FIU) SPI Controller
-
-NPCM FIU supports single, dual and quad communication interface.
-
-The NPCM7XX supports three FIU modules,
-FIU0 and FIUx supports two chip selects,
-FIU3 support four chip select.
-
-The NPCM8XX supports four FIU modules,
-FIU0 and FIUx supports two chip selects,
-FIU1 and FIU3 supports four chip selects.
-
-Required properties:
-  - compatible : "nuvoton,npcm750-fiu" for Poleg NPCM7XX BMC
-			     "nuvoton,npcm845-fiu" for Arbel NPCM8XX BMC
-  - #address-cells : should be 1.
-  - #size-cells : should be 0.
-  - reg : the first contains the register location and length,
-          the second contains the memory mapping address and length
-  - reg-names: Should contain the reg names "control" and "memory"
-  - clocks : phandle of FIU reference clock.
-
-Required properties in case the pins can be muxed:
-  - pinctrl-names : a pinctrl state named "default" must be defined.
-  - pinctrl-0 : phandle referencing pin configuration of the device.
-
-Optional property:
-  - nuvoton,spix-mode: enable spix-mode for an expansion bus to an ASIC or CPLD.
-
-Aliases:
-- All the FIU controller nodes should be represented in the aliases node using
-  the following format 'fiu{n}' where n is a unique number for the alias.
-  In the NPCM7XX BMC:
-  		fiu0 represent fiu 0 controller
-  		fiu1 represent fiu 3 controller
-  		fiu2 represent fiu x controller
-
-  In the NPCM8XX BMC:
-  		fiu0 represent fiu 0 controller
-  		fiu1 represent fiu 1 controller
-  		fiu2 represent fiu 3 controller
-  		fiu3 represent fiu x controller
-
-Example:
-fiu3: spi@c00000000 {
-	compatible = "nuvoton,npcm750-fiu";
-	#address-cells = <1>;
-	#size-cells = <0>;
-	reg = <0xfb000000 0x1000>, <0x80000000 0x10000000>;
-	reg-names = "control", "memory";
-	clocks = <&clk NPCM7XX_CLK_AHB>;
-	pinctrl-names = "default";
-	pinctrl-0 = <&spi3_pins>;
-	flash@0 {
-			...
-	};
-};
-
Documentation/devicetree/bindings/spi/nuvoton,npcm750-fiu.yaml new file +93 -0

93 lines not on this page · raw message

Signature · 1 line
2.34.1
Reviewed-by on v4 3/3
#9 lore raw

Re: [PATCH v4 3/3] spi: dt-bindings: nuvoton,npcm750-fiu: Convert to DT schema

Quoted 6 lines · Tomer Maimon

Convert the Nuvoton NPCM FIU binding to DT schema format.

Document the required control registers and the optional direct- mapped flash window separately, matching the driver behavior when the direct mapping is not described.

Quoted 7 lines
---
 .../bindings/spi/nuvoton,npcm-fiu.txt         | 58 ------------
 .../bindings/spi/nuvoton,npcm750-fiu.yaml     | 93 +++++++++++++++++++
 2 files changed, 93 insertions(+), 58 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/spi/nuvoton,npcm-fiu.txt
 create mode 100644 Documentation/devicetree/bindings/spi/nuvoton,npcm750-fiu.yaml

Best regards,
Krzysztof

maintainer
#10 lore raw

Re: (subset) [PATCH v4 0/3] Nuvoton NPCM FIU DTS fixes and binding conversion

In reply to Tomer Maimon
Quoted 10 lines · Tomer Maimon

Nuvoton NPCM FIU DTS fixes and binding conversion

This series fixes the in-tree NPCM7xx FIU controller nodes so their resources match what the DTS actually describes, and converts the legacy Nuvoton NPCM FIU binding to YAML DT schema.

Patch 1 drops the bogus "memory" entry from reg-names on the NPCM7xx FIU nodes.

[...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-7.2

Thanks!

[3/3] spi: dt-bindings: nuvoton,npcm750-fiu: Convert to DT schema

      https://git.kernel.org/broonie/misc/c/3da90b29241d

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark