Skip to contentSkip to conversation

Add SPI module for StarFive JH7110 SoC

PATCH RESEND Superseded William Qiu 4 participants 15 messages Jul 4, 2023 – Jul 5, 2023

Subsystems (5)
Device tree bindings RISC-V SPI StarFive devicetrees +1 more subsystems

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

William Qiu

author
#0 lore raw

cover Add SPI module for StarFive JH7110 SoC

Hi,

This patchset adds initial rudimentary support for the StarFive SPI controller. And this driver will be used in StarFive's VisionFive 2 board. The first patch constrain minItems of clocks for JH7110 SPI and Patch 2 adds support for StarFive JH7110 SPI.

The patch series is based on v6.4rc7.

William Qiu (2):
dt-binding: spi: constrain minItems of clocks and clock-names
riscv: dts: starfive: Add spi node for JH7110 SoC

 .../devicetree/bindings/spi/spi-pl022.yaml    | 11 ++-
 .../jh7110-starfive-visionfive-2.dtsi         | 52 ++++++++++
 arch/riscv/boot/dts/starfive/jh7110.dtsi      | 98 +++++++++++++++++++
 3 files changed, 158 insertions(+), 3 deletions(-)

--
2.34.1

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

author
#1 lore raw

PATCH RESEND 1/2 dt-binding: spi: constrain minItems of clocks and clock-names

The SPI controller only need apb_pclk clock to work properly on JH7110 SoC, so there add minItems whose value is equal to 1. Other platforms do not have this constraint.

Signed-off-by William Qiu <[email protected]>
---
 Documentation/devicetree/bindings/spi/spi-pl022.yaml | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/spi/spi-pl022.yaml +8 -3
index 91e540a92faf..42bb34c39971 100644
@@ -11,6 +11,7 @@ maintainers:
 
 allOf:
   - $ref: spi-controller.yaml#
+  - $ref: /schemas/arm/primecell.yaml#
 
 # We need a select here so we don't match all nodes with 'arm,primecell'
 select:

@@ -34,12 +35,16 @@ properties:
     maxItems: 1
 
   clocks:
+    minItems: 1
     maxItems: 2
 
   clock-names:
-    items:
-      - const: sspclk
-      - const: apb_pclk
+    oneOf:
+      - items:
+          - const: apb_pclk
+      - items:
+          - const: sspclk
+          - const: apb_pclk
 
   pl022,autosuspend-delay:
     description: delay in ms following transfer completion before the

--
2.34.1

#2 lore raw

Re: [RESEND v1 1/2] dt-binding: spi: constrain minItems of clocks and clock-names

Quoted 5 lines · William Qiu

The SPI controller only need apb_pclk clock to work properly on JH7110 SoC, so there add minItems whose value is equal to 1. Other platforms do not have this constraint.

Signed-off-by William Qiu <[email protected]>

I don't get why this is resent, but subject prefix is still wrong. It's dt-bindings.

Documentation/devicetree/bindings/spi/spi-pl022.yaml maintainers: · line 14

Quoted 13 lines
---
 Documentation/devicetree/bindings/spi/spi-pl022.yaml | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/spi/spi-pl022.yaml +1 -0
index 91e540a92faf..42bb34c39971 100644
@@ -11,6 +11,7 @@ maintainers:
 
 allOf:
   - $ref: spi-controller.yaml#
+  - $ref: /schemas/arm/primecell.yaml#

This looks unrelated, so keep it as separate commit with its own rationale.

Quoted 10 lines
Documentation/devicetree/bindings/spi/spi-pl022.yaml +1 -0
 # We need a select here so we don't match all nodes with 'arm,primecell'
 select:

@@ -34,12 +35,16 @@ properties:
     maxItems: 1
 
   clocks:
+    minItems: 1
     maxItems: 2

Documentation/devicetree/bindings/spi/spi-pl022.yaml properties: · line 47

Quoted 10 lines
Documentation/devicetree/bindings/spi/spi-pl022.yaml +6 -3
   clock-names:
-    items:
-      - const: sspclk
-      - const: apb_pclk
+    oneOf:
+      - items:
+          - const: apb_pclk
+      - items:
+          - const: sspclk
+          - const: apb_pclk

Are you sure that your clock is APB pclk in such case?

Best regards,
Krzysztof

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

author
#3 lore raw

Re: [RESEND v1 1/2] dt-binding: spi: constrain minItems of clocks and clock-names

Quoted 10 lines · Krzysztof Kozlowski
Quoted 5 lines · William Qiu

The SPI controller only need apb_pclk clock to work properly on JH7110 SoC, so there add minItems whose value is equal to 1. Other platforms do not have this constraint.

Signed-off-by William Qiu <[email protected]>

I don't get why this is resent, but subject prefix is still wrong. It's dt-bindings.

Will update.

