[PATCH v2 0/3] Add basic SPI support on TH1520

[PATCH v2 0/3] Add basic SPI support on TH1520

[Kanak Shilledar]

Implemented basic SPI support for TH1520 SoC. There are two SPIs reserved
on the LicheePi4A, one on the SPI Flash pads that are blanked out on the
back, and one on the pins. I implemented the one connected to the pad.

It is using a fixed clock of 396MHz. The address and clock frequency was
referenced from the TH1520 System Reference Manual [1].

I have tested this on the LicheePi4A.

Changes made in v2:
- Changed from single patch file to a patchset by splitting the single
commit into dt-bindings, driver and the devicetree.

[1] https://openbeagle.org/beaglev-ahead/beaglev-ahead/-/blob/main/docs/TH1520%20System%20User%20Manual.pdf

Kanak Shilledar (3):
  dt-bindings: spi: snps,dw-apb-ssi.yaml: update compatible property
  spi: dw-mmio: update dw_spi_mmio_of_match struct with thead
  riscv: dts: thead: add basic spi node

 .../devicetree/bindings/spi/snps,dw-apb-ssi.yaml |  4 ++++
 .../boot/dts/thead/th1520-beaglev-ahead.dts      |  9 +++++++++
 .../boot/dts/thead/th1520-lichee-module-4a.dtsi  |  4 ++++
 .../riscv/boot/dts/thead/th1520-lichee-pi-4a.dts |  5 +++++
 arch/riscv/boot/dts/thead/th1520.dtsi            | 16 ++++++++++++++++
 drivers/spi/spi-dw-mmio.c                        |  1 +
 6 files changed, 39 insertions(+)


base-commit: 6ba59ff4227927d3a8530fc2973b80e94b54d58f
-- 
2.45.2

[PATCH v2 1/3] dt-bindings: spi: snps,dw-apb-ssi.yaml: update compatible property

[Kanak Shilledar]

updated compatible property to include "thead,th1520-spi" for the
TH1520 SoC SPI Controller.

Signed-off-by: Kanak Shilledar <kanakshilledar@gmail.com>
---
Changes in v2:
- Separated from a single patch file
---
 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
index fde3776a558b..bccd00a1ddd0 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
@@ -88,6 +88,10 @@ properties:
               - renesas,r9a06g032-spi # RZ/N1D
               - renesas,r9a06g033-spi # RZ/N1S
           - const: renesas,rzn1-spi   # RZ/N1
+      - description: T-HEAD TH1520 SoC SPI Controller
+        items:
+          - const: thead,th1520-spi
+          - const: snps,dw-apb-ssi
 
   reg:
     minItems: 1
-- 
2.45.2

Re: [PATCH v2 1/3] dt-bindings: spi: snps,dw-apb-ssi.yaml: update compatible property

[Conor Dooley]

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

On Mon, Jul 01, 2024 at 05:43:52PM +0530, Kanak Shilledar wrote:

$subject: dt-bindings: spi: snps,dw-apb-ssi.yaml: update compatible property

Mark likes SPI bindings to be worked "spi: dt-bindings: ...", but
additionally I think the subject should say something like "add support
for T-Head th1520" or similar, what you've got is pretty generic.
Not requesting a respin for that alone, maybe Mark will grab it as-is.
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

> updated compatible property to include "thead,th1520-spi" for the
> TH1520 SoC SPI Controller.
> 
> Signed-off-by: Kanak Shilledar <kanakshilledar@gmail.com>
> ---
> Changes in v2:
> - Separated from a single patch file
> ---
>  Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> index fde3776a558b..bccd00a1ddd0 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> @@ -88,6 +88,10 @@ properties:
>                - renesas,r9a06g032-spi # RZ/N1D
>                - renesas,r9a06g033-spi # RZ/N1S
>            - const: renesas,rzn1-spi   # RZ/N1
> +      - description: T-HEAD TH1520 SoC SPI Controller
> +        items:
> +          - const: thead,th1520-spi
> +          - const: snps,dw-apb-ssi
>  
>    reg:
>      minItems: 1
> -- 
> 2.45.2
> 

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

[PATCH v2 2/3] spi: dw-mmio: update dw_spi_mmio_of_match struct with thead

[Kanak Shilledar]

updated the struct of_device_id dw_spi_mmio_of_match to include
the updated compatible value for TH1520 SoC ("thead,th1520-spi")
to initialize with dw_spi_pssi_init().

Signed-off-by: Kanak Shilledar <kanakshilledar@gmail.com>
---
Changes in v2:
- Separated from a single patch file.
---
 drivers/spi/spi-dw-mmio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index 819907e332c4..39e3d46ebf5d 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -419,6 +419,7 @@ static const struct of_device_id dw_spi_mmio_of_match[] = {
 	{ .compatible = "microchip,sparx5-spi", dw_spi_mscc_sparx5_init},
 	{ .compatible = "canaan,k210-spi", dw_spi_canaan_k210_init},
 	{ .compatible = "amd,pensando-elba-spi", .data = dw_spi_elba_init},
+	{ .compatible = "thead,th1520-spi", .data = dw_spi_pssi_init},
 	{ /* end of table */}
 };
 MODULE_DEVICE_TABLE(of, dw_spi_mmio_of_match);
-- 
2.45.2

