* Re: [PATCH v2] riscv: sophgo: dts: Add spi controller for SG2042
2025-02-28 12:40 [PATCH v2] riscv: sophgo: dts: Add spi controller for SG2042 Zixian Zeng
@ 2025-02-28 18:22 ` Conor Dooley
2025-03-01 1:58 ` Chen Wang
2025-03-01 13:41 ` damon
2025-03-02 2:18 ` Inochi Amaoto
2025-03-04 3:06 ` Chen Wang
2 siblings, 2 replies; 9+ messages in thread
From: Conor Dooley @ 2025-02-28 18:22 UTC (permalink / raw)
To: Zixian Zeng
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Chen Wang, Inochi Amaoto,
Alexandre Ghiti, devicetree, linux-riscv, linux-kernel, sophgo,
chao.wei, xiaoguang.xing
[-- Attachment #1: Type: text/plain, Size: 2196 bytes --]
On Fri, Feb 28, 2025 at 08:40:23PM +0800, Zixian Zeng wrote:
> Add spi controllers for SG2042.
>
> SG2042 uses the upstreamed Synopsys DW SPI IP.
>
> Signed-off-by: Zixian Zeng <sycamoremoon376@gmail.com>
> ---
> For this spi controller patch, only bindings are included.
^^^ you've not actually included any bindings in this patch, copy-paste
mistake?
> This is tested on milkv-pioneer board. Using driver/spi/spidev.c
> for creating /dev/spidevX.Y and tools/spi/spidev_test for testing
> functionality.
> ---
> Changes in v2:
> - rebase v1 to sophgo/master(github.com/sophgo/linux.git).
> - order properties in device node.
> - remove unevaluated properties `clock-frequency`.
> - set default status to disable.
> - Link to v1: https://lore.kernel.org/r/20250228-sfg-spi-v1-1-b989aed94911@gmail.com
> ---
> .../riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts | 8 +++++++
> arch/riscv/boot/dts/sophgo/sg2042.dtsi | 28 ++++++++++++++++++++++
> 2 files changed, 36 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts b/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
> index be596d01ff8d33bcdbe431d9731a55ee190ad5b3..c43a807af2f827b5267afe5e4fdf6e9e857dfa20 100644
> --- a/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
> +++ b/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
> @@ -72,6 +72,14 @@ &uart0 {
> status = "okay";
> };
>
> +&spi0 {
> + status = "okay";
> +};
> +
> +&spi1 {
> + status = "okay";
> +};
> +
> / {
> thermal-zones {
> soc-thermal {
> diff --git a/arch/riscv/boot/dts/sophgo/sg2042.dtsi b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> index e62ac51ac55abd922b5ef796ba8c2196383850c4..500645147b1f8ed0a08ad3cafb38ea79cf57d737 100644
> --- a/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> +++ b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> @@ -545,5 +545,33 @@ sd: mmc@704002b000 {
> "timer";
> status = "disabled";
> };
> +
> + spi0: spi@7040004000 {
> + compatible = "snps,dw-apb-ssi";
I thought were were dropping the use of "snps,dw-abp-ssi" in isolation,
and starting to require soc-specific compatibles now.
Rob, Krzysztof?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] riscv: sophgo: dts: Add spi controller for SG2042
2025-02-28 18:22 ` Conor Dooley
@ 2025-03-01 1:58 ` Chen Wang
2025-03-03 9:08 ` Conor Dooley
2025-03-01 13:41 ` damon
1 sibling, 1 reply; 9+ messages in thread
From: Chen Wang @ 2025-03-01 1:58 UTC (permalink / raw)
To: Conor Dooley, Zixian Zeng
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Inochi Amaoto, Alexandre Ghiti,
devicetree, linux-riscv, linux-kernel, sophgo, chao.wei,
xiaoguang.xing
On 2025/3/1 2:22, Conor Dooley wrote:
[......]
>> +
>> + spi0: spi@7040004000 {
>> + compatible = "snps,dw-apb-ssi";
> I thought were were dropping the use of "snps,dw-abp-ssi" in isolation,
> and starting to require soc-specific compatibles now.
>
> Rob, Krzysztof?
I'm also very interested to know why we can't just use
"snps,dw-abp-ssi", maybe I missed some discussion ...
I googled examples of soc-specific defined in the code, and it doesn't
seem to be much, only arch/mips/boot/dts/mscc/ocelot.dtsi and
arch/riscv/boot/dts/thead/th1520.dtsi.
Specially, I looked at the commits for th1520 and saw this
https://lore.kernel.org/linux-riscv/20240703-garbage-explicit-bd95f8deb716@wendy/.
It tells if the fallback works identically, then the specific compatible
is not needed.
This makes me more confused :)
Regards,
Chen
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] riscv: sophgo: dts: Add spi controller for SG2042
2025-03-01 1:58 ` Chen Wang
@ 2025-03-03 9:08 ` Conor Dooley
0 siblings, 0 replies; 9+ messages in thread
From: Conor Dooley @ 2025-03-03 9:08 UTC (permalink / raw)
To: Chen Wang
Cc: Conor Dooley, Zixian Zeng, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Inochi Amaoto, Alexandre Ghiti, devicetree, linux-riscv,
linux-kernel, sophgo, chao.wei, xiaoguang.xing
[-- Attachment #1: Type: text/plain, Size: 1466 bytes --]
On Sat, Mar 01, 2025 at 09:58:24AM +0800, Chen Wang wrote:
>
> On 2025/3/1 2:22, Conor Dooley wrote:
> [......]
> > > +
> > > + spi0: spi@7040004000 {
> > > + compatible = "snps,dw-apb-ssi";
> > I thought were were dropping the use of "snps,dw-abp-ssi" in isolation,
> > and starting to require soc-specific compatibles now.
> >
> > Rob, Krzysztof?
> I'm also very interested to know why we can't just use "snps,dw-abp-ssi",
> maybe I missed some discussion ...
Ordinarily you're not allowed to use generic compatibles at all, the
dw stuff has kinda been an ?unofficial? exception for a while and not
just for spi. I think that "exception" has been withdrawn though, and
dw stuff is being brought in line with everything else, but cannot
remember where that started.
> I googled examples of soc-specific defined in the code, and it doesn't seem
> to be much, only arch/mips/boot/dts/mscc/ocelot.dtsi and
> arch/riscv/boot/dts/thead/th1520.dtsi.
> Specially, I looked at the commits for th1520 and saw this https://lore.kernel.org/linux-riscv/20240703-garbage-explicit-bd95f8deb716@wendy/.
> It tells if the fallback works identically, then the specific compatible is
> not needed.
This link is talking about the driver, not the dts/binding. Adding the
device-specific compatible to the driver is only needed if the specific
one has some extra features above and beyond the compatible used as the
fallback.
Cheers,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] riscv: sophgo: dts: Add spi controller for SG2042
2025-02-28 18:22 ` Conor Dooley
2025-03-01 1:58 ` Chen Wang
@ 2025-03-01 13:41 ` damon
2025-03-03 9:12 ` Conor Dooley
1 sibling, 1 reply; 9+ messages in thread
From: damon @ 2025-03-01 13:41 UTC (permalink / raw)
To: Conor Dooley
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Chen Wang, Inochi Amaoto,
Alexandre Ghiti, devicetree, linux-riscv, linux-kernel, sophgo,
chao.wei, xiaoguang.xing
On 25/02/28 06:22PM, Conor Dooley wrote:
> On Fri, Feb 28, 2025 at 08:40:23PM +0800, Zixian Zeng wrote:
> > Add spi controllers for SG2042.
> >
> > SG2042 uses the upstreamed Synopsys DW SPI IP.
> >
> > Signed-off-by: Zixian Zeng <sycamoremoon376@gmail.com>
> > ---
> > For this spi controller patch, only bindings are included.
>
> ^^^ you've not actually included any bindings in this patch, copy-paste
> mistake?
>
My bad, I was confused about what’s the “binding” meaning. I will
remove this description in next revision.
> > This is tested on milkv-pioneer board. Using driver/spi/spidev.c
> > for creating /dev/spidevX.Y and tools/spi/spidev_test for testing
> > functionality.
> > ---
> > Changes in v2:
> > - rebase v1 to sophgo/master(github.com/sophgo/linux.git).
> > - order properties in device node.
> > - remove unevaluated properties `clock-frequency`.
> > - set default status to disable.
> > - Link to v1: https://lore.kernel.org/r/20250228-sfg-spi-v1-1-b989aed94911@gmail.com
> > ---
> > .../riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts | 8 +++++++
> > arch/riscv/boot/dts/sophgo/sg2042.dtsi | 28 ++++++++++++++++++++++
> > 2 files changed, 36 insertions(+)
> >
> > diff --git a/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts b/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
> > index be596d01ff8d33bcdbe431d9731a55ee190ad5b3..c43a807af2f827b5267afe5e4fdf6e9e857dfa20 100644
> > --- a/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
> > +++ b/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
> > @@ -72,6 +72,14 @@ &uart0 {
> > status = "okay";
> > };
> >
> > +&spi0 {
> > + status = "okay";
> > +};
> > +
> > +&spi1 {
> > + status = "okay";
> > +};
> > +
> > / {
> > thermal-zones {
> > soc-thermal {
> > diff --git a/arch/riscv/boot/dts/sophgo/sg2042.dtsi b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> > index e62ac51ac55abd922b5ef796ba8c2196383850c4..500645147b1f8ed0a08ad3cafb38ea79cf57d737 100644
> > --- a/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> > +++ b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> > @@ -545,5 +545,33 @@ sd: mmc@704002b000 {
> > "timer";
> > status = "disabled";
> > };
> > +
> > + spi0: spi@7040004000 {
> > + compatible = "snps,dw-apb-ssi";
>
> I thought were were dropping the use of "snps,dw-abp-ssi" in isolation,
> and starting to require soc-specific compatibles now.
>
That’s better, I will do it in next revision.
BTW, <https://lore.kernel.org/linux-riscv/20240703-garbage-explicit-bd95f8deb716@wendy>
has similar situation,
those patches only create binding but not implement the compatible
property of "thead,th1520-spi” in driver.
When kernel matching, it will find out there’s no compatible of
"thead,th1520-spi” so fallback to "snps,dw-apb-ssi”
May I take you a few minutes for reason behind it? My understanding is
that using soc-specific compatible is better
for future maintaining, If the device actually comes with some unknown
quirks not present in the generic IP.
And all we need to do for fixing is implementing the soc-specific
compatible property.
> Rob, Krzysztof?
>
Best regards,
Zixian Zeng
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] riscv: sophgo: dts: Add spi controller for SG2042
2025-03-01 13:41 ` damon
@ 2025-03-03 9:12 ` Conor Dooley
0 siblings, 0 replies; 9+ messages in thread
From: Conor Dooley @ 2025-03-03 9:12 UTC (permalink / raw)
To: damon
Cc: Conor Dooley, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Chen Wang,
Inochi Amaoto, Alexandre Ghiti, devicetree, linux-riscv,
linux-kernel, sophgo, chao.wei, xiaoguang.xing
[-- Attachment #1: Type: text/plain, Size: 2081 bytes --]
On Sat, Mar 01, 2025 at 09:41:14PM +0800, damon wrote:
> On 25/02/28 06:22PM, Conor Dooley wrote:
> > On Fri, Feb 28, 2025 at 08:40:23PM +0800, Zixian Zeng wrote:
> > > diff --git a/arch/riscv/boot/dts/sophgo/sg2042.dtsi b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> > > index e62ac51ac55abd922b5ef796ba8c2196383850c4..500645147b1f8ed0a08ad3cafb38ea79cf57d737 100644
> > > --- a/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> > > +++ b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> > > @@ -545,5 +545,33 @@ sd: mmc@704002b000 {
> > > "timer";
> > > status = "disabled";
> > > };
> > > +
> > > + spi0: spi@7040004000 {
> > > + compatible = "snps,dw-apb-ssi";
> >
> > I thought were were dropping the use of "snps,dw-abp-ssi" in isolation,
> > and starting to require soc-specific compatibles now.
> >
> That’s better, I will do it in next revision.
> BTW, <https://lore.kernel.org/linux-riscv/20240703-garbage-explicit-bd95f8deb716@wendy>
> has similar situation,
> those patches only create binding but not implement the compatible
> property of "thead,th1520-spi” in driver.
> When kernel matching, it will find out there’s no compatible of
> "thead,th1520-spi” so fallback to "snps,dw-apb-ssi”
> May I take you a few minutes for reason behind it? My understanding is
> that using soc-specific compatible is better
> for future maintaining, If the device actually comes with some unknown
> quirks not present in the generic IP.
Yup, that's pretty much it. Those quirks could be some sort of bug etc,
or they could be an extra feature. Sometimes the specific compatible is
only used to enforce constraints on what properties are allowed, but
that's more of a thing for the more configurable IPs like the dwmac.
> And all we need to do for fixing is implementing the soc-specific
> compatible property.
Correct. You just need to add a specific compatible to the binding and
to the dts, the driver will work just fine with the "fallback" dw one.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] riscv: sophgo: dts: Add spi controller for SG2042
2025-02-28 12:40 [PATCH v2] riscv: sophgo: dts: Add spi controller for SG2042 Zixian Zeng
2025-02-28 18:22 ` Conor Dooley
@ 2025-03-02 2:18 ` Inochi Amaoto
2025-03-04 6:36 ` Zixian Zeng
2025-03-04 3:06 ` Chen Wang
2 siblings, 1 reply; 9+ messages in thread
From: Inochi Amaoto @ 2025-03-02 2:18 UTC (permalink / raw)
To: Zixian Zeng, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Chen Wang,
Inochi Amaoto, Alexandre Ghiti
Cc: devicetree, linux-riscv, linux-kernel, sophgo, chao.wei,
xiaoguang.xing
On Fri, Feb 28, 2025 at 08:40:23PM +0800, Zixian Zeng wrote:
> Add spi controllers for SG2042.
>
> SG2042 uses the upstreamed Synopsys DW SPI IP.
>
> Signed-off-by: Zixian Zeng <sycamoremoon376@gmail.com>
> ---
> For this spi controller patch, only bindings are included.
> This is tested on milkv-pioneer board. Using driver/spi/spidev.c
> for creating /dev/spidevX.Y and tools/spi/spidev_test for testing
> functionality.
I wonder whether this patch is tested (or at least can be
tested), as I am not sure there are pins for spi or any
onboard device on existing SG2042 board.
> ---
> Changes in v2:
> - rebase v1 to sophgo/master(github.com/sophgo/linux.git).
> - order properties in device node.
> - remove unevaluated properties `clock-frequency`.
> - set default status to disable.
> - Link to v1: https://lore.kernel.org/r/20250228-sfg-spi-v1-1-b989aed94911@gmail.com
> ---
> .../riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts | 8 +++++++
> arch/riscv/boot/dts/sophgo/sg2042.dtsi | 28 ++++++++++++++++++++++
> 2 files changed, 36 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts b/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
> index be596d01ff8d33bcdbe431d9731a55ee190ad5b3..c43a807af2f827b5267afe5e4fdf6e9e857dfa20 100644
> --- a/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
> +++ b/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
> @@ -72,6 +72,14 @@ &uart0 {
> status = "okay";
> };
>
> +&spi0 {
> + status = "okay";
> +};
> +
> +&spi1 {
> + status = "okay";
> +};
> +
> / {
> thermal-zones {
> soc-thermal {
> diff --git a/arch/riscv/boot/dts/sophgo/sg2042.dtsi b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> index e62ac51ac55abd922b5ef796ba8c2196383850c4..500645147b1f8ed0a08ad3cafb38ea79cf57d737 100644
> --- a/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> +++ b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> @@ -545,5 +545,33 @@ sd: mmc@704002b000 {
> "timer";
> status = "disabled";
> };
> +
> + spi0: spi@7040004000 {
> + compatible = "snps,dw-apb-ssi";
> + reg = <0x70 0x40004000 0x00 0x1000>;
> + clocks = <&clkgen GATE_CLK_APB_SPI>,
> + <&clkgen GATE_CLK_SYSDMA_AXI>;
Is the sysdma axi clock for the this device. IIRC is APB interface.
Also, Are these clock aligned? If not, use space to align the two
clocks. You also need to add the clock-names here.
> + interrupt-parent = <&intc>;
> + interrupts = <110 IRQ_TYPE_LEVEL_HIGH>;
> + #address-cells = <0x01>;
> + #size-cells = <0x00>;
> + num-cs = <0x02>;
Just use decimal number, no hex there.
> + resets = <&rstgen RST_SPI0>;
> + status = "disabled";
> + };
> +
> + spi1: spi@7040005000 {
> + compatible = "snps,dw-apb-ssi";
> + reg = <0x70 0x40005000 0x00 0x1000>;
> + clocks = <&clkgen GATE_CLK_APB_SPI>,
> + <&clkgen GATE_CLK_SYSDMA_AXI>;
The same as above.
> + interrupt-parent = <&intc>;
> + interrupts = <111 IRQ_TYPE_LEVEL_HIGH>;
> + #address-cells = <0x01>;
> + #size-cells = <0x00>;
> + num-cs = <0x02>;
The same as above.
> + resets = <&rstgen RST_SPI1>;
> + status = "disabled";
> + };
> };
> };
>
> ---
> base-commit: aa5ee7180ec41bb77c3e327e95d119f2294babea
> change-id: 20250228-sfg-spi-e3f2aeca09ab
>
> Best regards,
> --
> Zixian Zeng <sycamoremoon376@gmail.com>
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2] riscv: sophgo: dts: Add spi controller for SG2042
2025-03-02 2:18 ` Inochi Amaoto
@ 2025-03-04 6:36 ` Zixian Zeng
0 siblings, 0 replies; 9+ messages in thread
From: Zixian Zeng @ 2025-03-04 6:36 UTC (permalink / raw)
To: Inochi Amaoto
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Chen Wang, Inochi Amaoto,
Alexandre Ghiti, devicetree, linux-riscv, linux-kernel, sophgo,
chao.wei, xiaoguang.xing
On 25/03/02 10:18AM, Inochi Amaoto wrote:
> On Fri, Feb 28, 2025 at 08:40:23PM +0800, Zixian Zeng wrote:
> > Add spi controllers for SG2042.
> >
> > SG2042 uses the upstreamed Synopsys DW SPI IP.
> >
> > Signed-off-by: Zixian Zeng <sycamoremoon376@gmail.com>
> > ---
> > For this spi controller patch, only bindings are included.
> > This is tested on milkv-pioneer board. Using driver/spi/spidev.c
> > for creating /dev/spidevX.Y and tools/spi/spidev_test for testing
> > functionality.
>
> I wonder whether this patch is tested (or at least can be
> tested), as I am not sure there are pins for spi or any
> onboard device on existing SG2042 board.
>
Yes, this is tested on milkv-pioneer board.
Since this patch is just for supporting SPI controller in BUS level
which not relates to the real hardware such as screen, I used
driver/spi/spidev.c for creating virtual device /dev/spidevX.Y by
adding nodes in DTS file as following:
spidev@0 {
compatible = "snps,dw-apb-ssi";
reg = <0>;
status = "okay";
};
spidev@1 {
compatible = "snps,dw-apb-ssi";
reg = <1>;
status = "okay";
};
And using tools/spi/spidev_{test,fdx} provided by kernel tree for
testing bus functionality.
> > ---
> > Changes in v2:
> > - rebase v1 to sophgo/master(github.com/sophgo/linux.git).
> > - order properties in device node.
> > - remove unevaluated properties `clock-frequency`.
> > - set default status to disable.
> > - Link to v1: https://lore.kernel.org/r/20250228-sfg-spi-v1-1-b989aed94911@gmail.com
> > ---
> > .../riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts | 8 +++++++
> > arch/riscv/boot/dts/sophgo/sg2042.dtsi | 28 ++++++++++++++++++++++
> > 2 files changed, 36 insertions(+)
> >
> > diff --git a/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts b/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
> > index be596d01ff8d33bcdbe431d9731a55ee190ad5b3..c43a807af2f827b5267afe5e4fdf6e9e857dfa20 100644
> > --- a/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
> > +++ b/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
> > @@ -72,6 +72,14 @@ &uart0 {
> > status = "okay";
> > };
> >
> > +&spi0 {
> > + status = "okay";
> > +};
> > +
> > +&spi1 {
> > + status = "okay";
> > +};
> > +
> > / {
> > thermal-zones {
> > soc-thermal {
> > diff --git a/arch/riscv/boot/dts/sophgo/sg2042.dtsi b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> > index e62ac51ac55abd922b5ef796ba8c2196383850c4..500645147b1f8ed0a08ad3cafb38ea79cf57d737 100644
> > --- a/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> > +++ b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> > @@ -545,5 +545,33 @@ sd: mmc@704002b000 {
> > "timer";
> > status = "disabled";
> > };
> > +
> > + spi0: spi@7040004000 {
> > + compatible = "snps,dw-apb-ssi";
> > + reg = <0x70 0x40004000 0x00 0x1000>;
>
> > + clocks = <&clkgen GATE_CLK_APB_SPI>,
> > + <&clkgen GATE_CLK_SYSDMA_AXI>;
>
> Is the sysdma axi clock for the this device. IIRC is APB interface.
>
Thanks, I will remove the GATE_CLK_SYSDMA_AXI in next revision.
> Also, Are these clock aligned? If not, use space to align the two
> clocks. You also need to add the clock-names here.
In the case of the only clock, I think it’s probably no need to add
the clock-names property here.
>
> > + interrupt-parent = <&intc>;
> > + interrupts = <110 IRQ_TYPE_LEVEL_HIGH>;
>
> > + #address-cells = <0x01>;
> > + #size-cells = <0x00>;
> > + num-cs = <0x02>;
>
> Just use decimal number, no hex there.
>
Thanks, I will fix it in next revision.
> > + resets = <&rstgen RST_SPI0>;
> > + status = "disabled";
> > + };
> > +
> > + spi1: spi@7040005000 {
> > + compatible = "snps,dw-apb-ssi";
> > + reg = <0x70 0x40005000 0x00 0x1000>;
>
> > + clocks = <&clkgen GATE_CLK_APB_SPI>,
> > + <&clkgen GATE_CLK_SYSDMA_AXI>;
>
> The same as above.
>
Thanks.
> > + interrupt-parent = <&intc>;
> > + interrupts = <111 IRQ_TYPE_LEVEL_HIGH>;
>
> > + #address-cells = <0x01>;
> > + #size-cells = <0x00>;
> > + num-cs = <0x02>;
>
> The same as above.
>
Thanks.
> > + resets = <&rstgen RST_SPI1>;
> > + status = "disabled";
> > + };
> > };
> > };
> >
> > ---
> > base-commit: aa5ee7180ec41bb77c3e327e95d119f2294babea
> > change-id: 20250228-sfg-spi-e3f2aeca09ab
> >
> > Best regards,
> > --
> > Zixian Zeng <sycamoremoon376@gmail.com>
> >
Best regards,
Zixian Zeng <sycamoremoon376@gmail.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] riscv: sophgo: dts: Add spi controller for SG2042
2025-02-28 12:40 [PATCH v2] riscv: sophgo: dts: Add spi controller for SG2042 Zixian Zeng
2025-02-28 18:22 ` Conor Dooley
2025-03-02 2:18 ` Inochi Amaoto
@ 2025-03-04 3:06 ` Chen Wang
2 siblings, 0 replies; 9+ messages in thread
From: Chen Wang @ 2025-03-04 3:06 UTC (permalink / raw)
To: Zixian Zeng, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Inochi Amaoto,
Alexandre Ghiti
Cc: devicetree, linux-riscv, linux-kernel, sophgo, chao.wei,
xiaoguang.xing
On 2025/2/28 20:40, Zixian Zeng wrote:
> Add spi controllers for SG2042.
>
> SG2042 uses the upstreamed Synopsys DW SPI IP.
>
> Signed-off-by: Zixian Zeng <sycamoremoon376@gmail.com>
> ---
> For this spi controller patch, only bindings are included.
> This is tested on milkv-pioneer board. Using driver/spi/spidev.c
> for creating /dev/spidevX.Y and tools/spi/spidev_test for testing
> functionality.
> ---
> Changes in v2:
> - rebase v1 to sophgo/master(github.com/sophgo/linux.git).
> - order properties in device node.
> - remove unevaluated properties `clock-frequency`.
> - set default status to disable.
> - Link to v1: https://lore.kernel.org/r/20250228-sfg-spi-v1-1-b989aed94911@gmail.com
> ---
> .../riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts | 8 +++++++
> arch/riscv/boot/dts/sophgo/sg2042.dtsi | 28 ++++++++++++++++++++++
> 2 files changed, 36 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts b/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
> index be596d01ff8d33bcdbe431d9731a55ee190ad5b3..c43a807af2f827b5267afe5e4fdf6e9e857dfa20 100644
> --- a/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
> +++ b/arch/riscv/boot/dts/sophgo/sg2042-milkv-pioneer.dts
> @@ -72,6 +72,14 @@ &uart0 {
> status = "okay";
> };
>
> +&spi0 {
> + status = "okay";
> +};
> +
> +&spi1 {
> + status = "okay";
> +};
> +
Pioneerbox doesn't use these SPI controllers, so just drop this change
and keep the change for sg2042.dtsi should be fine.
Thanks,
Chen
> / {
> thermal-zones {
> soc-thermal {
> diff --git a/arch/riscv/boot/dts/sophgo/sg2042.dtsi b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> index e62ac51ac55abd922b5ef796ba8c2196383850c4..500645147b1f8ed0a08ad3cafb38ea79cf57d737 100644
> --- a/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> +++ b/arch/riscv/boot/dts/sophgo/sg2042.dtsi
> @@ -545,5 +545,33 @@ sd: mmc@704002b000 {
> "timer";
> status = "disabled";
> };
> +
> + spi0: spi@7040004000 {
> + compatible = "snps,dw-apb-ssi";
> + reg = <0x70 0x40004000 0x00 0x1000>;
> + clocks = <&clkgen GATE_CLK_APB_SPI>,
> + <&clkgen GATE_CLK_SYSDMA_AXI>;
> + interrupt-parent = <&intc>;
> + interrupts = <110 IRQ_TYPE_LEVEL_HIGH>;
> + #address-cells = <0x01>;
> + #size-cells = <0x00>;
> + num-cs = <0x02>;
> + resets = <&rstgen RST_SPI0>;
> + status = "disabled";
> + };
> +
> + spi1: spi@7040005000 {
> + compatible = "snps,dw-apb-ssi";
> + reg = <0x70 0x40005000 0x00 0x1000>;
> + clocks = <&clkgen GATE_CLK_APB_SPI>,
> + <&clkgen GATE_CLK_SYSDMA_AXI>;
> + interrupt-parent = <&intc>;
> + interrupts = <111 IRQ_TYPE_LEVEL_HIGH>;
> + #address-cells = <0x01>;
> + #size-cells = <0x00>;
> + num-cs = <0x02>;
> + resets = <&rstgen RST_SPI1>;
> + status = "disabled";
> + };
> };
> };
>
> ---
> base-commit: aa5ee7180ec41bb77c3e327e95d119f2294babea
> change-id: 20250228-sfg-spi-e3f2aeca09ab
>
> Best regards,
^ permalink raw reply [flat|nested] 9+ messages in thread