[PATCH v3 0/3] spi: atmel: add support to FIFOs and the internal chip-select

[PATCH v3 0/3] spi: atmel: add support to FIFOs and the internal chip-select

[Cyrille Pitchen]

ChangeLog

v3:
- replace "Alternative Command Mode" by "Chip Select Active After Transfer" in
  the DT bindings documentation. The "Alternative Command Mode" is a feature of
  the I2C controller.
- rename somes "size" variables to "num_data" to ease the distinction between
  the number of bytes and the number of data to transfer.

v2:
- read the Status Register to clear its RX FIFO Threshold Flag (RXFTHF) before
  writing into the IER to enable the RXFTHF interrupt.
- remove the reset of the FIFO Mode Register (FMR) at the end of transfers
  because setting the RX FIFO Threshold to 0 also sets the RXFTHF bit in SR.
- add a 3rd patch in the series to update the documentation of DT bindings.

v1:
This series of patches add support to features introduced by the Atmel sama5d2x
SoC

Cyrille Pitchen (3):
  spi: atmel: add support for the internal chip-select of the spi
    controller
  spi: atmel: update DT bindings documentation
  spi: atmel: add support to FIFOs

 .../devicetree/bindings/spi/spi_atmel.txt          |   8 +-
 drivers/spi/spi-atmel.c                            | 292 +++++++++++++++++++--
 2 files changed, 280 insertions(+), 20 deletions(-)

-- 
1.8.2.2

[PATCH v3 2/3] spi: atmel: update DT bindings documentation

[Cyrille Pitchen]

- add new property "atmel,fifo-size"
- change "cs-gpios" to optional for SPI controller version >= 2.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
---
 Documentation/devicetree/bindings/spi/spi_atmel.txt | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/spi/spi_atmel.txt b/Documentation/devicetree/bindings/spi/spi_atmel.txt
index 4f8184d..fb588b3 100644
--- a/Documentation/devicetree/bindings/spi/spi_atmel.txt
+++ b/Documentation/devicetree/bindings/spi/spi_atmel.txt
@@ -4,11 +4,16 @@ Required properties:
 - compatible : should be "atmel,at91rm9200-spi".
 - reg: Address and length of the register set for the device
 - interrupts: Should contain spi interrupt
-- cs-gpios: chipselects
+- cs-gpios: chipselects (optional for SPI controller version >= 2 with the
+  Chip Select Active After Transfer feature).
 - clock-names: tuple listing input clock names.
 	Required elements: "spi_clk"
 - clocks: phandles to input clocks.
 
