[v3] spi: support the SpacemiT K1 SPI controller

[PATCH v3 3/3] riscv: dts: spacemit: define a SPI controller node

Posted by Alex Elder 1 week, 2 days ago

Define a node for the fourth SoC SPI controller (number 3) on
the SpacemiT K1 SoC.

Enable it on the Banana Pi BPI-F3 board, which exposes this feature
via its GPIO block:
  GPIO PIN 19:  MOSI
  GPIO PIN 21:  MISO
  GPIO PIN 23:  SCLK
  GPIO PIN 24:  SS (inverted)

Define pincontrol configurations for the pins as used on that board.

(This was tested using a GigaDevice GD25Q64E SPI NOR chip.)

Signed-off-by: Alex Elder <elder@riscstar.com>
---
v3: - Moved the SPI controller into the dma-bus memory region

 .../boot/dts/spacemit/k1-bananapi-f3.dts      |  7 +++++++
 arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi  | 20 +++++++++++++++++++
 arch/riscv/boot/dts/spacemit/k1.dtsi          | 16 +++++++++++++++
 3 files changed, 43 insertions(+)

diff --git a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
index 2aaaff77831e1..d9d865fbe320e 100644
--- a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
+++ b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
@@ -14,6 +14,7 @@ aliases {
 		ethernet0 = &eth0;
 		ethernet1 = &eth1;
 		serial0 = &uart0;
+		spi3 = &spi3;
 	};
 
 	chosen {
@@ -92,6 +93,12 @@ &pdma {
 	status = "okay";
 };
 
+&spi3 {
+	pinctrl-0 = <&ssp3_0_cfg>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
 &uart0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_2_cfg>;
diff --git a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
index aff19c86d5ff3..205c201a3005c 100644
--- a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
+++ b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
@@ -76,4 +76,24 @@ pwm14-1-pins {
 			drive-strength = <32>;
 		};
 	};
+
+	ssp3_0_cfg: ssp3-0-cfg {
+		ssp3-0-no-pull-pins {
+			pinmux = <K1_PADCONF(75, 2)>,	/* SCLK */
+				 <K1_PADCONF(77, 2)>,	/* MOSI  */
+				 <K1_PADCONF(78, 2)>;	/* MISO */
+
+			bias-disable;
+			drive-strength = <19>;
+			power-source = <3300>;
+		};
+
+		ssp3-0-frm-pins {
+			pinmux = <K1_PADCONF(76, 2)>;	/* FRM (frame) */
+
+			bias-pull-up = <0>;
+			drive-strength = <19>;
+			power-source = <3300>;
+		};
+	};
 };
diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
index 6cdcd80a7c83b..eb8a14dd72ea4 100644
--- a/arch/riscv/boot/dts/spacemit/k1.dtsi
+++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
@@ -797,6 +797,22 @@ uart9: serial@d4017800 {
 				status = "disabled";
 			};
 
+			spi3: spi@d401c000 {
+				compatible = "spacemit,k1-spi";
+				reg = <0x0 0xd401c000 0x0 0x30>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+				clocks = <&syscon_apbc CLK_SSP3>,
+					 <&syscon_apbc CLK_SSP3_BUS>;
+				clock-names = "core", "bus";
+				resets = <&syscon_apbc RESET_SSP3>;
+				interrupts = <55>;
+				dmas = <&pdma 20>,
+				       <&pdma 19>;
+				dma-names = "rx", "tx";
+				status = "disabled";
+			};
+
 			/* sec_uart1: 0xf0612000, not available from Linux */
 		};
 
-- 
2.48.1

Re: [PATCH v3 3/3] riscv: dts: spacemit: define a SPI controller node

Posted by Yixun Lan 1 week, 2 days ago

Hi Alex,

