[PATCH 00/10] spi: Adding support for Microchip Sparx5 SoC

[PATCH 00/10] spi: Adding support for Microchip Sparx5 SoC

[Lars Povlsen]

This is an add-on series to the main SoC Sparx5 series
(Message-ID: <20200513125532.24585-1-lars.povlsen@microchip.com>).

The series add support for Sparx5 on top of the existing
ocelot/jaguar2 spi driver.

It spins off the existing support for the MSCC platforms into a
separate driver, as adding new platforms from the MSCC/Microchip
product lines will further complicate (clutter) the original driver.

New YAML dt-bindings are provided for the resulting driver.

It is expected that the DT patches are to be taken directly by the arm-soc
maintainers.

Lars Povlsen (10):
  spi: dw: Add support for polled operation via no IRQ specified in DT
  spi: dw: Add support for RX sample delay register
  spi: dw: Add support for client driver memory operations
  dt-bindings: spi: Add bindings for spi-dw-mchp
  spi: spi-dw-mmio: Spin off MSCC platforms into spi-dw-mchp
  dt-bindings: spi: spi-dw-mchp: Add Sparx5 support
  spi: spi-dw-mchp: Add Sparx5 support
  arm64: dts: sparx5: Add SPI controller
  arm64: dts: sparx5: Add spi-nor support
  arm64: dts: sparx5: Add spi-nand devices

 .../bindings/spi/mscc,ocelot-spi.yaml         |  89 ++++
 .../bindings/spi/snps,dw-apb-ssi.txt          |   7 +-
 MAINTAINERS                                   |   2 +
 arch/arm64/boot/dts/microchip/sparx5.dtsi     |  37 ++
 .../boot/dts/microchip/sparx5_pcb125.dts      |  16 +
 .../boot/dts/microchip/sparx5_pcb134.dts      |  22 +
 .../dts/microchip/sparx5_pcb134_board.dtsi    |   9 +
 .../boot/dts/microchip/sparx5_pcb135.dts      |  23 +
 .../dts/microchip/sparx5_pcb135_board.dtsi    |   9 +
 arch/mips/configs/generic/board-ocelot.config |   2 +-
 drivers/spi/Kconfig                           |   7 +
 drivers/spi/Makefile                          |   1 +
 drivers/spi/spi-dw-mchp.c                     | 399 ++++++++++++++++++
 drivers/spi/spi-dw-mmio.c                     |  93 ----
 drivers/spi/spi-dw.c                          |  31 +-
 drivers/spi/spi-dw.h                          |   4 +
 16 files changed, 644 insertions(+), 107 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
 create mode 100644 drivers/spi/spi-dw-mchp.c

--
2.26.2

[PATCH 01/10] spi: dw: Add support for polled operation via no IRQ specified in DT

[Lars Povlsen]

With this change a SPI controller can be added without having a IRQ
associated, and causing all transfers to be polled. For SPI controllers
without DMA, this can significantly improve performance by less
interrupt handling overhead.

Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 drivers/spi/spi-dw.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index 31e3f866d11a7..e572eb34a3c1a 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -19,6 +19,8 @@
 #include <linux/debugfs.h>
 #endif

+#define VALID_IRQ(i) (i >= 0)
+
 /* Slave spi_dev related */
 struct chip_data {
 	u8 tmode;		/* TR/TO/RO/EEPROM */
@@ -359,7 +361,7 @@ static int dw_spi_transfer_one(struct spi_controller *master,
 			spi_enable_chip(dws, 1);
 			return ret;
 		}
-	} else if (!chip->poll_mode) {
+	} else if (!chip->poll_mode && VALID_IRQ(dws->irq)) {
 		txlevel = min_t(u16, dws->fifo_len / 2, dws->len / dws->n_bytes);
 		dw_writel(dws, DW_SPI_TXFLTR, txlevel);

@@ -379,7 +381,7 @@ static int dw_spi_transfer_one(struct spi_controller *master,
 			return ret;
 	}

-	if (chip->poll_mode)
+	if (chip->poll_mode || !VALID_IRQ(dws->irq))
 		return poll_transfer(dws);

 	return 1;
@@ -487,11 +489,13 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)

 	spi_controller_set_devdata(master, dws);

-	ret = request_irq(dws->irq, dw_spi_irq, IRQF_SHARED, dev_name(dev),
-			  master);
-	if (ret < 0) {
-		dev_err(dev, "can not get IRQ\n");
-		goto err_free_master;
+	if (VALID_IRQ(dws->irq)) {
+		ret = request_irq(dws->irq, dw_spi_irq, IRQF_SHARED,
+				  dev_name(dev), master);
+		if (ret < 0) {
+			dev_err(dev, "can not get IRQ\n");
+			goto err_free_master;
+		}
 	}

 	master->use_gpio_descriptors = true;
@@ -539,7 +543,8 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
 	if (dws->dma_ops && dws->dma_ops->dma_exit)
 		dws->dma_ops->dma_exit(dws);
 	spi_enable_chip(dws, 0);
-	free_irq(dws->irq, master);
+	if (VALID_IRQ(dws->irq))
+		free_irq(dws->irq, master);
 err_free_master:
 	spi_controller_put(master);
 	return ret;
--
2.26.2

Re: [PATCH 01/10] spi: dw: Add support for polled operation via no IRQ specified in DT

[Andy Shevchenko]

On Wed, May 13, 2020 at 5:03 PM Lars Povlsen <lars.povlsen@microchip.com> wrote:
>
> With this change a SPI controller can be added without having a IRQ
> associated, and causing all transfers to be polled. For SPI controllers
> without DMA, this can significantly improve performance by less
> interrupt handling overhead.

...

> +#define VALID_IRQ(i) (i >= 0)

drivers/rtc/rtc-cmos.c:95:#define is_valid_irq(n)               ((n) > 0)

Candidate to be in include/linux/irq.h ?

...

> +       if (VALID_IRQ(dws->irq))
> +               free_irq(dws->irq, master);

Isn't free_irq() aware of invalid ones (not found IRQ in the tree or
any other backend container won't do anything)?


>  err_free_master:
>         spi_controller_put(master);
>         return ret;
> --
> 2.26.2



--
With Best Regards,
Andy Shevchenko

Re: [PATCH 01/10] spi: dw: Add support for polled operation via no IRQ specified in DT

[Lars Povlsen]

Andy Shevchenko writes:

> On Wed, May 13, 2020 at 5:03 PM Lars Povlsen <lars.povlsen@microchip.com> wrote:
>>
>> With this change a SPI controller can be added without having a IRQ
>> associated, and causing all transfers to be polled. For SPI controllers
>> without DMA, this can significantly improve performance by less
>> interrupt handling overhead.
>
> ...
>
>> +#define VALID_IRQ(i) (i >= 0)
>
> drivers/rtc/rtc-cmos.c:95:#define is_valid_irq(n)               ((n) > 0)
>
> Candidate to be in include/linux/irq.h ?
>
> ...
>
>> +       if (VALID_IRQ(dws->irq))
>> +               free_irq(dws->irq, master);
>
> Isn't free_irq() aware of invalid ones (not found IRQ in the tree or
> any other backend container won't do anything)?
>
>
>>  err_free_master:
>>         spi_controller_put(master);
>>         return ret;
>> --
>> 2.26.2

I'll rework this with Serge.

Thank you!

-- 
Lars Povlsen,
Microchip

Re: [PATCH 01/10] spi: dw: Add support for polled operation via no IRQ specified in DT

[Serge Semin]

Hi Mark

On Wed, May 13, 2020 at 03:20:50PM +0100, Mark Brown wrote:
> On Wed, May 13, 2020 at 04:00:22PM +0200, Lars Povlsen wrote:
> > With this change a SPI controller can be added without having a IRQ
> > associated, and causing all transfers to be polled. For SPI controllers
> > without DMA, this can significantly improve performance by less
> > interrupt handling overhead.
> 
> This overlaps substantially with some work that Serge Semin (CCed) has
> in progress, please coordinate with him.

Thanks for copying me these mails. I haven't been Cc'ed in the series and
hasn't been subscribed to the SPI mailing list, so I would have definitely
missed that.

I would like to coordinate my efforts with Lars. I'll have the patchset reviewed
soon in addition providing my comments/suggestions of how to make it useful for
both mine and Lars solution.

One thing I can tell about the mem_ops he implemented, is that they aren't
mem_ops, but dirmap (as you remember it's also implemented in my code, but with
alignment specific), and the exec_mem_op partly consists of a code, which belong
to the supports_op() callback. The rest of my comments will be inlined in the
patches.

-Sergey

Re: [PATCH 01/10] spi: dw: Add support for polled operation via no IRQ specified in DT

[Lars Povlsen]

Serge Semin writes:

> Hi Mark
>
> On Wed, May 13, 2020 at 03:20:50PM +0100, Mark Brown wrote:
>> On Wed, May 13, 2020 at 04:00:22PM +0200, Lars Povlsen wrote:
>> > With this change a SPI controller can be added without having a IRQ
>> > associated, and causing all transfers to be polled. For SPI controllers
>> > without DMA, this can significantly improve performance by less
>> > interrupt handling overhead.
>>
>> This overlaps substantially with some work that Serge Semin (CCed) has
>> in progress, please coordinate with him.
>
> Thanks for copying me these mails. I haven't been Cc'ed in the series and
> hasn't been subscribed to the SPI mailing list, so I would have definitely
> missed that.
>
> I would like to coordinate my efforts with Lars. I'll have the patchset reviewed
> soon in addition providing my comments/suggestions of how to make it useful for
> both mine and Lars solution.

Serge - thanks for taking on this.

Note that my primary concern now is to get Sparx5 upstreamed. The
mem_ops (or dirmap) and polled mode are both performance enhancements,
which can be pulled from my series if it creates too much noise. I can
then add the necessary on top of your work/current kernel at a later
time.

> One thing I can tell about the mem_ops he implemented, is that they aren't
> mem_ops, but dirmap (as you remember it's also implemented in my code, but with
> alignment specific), and the exec_mem_op partly consists of a code, which belong
> to the supports_op() callback. The rest of my comments will be inlined in the
> patches.
>
> -Sergey

-- 
Lars Povlsen,
Microchip

Re: [PATCH 01/10] spi: dw: Add support for polled operation via no IRQ specified in DT

[Lars Povlsen]

On 13/05/20 15:37, Mark Brown wrote:
> Date: Wed, 13 May 2020 15:37:53 +0100
> From: Mark Brown <broonie@kernel.org>
> To: Lars Povlsen <lars.povlsen@microchip.com>
> Cc: SoC Team <soc@kernel.org>, Microchip Linux Driver Support
>  <UNGLinuxDriver@microchip.com>, linux-spi@vger.kernel.org,
>  devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
>  linux-arm-kernel@lists.infradead.org, Alexandre Belloni
>  <alexandre.belloni@bootlin.com>
> Subject: Re: [PATCH 01/10] spi: dw: Add support for polled operation via no
>  IRQ specified in DT
> User-Agent: Mutt/1.10.1 (2018-07-13)
> 
> On Wed, May 13, 2020 at 04:00:22PM +0200, Lars Povlsen wrote:
> 
> > +#define VALID_IRQ(i) (i >= 0)
> > +
> 
> This isn't something that should be defined by an individual driver, it
> should be in a generic header.

Thanks, I will work with Serge on getting this integrated right.

---Lars

Re: [PATCH 01/10] spi: dw: Add support for polled operation via no IRQ specified in DT

[Serge Semin]

On Wed, May 13, 2020 at 04:00:22PM +0200, Lars Povlsen wrote:
> With this change a SPI controller can be added without having a IRQ
> associated, and causing all transfers to be polled. For SPI controllers
> without DMA, this can significantly improve performance by less
> interrupt handling overhead.
> 
> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> ---
>  drivers/spi/spi-dw.c | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
> index 31e3f866d11a7..e572eb34a3c1a 100644
> --- a/drivers/spi/spi-dw.c
> +++ b/drivers/spi/spi-dw.c
> @@ -19,6 +19,8 @@
>  #include <linux/debugfs.h>
>  #endif
> 

> +#define VALID_IRQ(i) (i >= 0)

Mark and Andy are right. It is a good candidate to be in a generic IRQ-related
code as Anyd suggested:

> > drivers/rtc/rtc-cmos.c:95:#define is_valid_irq(n)               ((n) > 0)
> > Candidate to be in include/linux/irq.h ?

So if you feel like to author additional useful patch integrated into the
kernel, this one is a good chance for it.

> +
>  /* Slave spi_dev related */
>  struct chip_data {
>  	u8 tmode;		/* TR/TO/RO/EEPROM */
> @@ -359,7 +361,7 @@ static int dw_spi_transfer_one(struct spi_controller *master,
>  			spi_enable_chip(dws, 1);
>  			return ret;
>  		}
> -	} else if (!chip->poll_mode) {
> +	} else if (!chip->poll_mode && VALID_IRQ(dws->irq)) {
>  		txlevel = min_t(u16, dws->fifo_len / 2, dws->len / dws->n_bytes);
>  		dw_writel(dws, DW_SPI_TXFLTR, txlevel);
> 
> @@ -379,7 +381,7 @@ static int dw_spi_transfer_one(struct spi_controller *master,
>  			return ret;
>  	}
> 
> -	if (chip->poll_mode)
> +	if (chip->poll_mode || !VALID_IRQ(dws->irq))
>  		return poll_transfer(dws);

Please note. The chip->poll and the poll_transfer() methods've been discarded
from the driver, since commit 1ceb09717e98 ("spi: dw: remove cs_control and
poll_mode members from chip_data"). So you gonna have to get the
poll_transfer-like method back.

-Sergey

> 
>  	return 1;
> @@ -487,11 +489,13 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
> 
>  	spi_controller_set_devdata(master, dws);
> 
> -	ret = request_irq(dws->irq, dw_spi_irq, IRQF_SHARED, dev_name(dev),
> -			  master);
> -	if (ret < 0) {
> -		dev_err(dev, "can not get IRQ\n");
> -		goto err_free_master;
> +	if (VALID_IRQ(dws->irq)) {
> +		ret = request_irq(dws->irq, dw_spi_irq, IRQF_SHARED,
> +				  dev_name(dev), master);
> +		if (ret < 0) {
> +			dev_err(dev, "can not get IRQ\n");
> +			goto err_free_master;
> +		}
>  	}
> 
>  	master->use_gpio_descriptors = true;
> @@ -539,7 +543,8 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
>  	if (dws->dma_ops && dws->dma_ops->dma_exit)
>  		dws->dma_ops->dma_exit(dws);
>  	spi_enable_chip(dws, 0);
> -	free_irq(dws->irq, master);
> +	if (VALID_IRQ(dws->irq))
> +		free_irq(dws->irq, master);
>  err_free_master:
>  	spi_controller_put(master);
>  	return ret;
> --
> 2.26.2
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 01/10] spi: dw: Add support for polled operation via no IRQ specified in DT

[Lars Povlsen]

Serge Semin writes:

> On Wed, May 13, 2020 at 04:00:22PM +0200, Lars Povlsen wrote:
>> With this change a SPI controller can be added without having a IRQ
>> associated, and causing all transfers to be polled. For SPI controllers
>> without DMA, this can significantly improve performance by less
>> interrupt handling overhead.
>>
>> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>> ---
>>  drivers/spi/spi-dw.c | 21 +++++++++++++--------
>>  1 file changed, 13 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
>> index 31e3f866d11a7..e572eb34a3c1a 100644
>> --- a/drivers/spi/spi-dw.c
>> +++ b/drivers/spi/spi-dw.c
>> @@ -19,6 +19,8 @@
>>  #include <linux/debugfs.h>
>>  #endif
>>
>
>> +#define VALID_IRQ(i) (i >= 0)
>
> Mark and Andy are right. It is a good candidate to be in a generic IRQ-related
> code as Anyd suggested:
>
>> > drivers/rtc/rtc-cmos.c:95:#define is_valid_irq(n)               ((n) > 0)
>> > Candidate to be in include/linux/irq.h ?
>
> So if you feel like to author additional useful patch integrated into the
> kernel, this one is a good chance for it.
>

Yeah, but with the poll_transfer() gone below, I think I'll settle on
just getting this into the current framework. Optimization (as this is)
has less priority. I'll put this on the back burner.

>> +
>>  /* Slave spi_dev related */
>>  struct chip_data {
>>       u8 tmode;               /* TR/TO/RO/EEPROM */
>> @@ -359,7 +361,7 @@ static int dw_spi_transfer_one(struct spi_controller *master,
>>                       spi_enable_chip(dws, 1);
>>                       return ret;
>>               }
>> -     } else if (!chip->poll_mode) {
>> +     } else if (!chip->poll_mode && VALID_IRQ(dws->irq)) {
>>               txlevel = min_t(u16, dws->fifo_len / 2, dws->len / dws->n_bytes);
>>               dw_writel(dws, DW_SPI_TXFLTR, txlevel);
>>
>> @@ -379,7 +381,7 @@ static int dw_spi_transfer_one(struct spi_controller *master,
>>                       return ret;
>>       }
>>
>> -     if (chip->poll_mode)
>> +     if (chip->poll_mode || !VALID_IRQ(dws->irq))
>>               return poll_transfer(dws);
>
> Please note. The chip->poll and the poll_transfer() methods've been discarded
> from the driver, since commit 1ceb09717e98 ("spi: dw: remove cs_control and
> poll_mode members from chip_data"). So you gonna have to get the
> poll_transfer-like method back.
>
> -Sergey
>
>>
>>       return 1;
>> @@ -487,11 +489,13 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
>>
>>       spi_controller_set_devdata(master, dws);
>>
>> -     ret = request_irq(dws->irq, dw_spi_irq, IRQF_SHARED, dev_name(dev),
>> -                       master);
>> -     if (ret < 0) {
>> -             dev_err(dev, "can not get IRQ\n");
>> -             goto err_free_master;
>> +     if (VALID_IRQ(dws->irq)) {
>> +             ret = request_irq(dws->irq, dw_spi_irq, IRQF_SHARED,
>> +                               dev_name(dev), master);
>> +             if (ret < 0) {
>> +                     dev_err(dev, "can not get IRQ\n");
>> +                     goto err_free_master;
>> +             }
>>       }
>>
>>       master->use_gpio_descriptors = true;
>> @@ -539,7 +543,8 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
>>       if (dws->dma_ops && dws->dma_ops->dma_exit)
>>               dws->dma_ops->dma_exit(dws);
>>       spi_enable_chip(dws, 0);
>> -     free_irq(dws->irq, master);
>> +     if (VALID_IRQ(dws->irq))
>> +             free_irq(dws->irq, master);
>>  err_free_master:
>>       spi_controller_put(master);
>>       return ret;
>> --
>> 2.26.2
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Lars Povlsen,
Microchip

[PATCH 02/10] spi: dw: Add support for RX sample delay register

[Lars Povlsen]

This add support for the RX_SAMPLE_DLY register. If enabled in the
Designware IP, it allows tuning of the rx data signal by means of an
internal rx sample fifo.

The register is located at offset 0xf0, and if the option is not
enabled in the IP, changing the register will have no effect.

Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 drivers/spi/spi-dw.c | 7 +++++++
 drivers/spi/spi-dw.h | 2 ++
 2 files changed, 9 insertions(+)

diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index e572eb34a3c1a..32997f28fa5bb 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -81,6 +81,9 @@ static ssize_t dw_spi_show_regs(struct file *file, char __user *user_buf,
 			"DMATDLR: \t0x%08x\n", dw_readl(dws, DW_SPI_DMATDLR));
 	len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len,
 			"DMARDLR: \t0x%08x\n", dw_readl(dws, DW_SPI_DMARDLR));
+	len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len,
+			 "RX_SAMPLE_DLY: \t0x%08x\n",
+			 dw_readl(dws, DW_SPI_RX_SAMPLE_DLY));
 	len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len,
 			"=================================\n");

@@ -315,6 +318,10 @@ static int dw_spi_transfer_one(struct spi_controller *master,
 		spi_set_clk(dws, chip->clk_div);
 	}

+	/* Apply RX sample delay, iff requested (nonzero) */
+	if (dws->rx_sample_dly)
+		dw_writel(dws, DW_SPI_RX_SAMPLE_DLY, dws->rx_sample_dly);
+
 	dws->n_bytes = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);
 	dws->dma_width = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);

diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 1bf5713e047d3..ed6e47b3f50da 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -31,6 +31,7 @@
 #define DW_SPI_IDR			0x58
 #define DW_SPI_VERSION			0x5c
 #define DW_SPI_DR			0x60
+#define DW_SPI_RX_SAMPLE_DLY		0xf0
 #define DW_SPI_CS_OVERRIDE		0xf4

 /* Bit fields in CTRLR0 */
@@ -111,6 +112,7 @@ struct dw_spi {

 	int			cs_override;
 	u32			reg_io_width;	/* DR I/O width in bytes */
+	u8			rx_sample_dly;	/* RX fifo tuning (option) */
 	u16			bus_num;
 	u16			num_cs;		/* supported slave numbers */
 	void (*set_cs)(struct spi_device *spi, bool enable);
--
2.26.2

Re: [PATCH 02/10] spi: dw: Add support for RX sample delay register

[Serge Semin]

On Wed, May 13, 2020 at 04:00:23PM +0200, Lars Povlsen wrote:
> This add support for the RX_SAMPLE_DLY register. If enabled in the
> Designware IP, it allows tuning of the rx data signal by means of an
> internal rx sample fifo.
> 
> The register is located at offset 0xf0, and if the option is not
> enabled in the IP, changing the register will have no effect.
> 
> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> ---
>  drivers/spi/spi-dw.c | 7 +++++++
>  drivers/spi/spi-dw.h | 2 ++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
> index e572eb34a3c1a..32997f28fa5bb 100644
> --- a/drivers/spi/spi-dw.c
> +++ b/drivers/spi/spi-dw.c
> @@ -81,6 +81,9 @@ static ssize_t dw_spi_show_regs(struct file *file, char __user *user_buf,
>  			"DMATDLR: \t0x%08x\n", dw_readl(dws, DW_SPI_DMATDLR));
>  	len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len,
>  			"DMARDLR: \t0x%08x\n", dw_readl(dws, DW_SPI_DMARDLR));

> +	len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len,
> +			 "RX_SAMPLE_DLY: \t0x%08x\n",
> +			 dw_readl(dws, DW_SPI_RX_SAMPLE_DLY));