+Optional properties:
+- atmel,fifo-size: maximum number of data the RX and TX FIFOs can store for FIFO
+  capable SPI controllers.
+
 Example:
 
 spi1: spi@fffcc000 {
@@ -20,6 +25,7 @@ spi1: spi@fffcc000 {
 	clocks = <&spi1_clk>;
 	clock-names = "spi_clk";
 	cs-gpios = <&pioB 3 0>;
+	atmel,fifo-size = <32>;
 	status = "okay";
 
 	mmc-slot@0 {
-- 
1.8.2.2

Re: [PATCH v3 2/3] spi: atmel: update DT bindings documentation

[Nicolas Ferre]

Le 09/06/2015 13:53, Cyrille Pitchen a écrit :
> - add new property "atmel,fifo-size"
> - change "cs-gpios" to optional for SPI controller version >= 2.
> 
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>

Acked-by: Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>

> ---
>  Documentation/devicetree/bindings/spi/spi_atmel.txt | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi_atmel.txt b/Documentation/devicetree/bindings/spi/spi_atmel.txt
> index 4f8184d..fb588b3 100644
> --- a/Documentation/devicetree/bindings/spi/spi_atmel.txt
> +++ b/Documentation/devicetree/bindings/spi/spi_atmel.txt
> @@ -4,11 +4,16 @@ Required properties:
>  - compatible : should be "atmel,at91rm9200-spi".
>  - reg: Address and length of the register set for the device
>  - interrupts: Should contain spi interrupt
> -- cs-gpios: chipselects
> +- cs-gpios: chipselects (optional for SPI controller version >= 2 with the
> +  Chip Select Active After Transfer feature).
>  - clock-names: tuple listing input clock names.
>  	Required elements: "spi_clk"
>  - clocks: phandles to input clocks.
>  
> +Optional properties:
> +- atmel,fifo-size: maximum number of data the RX and TX FIFOs can store for FIFO
> +  capable SPI controllers.
> +
>  Example:
>  
>  spi1: spi@fffcc000 {
> @@ -20,6 +25,7 @@ spi1: spi@fffcc000 {
>  	clocks = <&spi1_clk>;
>  	clock-names = "spi_clk";
>  	cs-gpios = <&pioB 3 0>;
> +	atmel,fifo-size = <32>;
>  	status = "okay";
>  
>  	mmc-slot@0 {
> 


-- 
Nicolas Ferre
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/3] spi: atmel: update DT bindings documentation

[Mark Brown]

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

On Tue, Jun 09, 2015 at 01:53:53PM +0200, Cyrille Pitchen wrote:
> - add new property "atmel,fifo-size"

Why is this a property and not something we know from the IP version?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v3 2/3] spi: atmel: update DT bindings documentation

[Cyrille Pitchen]

Le 09/06/2015 19:25, Mark Brown a écrit :
> On Tue, Jun 09, 2015 at 01:53:53PM +0200, Cyrille Pitchen wrote:
>> - add new property "atmel,fifo-size"
> 
> Why is this a property and not something we know from the IP version?
>
Hi Mark,
 
Please be aware that the VERSION register can not be used to guess the
size of FIFOs. Indeed, for a given hardware version, the SPI controller
can be integrated on Atmel SoCs with different FIFO sizes. Also the
"atmel,fifo-size" property is optional as older SPI controllers don't
embed FIFO at all.

Besides, the FIFO size can not be read or guessed from other registers:
When designing the FIFO feature, no dedicated registers were added to
store this size. Unused spaces in the I/O register range are limited and
better reserved for future usages. Instead, the FIFO size of each
peripheral is documented in the programmer datasheet.

Finally, on a given SoC, there can be several instances of the SPI
controller with different FIFO sizes. This explain why we'd rather use a
dedicated DT property than use the "compatible" property.

I hope these pieces of information will help to clarify this point.
Of course, we are open to other suggestions.

Best Regards,

Cyrille
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/3] spi: atmel: update DT bindings documentation

[Mark Brown]

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

On Thu, Jun 11, 2015 at 06:37:51PM +0200, Cyrille Pitchen wrote:
> Le 09/06/2015 19:25, Mark Brown a écrit :
> > On Tue, Jun 09, 2015 at 01:53:53PM +0200, Cyrille Pitchen wrote:
> >> - add new property "atmel,fifo-size"

> > Why is this a property and not something we know from the IP version?

> Please be aware that the VERSION register can not be used to guess the
> size of FIFOs. Indeed, for a given hardware version, the SPI controller
> can be integrated on Atmel SoCs with different FIFO sizes. Also the
> "atmel,fifo-size" property is optional as older SPI controllers don't
> embed FIFO at all.

...

> Finally, on a given SoC, there can be several instances of the SPI
> controller with different FIFO sizes. This explain why we'd rather use a
> dedicated DT property than use the "compatible" property.

> I hope these pieces of information will help to clarify this point.
> Of course, we are open to other suggestions.

Ugh, what a wonderfully consistent hardware design :(  Please make it
clear in the documentation what is going on here, this looks like an
obvious bug in the DT binding - a very common pattern for bugs is to do
version quirks like this as properties.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[PATCH v3 1/3] spi: atmel: add support for the internal chip-select of the spi controller

[Cyrille Pitchen]

This patch relies on the CSAAT (Chip Select Active After Transfer) feature
introduced by the version 2 of the spi controller. This new mode allows to
use properly the internal chip-select output pin of the spi controller
instead of using external gpios. Consequently, the "cs-gpios" device-tree
property becomes optional.

When the new CSAAT bit is set into the Chip Select Register, the internal
chip-select output pin remains asserted till both the following conditions
become true:
- the LASTXFER bit is set into the Control Register (or the Transmit Data
  Register)
- the Transmit Data Register and its shift register are empty.

WARNING: if the LASTXFER bit is set into the Control Register then new
data are written into the Transmit Data Register fast enough to keep its
shifter not empty, the chip-select output pin remains asserted. Only when
the shifter becomes empty, the chip-select output pin is unasserted.

When the CSAAT bit is clear in the Chip Select Register, the LASTXFER bit
is ignored in both the Control Register and the Transmit Data Register.
The internal chip-select output pin remains active as long as the Transmit
Data Register or its shift register are not empty.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/spi-atmel.c | 37 ++++++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index a2f40b1..aa7d202 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -246,6 +246,7 @@ struct atmel_spi {
 
 	bool			use_dma;
 	bool			use_pdc;
+	bool			use_cs_gpios;
 	/* dmaengine data */
 	struct atmel_spi_dma	dma;
 
@@ -321,7 +322,8 @@ static void cs_activate(struct atmel_spi *as, struct spi_device *spi)
 		}
 
 		mr = spi_readl(as, MR);
-		gpio_set_value(asd->npcs_pin, active);
+		if (as->use_cs_gpios)
+			gpio_set_value(asd->npcs_pin, active);
 	} else {
 		u32 cpol = (spi->mode & SPI_CPOL) ? SPI_BIT(CPOL) : 0;
 		int i;
@@ -337,7 +339,7 @@ static void cs_activate(struct atmel_spi *as, struct spi_device *spi)
 
 		mr = spi_readl(as, MR);
 		mr = SPI_BFINS(PCS, ~(1 << spi->chip_select), mr);
-		if (spi->chip_select != 0)
+		if (as->use_cs_gpios && spi->chip_select != 0)
 			gpio_set_value(asd->npcs_pin, active);
 		spi_writel(as, MR, mr);
 	}
@@ -366,7 +368,9 @@ static void cs_deactivate(struct atmel_spi *as, struct spi_device *spi)
 			asd->npcs_pin, active ? " (low)" : "",
 			mr);
 
-	if (atmel_spi_is_v2(as) || spi->chip_select != 0)
+	if (!as->use_cs_gpios)
+		spi_writel(as, CR, SPI_BIT(LASTXFER));
+	else if (atmel_spi_is_v2(as) || spi->chip_select != 0)
 		gpio_set_value(asd->npcs_pin, !active);
 }
 
@@ -996,6 +1000,8 @@ static int atmel_spi_setup(struct spi_device *spi)
 		csr |= SPI_BIT(CPOL);
 	if (!(spi->mode & SPI_CPHA))
 		csr |= SPI_BIT(NCPHA);
+	if (!as->use_cs_gpios)
+		csr |= SPI_BIT(CSAAT);
 
 	/* DLYBS is mostly irrelevant since we manage chipselect using GPIOs.
 	 *
@@ -1009,7 +1015,9 @@ static int atmel_spi_setup(struct spi_device *spi)
 	/* chipselect must have been muxed as GPIO (e.g. in board setup) */
 	npcs_pin = (unsigned long)spi->controller_data;
 
-	if (gpio_is_valid(spi->cs_gpio))
+	if (!as->use_cs_gpios)
+		npcs_pin = spi->chip_select;
+	else if (gpio_is_valid(spi->cs_gpio))
 		npcs_pin = spi->cs_gpio;
 
 	asd = spi->controller_state;
@@ -1018,15 +1026,19 @@ static int atmel_spi_setup(struct spi_device *spi)
 		if (!asd)
 			return -ENOMEM;
 
-		ret = gpio_request(npcs_pin, dev_name(&spi->dev));
-		if (ret) {
-			kfree(asd);
-			return ret;
+		if (as->use_cs_gpios) {
+			ret = gpio_request(npcs_pin, dev_name(&spi->dev));
+			if (ret) {
+				kfree(asd);
+				return ret;
+			}
+
+			gpio_direction_output(npcs_pin,
+					      !(spi->mode & SPI_CS_HIGH));
 		}
 
 		asd->npcs_pin = npcs_pin;
 		spi->controller_state = asd;
-		gpio_direction_output(npcs_pin, !(spi->mode & SPI_CS_HIGH));
 	}
 
 	asd->csr = csr;
@@ -1338,6 +1350,13 @@ static int atmel_spi_probe(struct platform_device *pdev)
 
 	atmel_get_caps(as);
 
+	as->use_cs_gpios = true;
+	if (atmel_spi_is_v2(as) &&
+	    !of_get_property(pdev->dev.of_node, "cs-gpios", NULL)) {
+		as->use_cs_gpios = false;
+		master->num_chipselect = 4;
+	}
+
 	as->use_dma = false;
 	as->use_pdc = false;
 	if (as->caps.has_dma_support) {
-- 
1.8.2.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/3] spi: atmel: add support for the internal chip-select of the spi controller

[Nicolas Ferre]

Le 09/06/2015 13:53, Cyrille Pitchen a écrit :
> This patch relies on the CSAAT (Chip Select Active After Transfer) feature
> introduced by the version 2 of the spi controller. This new mode allows to
> use properly the internal chip-select output pin of the spi controller
> instead of using external gpios. Consequently, the "cs-gpios" device-tree
> property becomes optional.
> 
> When the new CSAAT bit is set into the Chip Select Register, the internal
> chip-select output pin remains asserted till both the following conditions
> become true:
> - the LASTXFER bit is set into the Control Register (or the Transmit Data
>   Register)
> - the Transmit Data Register and its shift register are empty.
> 
> WARNING: if the LASTXFER bit is set into the Control Register then new
> data are written into the Transmit Data Register fast enough to keep its
> shifter not empty, the chip-select output pin remains asserted. Only when
> the shifter becomes empty, the chip-select output pin is unasserted.
> 
> When the CSAAT bit is clear in the Chip Select Register, the LASTXFER bit
> is ignored in both the Control Register and the Transmit Data Register.
> The internal chip-select output pin remains active as long as the Transmit
> Data Register or its shift register are not empty.
> 
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
Acked-by: Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>

> ---
>  drivers/spi/spi-atmel.c | 37 ++++++++++++++++++++++++++++---------
>  1 file changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
> index a2f40b1..aa7d202 100644
> --- a/drivers/spi/spi-atmel.c
> +++ b/drivers/spi/spi-atmel.c
> @@ -246,6 +246,7 @@ struct atmel_spi {
>  
>  	bool			use_dma;
>  	bool			use_pdc;
> +	bool			use_cs_gpios;
>  	/* dmaengine data */
>  	struct atmel_spi_dma	dma;
>  
> @@ -321,7 +322,8 @@ static void cs_activate(struct atmel_spi *as, struct spi_device *spi)
>  		}
>  
>  		mr = spi_readl(as, MR);
> -		gpio_set_value(asd->npcs_pin, active);
> +		if (as->use_cs_gpios)
> +			gpio_set_value(asd->npcs_pin, active);
>  	} else {
>  		u32 cpol = (spi->mode & SPI_CPOL) ? SPI_BIT(CPOL) : 0;
>  		int i;
> @@ -337,7 +339,7 @@ static void cs_activate(struct atmel_spi *as, struct spi_device *spi)
>  
>  		mr = spi_readl(as, MR);
>  		mr = SPI_BFINS(PCS, ~(1 << spi->chip_select), mr);
> -		if (spi->chip_select != 0)
> +		if (as->use_cs_gpios && spi->chip_select != 0)
>  			gpio_set_value(asd->npcs_pin, active);
>  		spi_writel(as, MR, mr);
>  	}
> @@ -366,7 +368,9 @@ static void cs_deactivate(struct atmel_spi *as, struct spi_device *spi)
>  			asd->npcs_pin, active ? " (low)" : "",
>  			mr);
>  
> -	if (atmel_spi_is_v2(as) || spi->chip_select != 0)
> +	if (!as->use_cs_gpios)
> +		spi_writel(as, CR, SPI_BIT(LASTXFER));
> +	else if (atmel_spi_is_v2(as) || spi->chip_select != 0)
>  		gpio_set_value(asd->npcs_pin, !active);
>  }
>  
> @@ -996,6 +1000,8 @@ static int atmel_spi_setup(struct spi_device *spi)
>  		csr |= SPI_BIT(CPOL);
>  	if (!(spi->mode & SPI_CPHA))
>  		csr |= SPI_BIT(NCPHA);
> +	if (!as->use_cs_gpios)
> +		csr |= SPI_BIT(CSAAT);
>  
>  	/* DLYBS is mostly irrelevant since we manage chipselect using GPIOs.
>  	 *
> @@ -1009,7 +1015,9 @@ static int atmel_spi_setup(struct spi_device *spi)
>  	/* chipselect must have been muxed as GPIO (e.g. in board setup) */
>  	npcs_pin = (unsigned long)spi->controller_data;
>  
> -	if (gpio_is_valid(spi->cs_gpio))
> +	if (!as->use_cs_gpios)
> +		npcs_pin = spi->chip_select;
> +	else if (gpio_is_valid(spi->cs_gpio))
>  		npcs_pin = spi->cs_gpio;
>  
>  	asd = spi->controller_state;
> @@ -1018,15 +1026,19 @@ static int atmel_spi_setup(struct spi_device *spi)
>  		if (!asd)
>  			return -ENOMEM;
>  
> -		ret = gpio_request(npcs_pin, dev_name(&spi->dev));
> -		if (ret) {
> -			kfree(asd);
> -			return ret;
> +		if (as->use_cs_gpios) {
> +			ret = gpio_request(npcs_pin, dev_name(&spi->dev));
> +			if (ret) {
> +				kfree(asd);
> +				return ret;
> +			}
> +
> +			gpio_direction_output(npcs_pin,
> +					      !(spi->mode & SPI_CS_HIGH));
>  		}
>  
>  		asd->npcs_pin = npcs_pin;
>  		spi->controller_state = asd;
> -		gpio_direction_output(npcs_pin, !(spi->mode & SPI_CS_HIGH));
>  	}
>  
>  	asd->csr = csr;
> @@ -1338,6 +1350,13 @@ static int atmel_spi_probe(struct platform_device *pdev)
>  
>  	atmel_get_caps(as);
>  
> +	as->use_cs_gpios = true;
> +	if (atmel_spi_is_v2(as) &&
> +	    !of_get_property(pdev->dev.of_node, "cs-gpios", NULL)) {
> +		as->use_cs_gpios = false;
> +		master->num_chipselect = 4;
> +	}
> +
>  	as->use_dma = false;
>  	as->use_pdc = false;
>  	if (as->caps.has_dma_support) {
> 


-- 
Nicolas Ferre
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/3] spi: atmel: add support for the internal chip-select of the spi controller