On 11:17 Mon 22 Sep     , Alex Elder wrote:
> Define a node for the fourth SoC SPI controller (number 3) on
> the SpacemiT K1 SoC.
> 
> Enable it on the Banana Pi BPI-F3 board, which exposes this feature
> via its GPIO block:
>   GPIO PIN 19:  MOSI
>   GPIO PIN 21:  MISO
>   GPIO PIN 23:  SCLK
>   GPIO PIN 24:  SS (inverted)
> 
> Define pincontrol configurations for the pins as used on that board.
> 
> (This was tested using a GigaDevice GD25Q64E SPI NOR chip.)
> 
> Signed-off-by: Alex Elder <elder@riscstar.com>
> ---
> v3: - Moved the SPI controller into the dma-bus memory region
> 
>  .../boot/dts/spacemit/k1-bananapi-f3.dts      |  7 +++++++
>  arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi  | 20 +++++++++++++++++++
>  arch/riscv/boot/dts/spacemit/k1.dtsi          | 16 +++++++++++++++
>  3 files changed, 43 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> index 2aaaff77831e1..d9d865fbe320e 100644
> --- a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> +++ b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> @@ -14,6 +14,7 @@ aliases {
>  		ethernet0 = &eth0;
>  		ethernet1 = &eth1;
>  		serial0 = &uart0;
> +		spi3 = &spi3;
>  	};
>  
>  	chosen {
> @@ -92,6 +93,12 @@ &pdma {
>  	status = "okay";
>  };
>  
> +&spi3 {
> +	pinctrl-0 = <&ssp3_0_cfg>;
> +	pinctrl-names = "default";
> +	status = "okay";
> +};
> +
>  &uart0 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&uart0_2_cfg>;
> diff --git a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> index aff19c86d5ff3..205c201a3005c 100644
> --- a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> +++ b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> @@ -76,4 +76,24 @@ pwm14-1-pins {
>  			drive-strength = <32>;
>  		};
>  	};
> +
> +	ssp3_0_cfg: ssp3-0-cfg {
..
> +		ssp3-0-no-pull-pins {
I'd prefer not to enforce "pull" info inside the name, you can't embed
all property info, besides, what's if you want to change/override later?

how about just name it as ssp3-0-defaul-pins or simply ssp3-0-pins?

> +			pinmux = <K1_PADCONF(75, 2)>,	/* SCLK */
> +				 <K1_PADCONF(77, 2)>,	/* MOSI  */
> +				 <K1_PADCONF(78, 2)>;	/* MISO */
> +
> +			bias-disable;
> +			drive-strength = <19>;
> +			power-source = <3300>;
> +		};
> +
> +		ssp3-0-frm-pins {
> +			pinmux = <K1_PADCONF(76, 2)>;	/* FRM (frame) */
> +
> +			bias-pull-up = <0>;
> +			drive-strength = <19>;
> +			power-source = <3300>;
> +		};
> +	};
>  };
> diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
> index 6cdcd80a7c83b..eb8a14dd72ea4 100644
> --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
> +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
> @@ -797,6 +797,22 @@ uart9: serial@d4017800 {
>  				status = "disabled";
>  			};
>  
> +			spi3: spi@d401c000 {
> +				compatible = "spacemit,k1-spi";
> +				reg = <0x0 0xd401c000 0x0 0x30>;
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				clocks = <&syscon_apbc CLK_SSP3>,
> +					 <&syscon_apbc CLK_SSP3_BUS>;
> +				clock-names = "core", "bus";
> +				resets = <&syscon_apbc RESET_SSP3>;
> +				interrupts = <55>;
..
> +				dmas = <&pdma 20>,
> +				       <&pdma 19>;
can we also squash the dmas into one line? but, do split if there are too many..

yes, it's simply a style change that I'd like to keep them consistent at DT level,
besides you might also want to adjust dt-binding examples to align with them here..

thanks

> +				dma-names = "rx", "tx";
> +				status = "disabled";
> +			};
> +
>  			/* sec_uart1: 0xf0612000, not available from Linux */
>  		};
>  
> -- 
> 2.48.1
> 
> 

-- 
Yixun Lan (dlan)

Re: [PATCH v3 3/3] riscv: dts: spacemit: define a SPI controller node

Posted by Alex Elder 1 week, 1 day ago