Re: [PATCH v2 2/3] spi: dw-mmio: update dw_spi_mmio_of_match struct with thead

[Samuel Holland]

Hi Kanak,

On 2024-07-01 7:13 AM, Kanak Shilledar wrote:
> updated the struct of_device_id dw_spi_mmio_of_match to include
> the updated compatible value for TH1520 SoC ("thead,th1520-spi")
> to initialize with dw_spi_pssi_init().
> 
> Signed-off-by: Kanak Shilledar <kanakshilledar@gmail.com>
> ---
> Changes in v2:
> - Separated from a single patch file.
> ---
>  drivers/spi/spi-dw-mmio.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> index 819907e332c4..39e3d46ebf5d 100644
> --- a/drivers/spi/spi-dw-mmio.c
> +++ b/drivers/spi/spi-dw-mmio.c
> @@ -419,6 +419,7 @@ static const struct of_device_id dw_spi_mmio_of_match[] = {
>  	{ .compatible = "microchip,sparx5-spi", dw_spi_mscc_sparx5_init},
>  	{ .compatible = "canaan,k210-spi", dw_spi_canaan_k210_init},
>  	{ .compatible = "amd,pensando-elba-spi", .data = dw_spi_elba_init},
> +	{ .compatible = "thead,th1520-spi", .data = dw_spi_pssi_init},

Your binding requires snps,dw-apb-ssi as a fallback compatible string, which is
already supported by this driver and uses the same match data. So you don't need
this patch; its only effect is to make the kernel larger.

Regards,
Samuel

>  	{ /* end of table */}
>  };
>  MODULE_DEVICE_TABLE(of, dw_spi_mmio_of_match);

Re: [PATCH v2 2/3] spi: dw-mmio: update dw_spi_mmio_of_match struct with thead

[Serge Semin]

Hi folks

On Mon, Jul 01, 2024 at 08:17:29AM -0500, Samuel Holland wrote:
> Hi Kanak,
> 
> On 2024-07-01 7:13 AM, Kanak Shilledar wrote:
> > updated the struct of_device_id dw_spi_mmio_of_match to include
> > the updated compatible value for TH1520 SoC ("thead,th1520-spi")
> > to initialize with dw_spi_pssi_init().
> > 
> > Signed-off-by: Kanak Shilledar <kanakshilledar@gmail.com>
> > ---
> > Changes in v2:
> > - Separated from a single patch file.
> > ---
> >  drivers/spi/spi-dw-mmio.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> > index 819907e332c4..39e3d46ebf5d 100644
> > --- a/drivers/spi/spi-dw-mmio.c
> > +++ b/drivers/spi/spi-dw-mmio.c
> > @@ -419,6 +419,7 @@ static const struct of_device_id dw_spi_mmio_of_match[] = {
> >  	{ .compatible = "microchip,sparx5-spi", dw_spi_mscc_sparx5_init},
> >  	{ .compatible = "canaan,k210-spi", dw_spi_canaan_k210_init},
> >  	{ .compatible = "amd,pensando-elba-spi", .data = dw_spi_elba_init},
> > +	{ .compatible = "thead,th1520-spi", .data = dw_spi_pssi_init},
> 
> Your binding requires snps,dw-apb-ssi as a fallback compatible string, which is
> already supported by this driver and uses the same match data. So you don't need
> this patch; its only effect is to make the kernel larger.

Agree with Samuel comment. Indeed there is no point in adding the
vendor-specific device-name supported in the driver if the fallback
compatible works as-is.

From that perspective we shouldn't have merged in the patch adding the
Renesas RZN1 SPI device name support, since the generic fallback
compatible works for it. On the contrary the Microsemi Ocelot/Jaguar2
SoC SPI DT-bindings shouldn't have been defined with the generic
fallback compatible since should the device be bound via the generic
name it won't work as expected.

Although, it's better to hear out what Rob, Conor or Krzysztof think
about this.

-Serge(y)

> 
> Regards,
> Samuel
> 
> >  	{ /* end of table */}
> >  };
> >  MODULE_DEVICE_TABLE(of, dw_spi_mmio_of_match);
> 

Re: [PATCH v2 2/3] spi: dw-mmio: update dw_spi_mmio_of_match struct with thead

[Conor Dooley]

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

On Mon, Jul 01, 2024 at 09:57:20PM +0300, Serge Semin wrote:
> Hi folks
> 
> On Mon, Jul 01, 2024 at 08:17:29AM -0500, Samuel Holland wrote:
> > Hi Kanak,
> > 
> > On 2024-07-01 7:13 AM, Kanak Shilledar wrote:
> > > updated the struct of_device_id dw_spi_mmio_of_match to include
> > > the updated compatible value for TH1520 SoC ("thead,th1520-spi")
> > > to initialize with dw_spi_pssi_init().
> > > 
> > > Signed-off-by: Kanak Shilledar <kanakshilledar@gmail.com>
> > > ---
> > > Changes in v2:
> > > - Separated from a single patch file.
> > > ---
> > >  drivers/spi/spi-dw-mmio.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> > > index 819907e332c4..39e3d46ebf5d 100644
> > > --- a/drivers/spi/spi-dw-mmio.c
> > > +++ b/drivers/spi/spi-dw-mmio.c
> > > @@ -419,6 +419,7 @@ static const struct of_device_id dw_spi_mmio_of_match[] = {
> > >  	{ .compatible = "microchip,sparx5-spi", dw_spi_mscc_sparx5_init},
> > >  	{ .compatible = "canaan,k210-spi", dw_spi_canaan_k210_init},
> > >  	{ .compatible = "amd,pensando-elba-spi", .data = dw_spi_elba_init},
> > > +	{ .compatible = "thead,th1520-spi", .data = dw_spi_pssi_init},
> > 
> > Your binding requires snps,dw-apb-ssi as a fallback compatible string, which is
> > already supported by this driver and uses the same match data. So you don't need
> > this patch; its only effect is to make the kernel larger.
> 
> Agree with Samuel comment. Indeed there is no point in adding the
> vendor-specific device-name supported in the driver if the fallback
> compatible works as-is.

FWIW, Mark picked up the binding alone so I think there's nothing for
Kanak to do here & the driver patch should just be forgotten about :)