debugfs_reg32 interface is now utilized in the driver to dump the registers
state. So this will have to be converted to just a new entry in the
dw_spi_dbgfs_regs array.

>  	len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len,
>  			"=================================\n");
> 
> @@ -315,6 +318,10 @@ static int dw_spi_transfer_one(struct spi_controller *master,
>  		spi_set_clk(dws, chip->clk_div);
>  	}
> 

> +	/* Apply RX sample delay, iff requested (nonzero) */

s/iff/if

> +	if (dws->rx_sample_dly)
> +		dw_writel(dws, DW_SPI_RX_SAMPLE_DLY, dws->rx_sample_dly);
> +
>  	dws->n_bytes = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);
>  	dws->dma_width = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);
> 
> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> index 1bf5713e047d3..ed6e47b3f50da 100644
> --- a/drivers/spi/spi-dw.h
> +++ b/drivers/spi/spi-dw.h
> @@ -31,6 +31,7 @@
>  #define DW_SPI_IDR			0x58
>  #define DW_SPI_VERSION			0x5c
>  #define DW_SPI_DR			0x60
> +#define DW_SPI_RX_SAMPLE_DLY		0xf0
>  #define DW_SPI_CS_OVERRIDE		0xf4
> 
>  /* Bit fields in CTRLR0 */
> @@ -111,6 +112,7 @@ struct dw_spi {
> 
>  	int			cs_override;
>  	u32			reg_io_width;	/* DR I/O width in bytes */

> +	u8			rx_sample_dly;	/* RX fifo tuning (option) */

This doesn't seem like a good place for this parameter. The sample delay is
SPI-slave specific. So as I see it, the parameter should be moved to the
chip_data.

-Sergey

>  	u16			bus_num;
>  	u16			num_cs;		/* supported slave numbers */
>  	void (*set_cs)(struct spi_device *spi, bool enable);
> --
> 2.26.2
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 02/10] spi: dw: Add support for RX sample delay register

[Lars Povlsen]

Serge Semin writes:

> On Wed, May 13, 2020 at 04:00:23PM +0200, Lars Povlsen wrote:
>> This add support for the RX_SAMPLE_DLY register. If enabled in the
>> Designware IP, it allows tuning of the rx data signal by means of an
>> internal rx sample fifo.
>>
>> The register is located at offset 0xf0, and if the option is not
>> enabled in the IP, changing the register will have no effect.
>>
>> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>> ---
>>  drivers/spi/spi-dw.c | 7 +++++++
>>  drivers/spi/spi-dw.h | 2 ++
>>  2 files changed, 9 insertions(+)
>>
>> diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
>> index e572eb34a3c1a..32997f28fa5bb 100644
>> --- a/drivers/spi/spi-dw.c
>> +++ b/drivers/spi/spi-dw.c
>> @@ -81,6 +81,9 @@ static ssize_t dw_spi_show_regs(struct file *file, char __user *user_buf,
>>                       "DMATDLR: \t0x%08x\n", dw_readl(dws, DW_SPI_DMATDLR));
>>       len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len,
>>                       "DMARDLR: \t0x%08x\n", dw_readl(dws, DW_SPI_DMARDLR));
>
>> +     len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len,
>> +                      "RX_SAMPLE_DLY: \t0x%08x\n",
>> +                      dw_readl(dws, DW_SPI_RX_SAMPLE_DLY));
>
> debugfs_reg32 interface is now utilized in the driver to dump the registers
> state. So this will have to be converted to just a new entry in the
> dw_spi_dbgfs_regs array.
>

Ok, I'll have a look at this.

>>       len += scnprintf(buf + len, SPI_REGS_BUFSIZE - len,
>>                       "=================================\n");
>>
>> @@ -315,6 +318,10 @@ static int dw_spi_transfer_one(struct spi_controller *master,
>>               spi_set_clk(dws, chip->clk_div);
>>       }
>>
>
>> +     /* Apply RX sample delay, iff requested (nonzero) */
>
> s/iff/if
>
>> +     if (dws->rx_sample_dly)
>> +             dw_writel(dws, DW_SPI_RX_SAMPLE_DLY, dws->rx_sample_dly);
>> +
>>       dws->n_bytes = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);
>>       dws->dma_width = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);
>>
>> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
>> index 1bf5713e047d3..ed6e47b3f50da 100644
>> --- a/drivers/spi/spi-dw.h
>> +++ b/drivers/spi/spi-dw.h
>> @@ -31,6 +31,7 @@
>>  #define DW_SPI_IDR                   0x58
>>  #define DW_SPI_VERSION                       0x5c
>>  #define DW_SPI_DR                    0x60
>> +#define DW_SPI_RX_SAMPLE_DLY         0xf0
>>  #define DW_SPI_CS_OVERRIDE           0xf4
>>
>>  /* Bit fields in CTRLR0 */
>> @@ -111,6 +112,7 @@ struct dw_spi {
>>
>>       int                     cs_override;
>>       u32                     reg_io_width;   /* DR I/O width in bytes */
>
>> +     u8                      rx_sample_dly;  /* RX fifo tuning (option) */
>
> This doesn't seem like a good place for this parameter. The sample delay is
> SPI-slave specific. So as I see it, the parameter should be moved to the
> chip_data.
>

Yes, I got that in other comments, and you are right I guess.

I will apply that approach of having rx_sample_dly per SPI slave.

---Lars

> -Sergey
>
>>       u16                     bus_num;
>>       u16                     num_cs;         /* supported slave numbers */
>>       void (*set_cs)(struct spi_device *spi, bool enable);
>> --
>> 2.26.2
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Lars Povlsen,
Microchip

[PATCH 03/10] spi: dw: Add support for client driver memory operations

[Lars Povlsen]

This minor change allow dw-spi drivers to register
spi_controller_mem_ops memory operations if the platform supports it.

Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 drivers/spi/spi-dw.c | 3 +++
 drivers/spi/spi-dw.h | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index 32997f28fa5bb..d0c611c42421e 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -527,6 +527,9 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
 	/* Basic HW init */
 	spi_hw_init(dev, dws);

+	/* Memory ops? */
+	master->mem_ops = dws->mem_ops;
+
 	if (dws->dma_ops && dws->dma_ops->dma_init) {
 		ret = dws->dma_ops->dma_init(dws);
 		if (ret) {
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index ed6e47b3f50da..8ecccbde10a20 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -3,6 +3,7 @@
 #define DW_SPI_HEADER_H

 #include <linux/io.h>
+#include <linux/spi/spi-mem.h>
 #include <linux/scatterlist.h>

 /* Register offsets */
@@ -116,6 +117,7 @@ struct dw_spi {
 	u16			bus_num;
 	u16			num_cs;		/* supported slave numbers */
 	void (*set_cs)(struct spi_device *spi, bool enable);
+	const struct spi_controller_mem_ops *mem_ops;

 	/* Current message transfer state info */
 	size_t			len;
--
2.26.2

[PATCH 04/10] dt-bindings: spi: Add bindings for spi-dw-mchp

[Lars Povlsen]

This add DT bindings for the Microsemi/Microchip SPI controller used
in various SoC's. It describes the "mscc,ocelot-spi" and
"mscc,jaguar2-spi" bindings.

Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 .../bindings/spi/mscc,ocelot-spi.yaml         | 60 +++++++++++++++++++
 .../bindings/spi/snps,dw-apb-ssi.txt          |  7 +--
 MAINTAINERS                                   |  1 +
 3 files changed, 63 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml

diff --git a/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml b/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
new file mode 100644
index 0000000000000..a3ac0fa576553
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/spi/mscc,ocelot-spi.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Microsemi Vcore-III SPI Communication Controller
+
+maintainers:
+  - Alexandre Belloni <alexandre.belloni@bootlin.com>
+  - Lars Povlsen <lars.povlsen@microchip.com>
+
+allOf:
+  - $ref: "spi-controller.yaml#"
+
+description: |
+  The Microsemi Vcore-III SPI controller is a general purpose SPI
+  controller based upon the Designware SPI controller. It uses an 8
+  byte rx/tx fifo.
+
+properties:
+  compatible:
+    enum:
+      - mscc,ocelot-spi
+      - mscc,jaguar2-spi
+
+  interrupts:
+    maxItems: 1
+
+  reg:
+    minItems: 2
+    items:
+      - description: Designware SPI registers
+      - description: CS override registers
+
+  clocks:
+    maxItems: 1
+
+  reg-io-width:
+    description: |
+      The I/O register width (in bytes) implemented by this device.
+    items:
+       enum: [ 2, 4 ]
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+
+examples:
+  - |
+    spi0: spi@101000 {
+      compatible = "mscc,ocelot-spi";
+      #address-cells = <1>;
+      #size-cells = <0>;
+      reg = <0x101000 0x100>, <0x3c 0x18>;
+      interrupts = <9>;
+      clocks = <&ahb_clk>;
+    };
diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
index 3ed08ee9feba4..5e1849be7bae5 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
@@ -1,10 +1,8 @@
 Synopsys DesignWare AMBA 2.0 Synchronous Serial Interface.

 Required properties:
-- compatible : "snps,dw-apb-ssi" or "mscc,<soc>-spi", where soc is "ocelot" or
-  "jaguar2", or "amazon,alpine-dw-apb-ssi"
-- reg : The register base for the controller. For "mscc,<soc>-spi", a second
-  register set is required (named ICPU_CFG:SPI_MST)
+- compatible : "snps,dw-apb-ssi" or "amazon,alpine-dw-apb-ssi"
+- reg : The register base for the controller.
 - interrupts : One interrupt, used by the controller.
 - #address-cells : <1>, as required by generic SPI binding.
 - #size-cells : <0>, also as required by generic SPI binding.
@@ -38,4 +36,3 @@ Example:
 		cs-gpios = <&gpio0 13 0>,
 			   <&gpio0 14 0>;
 	};
-
diff --git a/MAINTAINERS b/MAINTAINERS
index 1db598723a1d8..6472240b8391b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11231,6 +11231,7 @@ L:	linux-mips@vger.kernel.org
 S:	Supported
 F:	Documentation/devicetree/bindings/mips/mscc.txt
 F:	Documentation/devicetree/bindings/power/reset/ocelot-reset.txt
+F:	Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
 F:	arch/mips/boot/dts/mscc/
 F:	arch/mips/configs/generic/board-ocelot.config
 F:	arch/mips/generic/board-ocelot.c
--
2.26.2

Re: [PATCH 04/10] dt-bindings: spi: Add bindings for spi-dw-mchp

[Lars Povlsen]

On 13/05/20 15:52, Mark Brown wrote:
> Date: Wed, 13 May 2020 15:52:13 +0100
> From: Mark Brown <broonie@kernel.org>
> To: Lars Povlsen <lars.povlsen@microchip.com>
> Cc: SoC Team <soc@kernel.org>, Rob Herring <robh+dt@kernel.org>, Microchip
>  Linux Driver Support <UNGLinuxDriver@microchip.com>,
>  linux-spi@vger.kernel.org, devicetree@vger.kernel.org,
>  linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
>  Alexandre Belloni <alexandre.belloni@bootlin.com>, Serge Semin
>  <Sergey.Semin@baikalelectronics.ru>, Serge Semin
>  <fancer.lancer@gmail.com>, Andy Shevchenko
>  <andriy.shevchenko@linux.intel.com>, Wan Ahmad Zainie
>  <wan.ahmad.zainie.wan.mohamad@intel.com>
> Subject: Re: [PATCH 04/10] dt-bindings: spi: Add bindings for spi-dw-mchp
> User-Agent: Mutt/1.10.1 (2018-07-13)
> 
> On Wed, May 13, 2020 at 04:00:25PM +0200, Lars Povlsen wrote:
> > This add DT bindings for the Microsemi/Microchip SPI controller used
> > in various SoC's. It describes the "mscc,ocelot-spi" and
> > "mscc,jaguar2-spi" bindings.
> 
> > Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> > Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> > ---
> >  .../bindings/spi/mscc,ocelot-spi.yaml         | 60 +++++++++++++++++++
> >  .../bindings/spi/snps,dw-apb-ssi.txt          |  7 +--
> 
> That's not what this change does.  It is removing the existing binding
> for Ocelot and Jaguar2 from the free format binding documentation and
> adds some entirely separate YAML bindings for them.  This conflicts with
> competing YAML conversions that both Serge Semin and Wan Ahmad Zainie
> (CCed) have in flight at the minute.  It also doesn't strike me as a
> good idea to fork the bindings, what's the motivation for doing that?

The reason for doing this was due to the fact that I felt adding
Sparx5 support only cluttered the original driver even more.

And since I was adding a new driver, I needed to add bindings for it -
and (re)move the old ones.

I have become aware of Serge and Wan Ahmad's work, and will work out
something on top of that.

Re: [PATCH 04/10] dt-bindings: spi: Add bindings for spi-dw-mchp

[Lars Povlsen]

On 19/05/20 12:58, Mark Brown wrote:
> Date: Tue, 19 May 2020 12:58:29 +0100
> From: Mark Brown <broonie@kernel.org>
> To: Lars Povlsen <lars.povlsen@microchip.com>
> Cc: SoC Team <soc@kernel.org>, Rob Herring <robh+dt@kernel.org>, Microchip
>  Linux Driver Support <UNGLinuxDriver@microchip.com>,
>  linux-spi@vger.kernel.org, devicetree@vger.kernel.org,
>  linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
>  Alexandre Belloni <alexandre.belloni@bootlin.com>, Serge Semin
>  <Sergey.Semin@baikalelectronics.ru>, Serge Semin
>  <fancer.lancer@gmail.com>, Andy Shevchenko
>  <andriy.shevchenko@linux.intel.com>, Wan Ahmad Zainie
>  <wan.ahmad.zainie.wan.mohamad@intel.com>
> Subject: Re: [PATCH 04/10] dt-bindings: spi: Add bindings for spi-dw-mchp
> User-Agent: Mutt/1.10.1 (2018-07-13)
> 
> On Tue, May 19, 2020 at 01:47:39PM +0200, Lars Povlsen wrote:
> > On 13/05/20 15:52, Mark Brown wrote:
> 
> > > On Wed, May 13, 2020 at 04:00:25PM +0200, Lars Povlsen wrote:
> > > > This add DT bindings for the Microsemi/Microchip SPI controller used
> > > > in various SoC's. It describes the "mscc,ocelot-spi" and
> > > > "mscc,jaguar2-spi" bindings.
> 
> > > That's not what this change does.  It is removing the existing binding
> > > for Ocelot and Jaguar2 from the free format binding documentation and
> 
> > The reason for doing this was due to the fact that I felt adding
> > Sparx5 support only cluttered the original driver even more.
> 
> That's not the issue I'm pointing out there.  The issue is that your
> changelog claims that the change does one thing and the change itself
> does something substantially different.

Ok, got it. I'll reword the changelog to be more precise.

Thanks again,

---Lars

Re: [PATCH 04/10] dt-bindings: spi: Add bindings for spi-dw-mchp

[Serge Semin]

On Wed, May 13, 2020 at 04:00:25PM +0200, Lars Povlsen wrote:
> This add DT bindings for the Microsemi/Microchip SPI controller used
> in various SoC's. It describes the "mscc,ocelot-spi" and
> "mscc,jaguar2-spi" bindings.

As I see it, there is no need in this patch at all. Current DT binding file
describes the MSCC version of the DW APB SSI IP pretty well. You can add an
example to the DT schema though with "mscc,ocelot-spi" or "mscc,jaguar2-spi"
compatible string and additional registers range. 

-Sergey

> 
> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> ---
>  .../bindings/spi/mscc,ocelot-spi.yaml         | 60 +++++++++++++++++++
>  .../bindings/spi/snps,dw-apb-ssi.txt          |  7 +--
>  MAINTAINERS                                   |  1 +
>  3 files changed, 63 insertions(+), 5 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml b/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
> new file mode 100644
> index 0000000000000..a3ac0fa576553
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/spi/mscc,ocelot-spi.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Microsemi Vcore-III SPI Communication Controller
> +
> +maintainers:
> +  - Alexandre Belloni <alexandre.belloni@bootlin.com>
> +  - Lars Povlsen <lars.povlsen@microchip.com>
> +
> +allOf:
> +  - $ref: "spi-controller.yaml#"
> +
> +description: |
> +  The Microsemi Vcore-III SPI controller is a general purpose SPI
> +  controller based upon the Designware SPI controller. It uses an 8
> +  byte rx/tx fifo.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - mscc,ocelot-spi
> +      - mscc,jaguar2-spi
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  reg:
> +    minItems: 2
> +    items:
> +      - description: Designware SPI registers
> +      - description: CS override registers
> +
> +  clocks:
> +    maxItems: 1
> +
> +  reg-io-width:
> +    description: |
> +      The I/O register width (in bytes) implemented by this device.
> +    items:
> +       enum: [ 2, 4 ]
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +
> +examples:
> +  - |
> +    spi0: spi@101000 {
> +      compatible = "mscc,ocelot-spi";
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      reg = <0x101000 0x100>, <0x3c 0x18>;
> +      interrupts = <9>;
> +      clocks = <&ahb_clk>;
> +    };
> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> index 3ed08ee9feba4..5e1849be7bae5 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
> @@ -1,10 +1,8 @@
>  Synopsys DesignWare AMBA 2.0 Synchronous Serial Interface.
> 
>  Required properties:
> -- compatible : "snps,dw-apb-ssi" or "mscc,<soc>-spi", where soc is "ocelot" or
> -  "jaguar2", or "amazon,alpine-dw-apb-ssi"
> -- reg : The register base for the controller. For "mscc,<soc>-spi", a second
> -  register set is required (named ICPU_CFG:SPI_MST)
> +- compatible : "snps,dw-apb-ssi" or "amazon,alpine-dw-apb-ssi"
> +- reg : The register base for the controller.
>  - interrupts : One interrupt, used by the controller.
>  - #address-cells : <1>, as required by generic SPI binding.
>  - #size-cells : <0>, also as required by generic SPI binding.
> @@ -38,4 +36,3 @@ Example:
>  		cs-gpios = <&gpio0 13 0>,
>  			   <&gpio0 14 0>;
>  	};
> -
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1db598723a1d8..6472240b8391b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11231,6 +11231,7 @@ L:	linux-mips@vger.kernel.org
>  S:	Supported
>  F:	Documentation/devicetree/bindings/mips/mscc.txt
>  F:	Documentation/devicetree/bindings/power/reset/ocelot-reset.txt
> +F:	Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
>  F:	arch/mips/boot/dts/mscc/
>  F:	arch/mips/configs/generic/board-ocelot.config
>  F:	arch/mips/generic/board-ocelot.c
> --
> 2.26.2
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 04/10] dt-bindings: spi: Add bindings for spi-dw-mchp

[Lars Povlsen]

Serge Semin writes:

> On Wed, May 13, 2020 at 04:00:25PM +0200, Lars Povlsen wrote:
>> This add DT bindings for the Microsemi/Microchip SPI controller used
>> in various SoC's. It describes the "mscc,ocelot-spi" and
>> "mscc,jaguar2-spi" bindings.
>
> As I see it, there is no need in this patch at all. Current DT binding file
> describes the MSCC version of the DW APB SSI IP pretty well. You can add an
> example to the DT schema though with "mscc,ocelot-spi" or "mscc,jaguar2-spi"
> compatible string and additional registers range.
>

Fine by me. I just had the understanding that a YAML binding had to be
given for a new driver.

I will add the bindings to the existing YAML with proper guards.

---Lars

> -Sergey
>
>>
>> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>> ---
>>  .../bindings/spi/mscc,ocelot-spi.yaml         | 60 +++++++++++++++++++
>>  .../bindings/spi/snps,dw-apb-ssi.txt          |  7 +--
>>  MAINTAINERS                                   |  1 +
>>  3 files changed, 63 insertions(+), 5 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml b/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
>> new file mode 100644
>> index 0000000000000..a3ac0fa576553
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
>> @@ -0,0 +1,60 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/spi/mscc,ocelot-spi.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>> +
>> +title: Microsemi Vcore-III SPI Communication Controller
>> +
>> +maintainers:
>> +  - Alexandre Belloni <alexandre.belloni@bootlin.com>
>> +  - Lars Povlsen <lars.povlsen@microchip.com>
>> +
>> +allOf:
>> +  - $ref: "spi-controller.yaml#"
>> +
>> +description: |
>> +  The Microsemi Vcore-III SPI controller is a general purpose SPI
>> +  controller based upon the Designware SPI controller. It uses an 8
>> +  byte rx/tx fifo.
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - mscc,ocelot-spi
>> +      - mscc,jaguar2-spi
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  reg:
>> +    minItems: 2
>> +    items:
>> +      - description: Designware SPI registers
>> +      - description: CS override registers
>> +
>> +  clocks:
>> +    maxItems: 1
>> +
>> +  reg-io-width:
>> +    description: |
>> +      The I/O register width (in bytes) implemented by this device.
>> +    items:
>> +       enum: [ 2, 4 ]
>> +    maxItems: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +
>> +examples:
>> +  - |
>> +    spi0: spi@101000 {
>> +      compatible = "mscc,ocelot-spi";
>> +      #address-cells = <1>;
>> +      #size-cells = <0>;
>> +      reg = <0x101000 0x100>, <0x3c 0x18>;
>> +      interrupts = <9>;
>> +      clocks = <&ahb_clk>;
>> +    };
>> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
>> index 3ed08ee9feba4..5e1849be7bae5 100644
>> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
>> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
>> @@ -1,10 +1,8 @@
>>  Synopsys DesignWare AMBA 2.0 Synchronous Serial Interface.
>>
>>  Required properties:
>> -- compatible : "snps,dw-apb-ssi" or "mscc,<soc>-spi", where soc is "ocelot" or
>> -  "jaguar2", or "amazon,alpine-dw-apb-ssi"
>> -- reg : The register base for the controller. For "mscc,<soc>-spi", a second
>> -  register set is required (named ICPU_CFG:SPI_MST)
>> +- compatible : "snps,dw-apb-ssi" or "amazon,alpine-dw-apb-ssi"
>> +- reg : The register base for the controller.
>>  - interrupts : One interrupt, used by the controller.
>>  - #address-cells : <1>, as required by generic SPI binding.
>>  - #size-cells : <0>, also as required by generic SPI binding.
>> @@ -38,4 +36,3 @@ Example:
>>               cs-gpios = <&gpio0 13 0>,
>>                          <&gpio0 14 0>;
>>       };
>> -
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 1db598723a1d8..6472240b8391b 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -11231,6 +11231,7 @@ L:    linux-mips@vger.kernel.org
>>  S:   Supported
>>  F:   Documentation/devicetree/bindings/mips/mscc.txt
>>  F:   Documentation/devicetree/bindings/power/reset/ocelot-reset.txt
>> +F:   Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
>>  F:   arch/mips/boot/dts/mscc/
>>  F:   arch/mips/configs/generic/board-ocelot.config
>>  F:   arch/mips/generic/board-ocelot.c
>> --
>> 2.26.2
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Lars Povlsen,
Microchip