On 9/22/25 7:19 PM, Yixun Lan wrote:
> Hi Alex,
> 
> On 11:17 Mon 22 Sep     , Alex Elder wrote:
>> Define a node for the fourth SoC SPI controller (number 3) on
>> the SpacemiT K1 SoC.
>>
>> Enable it on the Banana Pi BPI-F3 board, which exposes this feature
>> via its GPIO block:
>>    GPIO PIN 19:  MOSI
>>    GPIO PIN 21:  MISO
>>    GPIO PIN 23:  SCLK
>>    GPIO PIN 24:  SS (inverted)
>>
>> Define pincontrol configurations for the pins as used on that board.
>>
>> (This was tested using a GigaDevice GD25Q64E SPI NOR chip.)
>>
>> Signed-off-by: Alex Elder <elder@riscstar.com>
>> ---
>> v3: - Moved the SPI controller into the dma-bus memory region
>>
>>   .../boot/dts/spacemit/k1-bananapi-f3.dts      |  7 +++++++
>>   arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi  | 20 +++++++++++++++++++
>>   arch/riscv/boot/dts/spacemit/k1.dtsi          | 16 +++++++++++++++
>>   3 files changed, 43 insertions(+)
>>
>> diff --git a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
>> index 2aaaff77831e1..d9d865fbe320e 100644
>> --- a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
>> +++ b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
>> @@ -14,6 +14,7 @@ aliases {
>>   		ethernet0 = &eth0;
>>   		ethernet1 = &eth1;
>>   		serial0 = &uart0;
>> +		spi3 = &spi3;
>>   	};
>>   
>>   	chosen {
>> @@ -92,6 +93,12 @@ &pdma {
>>   	status = "okay";
>>   };
>>   
>> +&spi3 {
>> +	pinctrl-0 = <&ssp3_0_cfg>;
>> +	pinctrl-names = "default";
>> +	status = "okay";
>> +};
>> +
>>   &uart0 {
>>   	pinctrl-names = "default";
>>   	pinctrl-0 = <&uart0_2_cfg>;
>> diff --git a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
>> index aff19c86d5ff3..205c201a3005c 100644
>> --- a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
>> +++ b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
>> @@ -76,4 +76,24 @@ pwm14-1-pins {
>>   			drive-strength = <32>;
>>   		};
>>   	};
>> +
>> +	ssp3_0_cfg: ssp3-0-cfg {
> ..
>> +		ssp3-0-no-pull-pins {
> I'd prefer not to enforce "pull" info inside the name, you can't embed
> all property info, besides, what's if you want to change/override later?
> 
> how about just name it as ssp3-0-defaul-pins or simply ssp3-0-pins?

I should have looked more closely at this before I sent it out
for review; sorry about that.

Yes I agree with you, ssp-0-pins and ssp3-0-frm-pins would be
better.

> 
>> +			pinmux = <K1_PADCONF(75, 2)>,	/* SCLK */
>> +				 <K1_PADCONF(77, 2)>,	/* MOSI  */
>> +				 <K1_PADCONF(78, 2)>;	/* MISO */
>> +
>> +			bias-disable;
>> +			drive-strength = <19>;
>> +			power-source = <3300>;
>> +		};
>> +
>> +		ssp3-0-frm-pins {
>> +			pinmux = <K1_PADCONF(76, 2)>;	/* FRM (frame) */
>> +
>> +			bias-pull-up = <0>;
>> +			drive-strength = <19>;
>> +			power-source = <3300>;
>> +		};
>> +	};
>>   };
>> diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
>> index 6cdcd80a7c83b..eb8a14dd72ea4 100644
>> --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
>> +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
>> @@ -797,6 +797,22 @@ uart9: serial@d4017800 {
>>   				status = "disabled";
>>   			};
>>   
>> +			spi3: spi@d401c000 {
>> +				compatible = "spacemit,k1-spi";
>> +				reg = <0x0 0xd401c000 0x0 0x30>;
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +				clocks = <&syscon_apbc CLK_SSP3>,
>> +					 <&syscon_apbc CLK_SSP3_BUS>;
>> +				clock-names = "core", "bus";
>> +				resets = <&syscon_apbc RESET_SSP3>;
>> +				interrupts = <55>;
> ..
>> +				dmas = <&pdma 20>,
>> +				       <&pdma 19>;
> can we also squash the dmas into one line? but, do split if there are too many..
> 
> yes, it's simply a style change that I'd like to keep them consistent at DT level,
> besides you might also want to adjust dt-binding examples to align with them here..

Yes I'll do that.  Thanks for the review.

					-Alex

> thanks
> 
>> +				dma-names = "rx", "tx";
>> +				status = "disabled";
>> +			};
>> +
>>   			/* sec_uart1: 0xf0612000, not available from Linux */
>>   		};
>>   
>> -- 
>> 2.48.1
>>
>>
>

Re: [PATCH v3 3/3] riscv: dts: spacemit: define a SPI controller node

Posted by Troy Mitchell 1 week, 2 days ago