> >From that perspective we shouldn't have merged in the patch adding the
> Renesas RZN1 SPI device name support, since the generic fallback
> compatible works for it. On the contrary the Microsemi Ocelot/Jaguar2
> SoC SPI DT-bindings shouldn't have been defined with the generic
> fallback compatible since should the device be bound via the generic
> name it won't work as expected.
> 
> Although, it's better to hear out what Rob, Conor or Krzysztof think
> about this.

I agree with what you've written. If the fallback works identically, then
the specific compatible shouldn't be added here. And if the fallback
will cause the device to misbehave (or not behave at all), then it
should not have been added.
I'm not sure if the Microsemi stuff is in the "won't work {,properly}"
camp or in the "will work in a limited fashion" camp. The latter would
be suitable for a fallback, the former not.

Cheers,
Conor.


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

Re: [PATCH v2 2/3] spi: dw-mmio: update dw_spi_mmio_of_match struct with thead

[Kanak Shilledar]

Hi,
So, I will drop this patch.
In the next version (i.e. v2) of this patchset, do I need to include
the dt-binding patch as it is already in for-next.
I am waiting for comments on the devicetree files before sending the
v2 (if required).

Thanks and Regards,
Kanak Shilledar

On Wed, Jul 3, 2024 at 12:59 PM Conor Dooley <conor.dooley@microchip.com> wrote:
>
> On Mon, Jul 01, 2024 at 09:57:20PM +0300, Serge Semin wrote:
> > Hi folks
> >
> > On Mon, Jul 01, 2024 at 08:17:29AM -0500, Samuel Holland wrote:
> > > Hi Kanak,
> > >
> > > On 2024-07-01 7:13 AM, Kanak Shilledar wrote:
> > > > updated the struct of_device_id dw_spi_mmio_of_match to include
> > > > the updated compatible value for TH1520 SoC ("thead,th1520-spi")
> > > > to initialize with dw_spi_pssi_init().
> > > >
> > > > Signed-off-by: Kanak Shilledar <kanakshilledar@gmail.com>
> > > > ---
> > > > Changes in v2:
> > > > - Separated from a single patch file.
> > > > ---
> > > >  drivers/spi/spi-dw-mmio.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> > > > index 819907e332c4..39e3d46ebf5d 100644
> > > > --- a/drivers/spi/spi-dw-mmio.c
> > > > +++ b/drivers/spi/spi-dw-mmio.c
> > > > @@ -419,6 +419,7 @@ static const struct of_device_id dw_spi_mmio_of_match[] = {
> > > >   { .compatible = "microchip,sparx5-spi", dw_spi_mscc_sparx5_init},
> > > >   { .compatible = "canaan,k210-spi", dw_spi_canaan_k210_init},
> > > >   { .compatible = "amd,pensando-elba-spi", .data = dw_spi_elba_init},
> > > > + { .compatible = "thead,th1520-spi", .data = dw_spi_pssi_init},
> > >
> > > Your binding requires snps,dw-apb-ssi as a fallback compatible string, which is
> > > already supported by this driver and uses the same match data. So you don't need
> > > this patch; its only effect is to make the kernel larger.
> >
> > Agree with Samuel comment. Indeed there is no point in adding the
> > vendor-specific device-name supported in the driver if the fallback
> > compatible works as-is.
>
> FWIW, Mark picked up the binding alone so I think there's nothing for
> Kanak to do here & the driver patch should just be forgotten about :)
>
> > >From that perspective we shouldn't have merged in the patch adding the
> > Renesas RZN1 SPI device name support, since the generic fallback
> > compatible works for it. On the contrary the Microsemi Ocelot/Jaguar2
> > SoC SPI DT-bindings shouldn't have been defined with the generic
> > fallback compatible since should the device be bound via the generic
> > name it won't work as expected.
> >
> > Although, it's better to hear out what Rob, Conor or Krzysztof think
> > about this.
>
> I agree with what you've written. If the fallback works identically, then
> the specific compatible shouldn't be added here. And if the fallback
> will cause the device to misbehave (or not behave at all), then it
> should not have been added.
> I'm not sure if the Microsemi stuff is in the "won't work {,properly}"
> camp or in the "will work in a limited fashion" camp. The latter would
> be suitable for a fallback, the former not.
>
> Cheers,
> Conor.
>