[PATCH 05/10] spi: spi-dw-mmio: Spin off MSCC platforms into spi-dw-mchp

[Lars Povlsen]

This patch spins off the MSCC platforms into a separate driver, as
adding new platforms from the MSCC/Microchip product lines will
further complicate (clutter) the original driver.

The new 'spi-dw-mchp' driver still builds on the dw-spi foundation.

Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 MAINTAINERS                                   |   1 +
 arch/mips/configs/generic/board-ocelot.config |   2 +-
 drivers/spi/Kconfig                           |   7 +
 drivers/spi/Makefile                          |   1 +
 drivers/spi/spi-dw-mchp.c                     | 232 ++++++++++++++++++
 drivers/spi/spi-dw-mmio.c                     |  93 -------
 6 files changed, 242 insertions(+), 94 deletions(-)
 create mode 100644 drivers/spi/spi-dw-mchp.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 6472240b8391b..de64fd4548697 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2085,6 +2085,7 @@ M:	Steen Hegelund <Steen.Hegelund@microchip.com>
 M:	Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
 L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 F:	arch/arm64/boot/dts/microchip/
+F:	drivers/spi/spi-dw-mchp.c
 N:	sparx5
 S:	Supported

diff --git a/arch/mips/configs/generic/board-ocelot.config b/arch/mips/configs/generic/board-ocelot.config
index 7626f2a75b03f..6dbae76268a49 100644
--- a/arch/mips/configs/generic/board-ocelot.config
+++ b/arch/mips/configs/generic/board-ocelot.config
@@ -38,7 +38,7 @@ CONFIG_I2C_DESIGNWARE_PLATFORM=y
 CONFIG_SPI=y
 CONFIG_SPI_BITBANG=y
 CONFIG_SPI_DESIGNWARE=y
-CONFIG_SPI_DW_MMIO=y
+CONFIG_SPI_DW_MCHP=y
 CONFIG_SPI_SPIDEV=y

 CONFIG_PINCTRL=y
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 741b9140992a8..77eb580b9f51f 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -238,6 +238,13 @@ config SPI_DW_MMIO
 	tristate "Memory-mapped io interface driver for DW SPI core"
 	depends on SPI_DESIGNWARE

+config SPI_DW_MCHP
+	tristate "Memory-mapped io interface driver using DW SPI core of MSCC SoCs"
+	default y if ARCH_SPARX5
+	default y if SOC_VCOREIII
+	select SPI_DESIGNWARE
+	select SPI_DW_MMIO
+
 config SPI_DLN2
        tristate "Diolan DLN-2 USB SPI adapter"
        depends on MFD_DLN2
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 28f601327f8c7..be8a52d90721b 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -37,6 +37,7 @@ obj-$(CONFIG_SPI_DAVINCI)		+= spi-davinci.o
 obj-$(CONFIG_SPI_DLN2)			+= spi-dln2.o
 obj-$(CONFIG_SPI_DESIGNWARE)		+= spi-dw.o
 obj-$(CONFIG_SPI_DW_MMIO)		+= spi-dw-mmio.o
+obj-$(CONFIG_SPI_DW_MCHP)		+= spi-dw-mchp.o
 obj-$(CONFIG_SPI_DW_PCI)		+= spi-dw-midpci.o
 spi-dw-midpci-objs			:= spi-dw-pci.o spi-dw-mid.o
 obj-$(CONFIG_SPI_EFM32)			+= spi-efm32.o
diff --git a/drivers/spi/spi-dw-mchp.c b/drivers/spi/spi-dw-mchp.c
new file mode 100644
index 0000000000000..0828a7616d9ab
--- /dev/null
+++ b/drivers/spi/spi-dw-mchp.c
@@ -0,0 +1,232 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Memory-mapped interface driver for MSCC SoCs
+ *
+ * Copyright (c) 2020 Microchip Technology Inc. and its subsidiaries.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+#include <linux/mtd/spi-nor.h>
+#include <linux/spi/spi-mem.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_gpio.h>
+#include <linux/of_platform.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+
+#include "spi-dw.h"
+
+#define DRIVER_NAME "dw_spi_mchp"
+
+#define MAX_CS		4
+
+#define MSCC_CPU_SYSTEM_CTRL_GENERAL_CTRL	0x24
+#define OCELOT_IF_SI_OWNER_OFFSET		4
+#define JAGUAR2_IF_SI_OWNER_OFFSET		6
+#define MSCC_IF_SI_OWNER_MASK			GENMASK(1, 0)
+#define MSCC_IF_SI_OWNER_SISL			0
+#define MSCC_IF_SI_OWNER_SIBM			1
+#define MSCC_IF_SI_OWNER_SIMC			2
+
+#define MSCC_SPI_MST_SW_MODE			0x14
+#define MSCC_SPI_MST_SW_MODE_SW_PIN_CTRL_MODE	BIT(13)
+#define MSCC_SPI_MST_SW_MODE_SW_SPI_CS(x)	(x << 5)
+
+struct dw_spi_mchp_props {
+	const char *syscon_name;
+	u32 si_owner_bit;
+};
+
+struct dw_spi_mchp {
+	struct dw_spi  dws;
+	struct clk     *clk;
+	void __iomem   *read_map;
+	struct regmap			*syscon;
+	void __iomem			*spi_mst;
+	const struct dw_spi_mchp_props	*props;
+	u32				gen_owner;
+};
+
+static const struct dw_spi_mchp_props dw_spi_mchp_props_ocelot = {
+	.syscon_name		= "mscc,ocelot-cpu-syscon",
+	.si_owner_bit		= 4,
+};
+
+static const struct dw_spi_mchp_props dw_spi_mchp_props_jaguar2 = {
+	.syscon_name		= "mscc,ocelot-cpu-syscon",
+	.si_owner_bit		= 6,
+};
+
+/*
+ * The Designware SPI controller (referred to as master in the documentation)
+ * automatically deasserts chip select when the tx fifo is empty. The chip
+ * selects then needs to be either driven as GPIOs or, for the first 4 using the
+ * the SPI boot controller registers. the final chip select is an OR gate
+ * between the Designware SPI controller and the SPI boot controller.
+ */
+static void dw_spi_mchp_set_cs(struct spi_device *spi, bool enable)
+{
+	struct dw_spi *dws = spi_master_get_devdata(spi->master);
+	struct dw_spi_mchp *dwsmchp = container_of(dws, struct dw_spi_mchp,
+						   dws);
+	u32 cs = spi->chip_select;
+
+	if (cs < 4) {
+		u32 sw_mode = MSCC_SPI_MST_SW_MODE_SW_PIN_CTRL_MODE;
+
+		if (!enable)
+			sw_mode |= MSCC_SPI_MST_SW_MODE_SW_SPI_CS(BIT(cs));
+
+		writel(sw_mode, dwsmchp->spi_mst + MSCC_SPI_MST_SW_MODE);
+	}
+
+	dw_spi_set_cs(spi, enable);
+}
+
+static int dw_spi_mchp_init(struct platform_device *pdev,
+			    struct dw_spi *dws,
+			    struct dw_spi_mchp *dwsmchp,
+			    const struct dw_spi_mchp_props *props)
+{
+	struct resource *res;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (res && resource_size(res) > 0) {
+		dwsmchp->spi_mst = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(dwsmchp->spi_mst)) {
+			dev_err(&pdev->dev, "SPI_MST region map failed\n");
+			return PTR_ERR(dwsmchp->spi_mst);
+		}
+	}
+
+	dwsmchp->syscon =
+		syscon_regmap_lookup_by_compatible(props->syscon_name);
+	if (IS_ERR(dwsmchp->syscon)) {
+		dev_err(&pdev->dev, "No syscon map %s\n", props->syscon_name);
+		return PTR_ERR(dwsmchp->syscon);
+	}
+	dwsmchp->props = props;
+
+	/* Deassert all CS */
+	if (dwsmchp->spi_mst)
+		writel(0, dwsmchp->spi_mst + MSCC_SPI_MST_SW_MODE);
+
+	/* Select the owner of the SI interface */
+	regmap_update_bits(dwsmchp->syscon, MSCC_CPU_SYSTEM_CTRL_GENERAL_CTRL,
+			   MSCC_IF_SI_OWNER_MASK << props->si_owner_bit,
+			   MSCC_IF_SI_OWNER_SIMC << props->si_owner_bit);
+
+	dwsmchp->dws.set_cs = dw_spi_mchp_set_cs;
+
+	return 0;
+}
+
+static int dw_spi_mchp_probe(struct platform_device *pdev)
+{
+	const struct dw_spi_mchp_props *props;
+	struct dw_spi_mchp *dwsmchp;
+	struct dw_spi *dws;
+	int ret;
+	int num_cs, rx_sample_dly;
+
+	dwsmchp = devm_kzalloc(&pdev->dev, sizeof(struct dw_spi_mchp),
+			GFP_KERNEL);
+	if (!dwsmchp)
+		return -ENOMEM;
+
+	dws = &dwsmchp->dws;
+
+	/* Get basic io resource and map it */
+	dws->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(dws->regs)) {
+		dev_err(&pdev->dev, "SPI region map failed\n");
+		return PTR_ERR(dws->regs);
+	}
+
+	dws->irq = of_irq_get(pdev->dev.of_node, 0);
+	if (dws->irq < 0)
+		dev_info(&pdev->dev, "no irq, using polled mode\n");
+
+	dwsmchp->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(dwsmchp->clk))
+		return PTR_ERR(dwsmchp->clk);
+	ret = clk_prepare_enable(dwsmchp->clk);
+	if (ret)
+		return ret;
+
+	dws->bus_num = pdev->id;
+
+	dws->max_freq = clk_get_rate(dwsmchp->clk);
+
+	device_property_read_u32(&pdev->dev, "reg-io-width",
+				 &dws->reg_io_width);
+
+	num_cs = MAX_CS;
+
+	device_property_read_u32(&pdev->dev, "num-cs", &num_cs);
+
+	dws->num_cs = num_cs;
+
+	rx_sample_dly = 0;
+	device_property_read_u32(&pdev->dev, "spi-rx-delay-us", &rx_sample_dly);
+	dws->rx_sample_dly = DIV_ROUND_UP(rx_sample_dly,
+					  (dws->max_freq / 1000000));
+
+	props = device_get_match_data(&pdev->dev);
+	if (props)
+		ret = dw_spi_mchp_init(pdev, dws, dwsmchp, props);
+	else
+		ret = -EINVAL;
+	if (ret)
+		goto out;
+
+	ret = dw_spi_add_host(&pdev->dev, dws);
+	if (ret)
+		goto out;
+
+	platform_set_drvdata(pdev, dwsmchp);
+	return 0;
+
+out:
+	clk_disable_unprepare(dwsmchp->clk);
+	return ret;
+}
+
+static int dw_spi_mchp_remove(struct platform_device *pdev)
+{
+	struct dw_spi_mchp *dwsmchp = platform_get_drvdata(pdev);
+
+	dw_spi_remove_host(&dwsmchp->dws);
+	clk_disable_unprepare(dwsmchp->clk);
+
+	return 0;
+}
+
+static const struct of_device_id dw_spi_mchp_of_match[] = {
+	{ .compatible = "mscc,ocelot-spi", .data = &dw_spi_mchp_props_ocelot},
+	{ .compatible = "mscc,jaguar2-spi", .data = &dw_spi_mchp_props_jaguar2},
+	{ /* end of table */}
+};
+MODULE_DEVICE_TABLE(of, dw_spi_mchp_of_match);
+
+static struct platform_driver dw_spi_mchp_driver = {
+	.probe		= dw_spi_mchp_probe,
+	.remove		= dw_spi_mchp_remove,
+	.driver		= {
+		.name	= DRIVER_NAME,
+		.of_match_table = dw_spi_mchp_of_match,
+	},
+};
+module_platform_driver(dw_spi_mchp_driver);
+
+MODULE_AUTHOR("Lars Povlsen <lars.povlsen@microchip.com>");
+MODULE_DESCRIPTION("Memory-mapped I/O interface DW SPI driver for MSCC SoCs");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index 384a3ab6dc2d0..dc5db548fbcbc 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -32,97 +32,6 @@ struct dw_spi_mmio {
 	void           *priv;
 };