On Tue, Sep 23, 2025 at 08:19:30AM +0800, Yixun Lan wrote:
> Hi Alex,
> 
> On 11:17 Mon 22 Sep     , Alex Elder wrote:
> > Define a node for the fourth SoC SPI controller (number 3) on
> > the SpacemiT K1 SoC.
> > 
> > Enable it on the Banana Pi BPI-F3 board, which exposes this feature
> > via its GPIO block:
> >   GPIO PIN 19:  MOSI
> >   GPIO PIN 21:  MISO
> >   GPIO PIN 23:  SCLK
> >   GPIO PIN 24:  SS (inverted)
> > 
> > Define pincontrol configurations for the pins as used on that board.
> > 
> > (This was tested using a GigaDevice GD25Q64E SPI NOR chip.)
> > 
> > Signed-off-by: Alex Elder <elder@riscstar.com>
> > ---
> > v3: - Moved the SPI controller into the dma-bus memory region
> > 
> >  .../boot/dts/spacemit/k1-bananapi-f3.dts      |  7 +++++++
> >  arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi  | 20 +++++++++++++++++++
> >  arch/riscv/boot/dts/spacemit/k1.dtsi          | 16 +++++++++++++++
> >  3 files changed, 43 insertions(+)
> > 
> > diff --git a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> > index 2aaaff77831e1..d9d865fbe320e 100644
> > --- a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> > +++ b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> > @@ -14,6 +14,7 @@ aliases {
> >  		ethernet0 = &eth0;
> >  		ethernet1 = &eth1;
> >  		serial0 = &uart0;
> > +		spi3 = &spi3;
> >  	};
> >  
> >  	chosen {
> > @@ -92,6 +93,12 @@ &pdma {
> >  	status = "okay";
> >  };
> >  
> > +&spi3 {
> > +	pinctrl-0 = <&ssp3_0_cfg>;
> > +	pinctrl-names = "default";
> > +	status = "okay";
> > +};
> > +
> >  &uart0 {
> >  	pinctrl-names = "default";
> >  	pinctrl-0 = <&uart0_2_cfg>;
> > diff --git a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> > index aff19c86d5ff3..205c201a3005c 100644
> > --- a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> > +++ b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> > @@ -76,4 +76,24 @@ pwm14-1-pins {
> >  			drive-strength = <32>;
> >  		};
> >  	};
> > +
> > +	ssp3_0_cfg: ssp3-0-cfg {
> ..
> > +		ssp3-0-no-pull-pins {
> I'd prefer not to enforce "pull" info inside the name, you can't embed
> all property info, besides, what's if you want to change/override later?
> 
> how about just name it as ssp3-0-defaul-pins or simply ssp3-0-pins?
uart: uart0_2_cfg and function is 2.
pwm: pwm14_1_cfg and function is 4
spi: ssp3_0_cfg and function is 2

I’m a bit confused about the meaning of the second number here.
Is it intended to be an index, or the function number?

If it’s an index, should it start from 0 or 1?
The starting point seems inconsistent across pwm/spi/uart.
If it’s supposed to be the function number,
then the spi and pwm parts look incorrect.

Could you clarify this? Yixun.

                - Troy
> 
> > +			pinmux = <K1_PADCONF(75, 2)>,	/* SCLK */
> > +				 <K1_PADCONF(77, 2)>,	/* MOSI  */
> > +				 <K1_PADCONF(78, 2)>;	/* MISO */
> > +
> > +			bias-disable;
> > +			drive-strength = <19>;
> > +			power-source = <3300>;
> > +		};
> > +
> > +		ssp3-0-frm-pins {
> > +			pinmux = <K1_PADCONF(76, 2)>;	/* FRM (frame) */
> > +
> > +			bias-pull-up = <0>;
> > +			drive-strength = <19>;
> > +			power-source = <3300>;
> > +		};
> > +	};
> >  };
> > diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
> > index 6cdcd80a7c83b..eb8a14dd72ea4 100644
> > --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
> > +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
> > @@ -797,6 +797,22 @@ uart9: serial@d4017800 {
> >  				status = "disabled";
> >  			};
> >  
> > +			spi3: spi@d401c000 {
> > +				compatible = "spacemit,k1-spi";
> > +				reg = <0x0 0xd401c000 0x0 0x30>;
> > +				#address-cells = <1>;
> > +				#size-cells = <0>;
> > +				clocks = <&syscon_apbc CLK_SSP3>,
> > +					 <&syscon_apbc CLK_SSP3_BUS>;
> > +				clock-names = "core", "bus";
> > +				resets = <&syscon_apbc RESET_SSP3>;
> > +				interrupts = <55>;
> ..
> > +				dmas = <&pdma 20>,
> > +				       <&pdma 19>;
> can we also squash the dmas into one line? but, do split if there are too many..
> 
> yes, it's simply a style change that I'd like to keep them consistent at DT level,
> besides you might also want to adjust dt-binding examples to align with them here..
> 
> thanks
> 
> > +				dma-names = "rx", "tx";
> > +				status = "disabled";
> > +			};
> > +
> >  			/* sec_uart1: 0xf0612000, not available from Linux */
> >  		};
> >  
> > -- 
> > 2.48.1
> > 
> > 
> 
> -- 
> Yixun Lan (dlan)
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v3 3/3] riscv: dts: spacemit: define a SPI controller node

Posted by Alex Elder 1 week, 1 day ago

On 9/22/25 9:59 PM, Troy Mitchell wrote:
> On Tue, Sep 23, 2025 at 08:19:30AM +0800, Yixun Lan wrote:
>> Hi Alex,
>>
>> On 11:17 Mon 22 Sep     , Alex Elder wrote:
>>> Define a node for the fourth SoC SPI controller (number 3) on
>>> the SpacemiT K1 SoC.
>>>
>>> Enable it on the Banana Pi BPI-F3 board, which exposes this feature
>>> via its GPIO block:
>>>    GPIO PIN 19:  MOSI
>>>    GPIO PIN 21:  MISO
>>>    GPIO PIN 23:  SCLK
>>>    GPIO PIN 24:  SS (inverted)

Note that the pin numbers I'm mentioning above are the numbers
(1-26) on the 26-pin GPIO header on the BPI-F3 board.