Re: [PATCH v2 2/3] spi: dw-mmio: update dw_spi_mmio_of_match struct with thead

[Conor Dooley]

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

On Wed, Jul 03, 2024 at 06:42:46PM +0530, Kanak Shilledar wrote:
> Hi,
> So, I will drop this patch.
> In the next version (i.e. v2) of this patchset, do I need to include
> the dt-binding patch as it is already in for-next.

No, you do not need to include the binding.

> I am waiting for comments on the devicetree files before sending the
> v2 (if required).

I'll try to look at that today, not super sure if I wanna pick up more
patches for that platform with "fixed-clock"s, but I'll comment that on
the dts patch itself.

Cheers,
Conor.

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

Re: [PATCH v2 2/3] spi: dw-mmio: update dw_spi_mmio_of_match struct with thead

[Kanak Shilledar]

On Wed, Jul 3, 2024 at 7:57 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Wed, Jul 03, 2024 at 06:42:46PM +0530, Kanak Shilledar wrote:
> > Hi,
> > So, I will drop this patch.
> > In the next version (i.e. v2) of this patchset, do I need to include
> > the dt-binding patch as it is already in for-next.
>
> No, you do not need to include the binding.

Alright thanks for the clarification!

> > I am waiting for comments on the devicetree files before sending the
> > v2 (if required).
>
> I'll try to look at that today, not super sure if I wanna pick up more
> patches for that platform with "fixed-clock"s, but I'll comment that on
> the dts patch itself.
>
> Cheers,
> Conor.

Cheers,
Kanak Shilledar

[PATCH v2 3/3] riscv: dts: thead: add basic spi node

[Kanak Shilledar]

created spi0 node with fixed clock. the spi0 node
uses synopsis designware driver and has the following
compatible "snps,dw-apb-ssi". the spi0 node is connected
to a SPI NOR flash pad which is left unpopulated on the back
side of the board.

Signed-off-by: Kanak Shilledar <kanakshilledar@gmail.com>
---
Changes in v2:
- Separated from a single patch file
---
 .../boot/dts/thead/th1520-beaglev-ahead.dts      |  9 +++++++++
 .../boot/dts/thead/th1520-lichee-module-4a.dtsi  |  4 ++++
 .../riscv/boot/dts/thead/th1520-lichee-pi-4a.dts |  5 +++++
 arch/riscv/boot/dts/thead/th1520.dtsi            | 16 ++++++++++++++++
 4 files changed, 34 insertions(+)