-#define MSCC_CPU_SYSTEM_CTRL_GENERAL_CTRL	0x24
-#define OCELOT_IF_SI_OWNER_OFFSET		4
-#define JAGUAR2_IF_SI_OWNER_OFFSET		6
-#define MSCC_IF_SI_OWNER_MASK			GENMASK(1, 0)
-#define MSCC_IF_SI_OWNER_SISL			0
-#define MSCC_IF_SI_OWNER_SIBM			1
-#define MSCC_IF_SI_OWNER_SIMC			2
-
-#define MSCC_SPI_MST_SW_MODE			0x14
-#define MSCC_SPI_MST_SW_MODE_SW_PIN_CTRL_MODE	BIT(13)
-#define MSCC_SPI_MST_SW_MODE_SW_SPI_CS(x)	(x << 5)
-
-struct dw_spi_mscc {
-	struct regmap       *syscon;
-	void __iomem        *spi_mst;
-};
-
-/*
- * The Designware SPI controller (referred to as master in the documentation)
- * automatically deasserts chip select when the tx fifo is empty. The chip
- * selects then needs to be either driven as GPIOs or, for the first 4 using the
- * the SPI boot controller registers. the final chip select is an OR gate
- * between the Designware SPI controller and the SPI boot controller.
- */
-static void dw_spi_mscc_set_cs(struct spi_device *spi, bool enable)
-{
-	struct dw_spi *dws = spi_master_get_devdata(spi->master);
-	struct dw_spi_mmio *dwsmmio = container_of(dws, struct dw_spi_mmio, dws);
-	struct dw_spi_mscc *dwsmscc = dwsmmio->priv;
-	u32 cs = spi->chip_select;
-
-	if (cs < 4) {
-		u32 sw_mode = MSCC_SPI_MST_SW_MODE_SW_PIN_CTRL_MODE;
-
-		if (!enable)
-			sw_mode |= MSCC_SPI_MST_SW_MODE_SW_SPI_CS(BIT(cs));
-
-		writel(sw_mode, dwsmscc->spi_mst + MSCC_SPI_MST_SW_MODE);
-	}
-
-	dw_spi_set_cs(spi, enable);
-}
-
-static int dw_spi_mscc_init(struct platform_device *pdev,
-			    struct dw_spi_mmio *dwsmmio,
-			    const char *cpu_syscon, u32 if_si_owner_offset)
-{
-	struct dw_spi_mscc *dwsmscc;
-
-	dwsmscc = devm_kzalloc(&pdev->dev, sizeof(*dwsmscc), GFP_KERNEL);
-	if (!dwsmscc)
-		return -ENOMEM;
-
-	dwsmscc->spi_mst = devm_platform_ioremap_resource(pdev, 1);
-	if (IS_ERR(dwsmscc->spi_mst)) {
-		dev_err(&pdev->dev, "SPI_MST region map failed\n");
-		return PTR_ERR(dwsmscc->spi_mst);
-	}
-
-	dwsmscc->syscon = syscon_regmap_lookup_by_compatible(cpu_syscon);
-	if (IS_ERR(dwsmscc->syscon))
-		return PTR_ERR(dwsmscc->syscon);
-
-	/* Deassert all CS */
-	writel(0, dwsmscc->spi_mst + MSCC_SPI_MST_SW_MODE);
-
-	/* Select the owner of the SI interface */
-	regmap_update_bits(dwsmscc->syscon, MSCC_CPU_SYSTEM_CTRL_GENERAL_CTRL,
-			   MSCC_IF_SI_OWNER_MASK << if_si_owner_offset,
-			   MSCC_IF_SI_OWNER_SIMC << if_si_owner_offset);
-
-	dwsmmio->dws.set_cs = dw_spi_mscc_set_cs;
-	dwsmmio->priv = dwsmscc;
-
-	return 0;
-}
-
-static int dw_spi_mscc_ocelot_init(struct platform_device *pdev,
-				   struct dw_spi_mmio *dwsmmio)
-{
-	return dw_spi_mscc_init(pdev, dwsmmio, "mscc,ocelot-cpu-syscon",
-				OCELOT_IF_SI_OWNER_OFFSET);
-}
-
-static int dw_spi_mscc_jaguar2_init(struct platform_device *pdev,
-				    struct dw_spi_mmio *dwsmmio)
-{
-	return dw_spi_mscc_init(pdev, dwsmmio, "mscc,jaguar2-cpu-syscon",
-				JAGUAR2_IF_SI_OWNER_OFFSET);
-}
-
 static int dw_spi_alpine_init(struct platform_device *pdev,
 			      struct dw_spi_mmio *dwsmmio)
 {
@@ -225,8 +134,6 @@ static int dw_spi_mmio_remove(struct platform_device *pdev)

 static const struct of_device_id dw_spi_mmio_of_match[] = {
 	{ .compatible = "snps,dw-apb-ssi", },
-	{ .compatible = "mscc,ocelot-spi", .data = dw_spi_mscc_ocelot_init},
-	{ .compatible = "mscc,jaguar2-spi", .data = dw_spi_mscc_jaguar2_init},
 	{ .compatible = "amazon,alpine-dw-apb-ssi", .data = dw_spi_alpine_init},
 	{ .compatible = "renesas,rzn1-spi", },
 	{ /* end of table */}
--
2.26.2

Re: [PATCH 05/10] spi: spi-dw-mmio: Spin off MSCC platforms into spi-dw-mchp

[Lars Povlsen]

On 13/05/20 16:18, Mark Brown wrote:
> Date: Wed, 13 May 2020 16:18:11 +0100
> From: Mark Brown <broonie@kernel.org>
> To: Lars Povlsen <lars.povlsen@microchip.com>
> Cc: SoC Team <soc@kernel.org>, Microchip Linux Driver Support
>  <UNGLinuxDriver@microchip.com>, linux-spi@vger.kernel.org,
>  devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
>  linux-arm-kernel@lists.infradead.org, Alexandre Belloni
>  <alexandre.belloni@bootlin.com>
> Subject: Re: [PATCH 05/10] spi: spi-dw-mmio: Spin off MSCC platforms into
>  spi-dw-mchp
> User-Agent: Mutt/1.10.1 (2018-07-13)
> 
> On Wed, May 13, 2020 at 04:00:26PM +0200, Lars Povlsen wrote:
> 
> > +config SPI_DW_MCHP
> > +	tristate "Memory-mapped io interface driver using DW SPI core of MSCC SoCs"
> > +	default y if ARCH_SPARX5
> > +	default y if SOC_VCOREIII
> 
> Why the default ys?

The SoC will typically boot from SPI... But its not a requirement per
se. I will remove it.

> 
> > +++ b/drivers/spi/Makefile
> > @@ -37,6 +37,7 @@ obj-$(CONFIG_SPI_DAVINCI)		+= spi-davinci.o
> >  obj-$(CONFIG_SPI_DLN2)			+= spi-dln2.o
> >  obj-$(CONFIG_SPI_DESIGNWARE)		+= spi-dw.o
> >  obj-$(CONFIG_SPI_DW_MMIO)		+= spi-dw-mmio.o
> > +obj-$(CONFIG_SPI_DW_MCHP)		+= spi-dw-mchp.o
> >  obj-$(CONFIG_SPI_DW_PCI)		+= spi-dw-midpci.o
> >  spi-dw-midpci-objs			:= spi-dw-pci.o spi-dw-mid.o
> >  obj-$(CONFIG_SPI_EFM32)			+= spi-efm32.o
> 
> Please keep the file alphabetically sorted.
> 

Noted.

> > +++ b/drivers/spi/spi-dw-mchp.c
> > @@ -0,0 +1,232 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Memory-mapped interface driver for MSCC SoCs
> > + *
> 
> Please make the entire comment a C++ one so things look more
> intentional.

Sure, I can do that. The presented form matches that of the other
spi-dw-* drivers, but I can see other using // blocks. Ack.

> 
> > +#define MAX_CS		4
> 
> This should be namespaced.

Ack.

> 
> > +	rx_sample_dly = 0;
> > +	device_property_read_u32(&pdev->dev, "spi-rx-delay-us", &rx_sample_dly);
> > +	dws->rx_sample_dly = DIV_ROUND_UP(rx_sample_dly,
> > +					  (dws->max_freq / 1000000));
> 
> If this is a standard feature of the DesignWare IP why parse it here and
> not in the generic code?

This is a standard feature of the DesignWare IP, so good suggestion. I
will arrange with Serge.

Thank you for your comments!

---Lars

Re: [PATCH 05/10] spi: spi-dw-mmio: Spin off MSCC platforms into spi-dw-mchp

[Serge Semin]

On Tue, May 19, 2020 at 02:05:19PM +0200, Lars Povlsen wrote:
> On 13/05/20 16:18, Mark Brown wrote:
> > Date: Wed, 13 May 2020 16:18:11 +0100
> > From: Mark Brown <broonie@kernel.org>
> > To: Lars Povlsen <lars.povlsen@microchip.com>
> > Cc: SoC Team <soc@kernel.org>, Microchip Linux Driver Support
> >  <UNGLinuxDriver@microchip.com>, linux-spi@vger.kernel.org,
> >  devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
> >  linux-arm-kernel@lists.infradead.org, Alexandre Belloni
> >  <alexandre.belloni@bootlin.com>
> > Subject: Re: [PATCH 05/10] spi: spi-dw-mmio: Spin off MSCC platforms into
> >  spi-dw-mchp
> > User-Agent: Mutt/1.10.1 (2018-07-13)
> > 
> > On Wed, May 13, 2020 at 04:00:26PM +0200, Lars Povlsen wrote:
> > 
> > > +config SPI_DW_MCHP
> > > +	tristate "Memory-mapped io interface driver using DW SPI core of MSCC SoCs"
> > > +	default y if ARCH_SPARX5
> > > +	default y if SOC_VCOREIII
> > 
> > Why the default ys?
> 
> The SoC will typically boot from SPI... But its not a requirement per
> se. I will remove it.
> 
> > 
> > > +++ b/drivers/spi/Makefile
> > > @@ -37,6 +37,7 @@ obj-$(CONFIG_SPI_DAVINCI)		+= spi-davinci.o
> > >  obj-$(CONFIG_SPI_DLN2)			+= spi-dln2.o
> > >  obj-$(CONFIG_SPI_DESIGNWARE)		+= spi-dw.o
> > >  obj-$(CONFIG_SPI_DW_MMIO)		+= spi-dw-mmio.o
> > > +obj-$(CONFIG_SPI_DW_MCHP)		+= spi-dw-mchp.o
> > >  obj-$(CONFIG_SPI_DW_PCI)		+= spi-dw-midpci.o
> > >  spi-dw-midpci-objs			:= spi-dw-pci.o spi-dw-mid.o
> > >  obj-$(CONFIG_SPI_EFM32)			+= spi-efm32.o
> > 
> > Please keep the file alphabetically sorted.
> > 
> 
> Noted.
> 
> > > +++ b/drivers/spi/spi-dw-mchp.c
> > > @@ -0,0 +1,232 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Memory-mapped interface driver for MSCC SoCs
> > > + *
> > 
> > Please make the entire comment a C++ one so things look more
> > intentional.
> 
> Sure, I can do that. The presented form matches that of the other
> spi-dw-* drivers, but I can see other using // blocks. Ack.
> 
> > 
> > > +#define MAX_CS		4
> > 
> > This should be namespaced.
> 
> Ack.
> 

> > 
> > > +	rx_sample_dly = 0;
> > > +	device_property_read_u32(&pdev->dev, "spi-rx-delay-us", &rx_sample_dly);
> > > +	dws->rx_sample_dly = DIV_ROUND_UP(rx_sample_dly,
> > > +					  (dws->max_freq / 1000000));

Perhaps 100000 is better to be replace with macro USEC_PER_SEC...

Moreover are you sure the formulae is correct?
dws->rx_sample_dly - a number of ssi_clk periods/cycles to delay the Rx-data sample,
dws->max_freq - ssi_clk frequency (not period).

In real math the formulae would look like:
S = d * P [s], where d - number of delay cycles, P - ssi_clk period in seconds,
S - requested delay in seconds.
In the driver notation: d = dws->rx_sample_dly, P = 1 / dws->max_freq,
S = rx_sample_dly ("spi-rx-delay-us" property value).

dws->rx_sample_dly * (1 / dws->max_freq) = rx_sample_dly <=>
dws->rx_sample_dly = rx_sample_dly * dws->max_freq.

Though that's represented in seconds, so if rx_sample_dly is specified in usec,
then you'd need to scale it down dividing by USEC_PER_SEC.

For example, imagine we need a delay of 1 usec with ssi_clk of 50MHz.
By your formulae we'd have: 1 / (50000000 / 1000000) = 0 cycles (actually 1 due
to DIV_ROUND_UP, but incorrect anyway),
By mine: 1 * (500000000 / 1000000) = 50 cycles. Seems closer to reality.

Am I missing something?

> > 
> > If this is a standard feature of the DesignWare IP why parse it here and
> > not in the generic code?
> 
> This is a standard feature of the DesignWare IP, so good suggestion. I
> will arrange with Serge.

Regarding "spi-rx-delay-us" and the sampling delay the IP supports. Here is what
documentation says regarding the register, which is then initialized with this
parameter "This register controls the number of ssi_clk cycles that are
delayed from the default sample time before the actual sample of the rxd input
signal occurs." While the "spi-rx-delay-us" property is described as: "Delay, in
microseconds, after a read transfer." I may misunderstand something, but IMO
these descriptions don't refer to the same values. The only real use of the
"spi-rx-delay-us" property I've found in "./drivers/input/rmi4/rmi_spi.c".
That driver gets the value of the property and just sets the delay_usecs
of some transfers, which isn't even close to the functionality the RX_SAMPLE_DLY
register provides. 

To be clear the RX_SAMPLE_DLY register can be used to delay the RX-bits sample
with respect to the normal Rx sampling timing. The delay is measured in the 
numbers of the ssi_clk periods. (Note also that the maximum delay is limited
with a constant parameter pre-initialized at the IP-core synthesis stage. It can
be defined within a range [4, 255]. In our IP it's limited with just 4 periods.)

As I see it, a better way would be to either define a new vendor-specific
property like "snps,rx-sample-delay-ns" (note NS here, since normally the
ssi_clk is much higher than 1MHz), or define a new generic SPI property.
Mark, Andy?

-Sergey

> 
> Thank you for your comments!
> 
> ---Lars
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 05/10] spi: spi-dw-mmio: Spin off MSCC platforms into spi-dw-mchp

[Lars Povlsen]

Serge Semin writes:

> On Tue, May 19, 2020 at 02:05:19PM +0200, Lars Povlsen wrote:
>> On 13/05/20 16:18, Mark Brown wrote:
>> > Date: Wed, 13 May 2020 16:18:11 +0100
>> > From: Mark Brown <broonie@kernel.org>
>> > To: Lars Povlsen <lars.povlsen@microchip.com>
>> > Cc: SoC Team <soc@kernel.org>, Microchip Linux Driver Support
>> >  <UNGLinuxDriver@microchip.com>, linux-spi@vger.kernel.org,
>> >  devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
>> >  linux-arm-kernel@lists.infradead.org, Alexandre Belloni
>> >  <alexandre.belloni@bootlin.com>
>> > Subject: Re: [PATCH 05/10] spi: spi-dw-mmio: Spin off MSCC platforms into
>> >  spi-dw-mchp
>> > User-Agent: Mutt/1.10.1 (2018-07-13)
>> >
>> > On Wed, May 13, 2020 at 04:00:26PM +0200, Lars Povlsen wrote:
>> >
>> > > +config SPI_DW_MCHP
>> > > + tristate "Memory-mapped io interface driver using DW SPI core of MSCC SoCs"
>> > > + default y if ARCH_SPARX5
>> > > + default y if SOC_VCOREIII
>> >
>> > Why the default ys?
>>
>> The SoC will typically boot from SPI... But its not a requirement per
>> se. I will remove it.
>>
>> >
>> > > +++ b/drivers/spi/Makefile
>> > > @@ -37,6 +37,7 @@ obj-$(CONFIG_SPI_DAVINCI)               += spi-davinci.o
>> > >  obj-$(CONFIG_SPI_DLN2)                   += spi-dln2.o
>> > >  obj-$(CONFIG_SPI_DESIGNWARE)             += spi-dw.o
>> > >  obj-$(CONFIG_SPI_DW_MMIO)                += spi-dw-mmio.o
>> > > +obj-$(CONFIG_SPI_DW_MCHP)                += spi-dw-mchp.o
>> > >  obj-$(CONFIG_SPI_DW_PCI)         += spi-dw-midpci.o
>> > >  spi-dw-midpci-objs                       := spi-dw-pci.o spi-dw-mid.o
>> > >  obj-$(CONFIG_SPI_EFM32)                  += spi-efm32.o
>> >
>> > Please keep the file alphabetically sorted.
>> >
>>
>> Noted.
>>
>> > > +++ b/drivers/spi/spi-dw-mchp.c
>> > > @@ -0,0 +1,232 @@
>> > > +// SPDX-License-Identifier: GPL-2.0-only
>> > > +/*
>> > > + * Memory-mapped interface driver for MSCC SoCs
>> > > + *
>> >
>> > Please make the entire comment a C++ one so things look more
>> > intentional.
>>
>> Sure, I can do that. The presented form matches that of the other
>> spi-dw-* drivers, but I can see other using // blocks. Ack.
>>
>> >
>> > > +#define MAX_CS           4
>> >
>> > This should be namespaced.
>>
>> Ack.
>>
>
>> >
>> > > + rx_sample_dly = 0;
>> > > + device_property_read_u32(&pdev->dev, "spi-rx-delay-us", &rx_sample_dly);
>> > > + dws->rx_sample_dly = DIV_ROUND_UP(rx_sample_dly,
>> > > +                                   (dws->max_freq / 1000000));
>
> Perhaps 100000 is better to be replace with macro USEC_PER_SEC...
>
> Moreover are you sure the formulae is correct?
> dws->rx_sample_dly - a number of ssi_clk periods/cycles to delay the Rx-data sample,
> dws->max_freq - ssi_clk frequency (not period).
>
> In real math the formulae would look like:
> S = d * P [s], where d - number of delay cycles, P - ssi_clk period in seconds,
> S - requested delay in seconds.
> In the driver notation: d = dws->rx_sample_dly, P = 1 / dws->max_freq,
> S = rx_sample_dly ("spi-rx-delay-us" property value).
>
> dws->rx_sample_dly * (1 / dws->max_freq) = rx_sample_dly <=>
> dws->rx_sample_dly = rx_sample_dly * dws->max_freq.
>
> Though that's represented in seconds, so if rx_sample_dly is specified in usec,
> then you'd need to scale it down dividing by USEC_PER_SEC.
>
> For example, imagine we need a delay of 1 usec with ssi_clk of 50MHz.
> By your formulae we'd have: 1 / (50000000 / 1000000) = 0 cycles (actually 1 due
> to DIV_ROUND_UP, but incorrect anyway),
> By mine: 1 * (500000000 / 1000000) = 50 cycles. Seems closer to reality.
>
> Am I missing something?

No, you are perfectly right, the calculation was wrong - and I concur
the unit should be NS.

(your example threw me off, you are using 500Mhz, typo I guess)

I believe the calculation should be:

  device_property_read_u32(&pdev->dev, "snps,rx-sample-delay-ns", &rx_sample_dly);
  dws->rx_sample_dly = DIV_ROUND_CLOSEST(rx_sample_dly, NSEC_PER_SEC / dws->max_freq);
        
So for your example of 1us = 1000ns, we have a cycle time of 20 ns => 50 cycles.

And I assume DIV_ROUND_CLOSEST() is the better instead of explicit
rounding up/down. And I assume its fair to assume that the cycle time is
not a fraction.

Ok?

>
>> >
>> > If this is a standard feature of the DesignWare IP why parse it here and
>> > not in the generic code?
>>
>> This is a standard feature of the DesignWare IP, so good suggestion. I
>> will arrange with Serge.
>
> Regarding "spi-rx-delay-us" and the sampling delay the IP supports. Here is what
> documentation says regarding the register, which is then initialized with this
> parameter "This register controls the number of ssi_clk cycles that are
> delayed from the default sample time before the actual sample of the rxd input
> signal occurs." While the "spi-rx-delay-us" property is described as: "Delay, in
> microseconds, after a read transfer." I may misunderstand something, but IMO
> these descriptions don't refer to the same values. The only real use of the
> "spi-rx-delay-us" property I've found in "./drivers/input/rmi4/rmi_spi.c".
> That driver gets the value of the property and just sets the delay_usecs
> of some transfers, which isn't even close to the functionality the RX_SAMPLE_DLY
> register provides.
>
> To be clear the RX_SAMPLE_DLY register can be used to delay the RX-bits sample
> with respect to the normal Rx sampling timing. The delay is measured in the
> numbers of the ssi_clk periods. (Note also that the maximum delay is limited
> with a constant parameter pre-initialized at the IP-core synthesis stage. It can
> be defined within a range [4, 255]. In our IP it's limited with just 4 periods.)
>

Yes - I was not aware of the instantiation incurred limit before. Turned
our IP has up to 100ns worth of fifo depth - 25 cycles.

> As I see it, a better way would be to either define a new vendor-specific
> property like "snps,rx-sample-delay-ns" (note NS here, since normally the
> ssi_clk is much higher than 1MHz), or define a new generic SPI property.
> Mark, Andy?

I'll assume "snps,rx-sample-delay-ns" for now, its easy to rename if you
decide so.

Thanks again!

---Lars

>
> -Sergey
>
>>
>> Thank you for your comments!
>>
>> ---Lars
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Lars Povlsen,
Microchip

[PATCH 06/10] dt-bindings: spi: spi-dw-mchp: Add Sparx5 support

[Lars Povlsen]

This add DT bindings for the Sparx5 SPI driver.

Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 .../bindings/spi/mscc,ocelot-spi.yaml         | 49 +++++++++++++++----
 1 file changed, 39 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml b/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
index a3ac0fa576553..8beecde4b0880 100644
--- a/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
+++ b/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
@@ -23,15 +23,23 @@ properties:
     enum:
       - mscc,ocelot-spi
       - mscc,jaguar2-spi
+      - microchip,sparx5-spi

   interrupts:
     maxItems: 1

   reg:
     minItems: 2
-    items:
-      - description: Designware SPI registers
-      - description: CS override registers
+    maxItems: 3
+    oneOf:
+      - items:
+          - description: Designware SPI registers
+          - description: CS override registers (Not sparx5).
+      - items:
+          - description: Designware SPI registers
+          - description: CS override registers (Not sparx5).
+          - description: Direct mapped SPI read area. If provided, the
+              driver will register spi_mem_op's to take advantage of it.

   clocks:
     maxItems: 1
@@ -43,6 +51,23 @@ properties:
        enum: [ 2, 4 ]
     maxItems: 1

+  spi-rx-delay-us:
+    description: |
+      The delay (in usec) of the RX signal sample position. This can
+      be used to tne the RX timing in order to acheive higher
+      speeds. This is used for all devices on the bus.
+    default: 0
+    maxItems: 1
+
+  interface-mapping-mask:
+    description: |
+      On the Sparx5 variant, two different busses are connected to the
+      controller. This property is a mask per chip-select, indicating
+      whether the CS should go to one or the other interface.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 0
+    maxItems: 1
+
 required:
   - compatible
   - reg
@@ -50,11 +75,15 @@ required:

 examples:
   - |
-    spi0: spi@101000 {
-      compatible = "mscc,ocelot-spi";
-      #address-cells = <1>;
-      #size-cells = <0>;
-      reg = <0x101000 0x100>, <0x3c 0x18>;
-      interrupts = <9>;
-      clocks = <&ahb_clk>;
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    spi0: spi@600104000 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        compatible = "microchip,sparx5-spi";
+        reg = <0x00104000 0x40>, <0 0>, <0x3000000 0x4000000>;
+        num-cs = <16>;
+        reg-io-width = <4>;
+        reg-shift = <2>;
+        clocks = <&ahb_clk>;
+        interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
     };
--
2.26.2

Re: [PATCH 06/10] dt-bindings: spi: spi-dw-mchp: Add Sparx5 support

[Serge Semin]

On Wed, May 13, 2020 at 04:00:27PM +0200, Lars Povlsen wrote:
> This add DT bindings for the Sparx5 SPI driver.

This whole file can be easily merged in to the generic DW APB SSI DT
binding file. Just use "if: properties: compatible: const: ..." construction
to distinguish ocelot, jaguar, sparx5 and non-sparx5 nodes.

> 
> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> ---
>  .../bindings/spi/mscc,ocelot-spi.yaml         | 49 +++++++++++++++----
>  1 file changed, 39 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml b/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
> index a3ac0fa576553..8beecde4b0880 100644
> --- a/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
> +++ b/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
> @@ -23,15 +23,23 @@ properties:
>      enum:
>        - mscc,ocelot-spi
>        - mscc,jaguar2-spi
> +      - microchip,sparx5-spi
> 
>    interrupts:
>      maxItems: 1
> 
>    reg:
>      minItems: 2
> -    items:
> -      - description: Designware SPI registers
> -      - description: CS override registers
> +    maxItems: 3
> +    oneOf:
> +      - items:
> +          - description: Designware SPI registers
> +          - description: CS override registers (Not sparx5).
> +      - items:
> +          - description: Designware SPI registers
> +          - description: CS override registers (Not sparx5).
> +          - description: Direct mapped SPI read area. If provided, the
> +              driver will register spi_mem_op's to take advantage of it.
> 
>    clocks:
>      maxItems: 1
> @@ -43,6 +51,23 @@ properties:
>         enum: [ 2, 4 ]
>      maxItems: 1
> 

> +  spi-rx-delay-us:
> +    description: |
> +      The delay (in usec) of the RX signal sample position. This can
> +      be used to tne the RX timing in order to acheive higher
> +      speeds. This is used for all devices on the bus.
> +    default: 0
> +    maxItems: 1

spi-rx-delay-us is defined for a particular SPI-slave. Please see the
DT binding file: Documentation/devicetree/bindings/spi/spi-controller.yaml .
Although as I suggested before this delay isn't what the Dw APB SSI RX sample
delay functionality does. Probably a vendor-specific property would be better
here. But I'd also define it on a SPI-slave basis, not for all devices on the
bus.

> +
> +  interface-mapping-mask:
> +    description: |
> +      On the Sparx5 variant, two different busses are connected to the
> +      controller. This property is a mask per chip-select, indicating
> +      whether the CS should go to one or the other interface.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    default: 0
> +    maxItems: 1

As Mark rightfully suggested this seems like an SPI-slave related property, then
most likely it should be defined on the SPI-slave basis (probably as a bool
property). Additionally it's vendor-specific, so the property name should be
accordingly prefixed.

> +
>  required:
>    - compatible
>    - reg
> @@ -50,11 +75,15 @@ required:
> 
>  examples:
>    - |
> -    spi0: spi@101000 {
> -      compatible = "mscc,ocelot-spi";
> -      #address-cells = <1>;
> -      #size-cells = <0>;
> -      reg = <0x101000 0x100>, <0x3c 0x18>;
> -      interrupts = <9>;
> -      clocks = <&ahb_clk>;
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    spi0: spi@600104000 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        compatible = "microchip,sparx5-spi";

> +        reg = <0x00104000 0x40>, <0 0>, <0x3000000 0x4000000>;

I have a doubt that defining an empty reg region is a good idea, since you can
detect the reg requirements by the node compatible string.

-Sergey

> +        num-cs = <16>;
> +        reg-io-width = <4>;
> +        reg-shift = <2>;
> +        clocks = <&ahb_clk>;
> +        interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
>      };
> --
> 2.26.2
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 06/10] dt-bindings: spi: spi-dw-mchp: Add Sparx5 support

[Lars Povlsen]

Serge Semin writes:

> On Wed, May 13, 2020 at 04:00:27PM +0200, Lars Povlsen wrote:
>> This add DT bindings for the Sparx5 SPI driver.
>
> This whole file can be easily merged in to the generic DW APB SSI DT
> binding file. Just use "if: properties: compatible: const: ..." construction
> to distinguish ocelot, jaguar, sparx5 and non-sparx5 nodes.
>
>>
>> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>> ---
>>  .../bindings/spi/mscc,ocelot-spi.yaml         | 49 +++++++++++++++----
>>  1 file changed, 39 insertions(+), 10 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml b/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
>> index a3ac0fa576553..8beecde4b0880 100644
>> --- a/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
>> +++ b/Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
>> @@ -23,15 +23,23 @@ properties:
>>      enum:
>>        - mscc,ocelot-spi
>>        - mscc,jaguar2-spi
>> +      - microchip,sparx5-spi
>>
>>    interrupts:
>>      maxItems: 1
>>
>>    reg:
>>      minItems: 2
>> -    items:
>> -      - description: Designware SPI registers
>> -      - description: CS override registers
>> +    maxItems: 3
>> +    oneOf:
>> +      - items:
>> +          - description: Designware SPI registers
>> +          - description: CS override registers (Not sparx5).
>> +      - items:
>> +          - description: Designware SPI registers
>> +          - description: CS override registers (Not sparx5).
>> +          - description: Direct mapped SPI read area. If provided, the
>> +              driver will register spi_mem_op's to take advantage of it.
>>
>>    clocks:
>>      maxItems: 1
>> @@ -43,6 +51,23 @@ properties:
>>         enum: [ 2, 4 ]
>>      maxItems: 1
>>
>
>> +  spi-rx-delay-us:
>> +    description: |
>> +      The delay (in usec) of the RX signal sample position. This can
>> +      be used to tne the RX timing in order to acheive higher
>> +      speeds. This is used for all devices on the bus.
>> +    default: 0
>> +    maxItems: 1
>
> spi-rx-delay-us is defined for a particular SPI-slave. Please see the
> DT binding file: Documentation/devicetree/bindings/spi/spi-controller.yaml .
> Although as I suggested before this delay isn't what the Dw APB SSI RX sample
> delay functionality does. Probably a vendor-specific property would be better
> here. But I'd also define it on a SPI-slave basis, not for all devices on the
> bus.

Right, I was hunting for something "similar". As pointed out, this is
really different in nature, and the unit is also too coarse.

I will change this to "snps,rx-sample-delay-ns" as suggested in your
other comments.

>
>> +
>> +  interface-mapping-mask:
>> +    description: |
>> +      On the Sparx5 variant, two different busses are connected to the
>> +      controller. This property is a mask per chip-select, indicating
>> +      whether the CS should go to one or the other interface.
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    default: 0
>> +    maxItems: 1
>
> As Mark rightfully suggested this seems like an SPI-slave related property, then
> most likely it should be defined on the SPI-slave basis (probably as a bool
> property). Additionally it's vendor-specific, so the property name should be
> accordingly prefixed.

Yes, I'll change this to a per-device property. I need the same for the
above as well.

>
>> +
>>  required:
>>    - compatible
>>    - reg
>> @@ -50,11 +75,15 @@ required:
>>
>>  examples:
>>    - |
>> -    spi0: spi@101000 {
>> -      compatible = "mscc,ocelot-spi";
>> -      #address-cells = <1>;
>> -      #size-cells = <0>;
>> -      reg = <0x101000 0x100>, <0x3c 0x18>;
>> -      interrupts = <9>;
>> -      clocks = <&ahb_clk>;
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    spi0: spi@600104000 {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +        compatible = "microchip,sparx5-spi";
>
>> +        reg = <0x00104000 0x40>, <0 0>, <0x3000000 0x4000000>;
>
> I have a doubt that defining an empty reg region is a good idea, since you can
> detect the reg requirements by the node compatible string.

Yes, its probably better that way. It looks ugly too :-)

Thanks for your comments!

---Lars


> -Sergey
>
>> +        num-cs = <16>;
>> +        reg-io-width = <4>;
>> +        reg-shift = <2>;
>> +        clocks = <&ahb_clk>;
>> +        interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
>>      };
>> --
>> 2.26.2
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Lars Povlsen,
Microchip

[PATCH 07/10] spi: spi-dw-mchp: Add Sparx5 support

[Lars Povlsen]

This adds support for the Sparx5 SoC in the spi-dw-mchp SPI controller.

Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 drivers/spi/spi-dw-mchp.c | 211 ++++++++++++++++++++++++++++++++++----
 1 file changed, 189 insertions(+), 22 deletions(-)

diff --git a/drivers/spi/spi-dw-mchp.c b/drivers/spi/spi-dw-mchp.c
index 0828a7616d9ab..3abdd44a550ea 100644
--- a/drivers/spi/spi-dw-mchp.c
+++ b/drivers/spi/spi-dw-mchp.c
@@ -28,21 +28,22 @@

 #define MAX_CS		4

-#define MSCC_CPU_SYSTEM_CTRL_GENERAL_CTRL	0x24
-#define OCELOT_IF_SI_OWNER_OFFSET		4
-#define JAGUAR2_IF_SI_OWNER_OFFSET		6
 #define MSCC_IF_SI_OWNER_MASK			GENMASK(1, 0)
 #define MSCC_IF_SI_OWNER_SISL			0
 #define MSCC_IF_SI_OWNER_SIBM			1
 #define MSCC_IF_SI_OWNER_SIMC			2

 #define MSCC_SPI_MST_SW_MODE			0x14
-#define MSCC_SPI_MST_SW_MODE_SW_PIN_CTRL_MODE	BIT(13)
-#define MSCC_SPI_MST_SW_MODE_SW_SPI_CS(x)	(x << 5)

 struct dw_spi_mchp_props {
 	const char *syscon_name;
-	u32 si_owner_bit;
+	u32 general_ctrl_off;
+	u32 si_owner_bit, si_owner2_bit;
+	u32 pinctrl_bit_off;
+	u32 cs_bit_off;
+	u32 ss_force_ena_off;
+	u32 ss_force_val_off;
+	u32 bootmaster_cs;
 };

 struct dw_spi_mchp {
@@ -53,44 +54,176 @@ struct dw_spi_mchp {
 	void __iomem			*spi_mst;
 	const struct dw_spi_mchp_props	*props;
 	u32				gen_owner;
+	u32				if2mask;
 };

 static const struct dw_spi_mchp_props dw_spi_mchp_props_ocelot = {
 	.syscon_name		= "mscc,ocelot-cpu-syscon",
+	.general_ctrl_off	= 0x24,
 	.si_owner_bit		= 4,
+	.pinctrl_bit_off	= 13,
+	.cs_bit_off		= 5,
+	.bootmaster_cs		= 0,
 };

 static const struct dw_spi_mchp_props dw_spi_mchp_props_jaguar2 = {
 	.syscon_name		= "mscc,ocelot-cpu-syscon",
+	.general_ctrl_off	= 0x24,
 	.si_owner_bit		= 6,
+	.pinctrl_bit_off	= 13,
+	.cs_bit_off		= 5,
+	.bootmaster_cs		= 0,
+};
+
+static const struct dw_spi_mchp_props dw_spi_mchp_props_sparx5 = {
+	.syscon_name		= "microchip,sparx5-cpu-syscon",
+	.general_ctrl_off	= 0x88,
+	.si_owner_bit		= 6,
+	.si_owner2_bit		= 4,
+	.ss_force_ena_off	= 0xa4,
+	.ss_force_val_off	= 0xa8,
+	.bootmaster_cs		= 0,
 };

 /*
- * The Designware SPI controller (referred to as master in the documentation)
- * automatically deasserts chip select when the tx fifo is empty. The chip
- * selects then needs to be either driven as GPIOs or, for the first 4 using the
- * the SPI boot controller registers. the final chip select is an OR gate
- * between the Designware SPI controller and the SPI boot controller.
+ * Set the owner of the SPI interface
  */
-static void dw_spi_mchp_set_cs(struct spi_device *spi, bool enable)
+static void dw_spi_mchp_set_owner(struct dw_spi_mchp *dwsmchp,
+				  const struct dw_spi_mchp_props *props,
+				  u8 owner, u8 owner2)
+{
+	u32 val, msk;
+
+	val = (owner << props->si_owner_bit);
+	msk = (MSCC_IF_SI_OWNER_MASK << props->si_owner_bit);
+	if (props->si_owner2_bit) {
+		val |= owner2 << props->si_owner2_bit;
+		msk |= (MSCC_IF_SI_OWNER_MASK << props->si_owner2_bit);
+	}
+	if (dwsmchp->gen_owner != val) {
+		regmap_update_bits(dwsmchp->syscon, props->general_ctrl_off,
+				   msk, val);
+		dwsmchp->gen_owner = val;
+	}
+}
+
+static void dw_spi_mchp_set_cs_owner(struct dw_spi_mchp *dwsmchp,
+				     const struct dw_spi_mchp_props *props,
+				     u8 cs, u8 owner)
 {
+	u8 dummy = (owner == MSCC_IF_SI_OWNER_SIBM ?
+		    MSCC_IF_SI_OWNER_SIMC : MSCC_IF_SI_OWNER_SIBM);
+	if (props->si_owner2_bit && (dwsmchp->if2mask & BIT(cs))) {
+		/* SPI2 */
+		dw_spi_mchp_set_owner(dwsmchp, props, dummy, owner);
+	} else {
+		/* SPI1 */
+		dw_spi_mchp_set_owner(dwsmchp, props, owner, dummy);
+	}
+}
+
+/*
+ * The Designware SPI controller (referred to as master in the
+ * documentation) automatically deasserts chip select when the tx fifo
+ * is empty. The chip selects then needs to be either driven as GPIOs
+ * or, for the first 4 using the the SPI boot controller
+ * registers. the final chip select is an OR gate between the
+ * Designware SPI controller and the SPI boot controller.  nselect is
+ * an active low signal
+ */
+static void dw_spi_mchp_set_cs(struct spi_device *spi, bool nEnable)
+{
+	bool enable = !nEnable;	/* This keeps changing in the API... */
 	struct dw_spi *dws = spi_master_get_devdata(spi->master);
 	struct dw_spi_mchp *dwsmchp = container_of(dws, struct dw_spi_mchp,
 						   dws);
-	u32 cs = spi->chip_select;
+	const struct dw_spi_mchp_props *props = dwsmchp->props;
+	u8 cs = spi->chip_select;

-	if (cs < 4) {
-		u32 sw_mode = MSCC_SPI_MST_SW_MODE_SW_PIN_CTRL_MODE;
+	if (enable)
+		dw_spi_mchp_set_cs_owner(dwsmchp, props, cs,
+					 MSCC_IF_SI_OWNER_SIMC);

-		if (!enable)
-			sw_mode |= MSCC_SPI_MST_SW_MODE_SW_SPI_CS(BIT(cs));
+	if (dwsmchp->spi_mst && (cs < MAX_CS)) {
+		u32 sw_mode;

+		if (enable)
+			sw_mode = BIT(props->pinctrl_bit_off) |
+				(BIT(cs) << props->cs_bit_off);
+		else
+			sw_mode = 0;
 		writel(sw_mode, dwsmchp->spi_mst + MSCC_SPI_MST_SW_MODE);
+	} else if (props->ss_force_ena_off) {
+		if (enable) {
+			/* Ensure CS toggles, so start off all disabled */
+			regmap_write(dwsmchp->syscon, props->ss_force_val_off,
+				     ~0);
+			/* CS override drive enable */
+			regmap_write(dwsmchp->syscon, props->ss_force_ena_off,
+				     1);
+			/* Allow settle */
+			udelay(1);
+			/* Now set CSx enabled */
+			regmap_write(dwsmchp->syscon, props->ss_force_val_off,
+				     ~BIT(cs));
+		} else {
+			/* CS value */
+			regmap_write(dwsmchp->syscon, props->ss_force_val_off,
+				     ~0);
+			/* CS override drive disable */
+			regmap_write(dwsmchp->syscon, props->ss_force_ena_off,
+				     0);
+		}
 	}

-	dw_spi_set_cs(spi, enable);
+	dw_spi_set_cs(spi, nEnable);
+}
+
+static int dw_mchp_bootmaster_exec_mem_op(struct spi_mem *mem,
+					  const struct spi_mem_op *op)
+{
+	struct spi_device *spi = mem->spi;
+	int ret = -ENOTSUPP;
+
+	/* Only reads, addrsize 1..4 */
+	if (!op->data.nbytes || !op->addr.nbytes || op->addr.nbytes > 4 ||
+	    op->data.dir != SPI_MEM_DATA_IN)
+		return ret;
+
+	/* Only handle (normal+fast) 3/4 bytes read */
+	if (op->cmd.opcode != SPINOR_OP_READ &&
+	    op->cmd.opcode != SPINOR_OP_READ_FAST &&
+	    op->cmd.opcode != SPINOR_OP_READ_4B &&
+	    op->cmd.opcode != SPINOR_OP_READ_FAST_4B)
+		return ret;
+
+	/* CS0..3, only 16M reach */
+	if ((spi->chip_select < MAX_CS) &&
+	    (op->addr.val + op->data.nbytes) < SZ_16M) {
+		struct dw_spi *dws = spi_master_get_devdata(spi->master);
+		struct dw_spi_mchp *dwsmchp = container_of(dws,
+							   struct dw_spi_mchp,
+							   dws);
+		const struct dw_spi_mchp_props *props = dwsmchp->props;
+		u8 __iomem *src = dwsmchp->read_map +
+			(spi->chip_select * SZ_16M) + op->addr.val;
+
+		if (props->bootmaster_cs != spi->chip_select)
+			return ret;
+
+		/* Make boot master owner of SI interface */
+		dw_spi_mchp_set_cs_owner(dwsmchp, props, spi->chip_select,
+					 MSCC_IF_SI_OWNER_SIBM);
+		memcpy(op->data.buf.in, src, op->data.nbytes);
+		ret = op->data.nbytes;
+	}
+	return ret;
 }

+static const struct spi_controller_mem_ops dw_mchp_bootmaster_mem_ops = {
+	.exec_op = dw_mchp_bootmaster_exec_mem_op,
+};
+
 static int dw_spi_mchp_init(struct platform_device *pdev,
 			    struct dw_spi *dws,
 			    struct dw_spi_mchp *dwsmchp,
@@ -107,6 +240,18 @@ static int dw_spi_mchp_init(struct platform_device *pdev,
 		}
 	}

+	/* See if we have a direct read window */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+	if (res && resource_size(res) >= (SZ_16M*MAX_CS)) {
+		void __iomem *ptr = devm_ioremap_resource(&pdev->dev, res);
+
+		if (!IS_ERR(ptr)) {
+			dwsmchp->read_map = ptr;
+			dws->mem_ops = &dw_mchp_bootmaster_mem_ops;
+			dev_info(&pdev->dev, "Enabling fast memory operations\n");
+		}
+	}
+
 	dwsmchp->syscon =
 		syscon_regmap_lookup_by_compatible(props->syscon_name);
 	if (IS_ERR(dwsmchp->syscon)) {
@@ -119,10 +264,9 @@ static int dw_spi_mchp_init(struct platform_device *pdev,
 	if (dwsmchp->spi_mst)
 		writel(0, dwsmchp->spi_mst + MSCC_SPI_MST_SW_MODE);

-	/* Select the owner of the SI interface */
-	regmap_update_bits(dwsmchp->syscon, MSCC_CPU_SYSTEM_CTRL_GENERAL_CTRL,
-			   MSCC_IF_SI_OWNER_MASK << props->si_owner_bit,
-			   MSCC_IF_SI_OWNER_SIMC << props->si_owner_bit);
+	/* SPI2 mapping bitmask */
+	device_property_read_u32(&pdev->dev, "interface-mapping-mask",
+				 &dwsmchp->if2mask);

 	dwsmchp->dws.set_cs = dw_spi_mchp_set_cs;

@@ -180,6 +324,27 @@ static int dw_spi_mchp_probe(struct platform_device *pdev)
 	dws->rx_sample_dly = DIV_ROUND_UP(rx_sample_dly,
 					  (dws->max_freq / 1000000));

+	if (pdev->dev.of_node) {
+		int i;
+
+		for (i = 0; i < dws->num_cs; i++) {
+			int cs_gpio = of_get_named_gpio(pdev->dev.of_node,
+					"cs-gpios", i);
+
+			if (cs_gpio == -EPROBE_DEFER) {
+				ret = cs_gpio;
+				goto out;
+			}
+
+			if (gpio_is_valid(cs_gpio)) {
+				ret = devm_gpio_request(&pdev->dev, cs_gpio,
+						dev_name(&pdev->dev));
+				if (ret)
+					goto out;
+			}
+		}
+	}
+
 	props = device_get_match_data(&pdev->dev);
 	if (props)
 		ret = dw_spi_mchp_init(pdev, dws, dwsmchp, props);
@@ -213,6 +378,8 @@ static int dw_spi_mchp_remove(struct platform_device *pdev)
 static const struct of_device_id dw_spi_mchp_of_match[] = {
 	{ .compatible = "mscc,ocelot-spi", .data = &dw_spi_mchp_props_ocelot},
 	{ .compatible = "mscc,jaguar2-spi", .data = &dw_spi_mchp_props_jaguar2},
+	{ .compatible = "microchip,sparx5-spi",
+	  .data = &dw_spi_mchp_props_sparx5},
 	{ /* end of table */}
 };
 MODULE_DEVICE_TABLE(of, dw_spi_mchp_of_match);
--
2.26.2

Re: [PATCH 07/10] spi: spi-dw-mchp: Add Sparx5 support

[Lars Povlsen]

[Sorry about the slight delay on getting back on this]

On 14/05/20 11:25, Mark Brown wrote:

> Date: Thu, 14 May 2020 11:25:16 +0100
> From: Mark Brown <broonie@kernel.org>
> To: Lars Povlsen <lars.povlsen@microchip.com>
> Cc: SoC Team <soc@kernel.org>, Microchip Linux Driver Support
>  <UNGLinuxDriver@microchip.com>, linux-spi@vger.kernel.org,
>  devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
>  linux-arm-kernel@lists.infradead.org, Alexandre Belloni
>  <alexandre.belloni@bootlin.com>
> Subject: Re: [PATCH 07/10] spi: spi-dw-mchp: Add Sparx5 support
> User-Agent: Mutt/1.10.1 (2018-07-13)
> 
> On Wed, May 13, 2020 at 04:00:28PM +0200, Lars Povlsen wrote:
> 
> > +static void dw_spi_mchp_set_cs_owner(struct dw_spi_mchp *dwsmchp,
> > +				     const struct dw_spi_mchp_props *props,
> > +				     u8 cs, u8 owner)
> >  {
> > +	u8 dummy = (owner == MSCC_IF_SI_OWNER_SIBM ?
> > +		    MSCC_IF_SI_OWNER_SIMC : MSCC_IF_SI_OWNER_SIBM);
> 
> Please write normal conditional statements to improve legibility.
> 

I will take your recommendation to heart.

> > +static void dw_spi_mchp_set_cs(struct spi_device *spi, bool nEnable)
> > +{
> > +	bool enable = !nEnable;	/* This keeps changing in the API... */
> 
> No, it doesn't.  The API has not changed for more than a decade.
> 

I will remove the comment.

I think the comment was related to when we got bitten by the below
change, but alas.

commit ada9e3fcc175db4538f5b5e05abf5dedf626e550
Author: Charles Keepax <ckeepax@opensource.cirrus.com>
Date:   Wed Nov 27 15:39:36 2019 +0000

    spi: dw: Correct handling of native chipselect

    This patch reverts commit 6e0a32d6f376 ("spi: dw: Fix default polarity
    of native chipselect").
    
> > +	} else if (props->ss_force_ena_off) {
> > +		if (enable) {
> > +			/* Ensure CS toggles, so start off all disabled */
> > +			regmap_write(dwsmchp->syscon, props->ss_force_val_off,
> > +				     ~0);
> 
> What's all this force_ena_off stuff about?  The controller should not be
> making decisions about management of the chip select, this will break
> users.
> 

Our controller is not using DMA, but the FIFO interface. And as the DW
controller drops CS when the FIFO runs empty, this will upset SPI
devices. The "ss_force" is something the HW designes put on top to
"override" the CS. We could of course use the GPIO's specifically to
overcome this - but the "boot" CS 0 is a builtin CS, with no
underlying GPIO.

Add to this that the HW dept decided to add *2* physical SPI busses to
the same controller. That we also need to switch between. And ensure
CS gets dropped correctly before changing tracks...

Long story, lot of grief...

> > +	if (pdev->dev.of_node) {
> > +		int i;
> > +
> > +		for (i = 0; i < dws->num_cs; i++) {
> > +			int cs_gpio = of_get_named_gpio(pdev->dev.of_node,
> > +					"cs-gpios", i);
> > +
> > +			if (cs_gpio == -EPROBE_DEFER) {
> > +				ret = cs_gpio;
> > +				goto out;
> > +			}
> > +
> > +			if (gpio_is_valid(cs_gpio)) {
> > +				ret = devm_gpio_request(&pdev->dev, cs_gpio,
> > +						dev_name(&pdev->dev));
> > +				if (ret)
> > +					goto out;
> 
> Set use_gpio_descriptors and let the core manage the GPIO.

Good suggestion, just the ticket!

And thank you very much for your time & comments.

---Lars

Re: [PATCH 07/10] spi: spi-dw-mchp: Add Sparx5 support

[Serge Semin]

On Wed, May 13, 2020 at 04:00:28PM +0200, Lars Povlsen wrote:
> This adds support for the Sparx5 SoC in the spi-dw-mchp SPI controller.
> 
> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> ---
>  drivers/spi/spi-dw-mchp.c | 211 ++++++++++++++++++++++++++++++++++----
>  1 file changed, 189 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/spi/spi-dw-mchp.c b/drivers/spi/spi-dw-mchp.c
> index 0828a7616d9ab..3abdd44a550ea 100644
> --- a/drivers/spi/spi-dw-mchp.c
> +++ b/drivers/spi/spi-dw-mchp.c
> @@ -28,21 +28,22 @@
> 
>  #define MAX_CS		4
> 
> -#define MSCC_CPU_SYSTEM_CTRL_GENERAL_CTRL	0x24
> -#define OCELOT_IF_SI_OWNER_OFFSET		4
> -#define JAGUAR2_IF_SI_OWNER_OFFSET		6
>  #define MSCC_IF_SI_OWNER_MASK			GENMASK(1, 0)
>  #define MSCC_IF_SI_OWNER_SISL			0
>  #define MSCC_IF_SI_OWNER_SIBM			1
>  #define MSCC_IF_SI_OWNER_SIMC			2
> 
>  #define MSCC_SPI_MST_SW_MODE			0x14
> -#define MSCC_SPI_MST_SW_MODE_SW_PIN_CTRL_MODE	BIT(13)
> -#define MSCC_SPI_MST_SW_MODE_SW_SPI_CS(x)	(x << 5)
> 
>  struct dw_spi_mchp_props {
>  	const char *syscon_name;
> -	u32 si_owner_bit;
> +	u32 general_ctrl_off;
> +	u32 si_owner_bit, si_owner2_bit;
> +	u32 pinctrl_bit_off;
> +	u32 cs_bit_off;
> +	u32 ss_force_ena_off;
> +	u32 ss_force_val_off;
> +	u32 bootmaster_cs;
>  };
> 
>  struct dw_spi_mchp {
> @@ -53,44 +54,176 @@ struct dw_spi_mchp {
>  	void __iomem			*spi_mst;
>  	const struct dw_spi_mchp_props	*props;
>  	u32				gen_owner;
> +	u32				if2mask;
>  };
> 
>  static const struct dw_spi_mchp_props dw_spi_mchp_props_ocelot = {
>  	.syscon_name		= "mscc,ocelot-cpu-syscon",
> +	.general_ctrl_off	= 0x24,
>  	.si_owner_bit		= 4,
> +	.pinctrl_bit_off	= 13,
> +	.cs_bit_off		= 5,
> +	.bootmaster_cs		= 0,
>  };
> 
>  static const struct dw_spi_mchp_props dw_spi_mchp_props_jaguar2 = {
>  	.syscon_name		= "mscc,ocelot-cpu-syscon",
> +	.general_ctrl_off	= 0x24,
>  	.si_owner_bit		= 6,
> +	.pinctrl_bit_off	= 13,
> +	.cs_bit_off		= 5,
> +	.bootmaster_cs		= 0,
> +};
> +
> +static const struct dw_spi_mchp_props dw_spi_mchp_props_sparx5 = {
> +	.syscon_name		= "microchip,sparx5-cpu-syscon",
> +	.general_ctrl_off	= 0x88,
> +	.si_owner_bit		= 6,
> +	.si_owner2_bit		= 4,
> +	.ss_force_ena_off	= 0xa4,
> +	.ss_force_val_off	= 0xa8,
> +	.bootmaster_cs		= 0,
>  };
> 
>  /*
> - * The Designware SPI controller (referred to as master in the documentation)
> - * automatically deasserts chip select when the tx fifo is empty. The chip
> - * selects then needs to be either driven as GPIOs or, for the first 4 using the
> - * the SPI boot controller registers. the final chip select is an OR gate
> - * between the Designware SPI controller and the SPI boot controller.
> + * Set the owner of the SPI interface
>   */
> -static void dw_spi_mchp_set_cs(struct spi_device *spi, bool enable)
> +static void dw_spi_mchp_set_owner(struct dw_spi_mchp *dwsmchp,
> +				  const struct dw_spi_mchp_props *props,
> +				  u8 owner, u8 owner2)
> +{
> +	u32 val, msk;
> +
> +	val = (owner << props->si_owner_bit);
> +	msk = (MSCC_IF_SI_OWNER_MASK << props->si_owner_bit);
> +	if (props->si_owner2_bit) {
> +		val |= owner2 << props->si_owner2_bit;
> +		msk |= (MSCC_IF_SI_OWNER_MASK << props->si_owner2_bit);
> +	}
> +	if (dwsmchp->gen_owner != val) {
> +		regmap_update_bits(dwsmchp->syscon, props->general_ctrl_off,
> +				   msk, val);
> +		dwsmchp->gen_owner = val;
> +	}
> +}
> +
> +static void dw_spi_mchp_set_cs_owner(struct dw_spi_mchp *dwsmchp,
> +				     const struct dw_spi_mchp_props *props,
> +				     u8 cs, u8 owner)
>  {
> +	u8 dummy = (owner == MSCC_IF_SI_OWNER_SIBM ?
> +		    MSCC_IF_SI_OWNER_SIMC : MSCC_IF_SI_OWNER_SIBM);
> +	if (props->si_owner2_bit && (dwsmchp->if2mask & BIT(cs))) {
> +		/* SPI2 */
> +		dw_spi_mchp_set_owner(dwsmchp, props, dummy, owner);
> +	} else {
> +		/* SPI1 */
> +		dw_spi_mchp_set_owner(dwsmchp, props, owner, dummy);
> +	}
> +}
> +
> +/*
> + * The Designware SPI controller (referred to as master in the
> + * documentation) automatically deasserts chip select when the tx fifo
> + * is empty. The chip selects then needs to be either driven as GPIOs
> + * or, for the first 4 using the the SPI boot controller
> + * registers. the final chip select is an OR gate between the
> + * Designware SPI controller and the SPI boot controller.  nselect is
> + * an active low signal
> + */
> +static void dw_spi_mchp_set_cs(struct spi_device *spi, bool nEnable)
> +{
> +	bool enable = !nEnable;	/* This keeps changing in the API... */
>  	struct dw_spi *dws = spi_master_get_devdata(spi->master);
>  	struct dw_spi_mchp *dwsmchp = container_of(dws, struct dw_spi_mchp,
>  						   dws);
> -	u32 cs = spi->chip_select;
> +	const struct dw_spi_mchp_props *props = dwsmchp->props;
> +	u8 cs = spi->chip_select;
> 
> -	if (cs < 4) {
> -		u32 sw_mode = MSCC_SPI_MST_SW_MODE_SW_PIN_CTRL_MODE;
> +	if (enable)
> +		dw_spi_mchp_set_cs_owner(dwsmchp, props, cs,
> +					 MSCC_IF_SI_OWNER_SIMC);
> 
> -		if (!enable)
> -			sw_mode |= MSCC_SPI_MST_SW_MODE_SW_SPI_CS(BIT(cs));
> +	if (dwsmchp->spi_mst && (cs < MAX_CS)) {
> +		u32 sw_mode;
> 
> +		if (enable)
> +			sw_mode = BIT(props->pinctrl_bit_off) |
> +				(BIT(cs) << props->cs_bit_off);
> +		else
> +			sw_mode = 0;
>  		writel(sw_mode, dwsmchp->spi_mst + MSCC_SPI_MST_SW_MODE);
> +	} else if (props->ss_force_ena_off) {
> +		if (enable) {
> +			/* Ensure CS toggles, so start off all disabled */
> +			regmap_write(dwsmchp->syscon, props->ss_force_val_off,
> +				     ~0);
> +			/* CS override drive enable */
> +			regmap_write(dwsmchp->syscon, props->ss_force_ena_off,
> +				     1);
> +			/* Allow settle */
> +			udelay(1);
> +			/* Now set CSx enabled */
> +			regmap_write(dwsmchp->syscon, props->ss_force_val_off,
> +				     ~BIT(cs));
> +		} else {
> +			/* CS value */
> +			regmap_write(dwsmchp->syscon, props->ss_force_val_off,
> +				     ~0);
> +			/* CS override drive disable */
> +			regmap_write(dwsmchp->syscon, props->ss_force_ena_off,
> +				     0);
> +		}
>  	}
> 
> -	dw_spi_set_cs(spi, enable);
> +	dw_spi_set_cs(spi, nEnable);
> +}
> +
> +static int dw_mchp_bootmaster_exec_mem_op(struct spi_mem *mem,
> +					  const struct spi_mem_op *op)
> +{
> +	struct spi_device *spi = mem->spi;
> +	int ret = -ENOTSUPP;
> +

> +	/* Only reads, addrsize 1..4 */
> +	if (!op->data.nbytes || !op->addr.nbytes || op->addr.nbytes > 4 ||
> +	    op->data.dir != SPI_MEM_DATA_IN)
> +		return ret;
> +
> +	/* Only handle (normal+fast) 3/4 bytes read */
> +	if (op->cmd.opcode != SPINOR_OP_READ &&
> +	    op->cmd.opcode != SPINOR_OP_READ_FAST &&
> +	    op->cmd.opcode != SPINOR_OP_READ_4B &&
> +	    op->cmd.opcode != SPINOR_OP_READ_FAST_4B)
> +		return ret;

Hm, this part most like belongs to supports_op() callback.

> +
> +	/* CS0..3, only 16M reach */
> +	if ((spi->chip_select < MAX_CS) &&

> +	    (op->addr.val + op->data.nbytes) < SZ_16M) {

The driver shouldn't return a failure if more than available data requested.
Just return the length the driver managed to read.

> +		struct dw_spi *dws = spi_master_get_devdata(spi->master);
> +		struct dw_spi_mchp *dwsmchp = container_of(dws,
> +							   struct dw_spi_mchp,
> +							   dws);
> +		const struct dw_spi_mchp_props *props = dwsmchp->props;
> +		u8 __iomem *src = dwsmchp->read_map +
> +			(spi->chip_select * SZ_16M) + op->addr.val;
> +
> +		if (props->bootmaster_cs != spi->chip_select)
> +			return ret;
> +
> +		/* Make boot master owner of SI interface */
> +		dw_spi_mchp_set_cs_owner(dwsmchp, props, spi->chip_select,
> +					 MSCC_IF_SI_OWNER_SIBM);

> +		memcpy(op->data.buf.in, src, op->data.nbytes);

So after all it's just memcpy from the directly mapped SPI-flash memory, right?
Then it's not mem_op, but I supposed it should be implemented by means of the
dirmap_{create,read,destroy}.

-Sergey

> +		ret = op->data.nbytes;
> +	}
> +	return ret;
>  }
> 
> +static const struct spi_controller_mem_ops dw_mchp_bootmaster_mem_ops = {
> +	.exec_op = dw_mchp_bootmaster_exec_mem_op,
> +};
> +
>  static int dw_spi_mchp_init(struct platform_device *pdev,
>  			    struct dw_spi *dws,
>  			    struct dw_spi_mchp *dwsmchp,
> @@ -107,6 +240,18 @@ static int dw_spi_mchp_init(struct platform_device *pdev,
>  		}
>  	}
> 
> +	/* See if we have a direct read window */
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> +	if (res && resource_size(res) >= (SZ_16M*MAX_CS)) {
> +		void __iomem *ptr = devm_ioremap_resource(&pdev->dev, res);
> +
> +		if (!IS_ERR(ptr)) {
> +			dwsmchp->read_map = ptr;
> +			dws->mem_ops = &dw_mchp_bootmaster_mem_ops;
> +			dev_info(&pdev->dev, "Enabling fast memory operations\n");
> +		}
> +	}
> +
>  	dwsmchp->syscon =
>  		syscon_regmap_lookup_by_compatible(props->syscon_name);
>  	if (IS_ERR(dwsmchp->syscon)) {
> @@ -119,10 +264,9 @@ static int dw_spi_mchp_init(struct platform_device *pdev,
>  	if (dwsmchp->spi_mst)
>  		writel(0, dwsmchp->spi_mst + MSCC_SPI_MST_SW_MODE);
> 
> -	/* Select the owner of the SI interface */
> -	regmap_update_bits(dwsmchp->syscon, MSCC_CPU_SYSTEM_CTRL_GENERAL_CTRL,
> -			   MSCC_IF_SI_OWNER_MASK << props->si_owner_bit,
> -			   MSCC_IF_SI_OWNER_SIMC << props->si_owner_bit);
> +	/* SPI2 mapping bitmask */
> +	device_property_read_u32(&pdev->dev, "interface-mapping-mask",
> +				 &dwsmchp->if2mask);
> 
>  	dwsmchp->dws.set_cs = dw_spi_mchp_set_cs;
> 
> @@ -180,6 +324,27 @@ static int dw_spi_mchp_probe(struct platform_device *pdev)
>  	dws->rx_sample_dly = DIV_ROUND_UP(rx_sample_dly,
>  					  (dws->max_freq / 1000000));
> 
> +	if (pdev->dev.of_node) {
> +		int i;
> +
> +		for (i = 0; i < dws->num_cs; i++) {
> +			int cs_gpio = of_get_named_gpio(pdev->dev.of_node,
> +					"cs-gpios", i);
> +
> +			if (cs_gpio == -EPROBE_DEFER) {
> +				ret = cs_gpio;
> +				goto out;
> +			}
> +
> +			if (gpio_is_valid(cs_gpio)) {
> +				ret = devm_gpio_request(&pdev->dev, cs_gpio,
> +						dev_name(&pdev->dev));
> +				if (ret)
> +					goto out;
> +			}
> +		}
> +	}
> +
>  	props = device_get_match_data(&pdev->dev);
>  	if (props)
>  		ret = dw_spi_mchp_init(pdev, dws, dwsmchp, props);
> @@ -213,6 +378,8 @@ static int dw_spi_mchp_remove(struct platform_device *pdev)
>  static const struct of_device_id dw_spi_mchp_of_match[] = {
>  	{ .compatible = "mscc,ocelot-spi", .data = &dw_spi_mchp_props_ocelot},
>  	{ .compatible = "mscc,jaguar2-spi", .data = &dw_spi_mchp_props_jaguar2},
> +	{ .compatible = "microchip,sparx5-spi",
> +	  .data = &dw_spi_mchp_props_sparx5},
>  	{ /* end of table */}
>  };
>  MODULE_DEVICE_TABLE(of, dw_spi_mchp_of_match);
> --
> 2.26.2
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 08/10] arm64: dts: sparx5: Add SPI controller

[Lars Povlsen]

This adds a SPI controller to the Microchip Sparx5 SoC

Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 arch/arm64/boot/dts/microchip/sparx5.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/microchip/sparx5.dtsi b/arch/arm64/boot/dts/microchip/sparx5.dtsi
index b5f2d088af30e..daa216978887d 100644
--- a/arch/arm64/boot/dts/microchip/sparx5.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5.dtsi
@@ -14,6 +14,7 @@ / {
 	#size-cells = <1>;

 	aliases {
+		spi0 = &spi0;
 		serial0 = &uart0;
 		serial1 = &uart1;
 	};
@@ -144,6 +145,21 @@ uart1: serial@600102000 {
 			status = "disabled";
 		};

+		spi0: spi@600104000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "microchip,sparx5-spi";
+			reg = <0x6 0x00104000 0x40>, <0 0 0>,
+			      <0x3 0x0 0x4000000>;
+			num-cs = <16>;
+			reg-io-width = <4>;
+			reg-shift = <2>;
+			clocks = <&ahb_clk>;
+			interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
+
+			status = "disabled";
+		};
+
 		timer1: timer@600105000 {
 			compatible = "snps,dw-apb-timer";
 			reg = <0x6 0x00105000 0x1000>;
--
2.26.2

[PATCH 09/10] arm64: dts: sparx5: Add spi-nor support

[Lars Povlsen]

This add spi-nor device nodes to the Sparx5 reference boards.

Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 arch/arm64/boot/dts/microchip/sparx5.dtsi              | 5 +++--
 arch/arm64/boot/dts/microchip/sparx5_pcb125.dts        | 9 +++++++++
 arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi | 9 +++++++++
 arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi | 9 +++++++++
 4 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/microchip/sparx5.dtsi b/arch/arm64/boot/dts/microchip/sparx5.dtsi
index daa216978887d..330fd8b096d4c 100644
--- a/arch/arm64/boot/dts/microchip/sparx5.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5.dtsi
@@ -155,8 +155,9 @@ spi0: spi@600104000 {
 			reg-io-width = <4>;
 			reg-shift = <2>;
 			clocks = <&ahb_clk>;
-			interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
-
+			/* NB: Polled mode - next line commented out
+			 * interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
+			 */
 			status = "disabled";
 		};

diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts b/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts
index 573309fe45823..d8b5d23abfab0 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts
@@ -39,6 +39,15 @@ &sdhci0 {
 	microchip,clock-delay = <10>;
 };

+&spi0 {
+	status = "okay";
+	spi-flash@0 {
+		compatible = "jedec,spi-nor";
+		spi-max-frequency = <8000000>; /* input clock */
+		reg = <0>; /* CS0 */
+	};
+};
+
 &i2c1 {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi b/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
index 18a535a043686..628a05d3f57ce 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
@@ -38,6 +38,15 @@ gpio-restart {
 	};
 };

+&spi0 {
+	status = "okay";
+	spi-flash@0 {
+		compatible = "jedec,spi-nor";
+		spi-max-frequency = <8000000>;
+		reg = <0>;
+	};
+};
+
 &gpio {
 	i2cmux_pins_i: i2cmux-pins-i {
 	       pins = "GPIO_16", "GPIO_17", "GPIO_18", "GPIO_19",
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi b/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi
index d71f11a10b3d2..fb0bc3b241204 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi
@@ -51,6 +51,15 @@ i2cmux_s32: i2cmux-3 {
 	};
 };

+&spi0 {
+	status = "okay";
+	spi-flash@0 {
+		compatible = "jedec,spi-nor";
+		spi-max-frequency = <8000000>;
+		reg = <0>;
+	};
+};
+
 &axi {
 	i2c0_imux: i2c0-imux@0 {
 		compatible = "i2c-mux-pinctrl";
--
2.26.2

[PATCH 10/10] arm64: dts: sparx5: Add spi-nand devices

[Lars Povlsen]

This patch add spi-nand DT nodes to the applicable Sparx5 boards.

Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 arch/arm64/boot/dts/microchip/sparx5.dtsi     | 20 ++++++++++++++++
 .../boot/dts/microchip/sparx5_pcb125.dts      |  7 ++++++
 .../boot/dts/microchip/sparx5_pcb134.dts      | 22 ++++++++++++++++++
 .../boot/dts/microchip/sparx5_pcb135.dts      | 23 +++++++++++++++++++
 4 files changed, 72 insertions(+)

diff --git a/arch/arm64/boot/dts/microchip/sparx5.dtsi b/arch/arm64/boot/dts/microchip/sparx5.dtsi
index 330fd8b096d4c..60629861a5157 100644
--- a/arch/arm64/boot/dts/microchip/sparx5.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5.dtsi
@@ -193,6 +193,26 @@ gpio: pinctrl@6110101e0 {
 			interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
 			#interrupt-cells = <2>;

+			cs1_pins: cs1-pins {
+				pins = "GPIO_16";
+				function = "si";
+			};
+
+			cs2_pins: cs2-pins {
+				pins = "GPIO_17";
+				function = "si";
+			};
+
+			cs3_pins: cs3-pins {
+				pins = "GPIO_18";
+				function = "si";
+			};
+
+			si2_pins: si2-pins {
+				pins = "GPIO_39", "GPIO_40", "GPIO_41";
+				function = "si2";
+			};
+
 			uart_pins: uart-pins {
 				pins = "GPIO_10", "GPIO_11";
 				function = "uart";
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts b/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts
index d8b5d23abfab0..94c4c3fd5a786 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts
@@ -46,6 +46,13 @@ spi-flash@0 {
 		spi-max-frequency = <8000000>; /* input clock */
 		reg = <0>; /* CS0 */
 	};
+	spi-flash@1 {
+		compatible = "spi-nand";
+		pinctrl-0 = <&cs1_pins>;
+		pinctrl-names = "default";
+		spi-max-frequency = <8000000>;
+		reg = <1>; /* CS1 */
+	};
 };

 &i2c1 {
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb134.dts b/arch/arm64/boot/dts/microchip/sparx5_pcb134.dts
index feee4e99ff57c..9e8dc725a954a 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb134.dts
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb134.dts
@@ -15,3 +15,25 @@ memory@0 {
 		reg = <0x00000000 0x00000000 0x10000000>;
 	};
 };
+
+&gpio {
+	cs14_pins: cs14-pins {
+		pins = "GPIO_44";
+		function = "si";
+	};
+};
+
+&spi0 {
+	pinctrl-0 = <&si2_pins>;
+	pinctrl-names = "default";
+	interface-mapping-mask = <0x4000>;	/* NAND CS14 = SPI2 */
+	spi-rx-delay-us = <500>;		/* Tune for speed */
+	/* Dedicated SPI2 interface */
+	spi-flash@e {
+		compatible = "spi-nand";
+		pinctrl-0 = <&cs14_pins>;
+		pinctrl-names = "default";
+		spi-max-frequency = <42000000>;
+		reg = <14>;
+	};
+};
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb135.dts b/arch/arm64/boot/dts/microchip/sparx5_pcb135.dts
index 20e409a9be196..a31e10911dbaf 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb135.dts
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb135.dts
@@ -15,3 +15,26 @@ memory@0 {
 		reg = <0x00000000 0x00000000 0x10000000>;
 	};
 };
+
+&gpio {
+	cs14_pins: cs14-pins {
+		pins = "GPIO_44";
+		function = "si";
+	};
+};
+
+&spi0 {
+	status = "okay";
+	pinctrl-0 = <&si2_pins>;
+	pinctrl-names = "default";
+	interface-mapping-mask = <0x4000>;	/* NAND CS14 = SPI2 */
+	spi-rx-delay-us = <500>;		/* Tune for speed */
+	/* Dedicated SPI2 interface */
+	spi-flash@e {
+		compatible = "spi-nand";
+		pinctrl-0 = <&cs14_pins>;
+		pinctrl-names = "default";
+		spi-max-frequency = <42000000>;
+		reg = <14>;
+	};
+};
--
2.26.2

Re: [PATCH 00/10] spi: Adding support for Microchip Sparx5 SoC

[Serge Semin]

Hello Lars,

On Wed, May 13, 2020 at 04:00:21PM +0200, Lars Povlsen wrote:
> This is an add-on series to the main SoC Sparx5 series
> (Message-ID: <20200513125532.24585-1-lars.povlsen@microchip.com>).
> 
> The series add support for Sparx5 on top of the existing
> ocelot/jaguar2 spi driver.
> 
> It spins off the existing support for the MSCC platforms into a
> separate driver, as adding new platforms from the MSCC/Microchip
> product lines will further complicate (clutter) the original driver.
> 
> New YAML dt-bindings are provided for the resulting driver.
> 
> It is expected that the DT patches are to be taken directly by the arm-soc
> maintainers.

Regarding our cooperation. It can be implemented as follows. Since your patchset
is less cumbersome than mine and is more ready to be integrated into the generic DW
APB SSI code, it would be better to first make it through Mark', Andy' and my reviews
to be further merged into the kernel version of the driver. After that I'll have
my code altered so it could be applied on top of your patches. When everything
is done we'll have a more comprehensive DW APB SSI driver with poll-based
PIO operations support, new features like rx-delay, etc.

Thank you one more time for the series you've shared with us. Let's see what can
be done to improve it...

-Sergey

> 
> Lars Povlsen (10):
>   spi: dw: Add support for polled operation via no IRQ specified in DT
>   spi: dw: Add support for RX sample delay register
>   spi: dw: Add support for client driver memory operations
>   dt-bindings: spi: Add bindings for spi-dw-mchp
>   spi: spi-dw-mmio: Spin off MSCC platforms into spi-dw-mchp
>   dt-bindings: spi: spi-dw-mchp: Add Sparx5 support
>   spi: spi-dw-mchp: Add Sparx5 support
>   arm64: dts: sparx5: Add SPI controller
>   arm64: dts: sparx5: Add spi-nor support
>   arm64: dts: sparx5: Add spi-nand devices
> 
>  .../bindings/spi/mscc,ocelot-spi.yaml         |  89 ++++
>  .../bindings/spi/snps,dw-apb-ssi.txt          |   7 +-
>  MAINTAINERS                                   |   2 +
>  arch/arm64/boot/dts/microchip/sparx5.dtsi     |  37 ++
>  .../boot/dts/microchip/sparx5_pcb125.dts      |  16 +
>  .../boot/dts/microchip/sparx5_pcb134.dts      |  22 +
>  .../dts/microchip/sparx5_pcb134_board.dtsi    |   9 +
>  .../boot/dts/microchip/sparx5_pcb135.dts      |  23 +
>  .../dts/microchip/sparx5_pcb135_board.dtsi    |   9 +
>  arch/mips/configs/generic/board-ocelot.config |   2 +-
>  drivers/spi/Kconfig                           |   7 +
>  drivers/spi/Makefile                          |   1 +
>  drivers/spi/spi-dw-mchp.c                     | 399 ++++++++++++++++++
>  drivers/spi/spi-dw-mmio.c                     |  93 ----
>  drivers/spi/spi-dw.c                          |  31 +-
>  drivers/spi/spi-dw.h                          |   4 +
>  16 files changed, 644 insertions(+), 107 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
>  create mode 100644 drivers/spi/spi-dw-mchp.c
> 
> --
> 2.26.2
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 00/10] spi: Adding support for Microchip Sparx5 SoC

[Lars Povlsen]

Serge Semin writes:

> Hello Lars,
>
> On Wed, May 13, 2020 at 04:00:21PM +0200, Lars Povlsen wrote:
>> This is an add-on series to the main SoC Sparx5 series
>> (Message-ID: <20200513125532.24585-1-lars.povlsen@microchip.com>).
>>
>> The series add support for Sparx5 on top of the existing
>> ocelot/jaguar2 spi driver.
>>
>> It spins off the existing support for the MSCC platforms into a
>> separate driver, as adding new platforms from the MSCC/Microchip
>> product lines will further complicate (clutter) the original driver.
>>
>> New YAML dt-bindings are provided for the resulting driver.
>>
>> It is expected that the DT patches are to be taken directly by the arm-soc
>> maintainers.
>
> Regarding our cooperation. It can be implemented as follows. Since your patchset
> is less cumbersome than mine and is more ready to be integrated into the generic DW
> APB SSI code, it would be better to first make it through Mark', Andy' and my reviews
> to be further merged into the kernel version of the driver. After that I'll have
> my code altered so it could be applied on top of your patches. When everything
> is done we'll have a more comprehensive DW APB SSI driver with poll-based
> PIO operations support, new features like rx-delay, etc.
>

Hi Serge!

I think I would be able to work on the SPI patches this week. Should I
base it on the current spi-next or 5.7? Then address the comments and
send out a new revision?

Thanks for reaching out.

---Lars

> Thank you one more time for the series you've shared with us. Let's see what can
> be done to improve it...
>
> -Sergey
>
>>
>> Lars Povlsen (10):
>>   spi: dw: Add support for polled operation via no IRQ specified in DT
>>   spi: dw: Add support for RX sample delay register
>>   spi: dw: Add support for client driver memory operations
>>   dt-bindings: spi: Add bindings for spi-dw-mchp
>>   spi: spi-dw-mmio: Spin off MSCC platforms into spi-dw-mchp
>>   dt-bindings: spi: spi-dw-mchp: Add Sparx5 support
>>   spi: spi-dw-mchp: Add Sparx5 support
>>   arm64: dts: sparx5: Add SPI controller
>>   arm64: dts: sparx5: Add spi-nor support
>>   arm64: dts: sparx5: Add spi-nand devices
>>
>>  .../bindings/spi/mscc,ocelot-spi.yaml         |  89 ++++
>>  .../bindings/spi/snps,dw-apb-ssi.txt          |   7 +-
>>  MAINTAINERS                                   |   2 +
>>  arch/arm64/boot/dts/microchip/sparx5.dtsi     |  37 ++
>>  .../boot/dts/microchip/sparx5_pcb125.dts      |  16 +
>>  .../boot/dts/microchip/sparx5_pcb134.dts      |  22 +
>>  .../dts/microchip/sparx5_pcb134_board.dtsi    |   9 +
>>  .../boot/dts/microchip/sparx5_pcb135.dts      |  23 +
>>  .../dts/microchip/sparx5_pcb135_board.dtsi    |   9 +
>>  arch/mips/configs/generic/board-ocelot.config |   2 +-
>>  drivers/spi/Kconfig                           |   7 +
>>  drivers/spi/Makefile                          |   1 +
>>  drivers/spi/spi-dw-mchp.c                     | 399 ++++++++++++++++++
>>  drivers/spi/spi-dw-mmio.c                     |  93 ----
>>  drivers/spi/spi-dw.c                          |  31 +-
>>  drivers/spi/spi-dw.h                          |   4 +
>>  16 files changed, 644 insertions(+), 107 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
>>  create mode 100644 drivers/spi/spi-dw-mchp.c
>>
>> --
>> 2.26.2
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

--
Lars Povlsen,
Microchip

Re: [PATCH 00/10] spi: Adding support for Microchip Sparx5 SoC

[Serge Semin]

On Tue, Jun 02, 2020 at 10:18:28AM +0200, Lars Povlsen wrote:
> 
> Serge Semin writes:
> 
> > Hello Lars,
> >
> > On Wed, May 13, 2020 at 04:00:21PM +0200, Lars Povlsen wrote:
> >> This is an add-on series to the main SoC Sparx5 series
> >> (Message-ID: <20200513125532.24585-1-lars.povlsen@microchip.com>).
> >>
> >> The series add support for Sparx5 on top of the existing
> >> ocelot/jaguar2 spi driver.
> >>
> >> It spins off the existing support for the MSCC platforms into a
> >> separate driver, as adding new platforms from the MSCC/Microchip
> >> product lines will further complicate (clutter) the original driver.
> >>
> >> New YAML dt-bindings are provided for the resulting driver.
> >>
> >> It is expected that the DT patches are to be taken directly by the arm-soc
> >> maintainers.
> >
> > Regarding our cooperation. It can be implemented as follows. Since your patchset
> > is less cumbersome than mine and is more ready to be integrated into the generic DW
> > APB SSI code, it would be better to first make it through Mark', Andy' and my reviews
> > to be further merged into the kernel version of the driver. After that I'll have
> > my code altered so it could be applied on top of your patches. When everything
> > is done we'll have a more comprehensive DW APB SSI driver with poll-based
> > PIO operations support, new features like rx-delay, etc.
> >
> 
> Hi Serge!
> 
> I think I would be able to work on the SPI patches this week. Should I
> base it on the current spi-next or 5.7? Then address the comments and
> send out a new revision?
> 
> Thanks for reaching out.

Sorry for a delay. I had to finish urgent tasks first. I'll give my review comments
shortly today.

-Sergey

> 
> ---Lars
> 
> > Thank you one more time for the series you've shared with us. Let's see what can
> > be done to improve it...
> >
> > -Sergey
> >
> >>
> >> Lars Povlsen (10):
> >>   spi: dw: Add support for polled operation via no IRQ specified in DT
> >>   spi: dw: Add support for RX sample delay register
> >>   spi: dw: Add support for client driver memory operations
> >>   dt-bindings: spi: Add bindings for spi-dw-mchp
> >>   spi: spi-dw-mmio: Spin off MSCC platforms into spi-dw-mchp
> >>   dt-bindings: spi: spi-dw-mchp: Add Sparx5 support
> >>   spi: spi-dw-mchp: Add Sparx5 support
> >>   arm64: dts: sparx5: Add SPI controller
> >>   arm64: dts: sparx5: Add spi-nor support
> >>   arm64: dts: sparx5: Add spi-nand devices
> >>
> >>  .../bindings/spi/mscc,ocelot-spi.yaml         |  89 ++++
> >>  .../bindings/spi/snps,dw-apb-ssi.txt          |   7 +-
> >>  MAINTAINERS                                   |   2 +
> >>  arch/arm64/boot/dts/microchip/sparx5.dtsi     |  37 ++
> >>  .../boot/dts/microchip/sparx5_pcb125.dts      |  16 +
> >>  .../boot/dts/microchip/sparx5_pcb134.dts      |  22 +
> >>  .../dts/microchip/sparx5_pcb134_board.dtsi    |   9 +
> >>  .../boot/dts/microchip/sparx5_pcb135.dts      |  23 +
> >>  .../dts/microchip/sparx5_pcb135_board.dtsi    |   9 +
> >>  arch/mips/configs/generic/board-ocelot.config |   2 +-
> >>  drivers/spi/Kconfig                           |   7 +
> >>  drivers/spi/Makefile                          |   1 +
> >>  drivers/spi/spi-dw-mchp.c                     | 399 ++++++++++++++++++
> >>  drivers/spi/spi-dw-mmio.c                     |  93 ----
> >>  drivers/spi/spi-dw.c                          |  31 +-
> >>  drivers/spi/spi-dw.h                          |   4 +
> >>  16 files changed, 644 insertions(+), 107 deletions(-)
> >>  create mode 100644 Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
> >>  create mode 100644 drivers/spi/spi-dw-mchp.c
> >>
> >> --
> >> 2.26.2
> >>
> >> _______________________________________________
> >> linux-arm-kernel mailing list
> >> linux-arm-kernel@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> --
> Lars Povlsen,
> Microchip

Re: [PATCH 00/10] spi: Adding support for Microchip Sparx5 SoC

[Serge Semin]

On Tue, Jun 02, 2020 at 10:18:28AM +0200, Lars Povlsen wrote:
> 
> Serge Semin writes:
> 
> > Hello Lars,
> >
> > On Wed, May 13, 2020 at 04:00:21PM +0200, Lars Povlsen wrote:
> >> This is an add-on series to the main SoC Sparx5 series
> >> (Message-ID: <20200513125532.24585-1-lars.povlsen@microchip.com>).
> >>
> >> The series add support for Sparx5 on top of the existing
> >> ocelot/jaguar2 spi driver.
> >>
> >> It spins off the existing support for the MSCC platforms into a
> >> separate driver, as adding new platforms from the MSCC/Microchip
> >> product lines will further complicate (clutter) the original driver.
> >>
> >> New YAML dt-bindings are provided for the resulting driver.
> >>
> >> It is expected that the DT patches are to be taken directly by the arm-soc
> >> maintainers.
> >
> > Regarding our cooperation. It can be implemented as follows. Since your patchset
> > is less cumbersome than mine and is more ready to be integrated into the generic DW
> > APB SSI code, it would be better to first make it through Mark', Andy' and my reviews
> > to be further merged into the kernel version of the driver. After that I'll have
> > my code altered so it could be applied on top of your patches. When everything
> > is done we'll have a more comprehensive DW APB SSI driver with poll-based
> > PIO operations support, new features like rx-delay, etc.
> >
> 

> Hi Serge!
> 
> I think I would be able to work on the SPI patches this week. Should I
> base it on the current spi-next or 5.7? Then address the comments and
> send out a new revision?

Finally I've done a part of review. It must be enough for v2. As Mark said the
new version is supposed to be based on the spi-next, since that branch's got
all recent DW APB SSI patches applied.

-Sergey

> 
> Thanks for reaching out.
> 
> ---Lars
> 
> > Thank you one more time for the series you've shared with us. Let's see what can
> > be done to improve it...
> >
> > -Sergey
> >
> >>
> >> Lars Povlsen (10):
> >>   spi: dw: Add support for polled operation via no IRQ specified in DT
> >>   spi: dw: Add support for RX sample delay register
> >>   spi: dw: Add support for client driver memory operations
> >>   dt-bindings: spi: Add bindings for spi-dw-mchp
> >>   spi: spi-dw-mmio: Spin off MSCC platforms into spi-dw-mchp
> >>   dt-bindings: spi: spi-dw-mchp: Add Sparx5 support
> >>   spi: spi-dw-mchp: Add Sparx5 support
> >>   arm64: dts: sparx5: Add SPI controller
> >>   arm64: dts: sparx5: Add spi-nor support
> >>   arm64: dts: sparx5: Add spi-nand devices
> >>
> >>  .../bindings/spi/mscc,ocelot-spi.yaml         |  89 ++++
> >>  .../bindings/spi/snps,dw-apb-ssi.txt          |   7 +-
> >>  MAINTAINERS                                   |   2 +
> >>  arch/arm64/boot/dts/microchip/sparx5.dtsi     |  37 ++
> >>  .../boot/dts/microchip/sparx5_pcb125.dts      |  16 +
> >>  .../boot/dts/microchip/sparx5_pcb134.dts      |  22 +
> >>  .../dts/microchip/sparx5_pcb134_board.dtsi    |   9 +
> >>  .../boot/dts/microchip/sparx5_pcb135.dts      |  23 +
> >>  .../dts/microchip/sparx5_pcb135_board.dtsi    |   9 +
> >>  arch/mips/configs/generic/board-ocelot.config |   2 +-
> >>  drivers/spi/Kconfig                           |   7 +
> >>  drivers/spi/Makefile                          |   1 +
> >>  drivers/spi/spi-dw-mchp.c                     | 399 ++++++++++++++++++
> >>  drivers/spi/spi-dw-mmio.c                     |  93 ----
> >>  drivers/spi/spi-dw.c                          |  31 +-
> >>  drivers/spi/spi-dw.h                          |   4 +
> >>  16 files changed, 644 insertions(+), 107 deletions(-)
> >>  create mode 100644 Documentation/devicetree/bindings/spi/mscc,ocelot-spi.yaml
> >>  create mode 100644 drivers/spi/spi-dw-mchp.c
> >>
> >> --
> >> 2.26.2
> >>
> >> _______________________________________________
> >> linux-arm-kernel mailing list
> >> linux-arm-kernel@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> --
> Lars Povlsen,
> Microchip