>>>
>>> Define pincontrol configurations for the pins as used on that board.
>>>
>>> (This was tested using a GigaDevice GD25Q64E SPI NOR chip.)
>>>
>>> Signed-off-by: Alex Elder <elder@riscstar.com>
>>> ---
>>> v3: - Moved the SPI controller into the dma-bus memory region
>>>
>>>   .../boot/dts/spacemit/k1-bananapi-f3.dts      |  7 +++++++
>>>   arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi  | 20 +++++++++++++++++++
>>>   arch/riscv/boot/dts/spacemit/k1.dtsi          | 16 +++++++++++++++
>>>   3 files changed, 43 insertions(+)
>>>
>>> diff --git a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
>>> index 2aaaff77831e1..d9d865fbe320e 100644
>>> --- a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
>>> +++ b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
>>> @@ -14,6 +14,7 @@ aliases {
>>>   		ethernet0 = &eth0;
>>>   		ethernet1 = &eth1;
>>>   		serial0 = &uart0;
>>> +		spi3 = &spi3;
>>>   	};
>>>   
>>>   	chosen {
>>> @@ -92,6 +93,12 @@ &pdma {
>>>   	status = "okay";
>>>   };
>>>   
>>> +&spi3 {
>>> +	pinctrl-0 = <&ssp3_0_cfg>;
>>> +	pinctrl-names = "default";
>>> +	status = "okay";
>>> +};
>>> +
>>>   &uart0 {
>>>   	pinctrl-names = "default";
>>>   	pinctrl-0 = <&uart0_2_cfg>;
>>> diff --git a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
>>> index aff19c86d5ff3..205c201a3005c 100644
>>> --- a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
>>> +++ b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
>>> @@ -76,4 +76,24 @@ pwm14-1-pins {
>>>   			drive-strength = <32>;
>>>   		};
>>>   	};
>>> +
>>> +	ssp3_0_cfg: ssp3-0-cfg {
>> ..
>>> +		ssp3-0-no-pull-pins {
>> I'd prefer not to enforce "pull" info inside the name, you can't embed
>> all property info, besides, what's if you want to change/override later?
>>
>> how about just name it as ssp3-0-defaul-pins or simply ssp3-0-pins?
> uart: uart0_2_cfg and function is 2.
> pwm: pwm14_1_cfg and function is 4
> spi: ssp3_0_cfg and function is 2
> 
> I’m a bit confused about the meaning of the second number here.
> Is it intended to be an index, or the function number?

It is an index, and it seems arbitrary but it is based on the
order in which they occur in a spreadsheet that defines a set
of possible pin configurations.

For example, SPI3 lists 2 possible pin combinations:
SCLK	GPIO[75] function 2	GPIO[59] function 2
FRM	GPIO[76] function 2	GPIO[60] function 2
TXD	GPIO[77] function 2	GPIO[61] function 2
RXD	GPIO[78] function 2	GPIO[62] function 2

> If it’s an index, should it start from 0 or 1?

It starts with 0.

> The starting point seems inconsistent across pwm/spi/uart.
> If it’s supposed to be the function number,
> then the spi and pwm parts look incorrect.

The first one (index 0) shows up earlier (lower line number) in
the spreadsheet, even though the GPIO numbers used are higher
than those in the second one.  They're grouped, and the first
one is in GPIO group 2 and the second is in GPIO group 5.

					-Alex

> Could you clarify this? Yixun.
> 
>                  - Troy
>>
>>> +			pinmux = <K1_PADCONF(75, 2)>,	/* SCLK */
>>> +				 <K1_PADCONF(77, 2)>,	/* MOSI  */
>>> +				 <K1_PADCONF(78, 2)>;	/* MISO */
>>> +
>>> +			bias-disable;
>>> +			drive-strength = <19>;
>>> +			power-source = <3300>;
>>> +		};
>>> +
>>> +		ssp3-0-frm-pins {
>>> +			pinmux = <K1_PADCONF(76, 2)>;	/* FRM (frame) */
>>> +
>>> +			bias-pull-up = <0>;
>>> +			drive-strength = <19>;
>>> +			power-source = <3300>;
>>> +		};
>>> +	};
>>>   };
>>> diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
>>> index 6cdcd80a7c83b..eb8a14dd72ea4 100644
>>> --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
>>> +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
>>> @@ -797,6 +797,22 @@ uart9: serial@d4017800 {
>>>   				status = "disabled";
>>>   			};
>>>   
>>> +			spi3: spi@d401c000 {
>>> +				compatible = "spacemit,k1-spi";
>>> +				reg = <0x0 0xd401c000 0x0 0x30>;
>>> +				#address-cells = <1>;
>>> +				#size-cells = <0>;
>>> +				clocks = <&syscon_apbc CLK_SSP3>,
>>> +					 <&syscon_apbc CLK_SSP3_BUS>;
>>> +				clock-names = "core", "bus";
>>> +				resets = <&syscon_apbc RESET_SSP3>;
>>> +				interrupts = <55>;
>> ..
>>> +				dmas = <&pdma 20>,
>>> +				       <&pdma 19>;
>> can we also squash the dmas into one line? but, do split if there are too many..
>>
>> yes, it's simply a style change that I'd like to keep them consistent at DT level,
>> besides you might also want to adjust dt-binding examples to align with them here..
>>
>> thanks
>>
>>> +				dma-names = "rx", "tx";
>>> +				status = "disabled";
>>> +			};
>>> +
>>>   			/* sec_uart1: 0xf0612000, not available from Linux */
>>>   		};
>>>   
>>> -- 
>>> 2.48.1
>>>
>>>
>>
>> -- 
>> Yixun Lan (dlan)
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v3 3/3] riscv: dts: spacemit: define a SPI controller node

Posted by Troy Mitchell 1 week, 1 day ago

