[PATCH 0/3] ARM: dspi: Provide slave mode support for Vybryd vf610

[PATCH 0/3] ARM: dspi: Provide slave mode support for Vybryd vf610

[Lukasz Majewski]

This patch series provides support for DSPI slave mode operation.
It tries to maximally reuse current DMA driver (including its limitations).

For testing the spidev_test program has been used.
Test script for this patch can be found here:
https://github.com/lmajewski/tests-spi/blob/master/tests/spi/spi_tests.sh

Lukasz Majewski (3):
  dt-bindings: spi: Provide bindings for fsl dspi working in slave mode
  ARM: dspi: Provide per DSPI instance of the MCR register (SLAVE mode)
  ARM: dspi: Provide support for DSPI slave more operation (Vybryd
    vf610)

 .../devicetree/bindings/spi/spi-fsl-dspi.txt       |  8 ++++
 drivers/spi/spi-fsl-dspi.c                         | 54 ++++++++++++++++------
 2 files changed, 49 insertions(+), 13 deletions(-)

-- 
2.11.0

[PATCH 1/3] dt-bindings: spi: Provide bindings for fsl dspi working in slave mode

[Lukasz Majewski]

This commit provides the description of new property: "fsl,spi-slave-mode"
which enables support for DSPI driver working in slave mode.

As the new compatible shall be used with SPI bus equipped with master
device a new "spidev" based node has been introduced to avoid confusion.

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
 Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt b/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt
index 18eeafe359d8..b30af19a2bc5 100644
--- a/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt
+++ b/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt
@@ -26,6 +26,7 @@ Optional SPI properties for slave nodes:
 - fsl,spi-sck-cs-delay: a delay in nanoseconds between stopping the clock
   signal and deactivating chip select, at the end of a transfer.
 - bus-num : the slave chip chipselect signal number.
+- fsl,spi-slave-mode: if present, controller runs in slave mode.
 
 Example:
 
@@ -56,6 +57,13 @@ dspi0@4002c000 {
 		fsl,spi-cs-sck-delay = <100>;
 		fsl,spi-sck-cs-delay = <50>;
 	};
+
+	spidev3@1 {
+		compatible = "fsl,vf610-dspi";
+		spi-max-frequency = <30000000>;
+		reg = <1>;
+		fsl,spi-slave-mode;
+	};
 };
 
 
-- 
2.11.0

Re: [PATCH 1/3] dt-bindings: spi: Provide bindings for fsl dspi working in slave mode

[Lukasz Majewski]

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

Hi Rob,

> On Tue, Sep 18, 2018 at 11:34:35AM +0200, Lukasz Majewski wrote:
> > This commit provides the description of new property:
> > "fsl,spi-slave-mode" which enables support for DSPI driver working
> > in slave mode.
> > 
> > As the new compatible shall be used with SPI bus equipped with
> > master device a new "spidev" based node has been introduced to
> > avoid confusion.
> > 
> > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> > ---
> >  Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt
> > b/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt index
> > 18eeafe359d8..b30af19a2bc5 100644 ---
> > a/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt +++
> > b/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt @@ -26,6
> > +26,7 @@ Optional SPI properties for slave nodes:
> >  - fsl,spi-sck-cs-delay: a delay in nanoseconds between stopping
> > the clock signal and deactivating chip select, at the end of a
> > transfer.
> >  - bus-num : the slave chip chipselect signal number.
> > +- fsl,spi-slave-mode: if present, controller runs in slave mode.  
> 
> There's a standard property for this.

I've just realised that there is "spi-slave" property in the
Documentation/devicetree/bindings/spi/spi-bus.txt

I will convert the code to use it.

> 
> >  
> >  Example:
> >  
> > @@ -56,6 +57,13 @@ dspi0@4002c000 {
> >  		fsl,spi-cs-sck-delay = <100>;
> >  		fsl,spi-sck-cs-delay = <50>;
> >  	};
> > +
> > +	spidev3@1 {
> > +		compatible = "fsl,vf610-dspi";
> > +		spi-max-frequency = <30000000>;
> > +		reg = <1>;
> > +		fsl,spi-slave-mode;  
> 
> This doesn't look right for how slave mode is described.

You mean that I shall replace fsl,spi-slave-mode; with "spi-slave" ?

Or is there any other issue with this code?

Thanks for your review,

> 
> > +	};
> >  };
> >  
> >  
> > -- 
> > 2.11.0
> >   
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/3] dt-bindings: spi: Provide bindings for fsl dspi working in slave mode