Quoted 10 lines
Quoted 10 lines
---
 Documentation/devicetree/bindings/spi/spi-pl022.yaml | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/spi/spi-pl022.yaml +0 -0
index 91e540a92faf..42bb34c39971 100644
@@ -11,6 +11,7 @@ maintainers:
Quoted 6 lines

Documentation/devicetree/bindings/spi/spi-pl022.yaml maintainers: · line 14

Quoted 3 lines
Documentation/devicetree/bindings/spi/spi-pl022.yaml +1 -0
 allOf:
   - $ref: spi-controller.yaml#
+  - $ref: /schemas/arm/primecell.yaml#

This looks unrelated, so keep it as separate commit with its own rationale.

Because "arm,primecell-periphid" is need in JH7110 SoC, so I added them in one commit, so do I need to put them in two commit?

Quoted 10 lines
Quoted 10 lines
Documentation/devicetree/bindings/spi/spi-pl022.yaml +1 -0
 # We need a select here so we don't match all nodes with 'arm,primecell'
 select:

@@ -34,12 +35,16 @@ properties:
     maxItems: 1
 
   clocks:
+    minItems: 1
     maxItems: 2
Quoted 13 lines

Documentation/devicetree/bindings/spi/spi-pl022.yaml properties: · line 47

Quoted 10 lines
Documentation/devicetree/bindings/spi/spi-pl022.yaml +6 -3
   clock-names:
-    items:
-      - const: sspclk
-      - const: apb_pclk
+    oneOf:
+      - items:
+          - const: apb_pclk
+      - items:
+          - const: sspclk
+          - const: apb_pclk

Are you sure that your clock is APB pclk in such case?

Yes, in JH7110 SoC is APB pclk in such case.

Thanks for taking time to review this patch series.

Quoted 3 lines

Best regards,
Krzysztof

#4 lore raw

Re: [RESEND v1 1/2] dt-binding: spi: constrain minItems of clocks and clock-names

Quoted 10 lines · William Qiu
Quoted 10 lines
Quoted 10 lines
---
 Documentation/devicetree/bindings/spi/spi-pl022.yaml | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/spi/spi-pl022.yaml +0 -0
index 91e540a92faf..42bb34c39971 100644
@@ -11,6 +11,7 @@ maintainers:
Quoted 8 lines
Quoted 6 lines

Documentation/devicetree/bindings/spi/spi-pl022.yaml maintainers: · line 14

Quoted 3 lines
Documentation/devicetree/bindings/spi/spi-pl022.yaml +1 -0
 allOf:
   - $ref: spi-controller.yaml#
+  - $ref: /schemas/arm/primecell.yaml#

This looks unrelated, so keep it as separate commit with its own rationale.

Because "arm,primecell-periphid" is need in JH7110 SoC, so I added them in one commit, so do I need to put them in two commit?

You need to provide rationale why this is needed. I would assume this is needed for every primecell, not only JH7110, right?

Best regards,
Krzysztof

author
#5 lore raw

Re: [RESEND v1 1/2] dt-binding: spi: constrain minItems of clocks and clock-names

Quoted 17 lines · Krzysztof Kozlowski
Quoted 16 lines · William Qiu
Quoted 16 lines

Documentation/devicetree/bindings/spi/spi-pl022.yaml maintainers: · line 14

Quoted 13 lines
---
 Documentation/devicetree/bindings/spi/spi-pl022.yaml | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/spi/spi-pl022.yaml +1 -0
index 91e540a92faf..42bb34c39971 100644
@@ -11,6 +11,7 @@ maintainers:
 
 allOf:
   - $ref: spi-controller.yaml#
+  - $ref: /schemas/arm/primecell.yaml#

This looks unrelated, so keep it as separate commit with its own rationale.

Quoted 7 lines
Quoted 2 lines

Because "arm,primecell-periphid" is need in JH7110 SoC, so I added them in one commit, so do I need to put them in two commit?

You need to provide rationale why this is needed. I would assume this is needed for every primecell, not only JH7110, right?

All right, I'll keep it as separate commit.

Quoted 4 lines

Best regards,
Krzysztof

author
#6 lore raw

PATCH RESEND 2/2 riscv: dts: starfive: Add spi node for JH7110 SoC

In reply to William Qiu

Add spi node for JH7110 SoC.

Co-developed-by Xingyu Wu <[email protected]>
Signed-off-by William Qiu <[email protected]>
---
 .../jh7110-starfive-visionfive-2.dtsi         | 52 ++++++++++
 arch/riscv/boot/dts/starfive/jh7110.dtsi      | 98 +++++++++++++++++++
 2 files changed, 150 insertions(+)
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi +52 -0
index 2a6d81609284..a066d2e399c4 100644
@@ -126,6 +126,20 @@ &i2c6 {
 	status = "okay";
 };
 
