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

Open Source and information security mailing list archives

Message-ID: <1aa28123-cfa4-415a-9d1b-4d9edd62489b@riscstar.com>
Date: Tue, 23 Sep 2025 07:49:34 -0500
From: Alex Elder <elder@...cstar.com>
To: Troy Mitchell <troy.mitchell@...ux.spacemit.com>,
 Yixun Lan <dlan@...too.org>
Cc: broonie@...nel.org, robh@...nel.org, krzk+dt@...nel.org,
 conor+dt@...nel.org, ziyao@...root.org, linux-spi@...r.kernel.org,
 devicetree@...r.kernel.org, paul.walmsley@...ive.com, palmer@...belt.com,
 aou@...s.berkeley.edu, alex@...ti.fr, p.zabel@...gutronix.de,
 spacemit@...ts.linux.dev, linux-riscv@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/3] riscv: dts: spacemit: define a SPI controller node

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@...cstar.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@...17800 {
>>>   				status = "disabled";
>>>   			};
>>>   
>>> +			spi3: spi@...1c000 {
>>> +				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@...ts.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives