[PATCH v2 0/3] Add SPI module for StarFive JH7110 SoC

[PATCH v2 0/3] Add SPI module for StarFive JH7110 SoC

[William Qiu]

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.

Changes v1->v2:
- Rebaed to v6.5rc1.
- Submitted reference file separately.
- Dropped 'status' node as it was 'okay' by default.
- Dropped Co-developed-by message.

The patch series is based on v6.5rc1.

William Qiu (3):
  dt-bindings: spi: add reference file to YAML
  dt-bindings: 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         | 50 ++++++++++
 arch/riscv/boot/dts/starfive/jh7110.dtsi      | 98 +++++++++++++++++++
 3 files changed, 156 insertions(+), 3 deletions(-)

--
2.34.1

[PATCH v2 1/3] dt-bindings: spi: add reference file to YAML

[William Qiu]

Add primecell.yaml as a refereence file to YAML.

Signed-off-by: William Qiu <william.qiu@starfivetech.com>
---
 Documentation/devicetree/bindings/spi/spi-pl022.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/spi/spi-pl022.yaml b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
index 91e540a92faf..5e5a704a766e 100644
--- a/Documentation/devicetree/bindings/spi/spi-pl022.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
@@ -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:
-- 
2.34.1

Re: [PATCH v2 1/3] dt-bindings: spi: add reference file to YAML

[Krzysztof Kozlowski]

On 13/07/2023 11:00, William Qiu wrote:
> Add primecell.yaml as a refereence file to YAML.

This we see from the diff, but why you are doing it? Please provide
rationale and answer to "why" in your commits.

Also typo: reference.

> 
> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> ---
>  Documentation/devicetree/bindings/spi/spi-pl022.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-pl022.yaml b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
> index 91e540a92faf..5e5a704a766e 100644
> --- a/Documentation/devicetree/bindings/spi/spi-pl022.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
> @@ -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:

Best regards,
Krzysztof

Re: [PATCH v2 1/3] dt-bindings: spi: add reference file to YAML

[William Qiu]

On 2023/7/13 18:12, Krzysztof Kozlowski wrote:
> On 13/07/2023 11:00, William Qiu wrote:
>> Add primecell.yaml as a refereence file to YAML.
> 
> This we see from the diff, but why you are doing it? Please provide
> rationale and answer to "why" in your commits.
> 
> Also typo: reference.
> 
>> 
>> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
>> ---
>>  Documentation/devicetree/bindings/spi/spi-pl022.yaml | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/spi/spi-pl022.yaml b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
>> index 91e540a92faf..5e5a704a766e 100644
>> --- a/Documentation/devicetree/bindings/spi/spi-pl022.yaml
>> +++ b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
>> @@ -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:
> 
> Best regards,
> Krzysztof
> 
I will explain it in next version.

Best regards,
William

[PATCH v2 2/3] dt-bindings: spi: constrain minItems of clocks and clock-names

[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 <william.qiu@starfivetech.com>
---
 Documentation/devicetree/bindings/spi/spi-pl022.yaml | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-pl022.yaml b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
index 5e5a704a766e..42bb34c39971 100644
--- a/Documentation/devicetree/bindings/spi/spi-pl022.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-pl022.yaml
@@ -35,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

Re: [PATCH v2 2/3] dt-bindings: spi: constrain minItems of clocks and clock-names

[Krzysztof Kozlowski]

On 13/07/2023 11:00, William Qiu wrote:
> 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 <william.qiu@starfivetech.com>
> ---

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v2 2/3] dt-bindings: spi: constrain minItems of clocks and clock-names

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 379 bytes --]

On Thu, Jul 13, 2023 at 05:00:14PM +0800, William Qiu wrote:

> 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.

Presumably this means that this is some variant of the usual pl022 IP,
or that the clock is in fact present but is not modelled in your DT?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 2/3] dt-bindings: spi: constrain minItems of clocks and clock-names