[Geert Uytterhoeven]

Hi Lukasz,

On Wed, Sep 26, 2018 at 4:33 PM Lukasz Majewski <lukma@denx.de> wrote:
> > On Tue, Sep 18, 2018 at 11:34:35AM +0200, Lukasz Majewski wrote:
> > > This commit provides the description of new property:
> > > "fsl,spi-slave-mode" which enables support for DSPI driver working
> > > in slave mode.
> > >
> > > As the new compatible shall be used with SPI bus equipped with
> > > master device a new "spidev" based node has been introduced to
> > > avoid confusion.
> > >
> > > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> > > ---
> > >  Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt
> > > b/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt index
> > > 18eeafe359d8..b30af19a2bc5 100644 ---
> > > a/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt +++
> > > b/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt @@ -26,6
> > > +26,7 @@ Optional SPI properties for slave nodes:
> > >  - fsl,spi-sck-cs-delay: a delay in nanoseconds between stopping
> > > the clock signal and deactivating chip select, at the end of a
> > > transfer.
> > >  - bus-num : the slave chip chipselect signal number.
> > > +- fsl,spi-slave-mode: if present, controller runs in slave mode.
> >
> > There's a standard property for this.
>
> I've just realised that there is "spi-slave" property in the
> Documentation/devicetree/bindings/spi/spi-bus.txt
>
> I will convert the code to use it.
>
> >
> > >
> > >  Example:
> > >
> > > @@ -56,6 +57,13 @@ dspi0@4002c000 {
> > >             fsl,spi-cs-sck-delay = <100>;
> > >             fsl,spi-sck-cs-delay = <50>;
> > >     };
> > > +
> > > +   spidev3@1 {
> > > +           compatible = "fsl,vf610-dspi";
> > > +           spi-max-frequency = <30000000>;
> > > +           reg = <1>;
> > > +           fsl,spi-slave-mode;
> >
> > This doesn't look right for how slave mode is described.
>
> You mean that I shall replace fsl,spi-slave-mode; with "spi-slave" ?
>
> Or is there any other issue with this code?

According to Documentation/devicetree/bindings/spi/spi-bus.txt:
  1. "spi-slave" should be a property of the controller node, not of the
     slave node,
  2. the slave node should be called "slave", without unit address, and
     without "reg" property.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/3] dt-bindings: spi: Provide bindings for fsl dspi working in slave mode

[Lukasz Majewski]

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

Hi Geert,

> Hi Lukasz,
> 
> On Wed, Sep 26, 2018 at 4:33 PM Lukasz Majewski <lukma@denx.de> wrote:
> > > On Tue, Sep 18, 2018 at 11:34:35AM +0200, Lukasz Majewski wrote:  
> > > > This commit provides the description of new property:
> > > > "fsl,spi-slave-mode" which enables support for DSPI driver
> > > > working in slave mode.
> > > >
> > > > As the new compatible shall be used with SPI bus equipped with
> > > > master device a new "spidev" based node has been introduced to
> > > > avoid confusion.
> > > >
> > > > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> > > > ---
> > > >  Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt | 8
> > > > ++++++++ 1 file changed, 8 insertions(+)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt
> > > > b/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt index
> > > > 18eeafe359d8..b30af19a2bc5 100644 ---
> > > > a/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt +++
> > > > b/Documentation/devicetree/bindings/spi/spi-fsl-dspi.txt @@
> > > > -26,6 +26,7 @@ Optional SPI properties for slave nodes:
> > > >  - fsl,spi-sck-cs-delay: a delay in nanoseconds between stopping
> > > > the clock signal and deactivating chip select, at the end of a
> > > > transfer.
> > > >  - bus-num : the slave chip chipselect signal number.
> > > > +- fsl,spi-slave-mode: if present, controller runs in slave
> > > > mode.  
> > >
> > > There's a standard property for this.  
> >
> > I've just realised that there is "spi-slave" property in the
> > Documentation/devicetree/bindings/spi/spi-bus.txt
> >
> > I will convert the code to use it.
> >  
> > >  
> > > >
> > > >  Example:
> > > >
> > > > @@ -56,6 +57,13 @@ dspi0@4002c000 {
> > > >             fsl,spi-cs-sck-delay = <100>;
> > > >             fsl,spi-sck-cs-delay = <50>;
> > > >     };
> > > > +
> > > > +   spidev3@1 {
> > > > +           compatible = "fsl,vf610-dspi";
> > > > +           spi-max-frequency = <30000000>;
> > > > +           reg = <1>;
> > > > +           fsl,spi-slave-mode;  
> > >
> > > This doesn't look right for how slave mode is described.  
> >
> > You mean that I shall replace fsl,spi-slave-mode; with "spi-slave" ?
> >
> > Or is there any other issue with this code?  
> 
> According to Documentation/devicetree/bindings/spi/spi-bus.txt:
>   1. "spi-slave" should be a property of the controller node, not of
> the slave node,
>   2. the slave node should be called "slave", without unit address,
> and without "reg" property.

Thanks for clarification.

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 2/3] ARM: dspi: Provide per DSPI instance of the MCR register (SLAVE mode)

[Lukasz Majewski]

The vf610 Vybryd can work in two DSPI modes - namely master and slave.
Already we do support master mode.
This commit serves as a prerequisite to add support for slave mode. For
slave mode we do need to preserve the MCR register value to change its
"mode" bit (31) according to the supported state.

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
 drivers/spi/spi-fsl-dspi.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 7cb3ab0a35a0..472385f0a842 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -126,6 +126,7 @@
 #define DMA_COMPLETION_TIMEOUT	msecs_to_jiffies(3000)
 
 struct chip_data {
+	u32 mcr_val;
 	u32 ctar_val;
 	u16 void_write_data;
 };
@@ -696,9 +697,9 @@ static int dspi_transfer_one_message(struct spi_master *master,
 		else
 			dspi->bytes_per_word = 4;
 
-		regmap_update_bits(dspi->regmap, SPI_MCR,
-				   SPI_MCR_CLR_TXF | SPI_MCR_CLR_RXF,
-				   SPI_MCR_CLR_TXF | SPI_MCR_CLR_RXF);
+		regmap_write(dspi->regmap, SPI_MCR, dspi->cur_chip->mcr_val |
+			     SPI_MCR_CLR_TXF | SPI_MCR_CLR_RXF);
+
 		regmap_write(dspi->regmap, SPI_CTAR(0),
 			     dspi->cur_chip->ctar_val |
 			     SPI_FRAME_BITS(transfer->bits_per_word));
@@ -767,6 +768,11 @@ static int dspi_setup(struct spi_device *spi)
 			return -ENOMEM;
 	}
 
