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

[Yixun Lan <dlan@gentoo.org>]

Hi Alex,

On 17:07 Wed 17 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>
> ---
>  .../boot/dts/spacemit/k1-bananapi-f3.dts      |  6 ++++++
>  arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi  | 20 +++++++++++++++++++
>  arch/riscv/boot/dts/spacemit/k1.dtsi          | 19 ++++++++++++++++++
>  3 files changed, 45 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> index 6013be2585428..380d475d2f3f3 100644
> --- a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> +++ b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> @@ -44,6 +44,12 @@ &pdma {
>  	status = "okay";
>  };
>  
> +&spi3 {
..
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&ssp3_0_cfg>;
Can you swap the order of these two pinctrl properties?
Yes, we currently have some inconsistency in tree, I plan to fix during next cycle

> +	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 3810557374228..16c953eca2aaa 100644
> --- a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> +++ b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> @@ -28,4 +28,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 66b33a9110ccd..a826cc1ac83d5 100644
> --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
> +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
> @@ -834,6 +834,25 @@ storage-bus {
>  			#size-cells = <2>;
>  			dma-ranges = <0x0 0x00000000 0x0 0x00000000 0x0 0x80000000>;
>  
> +			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";
can you simply put them together in one line? it's kind of tedious to split..
> +				resets = <&syscon_apbc RESET_SSP3>;
> +				interrupts-extended = <&plic 55>;
why use interrupts-extended?
> +				spacemit,k1-ssp-id = <3>;
> +				dmas = <&pdma 20>,
> +				       <&pdma 19>;
.. em, so the SPI will use pdma, then probably you should also adjust Kconfig to
select PDMA driver?

> +				dma-names = "rx",
> +					    "tx";
> +				status = "disabled";
> +			};
> +
>  			emmc: mmc@d4281000 {
>  				compatible = "spacemit,k1-sdhci";
>  				reg = <0x0 0xd4281000 0x0 0x200>;
> -- 
> 2.48.1
> 
> 

-- 
Yixun Lan (dlan)