[Krzysztof Kozlowski]

On 13/07/2023 14:28, Mark Brown wrote:
> On Thu, Jul 13, 2023 at 05:00:14PM +0800, William Qiu wrote:
> 
>> 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.
> 
> Presumably this means that this is some variant of the usual pl022 IP,

Hm, in such case this could mean we need dedicated compatible.

> or that the clock is in fact present but is not modelled in your DT?

Best regards,
Krzysztof

Re: [PATCH v2 2/3] dt-bindings: spi: constrain minItems of clocks and clock-names

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 517 bytes --]

On Thu, Jul 13, 2023 at 02:39:19PM +0200, Krzysztof Kozlowski wrote:
> On 13/07/2023 14:28, Mark Brown wrote:
> > On Thu, Jul 13, 2023 at 05:00:14PM +0800, William Qiu wrote:

> >> 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.

> > Presumably this means that this is some variant of the usual pl022 IP,

> Hm, in such case this could mean we need dedicated compatible.

Yes, indeed.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 2/3] dt-bindings: spi: constrain minItems of clocks and clock-names

[Rob Herring]

On Thu, Jul 13, 2023 at 02:39:19PM +0200, Krzysztof Kozlowski wrote:
> On 13/07/2023 14:28, Mark Brown wrote:
> > On Thu, Jul 13, 2023 at 05:00:14PM +0800, William Qiu wrote:
> > 
> >> 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.
> > 
> > Presumably this means that this is some variant of the usual pl022 IP,
> 
> Hm, in such case this could mean we need dedicated compatible.

Except the vendor in the ID registers should be different if the IP is 
modified.

I suspect that PCLK and SSPCLK are tied to the same clock source. There 
must be an SSPCLK because that is the one used to clock the SPI bus and 
we need to know the frequency of it.

Rob

Re: [PATCH v2 2/3] dt-bindings: spi: constrain minItems of clocks and clock-names

[William Qiu]

On 2023/7/13 22:57, Rob Herring wrote:
> On Thu, Jul 13, 2023 at 02:39:19PM +0200, Krzysztof Kozlowski wrote:
>> On 13/07/2023 14:28, Mark Brown wrote:
>> > On Thu, Jul 13, 2023 at 05:00:14PM +0800, William Qiu wrote:
>> > 
>> >> 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.
>> > 
>> > Presumably this means that this is some variant of the usual pl022 IP,
>> 
>> Hm, in such case this could mean we need dedicated compatible.
> 
> Except the vendor in the ID registers should be different if the IP is 
> modified.
> 
> I suspect that PCLK and SSPCLK are tied to the same clock source. There 
> must be an SSPCLK because that is the one used to clock the SPI bus and 
> we need to know the frequency of it.
> 
> Rob

After communicating with colleagues in SoC FE, I learned that PCLK and
SSPCLK were homologous on JH7110. He said that SSPCLK would divide the
frequency internally anyway, and there was no need for external part frequency,
so he directly gave them together.

So, should I call this clock ssp_apb or keep it SSPCLK?

Best regards,
William

Re: [PATCH v2 2/3] dt-bindings: spi: constrain minItems of clocks and clock-names

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 830 bytes --]

On Fri, Jul 14, 2023 at 03:14:59PM +0800, William Qiu wrote:
> On 2023/7/13 22:57, Rob Herring wrote:

> > I suspect that PCLK and SSPCLK are tied to the same clock source. There 
> > must be an SSPCLK because that is the one used to clock the SPI bus and 
> > we need to know the frequency of it.

> After communicating with colleagues in SoC FE, I learned that PCLK and
> SSPCLK were homologous on JH7110. He said that SSPCLK would divide the
> frequency internally anyway, and there was no need for external part frequency,
> so he directly gave them together.

> So, should I call this clock ssp_apb or keep it SSPCLK?

I'd expect this to be handled in the DTS for the SoC - connect both
clocks the binding requires to whatever the upstream clock is, it's not
clear to me that any binding change is required.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 2/3] dt-bindings: spi: constrain minItems of clocks and clock-names

[William Qiu]

On 2023/7/14 19:52, Mark Brown wrote:
> On Fri, Jul 14, 2023 at 03:14:59PM +0800, William Qiu wrote:
>> On 2023/7/13 22:57, Rob Herring wrote:
> 
>> > I suspect that PCLK and SSPCLK are tied to the same clock source. There 
>> > must be an SSPCLK because that is the one used to clock the SPI bus and 
>> > we need to know the frequency of it.
> 
>> After communicating with colleagues in SoC FE, I learned that PCLK and
>> SSPCLK were homologous on JH7110. He said that SSPCLK would divide the
>> frequency internally anyway, and there was no need for external part frequency,
>> so he directly gave them together.
> 
>> So, should I call this clock ssp_apb or keep it SSPCLK?
> 
> I'd expect this to be handled in the DTS for the SoC - connect both
> clocks the binding requires to whatever the upstream clock is, it's not
> clear to me that any binding change is required.

You mean binding two clocks, with the same clock source? Then there is no
need to modify YAML.

Best regards,
William

Re: [PATCH v2 2/3] dt-bindings: spi: constrain minItems of clocks and clock-names

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 823 bytes --]

On Tue, Jul 18, 2023 at 02:06:01PM +0800, William Qiu wrote:
> On 2023/7/14 19:52, Mark Brown wrote:
> > On Fri, Jul 14, 2023 at 03:14:59PM +0800, William Qiu wrote:

> >> After communicating with colleagues in SoC FE, I learned that PCLK and
> >> SSPCLK were homologous on JH7110. He said that SSPCLK would divide the
> >> frequency internally anyway, and there was no need for external part frequency,
> >> so he directly gave them together.

> >> So, should I call this clock ssp_apb or keep it SSPCLK?

> > I'd expect this to be handled in the DTS for the SoC - connect both
> > clocks the binding requires to whatever the upstream clock is, it's not
> > clear to me that any binding change is required.

> You mean binding two clocks, with the same clock source? Then there is no
> need to modify YAML.

Yes, exactly.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2 3/3] riscv: dts: starfive: Add spi node for JH7110 SoC

[William Qiu]

Add spi node for JH7110 SoC.

Signed-off-by: William Qiu <william.qiu@starfivetech.com>
---
 .../jh7110-starfive-visionfive-2.dtsi         | 50 ++++++++++
 arch/riscv/boot/dts/starfive/jh7110.dtsi      | 98 +++++++++++++++++++
 2 files changed, 148 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
index fa0061eb33a7..ae3f39c33d3c 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
@@ -143,6 +143,18 @@ &i2c6 {
 	status = "okay";
 };
 