[Mark Brown]

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

On Tue, Jun 09, 2015 at 01:53:52PM +0200, Cyrille Pitchen wrote:
> This patch relies on the CSAAT (Chip Select Active After Transfer) feature
> introduced by the version 2 of the spi controller. This new mode allows to
> use properly the internal chip-select output pin of the spi controller
> instead of using external gpios. Consequently, the "cs-gpios" device-tree
> property becomes optional.

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v3 1/3] spi: atmel: add support for the internal chip-select of the spi controller

[Måns Rullgård]

Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org> writes:

> This patch relies on the CSAAT (Chip Select Active After Transfer) feature
> introduced by the version 2 of the spi controller. This new mode allows to
> use properly the internal chip-select output pin of the spi controller
> instead of using external gpios. Consequently, the "cs-gpios" device-tree
> property becomes optional.
>
> When the new CSAAT bit is set into the Chip Select Register, the internal
> chip-select output pin remains asserted till both the following conditions
> become true:
> - the LASTXFER bit is set into the Control Register (or the Transmit Data
>   Register)
> - the Transmit Data Register and its shift register are empty.
>
> WARNING: if the LASTXFER bit is set into the Control Register then new
> data are written into the Transmit Data Register fast enough to keep its
> shifter not empty, the chip-select output pin remains asserted. Only when
> the shifter becomes empty, the chip-select output pin is unasserted.
>
> When the CSAAT bit is clear in the Chip Select Register, the LASTXFER bit
> is ignored in both the Control Register and the Transmit Data Register.
> The internal chip-select output pin remains active as long as the Transmit
> Data Register or its shift register are not empty.
>
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/spi/spi-atmel.c | 37 ++++++++++++++++++++++++++++---------
>  1 file changed, 28 insertions(+), 9 deletions(-)

[...]

> @@ -1338,6 +1350,13 @@ static int atmel_spi_probe(struct platform_device *pdev)
>
>  	atmel_get_caps(as);
>
> +	as->use_cs_gpios = true;
> +	if (atmel_spi_is_v2(as) &&
> +	    !of_get_property(pdev->dev.of_node, "cs-gpios", NULL)) {
> +		as->use_cs_gpios = false;
> +		master->num_chipselect = 4;
> +	}

This part breaks the AVR32 boards and probably anything else that
doesn't use devicetree but does use GPIOs for chip select.

-- 
Måns Rullgård
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/3] spi: atmel: add support for the internal chip-select of the spi controller

[Mark Brown]

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

On Tue, Jan 05, 2016 at 09:50:54PM +0000, Måns Rullgård wrote:
> Cyrille Pitchen <cyrille.pitchen@atmel.com> writes:

> > +	as->use_cs_gpios = true;
> > +	if (atmel_spi_is_v2(as) &&
> > +	    !of_get_property(pdev->dev.of_node, "cs-gpios", NULL)) {
> > +		as->use_cs_gpios = false;
> > +		master->num_chipselect = 4;
> > +	}

> This part breaks the AVR32 boards and probably anything else that
> doesn't use devicetree but does use GPIOs for chip select.

Shouldn't this just be setting defaults for the case where nothing is
provided?

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

Re: [PATCH v3 1/3] spi: atmel: add support for the internal chip-select of the spi controller

[Måns Rullgård]

Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> writes:

> On Tue, Jan 05, 2016 at 09:50:54PM +0000, Måns Rullgård wrote:
>> Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org> writes:
>
>> > +	as->use_cs_gpios = true;
>> > +	if (atmel_spi_is_v2(as) &&
>> > +	    !of_get_property(pdev->dev.of_node, "cs-gpios", NULL)) {
>> > +		as->use_cs_gpios = false;
>> > +		master->num_chipselect = 4;
>> > +	}
>
>> This part breaks the AVR32 boards and probably anything else that
>> doesn't use devicetree but does use GPIOs for chip select.
>
> Shouldn't this just be setting defaults for the case where nothing is
> provided?

Traditionally the platform data included a GPIO number to use and that
was that.  At some point DT support was added wherein the platform data
is overridden by cs_gpio from struct spi_device if this is valid (it is
set to a negative value by default).  Thus far all was well.  Then came
this patch.  It assumes that everybody uses DT and treats a missing
cs-gpios property as indication that the controller's own pins should be
used.  It also assumes that if any device uses GPIO for CS all of them
do, even though the SPI core driver might provide a partial list
(probably since many boards don't use all the available chip selects,
but it doesn't prevent someone abusing this).

To work correctly in call cases, this driver should use, for each
peripheral, the following priority:

- spi->cs_gpio (filled from DT or -ENOENT)
- GPIO from platform data
- controller chip select pin

The trouble is that there is no way to reliably tell a valid GPIO number
of zero in the platform data from an unset value.  In practice, I
believe existing old boards using this driver all use a non-zero GPIO
(the AVR32 platform code requires this), so checking for a non-zero
number is probably sufficient.  I'll cook up a patch for this unless
someone objects.

-- 
Måns Rullgård
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/3] spi: atmel: add support for the internal chip-select of the spi controller

[Nicolas Ferre]

Le 05/01/2016 22:50, Måns Rullgård a écrit :
> Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org> writes:
> 
>> This patch relies on the CSAAT (Chip Select Active After Transfer) feature
>> introduced by the version 2 of the spi controller. This new mode allows to
>> use properly the internal chip-select output pin of the spi controller
>> instead of using external gpios. Consequently, the "cs-gpios" device-tree
>> property becomes optional.
>>
>> When the new CSAAT bit is set into the Chip Select Register, the internal
>> chip-select output pin remains asserted till both the following conditions
>> become true:
>> - the LASTXFER bit is set into the Control Register (or the Transmit Data
>>   Register)
>> - the Transmit Data Register and its shift register are empty.
>>
>> WARNING: if the LASTXFER bit is set into the Control Register then new
>> data are written into the Transmit Data Register fast enough to keep its
>> shifter not empty, the chip-select output pin remains asserted. Only when
>> the shifter becomes empty, the chip-select output pin is unasserted.
>>
>> When the CSAAT bit is clear in the Chip Select Register, the LASTXFER bit
>> is ignored in both the Control Register and the Transmit Data Register.
>> The internal chip-select output pin remains active as long as the Transmit
>> Data Register or its shift register are not empty.
>>
>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
>> ---
>>  drivers/spi/spi-atmel.c | 37 ++++++++++++++++++++++++++++---------
>>  1 file changed, 28 insertions(+), 9 deletions(-)
> 
> [...]
> 
>> @@ -1338,6 +1350,13 @@ static int atmel_spi_probe(struct platform_device *pdev)
>>
>>  	atmel_get_caps(as);
>>
>> +	as->use_cs_gpios = true;
>> +	if (atmel_spi_is_v2(as) &&
>> +	    !of_get_property(pdev->dev.of_node, "cs-gpios", NULL)) {
>> +		as->use_cs_gpios = false;
>> +		master->num_chipselect = 4;
>> +	}
> 
> This part breaks the AVR32 boards and probably anything else that
> doesn't use devicetree but does use GPIOs for chip select.

Hi Mans,

I have difficulties finding why you may enter this test. So, maybe you
can give me a clue by reading for me the value that resides in the SPI
version register: you can have it by reading at 0xFFE000FC for instance
(actually the atmel_get_caps() dev_info() call gives it as well in the
boot log which is somewhat easier: I tried to find one on the Internet
without success...).

So I think that just fixing the logic in atmel_get_caps() introduced by
d4820b7496219edd9a7055022681364d304525f7 can make it come back to a
situation where the ARV32 was more tested than nowadays.

Bye,
-- 
Nicolas Ferre
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/3] spi: atmel: add support for the internal chip-select of the spi controller

[Måns Rullgård]

Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org> writes:

> Le 05/01/2016 22:50, Måns Rullgård a écrit :
>> Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org> writes:
>> 
>>> This patch relies on the CSAAT (Chip Select Active After Transfer) feature
>>> introduced by the version 2 of the spi controller. This new mode allows to
>>> use properly the internal chip-select output pin of the spi controller
>>> instead of using external gpios. Consequently, the "cs-gpios" device-tree
>>> property becomes optional.
>>>
>>> When the new CSAAT bit is set into the Chip Select Register, the internal
>>> chip-select output pin remains asserted till both the following conditions
>>> become true:
>>> - the LASTXFER bit is set into the Control Register (or the Transmit Data
>>>   Register)
>>> - the Transmit Data Register and its shift register are empty.
>>>
>>> WARNING: if the LASTXFER bit is set into the Control Register then new
>>> data are written into the Transmit Data Register fast enough to keep its
>>> shifter not empty, the chip-select output pin remains asserted. Only when
>>> the shifter becomes empty, the chip-select output pin is unasserted.
>>>
>>> When the CSAAT bit is clear in the Chip Select Register, the LASTXFER bit
>>> is ignored in both the Control Register and the Transmit Data Register.
>>> The internal chip-select output pin remains active as long as the Transmit
>>> Data Register or its shift register are not empty.
>>>
>>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
>>> ---
>>>  drivers/spi/spi-atmel.c | 37 ++++++++++++++++++++++++++++---------
>>>  1 file changed, 28 insertions(+), 9 deletions(-)
>> 
>> [...]
>> 
>>> @@ -1338,6 +1350,13 @@ static int atmel_spi_probe(struct platform_device *pdev)
>>>
>>>  	atmel_get_caps(as);
>>>
>>> +	as->use_cs_gpios = true;
>>> +	if (atmel_spi_is_v2(as) &&
>>> +	    !of_get_property(pdev->dev.of_node, "cs-gpios", NULL)) {
>>> +		as->use_cs_gpios = false;
>>> +		master->num_chipselect = 4;
>>> +	}
>> 
>> This part breaks the AVR32 boards and probably anything else that
>> doesn't use devicetree but does use GPIOs for chip select.
>
> Hi Mans,
>
> I have difficulties finding why you may enter this test. So, maybe you
> can give me a clue by reading for me the value that resides in the SPI
> version register: you can have it by reading at 0xFFE000FC for instance
> (actually the atmel_get_caps() dev_info() call gives it as well in the
> boot log which is somewhat easier: I tried to find one on the Internet
> without success...).
>
> So I think that just fixing the logic in atmel_get_caps() introduced by
> d4820b7496219edd9a7055022681364d304525f7 can make it come back to a
> situation where the ARV32 was more tested than nowadays.

atmel_spi atmel_spi.0: version: 0x171
atmel_spi atmel_spi.0: Atmel SPI Controller at 0xffe00000 (irq 3)

atmel_spi_is_v2() returns true for version > 0x121.

-- 
Måns Rullgård
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/3] spi: atmel: add support for the internal chip-select of the spi controller

[Nicolas Ferre]

Le 27/01/2016 16:53, Måns Rullgård a écrit :
> Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org> writes:
> 
>> Le 05/01/2016 22:50, Måns Rullgård a écrit :
>>> Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org> writes:
>>>
>>>> This patch relies on the CSAAT (Chip Select Active After Transfer) feature
>>>> introduced by the version 2 of the spi controller. This new mode allows to
>>>> use properly the internal chip-select output pin of the spi controller
>>>> instead of using external gpios. Consequently, the "cs-gpios" device-tree
>>>> property becomes optional.
>>>>
>>>> When the new CSAAT bit is set into the Chip Select Register, the internal
>>>> chip-select output pin remains asserted till both the following conditions
>>>> become true:
>>>> - the LASTXFER bit is set into the Control Register (or the Transmit Data
>>>>   Register)
>>>> - the Transmit Data Register and its shift register are empty.
>>>>
>>>> WARNING: if the LASTXFER bit is set into the Control Register then new
>>>> data are written into the Transmit Data Register fast enough to keep its
>>>> shifter not empty, the chip-select output pin remains asserted. Only when
>>>> the shifter becomes empty, the chip-select output pin is unasserted.
>>>>
>>>> When the CSAAT bit is clear in the Chip Select Register, the LASTXFER bit
>>>> is ignored in both the Control Register and the Transmit Data Register.
>>>> The internal chip-select output pin remains active as long as the Transmit
>>>> Data Register or its shift register are not empty.
>>>>
>>>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
>>>> ---
>>>>  drivers/spi/spi-atmel.c | 37 ++++++++++++++++++++++++++++---------
>>>>  1 file changed, 28 insertions(+), 9 deletions(-)
>>>
>>> [...]
>>>
>>>> @@ -1338,6 +1350,13 @@ static int atmel_spi_probe(struct platform_device *pdev)
>>>>
>>>>  	atmel_get_caps(as);
>>>>
>>>> +	as->use_cs_gpios = true;
>>>> +	if (atmel_spi_is_v2(as) &&
>>>> +	    !of_get_property(pdev->dev.of_node, "cs-gpios", NULL)) {
>>>> +		as->use_cs_gpios = false;
>>>> +		master->num_chipselect = 4;
>>>> +	}
>>>
>>> This part breaks the AVR32 boards and probably anything else that
>>> doesn't use devicetree but does use GPIOs for chip select.
>>
>> Hi Mans,
>>
>> I have difficulties finding why you may enter this test. So, maybe you
>> can give me a clue by reading for me the value that resides in the SPI
>> version register: you can have it by reading at 0xFFE000FC for instance
>> (actually the atmel_get_caps() dev_info() call gives it as well in the
>> boot log which is somewhat easier: I tried to find one on the Internet
>> without success...).
>>
>> So I think that just fixing the logic in atmel_get_caps() introduced by
>> d4820b7496219edd9a7055022681364d304525f7 can make it come back to a
>> situation where the ARV32 was more tested than nowadays.
> 
> atmel_spi atmel_spi.0: version: 0x171
> atmel_spi atmel_spi.0: Atmel SPI Controller at 0xffe00000 (irq 3)
> 
> atmel_spi_is_v2() returns true for version > 0x121.

Ok, thanks: we thought that AVR32 didn't have a v2 IP: obviously it has.
So yes, I extract the patch by Cyrille to correct this and send it right
now.

If you can test it, it's even better ;-)

Thanks, bye.
-- 
Nicolas Ferre
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/3] spi: atmel: add support for the internal chip-select of the spi controller

[Måns Rullgård]

Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org> writes:

> Le 27/01/2016 16:53, Måns Rullgård a écrit :
>> Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org> writes:
>> 
>>> Le 05/01/2016 22:50, Måns Rullgård a écrit :
>>>> Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org> writes:
>>>>
>>>>> This patch relies on the CSAAT (Chip Select Active After Transfer) feature
>>>>> introduced by the version 2 of the spi controller. This new mode allows to
>>>>> use properly the internal chip-select output pin of the spi controller
>>>>> instead of using external gpios. Consequently, the "cs-gpios" device-tree
>>>>> property becomes optional.
>>>>>
>>>>> When the new CSAAT bit is set into the Chip Select Register, the internal
>>>>> chip-select output pin remains asserted till both the following conditions
>>>>> become true:
>>>>> - the LASTXFER bit is set into the Control Register (or the Transmit Data
>>>>>   Register)
>>>>> - the Transmit Data Register and its shift register are empty.
>>>>>
>>>>> WARNING: if the LASTXFER bit is set into the Control Register then new
>>>>> data are written into the Transmit Data Register fast enough to keep its
>>>>> shifter not empty, the chip-select output pin remains asserted. Only when
>>>>> the shifter becomes empty, the chip-select output pin is unasserted.
>>>>>
>>>>> When the CSAAT bit is clear in the Chip Select Register, the LASTXFER bit
>>>>> is ignored in both the Control Register and the Transmit Data Register.
>>>>> The internal chip-select output pin remains active as long as the Transmit
>>>>> Data Register or its shift register are not empty.
>>>>>
>>>>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
>>>>> ---
>>>>>  drivers/spi/spi-atmel.c | 37 ++++++++++++++++++++++++++++---------
>>>>>  1 file changed, 28 insertions(+), 9 deletions(-)
>>>>
>>>> [...]
>>>>
>>>>> @@ -1338,6 +1350,13 @@ static int atmel_spi_probe(struct platform_device *pdev)
>>>>>
>>>>>  	atmel_get_caps(as);
>>>>>
>>>>> +	as->use_cs_gpios = true;
>>>>> +	if (atmel_spi_is_v2(as) &&
>>>>> +	    !of_get_property(pdev->dev.of_node, "cs-gpios", NULL)) {
>>>>> +		as->use_cs_gpios = false;
>>>>> +		master->num_chipselect = 4;
>>>>> +	}
>>>>
>>>> This part breaks the AVR32 boards and probably anything else that
>>>> doesn't use devicetree but does use GPIOs for chip select.
>>>
>>> Hi Mans,
>>>
>>> I have difficulties finding why you may enter this test. So, maybe you
>>> can give me a clue by reading for me the value that resides in the SPI
>>> version register: you can have it by reading at 0xFFE000FC for instance
>>> (actually the atmel_get_caps() dev_info() call gives it as well in the
>>> boot log which is somewhat easier: I tried to find one on the Internet
>>> without success...).
>>>
>>> So I think that just fixing the logic in atmel_get_caps() introduced by
>>> d4820b7496219edd9a7055022681364d304525f7 can make it come back to a
>>> situation where the ARV32 was more tested than nowadays.
>> 
>> atmel_spi atmel_spi.0: version: 0x171
>> atmel_spi atmel_spi.0: Atmel SPI Controller at 0xffe00000 (irq 3)
>> 
>> atmel_spi_is_v2() returns true for version > 0x121.
>
> Ok, thanks: we thought that AVR32 didn't have a v2 IP: obviously it has.
> So yes, I extract the patch by Cyrille to correct this and send it right
> now.
>
> If you can test it, it's even better ;-)