+&spi0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&spi0_pins>;
+	status = "okay";
+
+	spi_dev0: spi@0 {
+		compatible = "st,m25p80";
+		pl022,com-mode = <1>;
+		spi-max-frequency = <10000000>;
+		reg = <0>;
+		status = "okay";
+	};
+};
+
 &sysgpio {
 	i2c0_pins: i2c0-0 {
 		i2c-pins {

@@ -183,6 +197,44 @@ GPOEN_SYS_I2C6_DATA,
 		};
 	};
 
+	spi0_pins: spi0-0 {
+		mosi-pins {
+			pinmux = <GPIOMUX(52, GPOUT_SYS_SPI0_TXD,
+					      GPOEN_ENABLE,
+					      GPI_NONE)>;
+			bias-disable;
+			input-disable;
+			input-schmitt-disable;
+		};
+
+		miso-pins {
+			pinmux = <GPIOMUX(53, GPOUT_LOW,
+					      GPOEN_DISABLE,
+					      GPI_SYS_SPI0_RXD)>;
+			bias-pull-up;
+			input-enable;
+			input-schmitt-enable;
+		};
+
+		sck-pins {
+			pinmux = <GPIOMUX(48, GPOUT_SYS_SPI0_CLK,
+					      GPOEN_ENABLE,
+					      GPI_SYS_SPI0_CLK)>;
+			bias-disable;
+			input-disable;
+			input-schmitt-disable;
+		};
+
+		ss-pins {
+			pinmux = <GPIOMUX(48, GPOUT_SYS_SPI0_FSS,
+					      GPOEN_ENABLE,
+					      GPI_SYS_SPI0_FSS)>;
+			bias-disable;
+			input-disable;
+			input-schmitt-disable;
+		};
+	};
+
 	uart0_pins: uart0-0 {
 		tx-pins {
 			pinmux = <GPIOMUX(5, GPOUT_SYS_UART0_TX,
arch/riscv/boot/dts/starfive/jh7110.dtsi +98 -0

110 lines not on this page · raw message

--
2.34.1

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

#7 lore raw

Re: [RESEND v1 2/2] riscv: dts: starfive: Add spi node for JH7110 SoC

Quoted 3 lines · William Qiu

Add spi node for JH7110 SoC.

Co-developed-by Xingyu Wu <[email protected]>

Missing SoB.

Quoted 14 lines
Signed-off-by William Qiu <[email protected]>
---
 .../jh7110-starfive-visionfive-2.dtsi         | 52 ++++++++++
 arch/riscv/boot/dts/starfive/jh7110.dtsi      | 98 +++++++++++++++++++
 2 files changed, 150 insertions(+)
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi +0 -0
index 2a6d81609284..a066d2e399c4 100644
@@ -126,6 +126,20 @@ &i2c6 {
 	status = "okay";
 };

arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi &i2c6 { · line 138

Quoted 10 lines
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi +10 -0
+&spi0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&spi0_pins>;
+	status = "okay";
+
+	spi_dev0: spi@0 {
+		compatible = "st,m25p80";
+		pl022,com-mode = <1>;
+		spi-max-frequency = <10000000>;
+		reg = <0>;

reg is always following compatible, not somewhere deep in properties.

arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi &i2c6 { · line 139

Quoted 1 line
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi +1 -0
+		status = "okay";

okay is by default

arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi &i2c6 { · line 141

Quoted 2 lines
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi +2 -0
+	};
+};

Best regards,
Krzysztof

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

maintainer
#8 lore raw

Re: [RESEND v1 2/2] riscv: dts: starfive: Add spi node for JH7110 SoC

Quoted 6 lines · Krzysztof Kozlowski
Quoted 3 lines · William Qiu

Add spi node for JH7110 SoC.

Co-developed-by Xingyu Wu <[email protected]>

Missing SoB.

It's fine not to have a signoff for the codeveloper of codeveloped patches, see case (a) for the DCO.

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

#9 lore raw

Re: [RESEND v1 2/2] riscv: dts: starfive: Add spi node for JH7110 SoC

Quoted 10 lines · Mark Brown
Quoted 6 lines · Krzysztof Kozlowski
Quoted 3 lines · William Qiu

Add spi node for JH7110 SoC.

Co-developed-by Xingyu Wu <[email protected]>

Missing SoB.

It's fine not to have a signoff for the codeveloper of codeveloped patches, see case (a) for the DCO.

Also see:
"every Co-developed-by: must be immediately followed by a Signed-off-by: of the associated co-author."

https://elixir.bootlin.com/linux/v6.4/source/Documentation/process/submitting-patches.rst#L467

Best regards,
Krzysztof

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

maintainer
#10 lore raw

Re: [RESEND v1 2/2] riscv: dts: starfive: Add spi node for JH7110 SoC

Quoted 5 lines · Krzysztof Kozlowski

Also see:
"every Co-developed-by: must be immediately followed by a Signed-off-by: of the associated co-author."

https://elixir.bootlin.com/linux/v6.4/source/Documentation/process/submitting-patches.rst#L467

Oh, that seems unhelpful especially with it not lining up with the DCO.

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

#11 lore raw

Re: [RESEND v1 2/2] riscv: dts: starfive: Add spi node for JH7110 SoC

Quoted 9 lines · Mark Brown
Quoted 5 lines · Krzysztof Kozlowski

Also see:
"every Co-developed-by: must be immediately followed by a Signed-off-by: of the associated co-author."

https://elixir.bootlin.com/linux/v6.4/source/Documentation/process/submitting-patches.rst#L467

Oh, that seems unhelpful especially with it not lining up with the DCO.

I assume the intention was here that if I attribute some co-author with Co-developed-by, then I know that author, therefore I expect author to explicitly participate in DCO chain.

Otherwise, just drop the Co-developed-by.

Best regards,
Krzysztof

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

maintainer
#12 lore raw

Re: [RESEND v1 2/2] riscv: dts: starfive: Add spi node for JH7110 SoC

Quoted 7 lines · Krzysztof Kozlowski
Quoted 5 lines · Mark Brown
Quoted 3 lines · Krzysztof Kozlowski

Also see:
"every Co-developed-by: must be immediately followed by a Signed-off-by: of the associated co-author."

Quoted 7 lines
Quoted 3 lines
Quoted 1 line

https://elixir.bootlin.com/linux/v6.4/source/Documentation/process/submitting-patches.rst#L467

Oh, that seems unhelpful especially with it not lining up with the DCO.

I assume the intention was here that if I attribute some co-author with Co-developed-by, then I know that author, therefore I expect author to explicitly participate in DCO chain.

Why? They're not the one sending the patch out, nor are they relying on someone else having certified anything.

Quoted 1 line

Otherwise, just drop the Co-developed-by.

It seems separately useful.

maintainer
#13 lore raw

Re: [RESEND v1 2/2] riscv: dts: starfive: Add spi node for JH7110 SoC

Quoted 10 lines · Mark Brown
Quoted 8 lines · Krzysztof Kozlowski
Quoted 7 lines · Mark Brown
Quoted 5 lines · Krzysztof Kozlowski

Also see:
"every Co-developed-by: must be immediately followed by a Signed-off-by: of the associated co-author."

https://elixir.bootlin.com/linux/v6.4/source/Documentation/process/submitting-patches.rst#L467

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

Oh, that seems unhelpful especially with it not lining up with the DCO.

I assume the intention was here that if I attribute some co-author with Co-developed-by, then I know that author, therefore I expect author to explicitly participate in DCO chain.

Why? They're not the one sending the patch out, nor are they relying on someone else having certified anything.

It's probably safe to say that StarFive owns the contributions anyway, so I doubt adding really makes a difference here.

Quoted 3 lines
Quoted 1 line

Otherwise, just drop the Co-developed-by.

It seems separately useful.

Yup, I'd rather have the people there if I ever have to run git blame on whatever commit this becomes.

author
#14 lore raw

Re: [RESEND v1 2/2] riscv: dts: starfive: Add spi node for JH7110 SoC

In reply to Krzysztof Kozlowski
Quoted 7 lines · Krzysztof Kozlowski
Quoted 3 lines · William Qiu

Add spi node for JH7110 SoC.

Co-developed-by Xingyu Wu <[email protected]>

Missing SoB.

It looks like that drop it is the best solution.

Quoted 14 lines
Quoted 14 lines
Signed-off-by William Qiu <[email protected]>
---
 .../jh7110-starfive-visionfive-2.dtsi         | 52 ++++++++++
 arch/riscv/boot/dts/starfive/jh7110.dtsi      | 98 +++++++++++++++++++
 2 files changed, 150 insertions(+)
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi +0 -0
index 2a6d81609284..a066d2e399c4 100644
@@ -126,6 +126,20 @@ &i2c6 {
 	status = "okay";
 };
Quoted 13 lines

arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi &i2c6 { · line 138

Quoted 10 lines
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi +10 -0
+&spi0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&spi0_pins>;
+	status = "okay";
+
+	spi_dev0: spi@0 {
+		compatible = "st,m25p80";
+		pl022,com-mode = <1>;
+		spi-max-frequency = <10000000>;
+		reg = <0>;

reg is always following compatible, not somewhere deep in properties.

Will update.

Quoted 4 lines

arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi &i2c6 { · line 139

Quoted 1 line
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi +1 -0
+		status = "okay";

okay is by default

Will drop.

Quoted 7 lines

arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi &i2c6 { · line 141

Quoted 2 lines
arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi +2 -0
+	};
+};

Best regards,
Krzysztof

Thanks for taking time to review this patch series.