+	chip->mcr_val = SPI_MCR_MASTER | SPI_MCR_PCSIS;
+
+	if (dspi->devtype_data->xspi_mode)
+		chip->mcr_val |= SPI_MCR_XSPI;
+
 	pdata = dev_get_platdata(&dspi->pdev->dev);
 
 	if (!pdata) {
@@ -964,8 +970,6 @@ static const struct regmap_config dspi_xspi_regmap_config[] = {
 
 static void dspi_init(struct fsl_dspi *dspi)
 {
-	regmap_write(dspi->regmap, SPI_MCR, SPI_MCR_MASTER | SPI_MCR_PCSIS |
-		     (dspi->devtype_data->xspi_mode ? SPI_MCR_XSPI : 0));
 	regmap_write(dspi->regmap, SPI_SR, SPI_SR_CLEAR);
 	if (dspi->devtype_data->xspi_mode)
 		regmap_write(dspi->regmap, SPI_CTARE(0),
-- 
2.11.0

[PATCH 3/3] ARM: dspi: Provide support for DSPI slave more operation (Vybryd vf610)

[Lukasz Majewski]

The NXP's Vybryd vf610 can work as a SPI slave device (the CS and clock
signal are provided by master).

It is possible to specify a single device to work in that mode. As we do
use DMA for transferring data, the RX channel must be prepared for
incoming data.
Moreover, in slave mode we just set a subset of control fields in
configuration registers (CTAR0, PUSHR).

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
 drivers/spi/spi-fsl-dspi.c | 40 ++++++++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 472385f0a842..1d71f6fd2e0b 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -209,6 +209,14 @@ struct fsl_dspi {
 	struct fsl_dspi_dma	*dma;
 };
 
+static inline bool dspi_slave_mode(struct fsl_dspi *dspi)
+{
+	if (!(dspi->cur_chip->mcr_val & SPI_MCR_MASTER))
+		return true;
+
+	return false;
+}
+
 static u32 dspi_pop_tx(struct fsl_dspi *dspi)
 {
 	u32 txdata = 0;
@@ -230,6 +238,9 @@ static u32 dspi_pop_tx_pushr(struct fsl_dspi *dspi)
 {
 	u16 cmd = dspi->tx_cmd, data = dspi_pop_tx(dspi);
 
+	if (dspi_slave_mode(dspi))
+		return data;
+
 	if (dspi->len > 0)
 		cmd |= SPI_PUSHR_CMD_CONT;
 	return cmd << 16 | data;
@@ -326,6 +337,11 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
 	dma_async_issue_pending(dma->chan_rx);
 	dma_async_issue_pending(dma->chan_tx);
 
+	if (dspi_slave_mode(dspi)) {
+		wait_for_completion_interruptible(&dspi->dma->cmd_rx_complete);
+		return 0;
+	}
+
 	time_left = wait_for_completion_timeout(&dspi->dma->cmd_tx_complete,
 					DMA_COMPLETION_TIMEOUT);
 	if (time_left == 0) {
@@ -781,6 +797,10 @@ static int dspi_setup(struct spi_device *spi)
 
 		of_property_read_u32(spi->dev.of_node, "fsl,spi-sck-cs-delay",
 				&sck_cs_delay);
+
+		if (of_property_read_bool(spi->dev.of_node,
+					  "fsl,spi-slave-mode"))
+			chip->mcr_val &= ~SPI_MCR_MASTER;
 	} else {
 		cs_sck_delay = pdata->cs_sck_delay;
 		sck_cs_delay = pdata->sck_cs_delay;
@@ -798,14 +818,18 @@ static int dspi_setup(struct spi_device *spi)
 	ns_delay_scale(&pasc, &asc, sck_cs_delay, clkrate);
 
 	chip->ctar_val = SPI_CTAR_CPOL(spi->mode & SPI_CPOL ? 1 : 0)
-		| SPI_CTAR_CPHA(spi->mode & SPI_CPHA ? 1 : 0)
-		| SPI_CTAR_LSBFE(spi->mode & SPI_LSB_FIRST ? 1 : 0)
-		| SPI_CTAR_PCSSCK(pcssck)
-		| SPI_CTAR_CSSCK(cssck)
-		| SPI_CTAR_PASC(pasc)
-		| SPI_CTAR_ASC(asc)
-		| SPI_CTAR_PBR(pbr)
-		| SPI_CTAR_BR(br);
+		| SPI_CTAR_CPHA(spi->mode & SPI_CPHA ? 1 : 0);
+
+	if (chip->mcr_val & SPI_MCR_MASTER) {
+		chip->ctar_val |= SPI_CTAR_LSBFE(spi->mode &
+						 SPI_LSB_FIRST ? 1 : 0)
+			| SPI_CTAR_PCSSCK(pcssck)
+			| SPI_CTAR_CSSCK(cssck)
+			| SPI_CTAR_PASC(pasc)
+			| SPI_CTAR_ASC(asc)
+			| SPI_CTAR_PBR(pbr)
+			| SPI_CTAR_BR(br);
+	}
 
 	spi_set_ctldata(spi, chip);
 
-- 
2.11.0

Re: [PATCH 0/3] ARM: dspi: Provide slave mode support for Vybryd vf610

[Lukasz Majewski]

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

Dear All,

By a mistake I've forgotten to add (to CC/TO) some developers who worked
on this driver before (they were not generated from get_maintainers.py
script).

I'm sending the notification now.

> This patch series provides support for DSPI slave mode operation.
> It tries to maximally reuse current DMA driver (including its
> limitations).
> 
> For testing the spidev_test program has been used.
> Test script for this patch can be found here:
> https://github.com/lmajewski/tests-spi/blob/master/tests/spi/spi_tests.sh
> 
> Lukasz Majewski (3):
>   dt-bindings: spi: Provide bindings for fsl dspi working in slave
> mode ARM: dspi: Provide per DSPI instance of the MCR register (SLAVE
> mode) ARM: dspi: Provide support for DSPI slave more operation (Vybryd
>     vf610)
> 
>  .../devicetree/bindings/spi/spi-fsl-dspi.txt       |  8 ++++
>  drivers/spi/spi-fsl-dspi.c                         | 54
> ++++++++++++++++------ 2 files changed, 49 insertions(+), 13
> deletions(-)
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]