On Tue, Sep 23, 2025 at 07:49:34AM -0500, Alex Elder wrote:
> On 9/22/25 9:59 PM, Troy Mitchell wrote:
> > On Tue, Sep 23, 2025 at 08:19:30AM +0800, Yixun Lan wrote:
> > > Hi Alex,
> > > 
> > > On 11:17 Mon 22 Sep     , Alex Elder wrote:
> > > > Define a node for the fourth SoC SPI controller (number 3) on
> > > > the SpacemiT K1 SoC.
> > > > 
> > > > Enable it on the Banana Pi BPI-F3 board, which exposes this feature
> > > > via its GPIO block:
> > > >    GPIO PIN 19:  MOSI
> > > >    GPIO PIN 21:  MISO
> > > >    GPIO PIN 23:  SCLK
> > > >    GPIO PIN 24:  SS (inverted)
> 
> Note that the pin numbers I'm mentioning above are the numbers
> (1-26) on the 26-pin GPIO header on the BPI-F3 board.
> 
> > > > 
> > > > Define pincontrol configurations for the pins as used on that board.
> > > > 
> > > > (This was tested using a GigaDevice GD25Q64E SPI NOR chip.)
> > > > 
> > > > Signed-off-by: Alex Elder <elder@riscstar.com>
> > > > ---
> > > > v3: - Moved the SPI controller into the dma-bus memory region
> > > > 
> > > >   .../boot/dts/spacemit/k1-bananapi-f3.dts      |  7 +++++++
> > > >   arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi  | 20 +++++++++++++++++++
> > > >   arch/riscv/boot/dts/spacemit/k1.dtsi          | 16 +++++++++++++++
> > > >   3 files changed, 43 insertions(+)
> > > > 
> > > > diff --git a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> > > > index 2aaaff77831e1..d9d865fbe320e 100644
> > > > --- a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> > > > +++ b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> > > > @@ -14,6 +14,7 @@ aliases {
> > > >   		ethernet0 = &eth0;
> > > >   		ethernet1 = &eth1;
> > > >   		serial0 = &uart0;
> > > > +		spi3 = &spi3;
> > > >   	};
> > > >   	chosen {
> > > > @@ -92,6 +93,12 @@ &pdma {
> > > >   	status = "okay";
> > > >   };
> > > > +&spi3 {
> > > > +	pinctrl-0 = <&ssp3_0_cfg>;
> > > > +	pinctrl-names = "default";
> > > > +	status = "okay";
> > > > +};
> > > > +
> > > >   &uart0 {
> > > >   	pinctrl-names = "default";
> > > >   	pinctrl-0 = <&uart0_2_cfg>;
> > > > diff --git a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> > > > index aff19c86d5ff3..205c201a3005c 100644
> > > > --- a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> > > > +++ b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> > > > @@ -76,4 +76,24 @@ pwm14-1-pins {
> > > >   			drive-strength = <32>;
> > > >   		};
> > > >   	};
> > > > +
> > > > +	ssp3_0_cfg: ssp3-0-cfg {
> > > ..
> > > > +		ssp3-0-no-pull-pins {
> > > I'd prefer not to enforce "pull" info inside the name, you can't embed
> > > all property info, besides, what's if you want to change/override later?
> > > 
> > > how about just name it as ssp3-0-defaul-pins or simply ssp3-0-pins?
> > uart: uart0_2_cfg and function is 2.
> > pwm: pwm14_1_cfg and function is 4
> > spi: ssp3_0_cfg and function is 2
> > 
> > I’m a bit confused about the meaning of the second number here.
> > Is it intended to be an index, or the function number?
> 
> It is an index, and it seems arbitrary but it is based on the
> order in which they occur in a spreadsheet that defines a set
> of possible pin configurations.
> 
> For example, SPI3 lists 2 possible pin combinations:
> SCLK	GPIO[75] function 2	GPIO[59] function 2
> FRM	GPIO[76] function 2	GPIO[60] function 2
> TXD	GPIO[77] function 2	GPIO[61] function 2
> RXD	GPIO[78] function 2	GPIO[62] function 2
> 
> > If it’s an index, should it start from 0 or 1?
> 
> It starts with 0.
> 
> > The starting point seems inconsistent across pwm/spi/uart.
> > If it’s supposed to be the function number,
> > then the spi and pwm parts look incorrect.
> 
> The first one (index 0) shows up earlier (lower line number) in
> the spreadsheet, even though the GPIO numbers used are higher
> than those in the second one.  They're grouped, and the first
> one is in GPIO group 2 and the second is in GPIO group 5.
Thanks! It makes sense.

                - Troy

> 
> 					-Alex
> 
> > Could you clarify this? Yixun.
> > 
> >                  - Troy
> > > 
> > > > +			pinmux = <K1_PADCONF(75, 2)>,	/* SCLK */
> > > > +				 <K1_PADCONF(77, 2)>,	/* MOSI  */
> > > > +				 <K1_PADCONF(78, 2)>;	/* MISO */
> > > > +
> > > > +			bias-disable;
> > > > +			drive-strength = <19>;
> > > > +			power-source = <3300>;
> > > > +		};
> > > > +
> > > > +		ssp3-0-frm-pins {
> > > > +			pinmux = <K1_PADCONF(76, 2)>;	/* FRM (frame) */
> > > > +
> > > > +			bias-pull-up = <0>;
> > > > +			drive-strength = <19>;
> > > > +			power-source = <3300>;
> > > > +		};
> > > > +	};
> > > >   };
> > > > diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
> > > > index 6cdcd80a7c83b..eb8a14dd72ea4 100644
> > > > --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
> > > > +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
> > > > @@ -797,6 +797,22 @@ uart9: serial@d4017800 {
> > > >   				status = "disabled";
> > > >   			};
> > > > +			spi3: spi@d401c000 {
> > > > +				compatible = "spacemit,k1-spi";
> > > > +				reg = <0x0 0xd401c000 0x0 0x30>;
> > > > +				#address-cells = <1>;
> > > > +				#size-cells = <0>;
> > > > +				clocks = <&syscon_apbc CLK_SSP3>,
> > > > +					 <&syscon_apbc CLK_SSP3_BUS>;
> > > > +				clock-names = "core", "bus";
> > > > +				resets = <&syscon_apbc RESET_SSP3>;
> > > > +				interrupts = <55>;
> > > ..
> > > > +				dmas = <&pdma 20>,
> > > > +				       <&pdma 19>;
> > > can we also squash the dmas into one line? but, do split if there are too many..
> > > 
> > > yes, it's simply a style change that I'd like to keep them consistent at DT level,
> > > besides you might also want to adjust dt-binding examples to align with them here..
> > > 
> > > thanks
> > > 
> > > > +				dma-names = "rx", "tx";
> > > > +				status = "disabled";
> > > > +			};
> > > > +
> > > >   			/* sec_uart1: 0xf0612000, not available from Linux */
> > > >   		};
> > > > -- 
> > > > 2.48.1
> > > > 
> > > > 
> > > 
> > > -- 
> > > Yixun Lan (dlan)
> > > 
> > > _______________________________________________
> > > linux-riscv mailing list
> > > linux-riscv@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> 
>