+&spi0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&spi0_pins>;
+	status = "okay";
+
+	spi_dev0: flash@0 {
+		compatible = "rohm,dh2228fv";
+		reg = <0>;
+		spi-max-frequency = <10000000>;
+	};
+};
+
 &sysgpio {
 	i2c0_pins: i2c0-0 {
 		i2c-pins {
@@ -200,6 +212,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,
diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index ec2e70011a73..13da297c18fe 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -386,6 +386,48 @@ i2c2: i2c@10050000 {
 			status = "disabled";
 		};
 
+		spi0: spi@10060000 {
+			compatible = "arm,pl022", "arm,primecell";
+			reg = <0x0 0x10060000 0x0 0x10000>;
+			clocks = <&syscrg JH7110_SYSCLK_SPI0_APB>;
+			clock-names = "apb_pclk";
+			resets = <&syscrg JH7110_SYSRST_SPI0_APB>;
+			interrupts = <38>;
+			arm,primecell-periphid = <0x00041022>;
+			num-cs = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		spi1: spi@10070000 {
+			compatible = "arm,pl022", "arm,primecell";
+			reg = <0x0 0x10070000 0x0 0x10000>;
+			clocks = <&syscrg JH7110_SYSCLK_SPI1_APB>;
+			clock-names = "apb_pclk";
+			resets = <&syscrg JH7110_SYSRST_SPI1_APB>;
+			interrupts = <39>;
+			arm,primecell-periphid = <0x00041022>;
+			num-cs = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		spi2: spi@10080000 {
+			compatible = "arm,pl022", "arm,primecell";
+			reg = <0x0 0x10080000 0x0 0x10000>;
+			clocks = <&syscrg JH7110_SYSCLK_SPI2_APB>;
+			clock-names = "apb_pclk";
+			resets = <&syscrg JH7110_SYSRST_SPI2_APB>;
+			interrupts = <40>;
+			arm,primecell-periphid = <0x00041022>;
+			num-cs = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
 		uart3: serial@12000000 {
 			compatible = "snps,dw-apb-uart";
 			reg = <0x0 0x12000000 0x0 0x10000>;
@@ -473,6 +515,62 @@ i2c6: i2c@12060000 {
 			status = "disabled";
 		};
 
+		spi3: spi@12070000 {
+			compatible = "arm,pl022", "arm,primecell";
+			reg = <0x0 0x12070000 0x0 0x10000>;
+			clocks = <&syscrg JH7110_SYSCLK_SPI3_APB>;
+			clock-names = "apb_pclk";
+			resets = <&syscrg JH7110_SYSRST_SPI3_APB>;
+			interrupts = <52>;
+			arm,primecell-periphid = <0x00041022>;
+			num-cs = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		spi4: spi@12080000 {
+			compatible = "arm,pl022", "arm,primecell";
+			reg = <0x0 0x12080000 0x0 0x10000>;
+			clocks = <&syscrg JH7110_SYSCLK_SPI4_APB>;
+			clock-names = "apb_pclk";
+			resets = <&syscrg JH7110_SYSRST_SPI4_APB>;
+			interrupts = <53>;
+			arm,primecell-periphid = <0x00041022>;
+			num-cs = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		spi5: spi@12090000 {
+			compatible = "arm,pl022", "arm,primecell";
+			reg = <0x0 0x12090000 0x0 0x10000>;
+			clocks = <&syscrg JH7110_SYSCLK_SPI5_APB>;
+			clock-names = "apb_pclk";
+			resets = <&syscrg JH7110_SYSRST_SPI5_APB>;
+			interrupts = <54>;
+			arm,primecell-periphid = <0x00041022>;
+			num-cs = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		spi6: spi@120a0000 {
+			compatible = "arm,pl022", "arm,primecell";
+			reg = <0x0 0x120A0000 0x0 0x10000>;
+			clocks = <&syscrg JH7110_SYSCLK_SPI6_APB>;
+			clock-names = "apb_pclk";
+			resets = <&syscrg JH7110_SYSRST_SPI6_APB>;
+			interrupts = <55>;
+			arm,primecell-periphid = <0x00041022>;
+			num-cs = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
 		syscrg: clock-controller@13020000 {
 			compatible = "starfive,jh7110-syscrg";
 			reg = <0x0 0x13020000 0x0 0x10000>;
-- 
2.34.1

Re: (subset) [PATCH v2 0/3] Add SPI module for StarFive JH7110 SoC

[Mark Brown]

On Thu, 13 Jul 2023 17:00:12 +0800, William Qiu wrote:
> 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.
> 
> Changes v1->v2:
> - Rebaed to v6.5rc1.
> - Submitted reference file separately.
> - Dropped 'status' node as it was 'okay' by default.
> - Dropped Co-developed-by message.
> 
> [...]

Applied to

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

Thanks!

[1/3] dt-bindings: spi: add reference file to YAML
      commit: 8858babff615ee366bf570c1facfbd0845dd924e
[2/3] dt-bindings: spi: constrain minItems of clocks and clock-names
      (no commit info)

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