diff --git a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
index d9b4de9e4757..3103b74e0288 100644
--- a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
+++ b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
@@ -17,6 +17,7 @@ aliases {
 		gpio1 = &gpio1;
 		gpio2 = &gpio2;
 		gpio3 = &gpio3;
+		spi0 = &spi0;
 		serial0 = &uart0;
 		serial1 = &uart1;
 		serial2 = &uart2;
@@ -52,6 +53,10 @@ &sdhci_clk {
 	clock-frequency = <198000000>;
 };
 
+&spi_clk {
+	clock-frequency = <396000000>;
+};
+
 &uart_sclk {
 	clock-frequency = <100000000>;
 };
@@ -79,3 +84,7 @@ &sdio0 {
 &uart0 {
 	status = "okay";
 };
+
+&spi0 {
+	status = "okay";
+};
diff --git a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
index 1365d3a512a3..6939bd36560c 100644
--- a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
+++ b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
@@ -33,6 +33,10 @@ &sdhci_clk {
 	clock-frequency = <198000000>;
 };
 
+&spi_clk {
+	clock-frequency = <396000000>;
+};
+
 &uart_sclk {
 	clock-frequency = <100000000>;
 };
diff --git a/arch/riscv/boot/dts/thead/th1520-lichee-pi-4a.dts b/arch/riscv/boot/dts/thead/th1520-lichee-pi-4a.dts
index 9a3884a73e13..14b06dd81a9a 100644
--- a/arch/riscv/boot/dts/thead/th1520-lichee-pi-4a.dts
+++ b/arch/riscv/boot/dts/thead/th1520-lichee-pi-4a.dts
@@ -14,6 +14,7 @@ aliases {
 		gpio1 = &gpio1;
 		gpio2 = &gpio2;
 		gpio3 = &gpio3;
+		spi0 = &spi0;
 		serial0 = &uart0;
 		serial1 = &uart1;
 		serial2 = &uart2;
@@ -30,3 +31,7 @@ chosen {
 &uart0 {
 	status = "okay";
 };
+
+&spi0 {
+	status = "okay";
+};
diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
index d2fa25839012..f962de663e7e 100644
--- a/arch/riscv/boot/dts/thead/th1520.dtsi
+++ b/arch/riscv/boot/dts/thead/th1520.dtsi
@@ -140,6 +140,12 @@ apb_clk: apb-clk-clock {
 		#clock-cells = <0>;
 	};
 
+	spi_clk: spi-clock {
+		compatible = "fixed-clock";
+		clock-output-names = "spi_clk";
+		#clock-cells = <0>;
+	};
+
 	uart_sclk: uart-sclk-clock {
 		compatible = "fixed-clock";
 		clock-output-names = "uart_sclk";
@@ -183,6 +189,16 @@ clint: timer@ffdc000000 {
 					      <&cpu3_intc 3>, <&cpu3_intc 7>;
 		};
 
+		spi0: spi@ffe700c000 {
+			compatible = "thead,th1520-spi", "snps,dw-apb-ssi";
+			reg = <0xff 0xe700c000 0x0 0x1000>;
+			interrupts = <54 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&spi_clk>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
 		uart0: serial@ffe7014000 {
 			compatible = "snps,dw-apb-uart";
 			reg = <0xff 0xe7014000 0x0 0x100>;
-- 
2.45.2

Re: [PATCH v2 3/3] riscv: dts: thead: add basic spi node

[Conor Dooley]

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

Kanak, Drew,

On Mon, Jul 01, 2024 at 05:43:54PM +0530, Kanak Shilledar wrote:
> created spi0 node with fixed clock. the spi0 node
> uses synopsis designware driver and has the following
> compatible "snps,dw-apb-ssi". the spi0 node is connected
> to a SPI NOR flash pad which is left unpopulated on the back
> side of the board.
> 
> Signed-off-by: Kanak Shilledar <kanakshilledar@gmail.com>
> ---
> Changes in v2:
> - Separated from a single patch file
> ---
>  .../boot/dts/thead/th1520-beaglev-ahead.dts      |  9 +++++++++
>  .../boot/dts/thead/th1520-lichee-module-4a.dtsi  |  4 ++++
>  .../riscv/boot/dts/thead/th1520-lichee-pi-4a.dts |  5 +++++

Didn't you say there was a flash on one of these two boards?

>  arch/riscv/boot/dts/thead/th1520.dtsi            | 16 ++++++++++++++++
>  4 files changed, 34 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> index d9b4de9e4757..3103b74e0288 100644
> --- a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> +++ b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> @@ -17,6 +17,7 @@ aliases {
>  		gpio1 = &gpio1;
>  		gpio2 = &gpio2;
>  		gpio3 = &gpio3;
> +		spi0 = &spi0;

"spi" would sort after "serial".

>  		serial0 = &uart0;
>  		serial1 = &uart1;
>  		serial2 = &uart2;
> @@ -52,6 +53,10 @@ &sdhci_clk {
>  	clock-frequency = <198000000>;
>  };
>  
> +&spi_clk {
> +	clock-frequency = <396000000>;
> +};

I'm pretty sceptical about adding more of these fixed clocks, rather
than waiting for the clock driver. Drew, what do you think? Should we
just add one more to your fixup list or would you rather delay? Guess it
depends on how long more you think that clock driver is likely to take.

Thanks,
Conor.

> +
>  &uart_sclk {
>  	clock-frequency = <100000000>;
>  };
> @@ -79,3 +84,7 @@ &sdio0 {
>  &uart0 {
>  	status = "okay";
>  };
> +
> +&spi0 {
> +	status = "okay";
> +};
> diff --git a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> index 1365d3a512a3..6939bd36560c 100644
> --- a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> +++ b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> @@ -33,6 +33,10 @@ &sdhci_clk {
>  	clock-frequency = <198000000>;
>  };
>  
> +&spi_clk {
> +	clock-frequency = <396000000>;
> +};
> +
>  &uart_sclk {
>  	clock-frequency = <100000000>;
>  };
> diff --git a/arch/riscv/boot/dts/thead/th1520-lichee-pi-4a.dts b/arch/riscv/boot/dts/thead/th1520-lichee-pi-4a.dts
> index 9a3884a73e13..14b06dd81a9a 100644
> --- a/arch/riscv/boot/dts/thead/th1520-lichee-pi-4a.dts
> +++ b/arch/riscv/boot/dts/thead/th1520-lichee-pi-4a.dts
> @@ -14,6 +14,7 @@ aliases {
>  		gpio1 = &gpio1;
>  		gpio2 = &gpio2;
>  		gpio3 = &gpio3;
> +		spi0 = &spi0;
>  		serial0 = &uart0;
>  		serial1 = &uart1;
>  		serial2 = &uart2;
> @@ -30,3 +31,7 @@ chosen {
>  &uart0 {
>  	status = "okay";
>  };
> +
> +&spi0 {
> +	status = "okay";
> +};
> diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
> index d2fa25839012..f962de663e7e 100644
> --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> @@ -140,6 +140,12 @@ apb_clk: apb-clk-clock {
>  		#clock-cells = <0>;
>  	};
>  
> +	spi_clk: spi-clock {
> +		compatible = "fixed-clock";
> +		clock-output-names = "spi_clk";
> +		#clock-cells = <0>;
> +	};
> +
>  	uart_sclk: uart-sclk-clock {
>  		compatible = "fixed-clock";
>  		clock-output-names = "uart_sclk";
> @@ -183,6 +189,16 @@ clint: timer@ffdc000000 {
>  					      <&cpu3_intc 3>, <&cpu3_intc 7>;
>  		};
>  
> +		spi0: spi@ffe700c000 {
> +			compatible = "thead,th1520-spi", "snps,dw-apb-ssi";
> +			reg = <0xff 0xe700c000 0x0 0x1000>;
> +			interrupts = <54 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&spi_clk>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			status = "disabled";
> +		};
> +
>  		uart0: serial@ffe7014000 {
>  			compatible = "snps,dw-apb-uart";
>  			reg = <0xff 0xe7014000 0x0 0x100>;
> -- 
> 2.45.2
> 

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

Re: [PATCH v2 3/3] riscv: dts: thead: add basic spi node

[Kanak Shilledar]

Hi

On Wed, Jul 3, 2024 at 8:15 PM Conor Dooley <conor@kernel.org> wrote:
>
> Kanak, Drew,
>
> On Mon, Jul 01, 2024 at 05:43:54PM +0530, Kanak Shilledar wrote:
> > created spi0 node with fixed clock. the spi0 node
> > uses synopsis designware driver and has the following
> > compatible "snps,dw-apb-ssi". the spi0 node is connected
> > to a SPI NOR flash pad which is left unpopulated on the back
> > side of the board.
> >
> > Signed-off-by: Kanak Shilledar <kanakshilledar@gmail.com>
> > ---
> > Changes in v2:
> > - Separated from a single patch file
> > ---
> >  .../boot/dts/thead/th1520-beaglev-ahead.dts      |  9 +++++++++
> >  .../boot/dts/thead/th1520-lichee-module-4a.dtsi  |  4 ++++
> >  .../riscv/boot/dts/thead/th1520-lichee-pi-4a.dts |  5 +++++
>
> Didn't you say there was a flash on one of these two boards?
Yes, there is a SPI nor flash pad left unpopulated on the bottom side
of the LicheePi 4a
carrier board. https://wiki.sipeed.com/hardware/en/lichee/th1520/lpi4a/2_unbox.html#Board-hardware-overview
notice the reserved pad in the bottom part of the image.

> >  arch/riscv/boot/dts/thead/th1520.dtsi            | 16 ++++++++++++++++
> >  4 files changed, 34 insertions(+)
> >
> > diff --git a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > index d9b4de9e4757..3103b74e0288 100644
> > --- a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > +++ b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > @@ -17,6 +17,7 @@ aliases {
> >               gpio1 = &gpio1;
> >               gpio2 = &gpio2;
> >               gpio3 = &gpio3;
> > +             spi0 = &spi0;
>
> "spi" would sort after "serial".
i will fix that.

> >               serial0 = &uart0;
> >               serial1 = &uart1;
> >               serial2 = &uart2;
> > @@ -52,6 +53,10 @@ &sdhci_clk {
> >       clock-frequency = <198000000>;
> >  };
> >
> > +&spi_clk {
> > +     clock-frequency = <396000000>;
> > +};
>
> I'm pretty sceptical about adding more of these fixed clocks, rather
> than waiting for the clock driver. Drew, what do you think? Should we
> just add one more to your fixup list or would you rather delay? Guess it
> depends on how long more you think that clock driver is likely to take.
>
> Thanks,
> Conor.
>

Thanks and Regards,
Kanak Shilledar

> > +
> >  &uart_sclk {
> >       clock-frequency = <100000000>;
> >  };
> > @@ -79,3 +84,7 @@ &sdio0 {
> >  &uart0 {
> >       status = "okay";
> >  };
> > +
> > +&spi0 {
> > +     status = "okay";
> > +};
> > diff --git a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> > index 1365d3a512a3..6939bd36560c 100644
> > --- a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> > +++ b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> > @@ -33,6 +33,10 @@ &sdhci_clk {
> >       clock-frequency = <198000000>;
> >  };
> >
> > +&spi_clk {
> > +     clock-frequency = <396000000>;
> > +};
> > +
> >  &uart_sclk {
> >       clock-frequency = <100000000>;
> >  };
> > diff --git a/arch/riscv/boot/dts/thead/th1520-lichee-pi-4a.dts b/arch/riscv/boot/dts/thead/th1520-lichee-pi-4a.dts
> > index 9a3884a73e13..14b06dd81a9a 100644
> > --- a/arch/riscv/boot/dts/thead/th1520-lichee-pi-4a.dts
> > +++ b/arch/riscv/boot/dts/thead/th1520-lichee-pi-4a.dts
> > @@ -14,6 +14,7 @@ aliases {
> >               gpio1 = &gpio1;
> >               gpio2 = &gpio2;
> >               gpio3 = &gpio3;
> > +             spi0 = &spi0;
> >               serial0 = &uart0;
> >               serial1 = &uart1;
> >               serial2 = &uart2;
> > @@ -30,3 +31,7 @@ chosen {
> >  &uart0 {
> >       status = "okay";
> >  };
> > +
> > +&spi0 {
> > +     status = "okay";
> > +};
> > diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
> > index d2fa25839012..f962de663e7e 100644
> > --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> > +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> > @@ -140,6 +140,12 @@ apb_clk: apb-clk-clock {
> >               #clock-cells = <0>;
> >       };
> >
> > +     spi_clk: spi-clock {
> > +             compatible = "fixed-clock";
> > +             clock-output-names = "spi_clk";
> > +             #clock-cells = <0>;
> > +     };
> > +
> >       uart_sclk: uart-sclk-clock {
> >               compatible = "fixed-clock";
> >               clock-output-names = "uart_sclk";
> > @@ -183,6 +189,16 @@ clint: timer@ffdc000000 {
> >                                             <&cpu3_intc 3>, <&cpu3_intc 7>;
> >               };
> >
> > +             spi0: spi@ffe700c000 {
> > +                     compatible = "thead,th1520-spi", "snps,dw-apb-ssi";
> > +                     reg = <0xff 0xe700c000 0x0 0x1000>;
> > +                     interrupts = <54 IRQ_TYPE_LEVEL_HIGH>;
> > +                     clocks = <&spi_clk>;
> > +                     #address-cells = <1>;
> > +                     #size-cells = <0>;
> > +                     status = "disabled";
> > +             };
> > +
> >               uart0: serial@ffe7014000 {
> >                       compatible = "snps,dw-apb-uart";
> >                       reg = <0xff 0xe7014000 0x0 0x100>;
> > --
> > 2.45.2
> >

Re: [PATCH v2 3/3] riscv: dts: thead: add basic spi node

[Conor Dooley]

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

On Thu, Jul 04, 2024 at 11:12:43AM +0530, Kanak Shilledar wrote:
> Hi
> 
> On Wed, Jul 3, 2024 at 8:15 PM Conor Dooley <conor@kernel.org> wrote:
> >
> > Kanak, Drew,
> >
> > On Mon, Jul 01, 2024 at 05:43:54PM +0530, Kanak Shilledar wrote:
> > > created spi0 node with fixed clock. the spi0 node
> > > uses synopsis designware driver and has the following
> > > compatible "snps,dw-apb-ssi". the spi0 node is connected
> > > to a SPI NOR flash pad which is left unpopulated on the back
> > > side of the board.
> > >
> > > Signed-off-by: Kanak Shilledar <kanakshilledar@gmail.com>
> > > ---
> > > Changes in v2:
> > > - Separated from a single patch file
> > > ---
> > >  .../boot/dts/thead/th1520-beaglev-ahead.dts      |  9 +++++++++
> > >  .../boot/dts/thead/th1520-lichee-module-4a.dtsi  |  4 ++++
> > >  .../riscv/boot/dts/thead/th1520-lichee-pi-4a.dts |  5 +++++
> >
> > Didn't you say there was a flash on one of these two boards?
> Yes, there is a SPI nor flash pad left unpopulated on the bottom side
> of the LicheePi 4a
> carrier board. https://wiki.sipeed.com/hardware/en/lichee/th1520/lpi4a/2_unbox.html#Board-hardware-overview
> notice the reserved pad in the bottom part of the image.

Ah right, unpopulated. That makes sense, thanks.

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

Re: [PATCH v2 3/3] riscv: dts: thead: add basic spi node

[Drew Fustini]

On Wed, Jul 03, 2024 at 03:45:37PM +0100, Conor Dooley wrote:
> Kanak, Drew,
> 
> On Mon, Jul 01, 2024 at 05:43:54PM +0530, Kanak Shilledar wrote:
> > created spi0 node with fixed clock. the spi0 node
> > uses synopsis designware driver and has the following
> > compatible "snps,dw-apb-ssi". the spi0 node is connected
> > to a SPI NOR flash pad which is left unpopulated on the back
> > side of the board.
> > 
> > Signed-off-by: Kanak Shilledar <kanakshilledar@gmail.com>
> > ---
> > Changes in v2:
> > - Separated from a single patch file
> > ---
> >  .../boot/dts/thead/th1520-beaglev-ahead.dts      |  9 +++++++++
> >  .../boot/dts/thead/th1520-lichee-module-4a.dtsi  |  4 ++++
> >  .../riscv/boot/dts/thead/th1520-lichee-pi-4a.dts |  5 +++++
> 
> Didn't you say there was a flash on one of these two boards?
> 
> >  arch/riscv/boot/dts/thead/th1520.dtsi            | 16 ++++++++++++++++
> >  4 files changed, 34 insertions(+)
> > 
> > diff --git a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > index d9b4de9e4757..3103b74e0288 100644
> > --- a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > +++ b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > @@ -17,6 +17,7 @@ aliases {
> >  		gpio1 = &gpio1;
> >  		gpio2 = &gpio2;
> >  		gpio3 = &gpio3;
> > +		spi0 = &spi0;
> 
> "spi" would sort after "serial".
> 
> >  		serial0 = &uart0;
> >  		serial1 = &uart1;
> >  		serial2 = &uart2;
> > @@ -52,6 +53,10 @@ &sdhci_clk {
> >  	clock-frequency = <198000000>;
> >  };
> >  
> > +&spi_clk {
> > +	clock-frequency = <396000000>;
> > +};
> 
> I'm pretty sceptical about adding more of these fixed clocks, rather
> than waiting for the clock driver. Drew, what do you think? Should we
> just add one more to your fixup list or would you rather delay? Guess it
> depends on how long more you think that clock driver is likely to take.

I think the clk driver [1] is in good shape but it has not been reviewed
by the clk maintainer yet. Thus it is hard to predict any timeline for
it getting merged.

SPI support doesn't require any driver changes so I'd be inclined to
allow the fixed clock in this case. It will be simple to change it over
to a real clock once the clk driver is upstream.

Acked-by: Drew Fustini <drew@pdp7.com>

Thanks,
Drew

Re: [PATCH v2 3/3] riscv: dts: thead: add basic spi node

[Drew Fustini]

On Thu, Jul 04, 2024 at 09:59:20AM -0700, Drew Fustini wrote:
> On Wed, Jul 03, 2024 at 03:45:37PM +0100, Conor Dooley wrote:
> > Kanak, Drew,
> > 
> > On Mon, Jul 01, 2024 at 05:43:54PM +0530, Kanak Shilledar wrote:
> > > created spi0 node with fixed clock. the spi0 node
> > > uses synopsis designware driver and has the following
> > > compatible "snps,dw-apb-ssi". the spi0 node is connected
> > > to a SPI NOR flash pad which is left unpopulated on the back
> > > side of the board.
> > > 
> > > Signed-off-by: Kanak Shilledar <kanakshilledar@gmail.com>
> > > ---
> > > Changes in v2:
> > > - Separated from a single patch file
> > > ---
> > >  .../boot/dts/thead/th1520-beaglev-ahead.dts      |  9 +++++++++
> > >  .../boot/dts/thead/th1520-lichee-module-4a.dtsi  |  4 ++++
> > >  .../riscv/boot/dts/thead/th1520-lichee-pi-4a.dts |  5 +++++
> > 
> > Didn't you say there was a flash on one of these two boards?
> > 
> > >  arch/riscv/boot/dts/thead/th1520.dtsi            | 16 ++++++++++++++++
> > >  4 files changed, 34 insertions(+)
> > > 
> > > diff --git a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > > index d9b4de9e4757..3103b74e0288 100644
> > > --- a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > > +++ b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > > @@ -17,6 +17,7 @@ aliases {
> > >  		gpio1 = &gpio1;
> > >  		gpio2 = &gpio2;
> > >  		gpio3 = &gpio3;
> > > +		spi0 = &spi0;
> > 
> > "spi" would sort after "serial".
> > 
> > >  		serial0 = &uart0;
> > >  		serial1 = &uart1;
> > >  		serial2 = &uart2;
> > > @@ -52,6 +53,10 @@ &sdhci_clk {
> > >  	clock-frequency = <198000000>;
> > >  };
> > >  
> > > +&spi_clk {
> > > +	clock-frequency = <396000000>;
> > > +};
> > 
> > I'm pretty sceptical about adding more of these fixed clocks, rather
> > than waiting for the clock driver. Drew, what do you think? Should we
> > just add one more to your fixup list or would you rather delay? Guess it
> > depends on how long more you think that clock driver is likely to take.
> 
> I think the clk driver [1] is in good shape but it has not been reviewed
> by the clk maintainer yet. Thus it is hard to predict any timeline for
> it getting merged.
> 
> SPI support doesn't require any driver changes so I'd be inclined to
> allow the fixed clock in this case. It will be simple to change it over
> to a real clock once the clk driver is upstream.
> 
> Acked-by: Drew Fustini <drew@pdp7.com>
> 
> Thanks,
> Drew

Sorry, I meant to include the link to the most recent clk series:
https://lore.kernel.org/linux-riscv/20240623-th1520-clk-v2-0-ad8d6432d9fb@tenstorrent.com/

-Drew

Re: [PATCH v2 3/3] riscv: dts: thead: add basic spi node

[Kanak Shilledar]

> > > > --- a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > > > +++ b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > > > @@ -17,6 +17,7 @@ aliases {
> > > >           gpio1 = &gpio1;
> > > >           gpio2 = &gpio2;
> > > >           gpio3 = &gpio3;
> > > > +         spi0 = &spi0;
> > >
> > > "spi" would sort after "serial".
> > >
> > > >           serial0 = &uart0;
> > > >           serial1 = &uart1;
> > > >           serial2 = &uart2;

> > > I'm pretty sceptical about adding more of these fixed clocks, rather
> > > than waiting for the clock driver. Drew, what do you think? Should we
> > > just add one more to your fixup list or would you rather delay? Guess it
> > > depends on how long more you think that clock driver is likely to take.
> >
> > I think the clk driver [1] is in good shape but it has not been reviewed
> > by the clk maintainer yet. Thus it is hard to predict any timeline for
> > it getting merged.
> >
> > SPI support doesn't require any driver changes so I'd be inclined to
> > allow the fixed clock in this case. It will be simple to change it over
> > to a real clock once the clk driver is upstream.
> >
> > Acked-by: Drew Fustini <drew@pdp7.com>

Alright! I do the suggested edits and send a new patch.

> > Thanks,
> > Drew
>
> Sorry, I meant to include the link to the most recent clk series:
> https://lore.kernel.org/linux-riscv/20240623-th1520-clk-v2-0-ad8d6432d9fb@tenstorrent.com/
>
> -Drew

Thanks and Regards,
Kanak Shilledar

Re: (subset) [PATCH v2 0/3] Add basic SPI support on TH1520

[Mark Brown]

On Mon, 01 Jul 2024 17:43:51 +0530, Kanak Shilledar wrote:
> Implemented basic SPI support for TH1520 SoC. There are two SPIs reserved
> on the LicheePi4A, one on the SPI Flash pads that are blanked out on the
> back, and one on the pins. I implemented the one connected to the pad.
> 
> It is using a fixed clock of 396MHz. The address and clock frequency was
> referenced from the TH1520 System Reference Manual [1].
> 
> [...]

Applied to

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

Thanks!

[1/3] dt-bindings: spi: snps,dw-apb-ssi.yaml: update compatible property
      commit: 6c387fb263363347185bd7a213ad175c174d35dc

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