Re: [PATCH v3 3/3] riscv: dts: spacemit: define a SPI controller node

Posted by Junhui Liu 1 week, 1 day ago

Hi Troy,

On 9/23/25 10:59 AM, Troy Mitchell wrote:
> On Tue, Sep 23, 2025 at 08:19:30AM +0800, Yixun Lan wrote:
>> Hi Alex,
>>
>> On 11:17 Mon 22 Sep     , Alex Elder wrote:
>>> Define a node for the fourth SoC SPI controller (number 3) on
>>> the SpacemiT K1 SoC.
>>>
>>> Enable it on the Banana Pi BPI-F3 board, which exposes this feature
>>> via its GPIO block:
>>>    GPIO PIN 19:  MOSI
>>>    GPIO PIN 21:  MISO
>>>    GPIO PIN 23:  SCLK
>>>    GPIO PIN 24:  SS (inverted)
>>>
>>> Define pincontrol configurations for the pins as used on that board.
>>>
>>> (This was tested using a GigaDevice GD25Q64E SPI NOR chip.)
>>>
>>> Signed-off-by: Alex Elder <elder@riscstar.com>
>>> ---
>>> v3: - Moved the SPI controller into the dma-bus memory region
>>>
>>>   .../boot/dts/spacemit/k1-bananapi-f3.dts      |  7 +++++++
>>>   arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi  | 20 +++++++++++++++++++
>>>   arch/riscv/boot/dts/spacemit/k1.dtsi          | 16 +++++++++++++++
>>>   3 files changed, 43 insertions(+)
>>>
>>> diff --git a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
>>> index 2aaaff77831e1..d9d865fbe320e 100644
>>> --- a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
>>> +++ b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
>>> @@ -14,6 +14,7 @@ aliases {
>>>   		ethernet0 = &eth0;
>>>   		ethernet1 = &eth1;
>>>   		serial0 = &uart0;
>>> +		spi3 = &spi3;
>>>   	};
>>>   
>>>   	chosen {
>>> @@ -92,6 +93,12 @@ &pdma {
>>>   	status = "okay";
>>>   };
>>>   
>>> +&spi3 {
>>> +	pinctrl-0 = <&ssp3_0_cfg>;
>>> +	pinctrl-names = "default";
>>> +	status = "okay";
>>> +};
>>> +
>>>   &uart0 {
>>>   	pinctrl-names = "default";
>>>   	pinctrl-0 = <&uart0_2_cfg>;
>>> diff --git a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
>>> index aff19c86d5ff3..205c201a3005c 100644
>>> --- a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
>>> +++ b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
>>> @@ -76,4 +76,24 @@ pwm14-1-pins {
>>>   			drive-strength = <32>;
>>>   		};
>>>   	};
>>> +
>>> +	ssp3_0_cfg: ssp3-0-cfg {
>> ..
>>> +		ssp3-0-no-pull-pins {
>> I'd prefer not to enforce "pull" info inside the name, you can't embed
>> all property info, besides, what's if you want to change/override later?
>>
>> how about just name it as ssp3-0-defaul-pins or simply ssp3-0-pins?
> uart: uart0_2_cfg and function is 2.
> pwm: pwm14_1_cfg and function is 4
> spi: ssp3_0_cfg and function is 2
>
> I’m a bit confused about the meaning of the second number here.
> Is it intended to be an index, or the function number?
>
> If it’s an index, should it start from 0 or 1?
> The starting point seems inconsistent across pwm/spi/uart.
> If it’s supposed to be the function number,
> then the spi and pwm parts look incorrect.
>
> Could you clarify this? Yixun.

I think the second number represents the index of the pin group available
for this device.

Take pwm14 as an example: according to the manual, the first pin group
(index 0) available for pwm14 is GPIO6 with function 3, while the second
group (index 1) is GPIO44 with function 4.

>                  - Troy

-- 
Best regards,
Junhui Liu

Re: [PATCH v3 3/3] riscv: dts: spacemit: define a SPI controller node

Posted by Troy Mitchell 1 week, 1 day ago

On Tue, Sep 23, 2025 at 02:31:31PM +0800, Junhui Liu wrote:
> Hi Troy,
> 
> On 9/23/25 10:59 AM, Troy Mitchell wrote:
> > On Tue, Sep 23, 2025 at 08:19:30AM +0800, Yixun Lan wrote:
> > > Hi Alex,
> > > 
> > > On 11:17 Mon 22 Sep     , Alex Elder wrote:
> > > > Define a node for the fourth SoC SPI controller (number 3) on
> > > > the SpacemiT K1 SoC.
> > > > 
> > > > Enable it on the Banana Pi BPI-F3 board, which exposes this feature
> > > > via its GPIO block:
> > > >    GPIO PIN 19:  MOSI
> > > >    GPIO PIN 21:  MISO
> > > >    GPIO PIN 23:  SCLK
> > > >    GPIO PIN 24:  SS (inverted)
> > > > 
> > > > Define pincontrol configurations for the pins as used on that board.
> > > > 
> > > > (This was tested using a GigaDevice GD25Q64E SPI NOR chip.)
> > > > 
> > > > Signed-off-by: Alex Elder <elder@riscstar.com>
> > > > ---
> > > > v3: - Moved the SPI controller into the dma-bus memory region
> > > > 
> > > >   .../boot/dts/spacemit/k1-bananapi-f3.dts      |  7 +++++++
> > > >   arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi  | 20 +++++++++++++++++++
> > > >   arch/riscv/boot/dts/spacemit/k1.dtsi          | 16 +++++++++++++++
> > > >   3 files changed, 43 insertions(+)
> > > > 
> > > > diff --git a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> > > > index 2aaaff77831e1..d9d865fbe320e 100644
> > > > --- a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> > > > +++ b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> > > > @@ -14,6 +14,7 @@ aliases {
> > > >   		ethernet0 = &eth0;
> > > >   		ethernet1 = &eth1;
> > > >   		serial0 = &uart0;
> > > > +		spi3 = &spi3;
> > > >   	};
> > > >   	chosen {
> > > > @@ -92,6 +93,12 @@ &pdma {
> > > >   	status = "okay";
> > > >   };
> > > > +&spi3 {
> > > > +	pinctrl-0 = <&ssp3_0_cfg>;
> > > > +	pinctrl-names = "default";
> > > > +	status = "okay";
> > > > +};
> > > > +
> > > >   &uart0 {
> > > >   	pinctrl-names = "default";
> > > >   	pinctrl-0 = <&uart0_2_cfg>;
> > > > diff --git a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> > > > index aff19c86d5ff3..205c201a3005c 100644
> > > > --- a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> > > > +++ b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> > > > @@ -76,4 +76,24 @@ pwm14-1-pins {
> > > >   			drive-strength = <32>;
> > > >   		};
> > > >   	};
> > > > +
> > > > +	ssp3_0_cfg: ssp3-0-cfg {
> > > ..
> > > > +		ssp3-0-no-pull-pins {
> > > I'd prefer not to enforce "pull" info inside the name, you can't embed
> > > all property info, besides, what's if you want to change/override later?
> > > 
> > > how about just name it as ssp3-0-defaul-pins or simply ssp3-0-pins?
> > uart: uart0_2_cfg and function is 2.
> > pwm: pwm14_1_cfg and function is 4
> > spi: ssp3_0_cfg and function is 2
> > 
> > I’m a bit confused about the meaning of the second number here.
> > Is it intended to be an index, or the function number?
> > 
> > If it’s an index, should it start from 0 or 1?
> > The starting point seems inconsistent across pwm/spi/uart.
> > If it’s supposed to be the function number,
> > then the spi and pwm parts look incorrect.
> > 
> > Could you clarify this? Yixun.
> 
> I think the second number represents the index of the pin group available
> for this device.
> 
> Take pwm14 as an example: according to the manual, the first pin group
> (index 0) available for pwm14 is GPIO6 with function 3, while the second
> group (index 1) is GPIO44 with function 4.
Thanks. Junhui.
with the hints from you and Alex, it’s now much clearer to me.

                    - Troy
> 
> >                  - Troy
> 
> -- 
> Best regards,
> Junhui Liu
> 
>

[PATCH v3 1/3] dt-bindings: spi: add SpacemiT K1 SPI support
[PATCH v3 2/3] spi: spacemit: introduce SpacemiT K1 SPI controller driver
[PATCH v3 3/3] riscv: dts: spacemit: define a SPI controller node