I saw the patch, will test it later.

-- 
Måns Rullgård
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 3/3] spi: atmel: add support to FIFOs

[Cyrille Pitchen]

To enable the FIFO feature a "atmel,fifo-size" attribute with a strictly
positive value must be added into the node of the device-tree describing
the spi controller.

When FIFOs are enabled, the RX one is forced to operate in SINGLE data
mode because this driver configures the spi controller as a master. In
master mode only, the Received Data Register has an additionnal Peripheral
Chip Select field, which prevents us from reading more than a single data
at each register access.

Besides, the TX FIFO operates in MULTIPLE data mode. However, even when a
8bit data size is used, only two data by access could be written into the
Transmit Data Register. Indeed the first data has to be written into the
lowest 16 bits whereas the second data has to be written into the highest
16 bits of the TDR. When DMA transfers are used to send data, we don't
rework the transmit buffer to cope with this hardware limitation: the
additional copies required to prepare a new input buffer suited to both
the DMA controller and the spi controller would waste all the benefit of
the DMA transfer. Instead, the DMA controller is configured to write only
one data at time into the TDR.

In pio mode, two data are written in the TDR in a single access.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/spi-atmel.c | 255 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 245 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index aa7d202..c9eca34 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -41,6 +41,8 @@
 #define SPI_CSR1				0x0034
 #define SPI_CSR2				0x0038
 #define SPI_CSR3				0x003c
+#define SPI_FMR					0x0040
+#define SPI_FLR					0x0044
 #define SPI_VERSION				0x00fc
 #define SPI_RPR					0x0100
 #define SPI_RCR					0x0104
@@ -62,6 +64,14 @@
 #define SPI_SWRST_SIZE				1
 #define SPI_LASTXFER_OFFSET			24
 #define SPI_LASTXFER_SIZE			1
+#define SPI_TXFCLR_OFFSET			16
+#define SPI_TXFCLR_SIZE				1
+#define SPI_RXFCLR_OFFSET			17
+#define SPI_RXFCLR_SIZE				1
+#define SPI_FIFOEN_OFFSET			30
+#define SPI_FIFOEN_SIZE				1
+#define SPI_FIFODIS_OFFSET			31
+#define SPI_FIFODIS_SIZE			1
 
 /* Bitfields in MR */
 #define SPI_MSTR_OFFSET				0
@@ -114,6 +124,22 @@
 #define SPI_TXEMPTY_SIZE			1
 #define SPI_SPIENS_OFFSET			16
 #define SPI_SPIENS_SIZE				1
+#define SPI_TXFEF_OFFSET			24
+#define SPI_TXFEF_SIZE				1
+#define SPI_TXFFF_OFFSET			25
+#define SPI_TXFFF_SIZE				1
+#define SPI_TXFTHF_OFFSET			26
+#define SPI_TXFTHF_SIZE				1
+#define SPI_RXFEF_OFFSET			27
+#define SPI_RXFEF_SIZE				1
+#define SPI_RXFFF_OFFSET			28
+#define SPI_RXFFF_SIZE				1
+#define SPI_RXFTHF_OFFSET			29
+#define SPI_RXFTHF_SIZE				1
+#define SPI_TXFPTEF_OFFSET			30
+#define SPI_TXFPTEF_SIZE			1
+#define SPI_RXFPTEF_OFFSET			31
+#define SPI_RXFPTEF_SIZE			1
 
 /* Bitfields in CSR0 */
 #define SPI_CPOL_OFFSET				0
@@ -157,6 +183,22 @@
 #define SPI_TXTDIS_OFFSET			9
 #define SPI_TXTDIS_SIZE				1
 
+/* Bitfields in FMR */
+#define SPI_TXRDYM_OFFSET			0
+#define SPI_TXRDYM_SIZE				2
+#define SPI_RXRDYM_OFFSET			4
+#define SPI_RXRDYM_SIZE				2
+#define SPI_TXFTHRES_OFFSET			16
+#define SPI_TXFTHRES_SIZE			6
+#define SPI_RXFTHRES_OFFSET			24
+#define SPI_RXFTHRES_SIZE			6
+
+/* Bitfields in FLR */
+#define SPI_TXFL_OFFSET				0
+#define SPI_TXFL_SIZE				6
+#define SPI_RXFL_OFFSET				16
+#define SPI_RXFL_SIZE				6
+
 /* Constants for BITS */
 #define SPI_BITS_8_BPT				0
 #define SPI_BITS_9_BPT				1
@@ -167,6 +209,9 @@
 #define SPI_BITS_14_BPT				6
 #define SPI_BITS_15_BPT				7
 #define SPI_BITS_16_BPT				8
+#define SPI_ONE_DATA				0
+#define SPI_TWO_DATA				1
+#define SPI_FOUR_DATA				2
 
 /* Bit manipulation macros */
 #define SPI_BIT(name) \
@@ -185,11 +230,31 @@
 	__raw_readl((port)->regs + SPI_##reg)
 #define spi_writel(port, reg, value) \
 	__raw_writel((value), (port)->regs + SPI_##reg)
+
+#define spi_readw(port, reg) \
+	__raw_readw((port)->regs + SPI_##reg)
+#define spi_writew(port, reg, value) \
+	__raw_writew((value), (port)->regs + SPI_##reg)
+
+#define spi_readb(port, reg) \
+	__raw_readb((port)->regs + SPI_##reg)
+#define spi_writeb(port, reg, value) \
+	__raw_writeb((value), (port)->regs + SPI_##reg)
 #else
 #define spi_readl(port, reg) \
 	readl_relaxed((port)->regs + SPI_##reg)
 #define spi_writel(port, reg, value) \
 	writel_relaxed((value), (port)->regs + SPI_##reg)
+
+#define spi_readw(port, reg) \
+	readw_relaxed((port)->regs + SPI_##reg)
+#define spi_writew(port, reg, value) \
+	writew_relaxed((value), (port)->regs + SPI_##reg)
+
+#define spi_readb(port, reg) \
+	readb_relaxed((port)->regs + SPI_##reg)
+#define spi_writeb(port, reg, value) \
+	writeb_relaxed((value), (port)->regs + SPI_##reg)
 #endif
 /* use PIO for small transfers, avoiding DMA setup/teardown overhead and
  * cache operations; better heuristics consider wordsize and bitrate.
@@ -252,6 +317,8 @@ struct atmel_spi {
 
 	bool			keep_cs;
 	bool			cs_active;
+
+	u32			fifo_size;
 };
 
 /* Controller-specific per-slave state */
@@ -410,6 +477,20 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as,
 	slave_config->dst_maxburst = 1;
 	slave_config->device_fc = false;
 
+	/*
+	 * This driver uses fixed peripheral select mode (PS bit set to '0' in
+	 * the Mode Register).
+	 * So according to the datasheet, when FIFOs are available (and
+	 * enabled), the Transmit FIFO operates in Multiple Data Mode.
+	 * In this mode, up to 2 data, not 4, can be written into the Transmit
+	 * Data Register in a single access.
+	 * However, the first data has to be written into the lowest 16 bits and
+	 * the second data into the highest 16 bits of the Transmit
+	 * Data Register. For 8bit data (the most frequent case), it would
+	 * require to rework tx_buf so each data would actualy fit 16 bits.
+	 * So we'd rather write only one data at the time. Hence the transmit
+	 * path works the same whether FIFOs are available (and enabled) or not.
+	 */
 	slave_config->direction = DMA_MEM_TO_DEV;
 	if (dmaengine_slave_config(as->dma.chan_tx, slave_config)) {
 		dev_err(&as->pdev->dev,
@@ -417,6 +498,14 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as,
 		err = -EINVAL;
 	}
 
+	/*
+	 * This driver configures the spi controller for master mode (MSTR bit
+	 * set to '1' in the Mode Register).
+	 * So according to the datasheet, when FIFOs are available (and
+	 * enabled), the Receive FIFO operates in Single Data Mode.
+	 * So the receive path works the same whether FIFOs are available (and
+	 * enabled) or not.
+	 */
 	slave_config->direction = DMA_DEV_TO_MEM;
 	if (dmaengine_slave_config(as->dma.chan_rx, slave_config)) {
 		dev_err(&as->pdev->dev,
@@ -506,10 +595,10 @@ static void dma_callback(void *data)
 }
 
 /*
- * Next transfer using PIO.
+ * Next transfer using PIO without FIFO.
  */
-static void atmel_spi_next_xfer_pio(struct spi_master *master,
-				struct spi_transfer *xfer)
+static void atmel_spi_next_xfer_single(struct spi_master *master,
+				       struct spi_transfer *xfer)
 {
 	struct atmel_spi	*as = spi_master_get_devdata(master);
 	unsigned long xfer_pos = xfer->len - as->current_remaining_bytes;
@@ -542,6 +631,99 @@ static void atmel_spi_next_xfer_pio(struct spi_master *master,
 }
 
 /*
+ * Next transfer using PIO with FIFO.
+ */
+static void atmel_spi_next_xfer_fifo(struct spi_master *master,
+				     struct spi_transfer *xfer)
+{
+	struct atmel_spi *as = spi_master_get_devdata(master);
+	u32 current_remaining_data, num_data;
+	u32 offset = xfer->len - as->current_remaining_bytes;
+	const u16 *words = (const u16 *)((u8 *)xfer->tx_buf + offset);
+	const u8  *bytes = (const u8  *)((u8 *)xfer->tx_buf + offset);
+	u16 td0, td1;
+	u32 fifomr;
+
+	dev_vdbg(master->dev.parent, "atmel_spi_next_xfer_fifo\n");
+
+	/* Compute the number of data to transfer in the current iteration */
+	current_remaining_data = ((xfer->bits_per_word > 8) ?
+				  ((u32)as->current_remaining_bytes >> 1) :
+				  (u32)as->current_remaining_bytes);
+	num_data = min(current_remaining_data, as->fifo_size);
+
+	/* Flush RX and TX FIFOs */
+	spi_writel(as, CR, SPI_BIT(RXFCLR) | SPI_BIT(TXFCLR));
+	while (spi_readl(as, FLR))
+		cpu_relax();
+
+	/* Set RX FIFO Threshold to the number of data to transfer */
+	fifomr = spi_readl(as, FMR);
+	spi_writel(as, FMR, SPI_BFINS(RXFTHRES, num_data, fifomr));
+
+	/* Clear FIFO flags in the Status Register, especially RXFTHF */
+	(void)spi_readl(as, SR);
+
+	/* Fill TX FIFO */
+	while (num_data >= 2) {
+		if (xfer->tx_buf) {
+			if (xfer->bits_per_word > 8) {
+				td0 = *words++;
+				td1 = *words++;
+			} else {
+				td0 = *bytes++;
+				td1 = *bytes++;
+			}
+		} else {
+			td0 = 0;
+			td1 = 0;
+		}
+
+		spi_writel(as, TDR, (td1 << 16) | td0);
+		num_data -= 2;
+	}
+
+	if (num_data) {
+		if (xfer->tx_buf) {
+			if (xfer->bits_per_word > 8)
+				td0 = *words++;
+			else
+				td0 = *bytes++;
+		} else {
+			td0 = 0;
+		}
+
+		spi_writew(as, TDR, td0);
+		num_data--;
+	}
+
+	dev_dbg(master->dev.parent,
+		"  start fifo xfer %p: len %u tx %p rx %p bitpw %d\n",
+		xfer, xfer->len, xfer->tx_buf, xfer->rx_buf,
+		xfer->bits_per_word);
+
+	/*
+	 * Enable RX FIFO Threshold Flag interrupt to be notified about
+	 * transfer completion.
+	 */
+	spi_writel(as, IER, SPI_BIT(RXFTHF) | SPI_BIT(OVRES));
+}
+
+/*
+ * Next transfer using PIO.
+ */
+static void atmel_spi_next_xfer_pio(struct spi_master *master,
+				    struct spi_transfer *xfer)
+{
+	struct atmel_spi *as = spi_master_get_devdata(master);
+
+	if (as->fifo_size)
+		atmel_spi_next_xfer_fifo(master, xfer);
+	else
+		atmel_spi_next_xfer_single(master, xfer);
+}
+
+/*
  * Submit next transfer for DMA.
  */
 static int atmel_spi_next_xfer_dma_submit(struct spi_master *master,
@@ -843,13 +1025,8 @@ static void atmel_spi_disable_pdc_transfer(struct atmel_spi *as)
 	spi_writel(as, PTCR, SPI_BIT(RXTDIS) | SPI_BIT(TXTDIS));
 }
 
-/* Called from IRQ
- *
- * Must update "current_remaining_bytes" to keep track of data
- * to transfer.
- */
 static void
-atmel_spi_pump_pio_data(struct atmel_spi *as, struct spi_transfer *xfer)
+atmel_spi_pump_single_data(struct atmel_spi *as, struct spi_transfer *xfer)
 {
 	u8		*rxp;
 	u16		*rxp16;
@@ -876,6 +1053,57 @@ atmel_spi_pump_pio_data(struct atmel_spi *as, struct spi_transfer *xfer)
 	}
 }
 
+static void
+atmel_spi_pump_fifo_data(struct atmel_spi *as, struct spi_transfer *xfer)
+{
+	u32 fifolr = spi_readl(as, FLR);
+	u32 num_bytes, num_data = SPI_BFEXT(RXFL, fifolr);
+	u32 offset = xfer->len - as->current_remaining_bytes;
+	u16 *words = (u16 *)((u8 *)xfer->rx_buf + offset);
+	u8  *bytes = (u8  *)((u8 *)xfer->rx_buf + offset);
+	u16 rd; /* RD field is the lowest 16 bits of RDR */
+
+	/* Update the number of remaining bytes to transfer */
+	num_bytes = ((xfer->bits_per_word > 8) ?
+		     (num_data << 1) :
+		     num_data);
+
+	if (as->current_remaining_bytes > num_bytes)
+		as->current_remaining_bytes -= num_bytes;
+	else
+		as->current_remaining_bytes = 0;
+
+	/* Handle odd number of bytes when data are more than 8bit width */
+	if (xfer->bits_per_word > 8)
+		as->current_remaining_bytes &= ~0x1;
+
+	/* Read data */
+	while (num_data) {
+		rd = spi_readl(as, RDR);
+		if (xfer->rx_buf) {
+			if (xfer->bits_per_word > 8)
+				*words++ = rd;
+			else
+				*bytes++ = rd;
+		}
+		num_data--;
+	}
+}
+
+/* Called from IRQ
+ *
+ * Must update "current_remaining_bytes" to keep track of data
+ * to transfer.
+ */
+static void
+atmel_spi_pump_pio_data(struct atmel_spi *as, struct spi_transfer *xfer)
+{
+	if (as->fifo_size)
+		atmel_spi_pump_fifo_data(as, xfer);
+	else
+		atmel_spi_pump_single_data(as, xfer);
+}
+
 /* Interrupt
  *
  * No need for locking in this Interrupt handler: done_status is the
@@ -916,7 +1144,7 @@ atmel_spi_pio_interrupt(int irq, void *dev_id)
 
 		complete(&as->xfer_completion);
 
-	} else if (pending & SPI_BIT(RDRF)) {
+	} else if (pending & (SPI_BIT(RDRF) | SPI_BIT(RXFTHF))) {
 		atmel_spi_lock(as);
 
 		if (as->current_remaining_bytes) {
@@ -1399,6 +1627,13 @@ static int atmel_spi_probe(struct platform_device *pdev)
 		spi_writel(as, PTCR, SPI_BIT(RXTDIS) | SPI_BIT(TXTDIS));
 	spi_writel(as, CR, SPI_BIT(SPIEN));
 
+	as->fifo_size = 0;
+	if (!of_property_read_u32(pdev->dev.of_node, "atmel,fifo-size",
+				  &as->fifo_size)) {
+		dev_info(&pdev->dev, "Using FIFO (%u data)\n", as->fifo_size);
+		spi_writel(as, CR, SPI_BIT(FIFOEN));
+	}
+
 	/* go! */
 	dev_info(&pdev->dev, "Atmel SPI Controller at 0x%08lx (irq %d)\n",
 			(unsigned long)regs->start, irq);
-- 
1.8.2.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/3] spi: atmel: add support to FIFOs

[Nicolas Ferre]

Le 09/06/2015 13:53, Cyrille Pitchen a écrit :
> To enable the FIFO feature a "atmel,fifo-size" attribute with a strictly
> positive value must be added into the node of the device-tree describing
> the spi controller.
> 
> When FIFOs are enabled, the RX one is forced to operate in SINGLE data
> mode because this driver configures the spi controller as a master. In
> master mode only, the Received Data Register has an additionnal Peripheral
> Chip Select field, which prevents us from reading more than a single data
> at each register access.
> 
> Besides, the TX FIFO operates in MULTIPLE data mode. However, even when a
> 8bit data size is used, only two data by access could be written into the
> Transmit Data Register. Indeed the first data has to be written into the
> lowest 16 bits whereas the second data has to be written into the highest
> 16 bits of the TDR. When DMA transfers are used to send data, we don't
> rework the transmit buffer to cope with this hardware limitation: the
> additional copies required to prepare a new input buffer suited to both
> the DMA controller and the spi controller would waste all the benefit of
> the DMA transfer. Instead, the DMA controller is configured to write only
> one data at time into the TDR.
> 
> In pio mode, two data are written in the TDR in a single access.
> 
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>

It's neat: thanks Cyrille:
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

> ---
>  drivers/spi/spi-atmel.c | 255 ++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 245 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
> index aa7d202..c9eca34 100644
> --- a/drivers/spi/spi-atmel.c
> +++ b/drivers/spi/spi-atmel.c
> @@ -41,6 +41,8 @@
>  #define SPI_CSR1				0x0034
>  #define SPI_CSR2				0x0038
>  #define SPI_CSR3				0x003c
> +#define SPI_FMR					0x0040
> +#define SPI_FLR					0x0044
>  #define SPI_VERSION				0x00fc
>  #define SPI_RPR					0x0100
>  #define SPI_RCR					0x0104
> @@ -62,6 +64,14 @@
>  #define SPI_SWRST_SIZE				1
>  #define SPI_LASTXFER_OFFSET			24
>  #define SPI_LASTXFER_SIZE			1
> +#define SPI_TXFCLR_OFFSET			16
> +#define SPI_TXFCLR_SIZE				1
> +#define SPI_RXFCLR_OFFSET			17
> +#define SPI_RXFCLR_SIZE				1
> +#define SPI_FIFOEN_OFFSET			30
> +#define SPI_FIFOEN_SIZE				1
> +#define SPI_FIFODIS_OFFSET			31
> +#define SPI_FIFODIS_SIZE			1
>  
>  /* Bitfields in MR */
>  #define SPI_MSTR_OFFSET				0
> @@ -114,6 +124,22 @@
>  #define SPI_TXEMPTY_SIZE			1
>  #define SPI_SPIENS_OFFSET			16
>  #define SPI_SPIENS_SIZE				1
> +#define SPI_TXFEF_OFFSET			24
> +#define SPI_TXFEF_SIZE				1
> +#define SPI_TXFFF_OFFSET			25
> +#define SPI_TXFFF_SIZE				1
> +#define SPI_TXFTHF_OFFSET			26
> +#define SPI_TXFTHF_SIZE				1
> +#define SPI_RXFEF_OFFSET			27
> +#define SPI_RXFEF_SIZE				1
> +#define SPI_RXFFF_OFFSET			28
> +#define SPI_RXFFF_SIZE				1
> +#define SPI_RXFTHF_OFFSET			29
> +#define SPI_RXFTHF_SIZE				1
> +#define SPI_TXFPTEF_OFFSET			30
> +#define SPI_TXFPTEF_SIZE			1
> +#define SPI_RXFPTEF_OFFSET			31
> +#define SPI_RXFPTEF_SIZE			1
>  
>  /* Bitfields in CSR0 */
>  #define SPI_CPOL_OFFSET				0
> @@ -157,6 +183,22 @@
>  #define SPI_TXTDIS_OFFSET			9
>  #define SPI_TXTDIS_SIZE				1
>  
> +/* Bitfields in FMR */
> +#define SPI_TXRDYM_OFFSET			0
> +#define SPI_TXRDYM_SIZE				2
> +#define SPI_RXRDYM_OFFSET			4
> +#define SPI_RXRDYM_SIZE				2
> +#define SPI_TXFTHRES_OFFSET			16
> +#define SPI_TXFTHRES_SIZE			6
> +#define SPI_RXFTHRES_OFFSET			24
> +#define SPI_RXFTHRES_SIZE			6
> +
> +/* Bitfields in FLR */
> +#define SPI_TXFL_OFFSET				0
> +#define SPI_TXFL_SIZE				6
> +#define SPI_RXFL_OFFSET				16
> +#define SPI_RXFL_SIZE				6
> +
>  /* Constants for BITS */
>  #define SPI_BITS_8_BPT				0
>  #define SPI_BITS_9_BPT				1
> @@ -167,6 +209,9 @@
>  #define SPI_BITS_14_BPT				6
>  #define SPI_BITS_15_BPT				7
>  #define SPI_BITS_16_BPT				8
> +#define SPI_ONE_DATA				0
> +#define SPI_TWO_DATA				1
> +#define SPI_FOUR_DATA				2
>  
>  /* Bit manipulation macros */
>  #define SPI_BIT(name) \
> @@ -185,11 +230,31 @@
>  	__raw_readl((port)->regs + SPI_##reg)
>  #define spi_writel(port, reg, value) \
>  	__raw_writel((value), (port)->regs + SPI_##reg)
> +
> +#define spi_readw(port, reg) \
> +	__raw_readw((port)->regs + SPI_##reg)
> +#define spi_writew(port, reg, value) \
> +	__raw_writew((value), (port)->regs + SPI_##reg)
> +
> +#define spi_readb(port, reg) \
> +	__raw_readb((port)->regs + SPI_##reg)
> +#define spi_writeb(port, reg, value) \
> +	__raw_writeb((value), (port)->regs + SPI_##reg)
>  #else
>  #define spi_readl(port, reg) \
>  	readl_relaxed((port)->regs + SPI_##reg)
>  #define spi_writel(port, reg, value) \
>  	writel_relaxed((value), (port)->regs + SPI_##reg)
> +
> +#define spi_readw(port, reg) \
> +	readw_relaxed((port)->regs + SPI_##reg)
> +#define spi_writew(port, reg, value) \
> +	writew_relaxed((value), (port)->regs + SPI_##reg)
> +
> +#define spi_readb(port, reg) \
> +	readb_relaxed((port)->regs + SPI_##reg)
> +#define spi_writeb(port, reg, value) \
> +	writeb_relaxed((value), (port)->regs + SPI_##reg)
>  #endif
>  /* use PIO for small transfers, avoiding DMA setup/teardown overhead and
>   * cache operations; better heuristics consider wordsize and bitrate.
> @@ -252,6 +317,8 @@ struct atmel_spi {
>  
>  	bool			keep_cs;
>  	bool			cs_active;
> +
> +	u32			fifo_size;
>  };
>  
>  /* Controller-specific per-slave state */
> @@ -410,6 +477,20 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as,
>  	slave_config->dst_maxburst = 1;
>  	slave_config->device_fc = false;
>  
> +	/*
> +	 * This driver uses fixed peripheral select mode (PS bit set to '0' in
> +	 * the Mode Register).
> +	 * So according to the datasheet, when FIFOs are available (and
> +	 * enabled), the Transmit FIFO operates in Multiple Data Mode.
> +	 * In this mode, up to 2 data, not 4, can be written into the Transmit
> +	 * Data Register in a single access.
> +	 * However, the first data has to be written into the lowest 16 bits and
> +	 * the second data into the highest 16 bits of the Transmit
> +	 * Data Register. For 8bit data (the most frequent case), it would
> +	 * require to rework tx_buf so each data would actualy fit 16 bits.
> +	 * So we'd rather write only one data at the time. Hence the transmit
> +	 * path works the same whether FIFOs are available (and enabled) or not.
> +	 */
>  	slave_config->direction = DMA_MEM_TO_DEV;
>  	if (dmaengine_slave_config(as->dma.chan_tx, slave_config)) {
>  		dev_err(&as->pdev->dev,
> @@ -417,6 +498,14 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as,
>  		err = -EINVAL;
>  	}
>  
> +	/*
> +	 * This driver configures the spi controller for master mode (MSTR bit
> +	 * set to '1' in the Mode Register).
> +	 * So according to the datasheet, when FIFOs are available (and
> +	 * enabled), the Receive FIFO operates in Single Data Mode.
> +	 * So the receive path works the same whether FIFOs are available (and
> +	 * enabled) or not.
> +	 */
>  	slave_config->direction = DMA_DEV_TO_MEM;
>  	if (dmaengine_slave_config(as->dma.chan_rx, slave_config)) {
>  		dev_err(&as->pdev->dev,
> @@ -506,10 +595,10 @@ static void dma_callback(void *data)
>  }
>  
>  /*
> - * Next transfer using PIO.
> + * Next transfer using PIO without FIFO.
>   */
> -static void atmel_spi_next_xfer_pio(struct spi_master *master,
> -				struct spi_transfer *xfer)
> +static void atmel_spi_next_xfer_single(struct spi_master *master,
> +				       struct spi_transfer *xfer)
>  {
>  	struct atmel_spi	*as = spi_master_get_devdata(master);
>  	unsigned long xfer_pos = xfer->len - as->current_remaining_bytes;
> @@ -542,6 +631,99 @@ static void atmel_spi_next_xfer_pio(struct spi_master *master,
>  }
>  
>  /*
> + * Next transfer using PIO with FIFO.
> + */
> +static void atmel_spi_next_xfer_fifo(struct spi_master *master,
> +				     struct spi_transfer *xfer)
> +{
> +	struct atmel_spi *as = spi_master_get_devdata(master);
> +	u32 current_remaining_data, num_data;
> +	u32 offset = xfer->len - as->current_remaining_bytes;
> +	const u16 *words = (const u16 *)((u8 *)xfer->tx_buf + offset);
> +	const u8  *bytes = (const u8  *)((u8 *)xfer->tx_buf + offset);
> +	u16 td0, td1;
> +	u32 fifomr;
> +
> +	dev_vdbg(master->dev.parent, "atmel_spi_next_xfer_fifo\n");
> +
> +	/* Compute the number of data to transfer in the current iteration */
> +	current_remaining_data = ((xfer->bits_per_word > 8) ?
> +				  ((u32)as->current_remaining_bytes >> 1) :
> +				  (u32)as->current_remaining_bytes);
> +	num_data = min(current_remaining_data, as->fifo_size);
> +
> +	/* Flush RX and TX FIFOs */
> +	spi_writel(as, CR, SPI_BIT(RXFCLR) | SPI_BIT(TXFCLR));
> +	while (spi_readl(as, FLR))
> +		cpu_relax();
> +
> +	/* Set RX FIFO Threshold to the number of data to transfer */
> +	fifomr = spi_readl(as, FMR);
> +	spi_writel(as, FMR, SPI_BFINS(RXFTHRES, num_data, fifomr));
> +
> +	/* Clear FIFO flags in the Status Register, especially RXFTHF */
> +	(void)spi_readl(as, SR);
> +
> +	/* Fill TX FIFO */
> +	while (num_data >= 2) {
> +		if (xfer->tx_buf) {
> +			if (xfer->bits_per_word > 8) {
> +				td0 = *words++;
> +				td1 = *words++;
> +			} else {
> +				td0 = *bytes++;
> +				td1 = *bytes++;
> +			}
> +		} else {
> +			td0 = 0;
> +			td1 = 0;
> +		}
> +
> +		spi_writel(as, TDR, (td1 << 16) | td0);
> +		num_data -= 2;
> +	}
> +
> +	if (num_data) {
> +		if (xfer->tx_buf) {
> +			if (xfer->bits_per_word > 8)
> +				td0 = *words++;
> +			else
> +				td0 = *bytes++;
> +		} else {
> +			td0 = 0;
> +		}
> +
> +		spi_writew(as, TDR, td0);
> +		num_data--;
> +	}
> +
> +	dev_dbg(master->dev.parent,
> +		"  start fifo xfer %p: len %u tx %p rx %p bitpw %d\n",
> +		xfer, xfer->len, xfer->tx_buf, xfer->rx_buf,
> +		xfer->bits_per_word);
> +
> +	/*
> +	 * Enable RX FIFO Threshold Flag interrupt to be notified about
> +	 * transfer completion.
> +	 */
> +	spi_writel(as, IER, SPI_BIT(RXFTHF) | SPI_BIT(OVRES));
> +}
> +
> +/*
> + * Next transfer using PIO.
> + */
> +static void atmel_spi_next_xfer_pio(struct spi_master *master,
> +				    struct spi_transfer *xfer)
> +{
> +	struct atmel_spi *as = spi_master_get_devdata(master);
> +
> +	if (as->fifo_size)
> +		atmel_spi_next_xfer_fifo(master, xfer);
> +	else
> +		atmel_spi_next_xfer_single(master, xfer);
> +}
> +
> +/*
>   * Submit next transfer for DMA.
>   */
>  static int atmel_spi_next_xfer_dma_submit(struct spi_master *master,
> @@ -843,13 +1025,8 @@ static void atmel_spi_disable_pdc_transfer(struct atmel_spi *as)
>  	spi_writel(as, PTCR, SPI_BIT(RXTDIS) | SPI_BIT(TXTDIS));
>  }
>  
> -/* Called from IRQ
> - *
> - * Must update "current_remaining_bytes" to keep track of data
> - * to transfer.
> - */
>  static void
> -atmel_spi_pump_pio_data(struct atmel_spi *as, struct spi_transfer *xfer)
> +atmel_spi_pump_single_data(struct atmel_spi *as, struct spi_transfer *xfer)
>  {
>  	u8		*rxp;
>  	u16		*rxp16;
> @@ -876,6 +1053,57 @@ atmel_spi_pump_pio_data(struct atmel_spi *as, struct spi_transfer *xfer)
>  	}
>  }
>  
> +static void
> +atmel_spi_pump_fifo_data(struct atmel_spi *as, struct spi_transfer *xfer)
> +{
> +	u32 fifolr = spi_readl(as, FLR);
> +	u32 num_bytes, num_data = SPI_BFEXT(RXFL, fifolr);
> +	u32 offset = xfer->len - as->current_remaining_bytes;
> +	u16 *words = (u16 *)((u8 *)xfer->rx_buf + offset);
> +	u8  *bytes = (u8  *)((u8 *)xfer->rx_buf + offset);
> +	u16 rd; /* RD field is the lowest 16 bits of RDR */
> +
> +	/* Update the number of remaining bytes to transfer */
> +	num_bytes = ((xfer->bits_per_word > 8) ?
> +		     (num_data << 1) :
> +		     num_data);
> +
> +	if (as->current_remaining_bytes > num_bytes)
> +		as->current_remaining_bytes -= num_bytes;
> +	else
> +		as->current_remaining_bytes = 0;
> +
> +	/* Handle odd number of bytes when data are more than 8bit width */
> +	if (xfer->bits_per_word > 8)
> +		as->current_remaining_bytes &= ~0x1;
> +
> +	/* Read data */
> +	while (num_data) {
> +		rd = spi_readl(as, RDR);
> +		if (xfer->rx_buf) {
> +			if (xfer->bits_per_word > 8)
> +				*words++ = rd;
> +			else
> +				*bytes++ = rd;
> +		}
> +		num_data--;
> +	}
> +}
> +
> +/* Called from IRQ
> + *
> + * Must update "current_remaining_bytes" to keep track of data
> + * to transfer.
> + */
> +static void
> +atmel_spi_pump_pio_data(struct atmel_spi *as, struct spi_transfer *xfer)
> +{
> +	if (as->fifo_size)
> +		atmel_spi_pump_fifo_data(as, xfer);
> +	else
> +		atmel_spi_pump_single_data(as, xfer);
> +}
> +
>  /* Interrupt
>   *
>   * No need for locking in this Interrupt handler: done_status is the
> @@ -916,7 +1144,7 @@ atmel_spi_pio_interrupt(int irq, void *dev_id)
>  
>  		complete(&as->xfer_completion);
>  
> -	} else if (pending & SPI_BIT(RDRF)) {
> +	} else if (pending & (SPI_BIT(RDRF) | SPI_BIT(RXFTHF))) {
>  		atmel_spi_lock(as);
>  
>  		if (as->current_remaining_bytes) {
> @@ -1399,6 +1627,13 @@ static int atmel_spi_probe(struct platform_device *pdev)
>  		spi_writel(as, PTCR, SPI_BIT(RXTDIS) | SPI_BIT(TXTDIS));
>  	spi_writel(as, CR, SPI_BIT(SPIEN));
>  
> +	as->fifo_size = 0;
> +	if (!of_property_read_u32(pdev->dev.of_node, "atmel,fifo-size",
> +				  &as->fifo_size)) {
> +		dev_info(&pdev->dev, "Using FIFO (%u data)\n", as->fifo_size);
> +		spi_writel(as, CR, SPI_BIT(FIFOEN));
> +	}
> +
>  	/* go! */
>  	dev_info(&pdev->dev, "Atmel SPI Controller at 0x%08lx (irq %d)\n",
>  			(unsigned long)regs->start, irq);
> 


-- 
Nicolas Ferre

Re: [PATCH v3 3/3] spi: atmel: add support to FIFOs

[Mark Brown]

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

On Tue, Jun 09, 2015 at 01:53:54PM +0200, Cyrille Pitchen wrote:

> To enable the FIFO feature a "atmel,fifo-size" attribute with a strictly
> positive value must be added into the node of the device-tree describing
> the spi controller.

I'd expect the driver to use FIFOs any time they make sense, I wouldn't
expect this to be something that requires configuration - I'd expect
that there's going to be at least some FIFO in all versions of the IP,
even if the size varies.  Is that not the case?

> When FIFOs are enabled, the RX one is forced to operate in SINGLE data
> mode because this driver configures the spi controller as a master. In
> master mode only, the Received Data Register has an additionnal Peripheral
> Chip Select field, which prevents us from reading more than a single data
> at each register access.
> 
> Besides, the TX FIFO operates in MULTIPLE data mode. However, even when a

This is very hard to understand as I have no idea what single and
multiple data modes are, sorry.  Please write your commit messages so
they can be read by people who aren't familiar with the internals of the
IP.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]