diff mbox

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

Message ID c05f4d9abfdabb62b3b3cfeb57a3f9c138b4e45d.1433850255.git.cyrille.pitchen@atmel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Cyrille Pitchen June 9, 2015, 11:53 a.m. UTC
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@atmel.com>
---
 drivers/spi/spi-atmel.c | 37 ++++++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 9 deletions(-)

Comments

Nicolas Ferre June 9, 2015, 12:15 p.m. UTC | #1
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@atmel.com>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

> ---
>  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) {
>
Mark Brown June 9, 2015, 5:26 p.m. UTC | #2
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.
Måns Rullgård Jan. 5, 2016, 9:50 p.m. UTC | #3
Cyrille Pitchen <cyrille.pitchen@atmel.com> 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@atmel.com>
> ---
>  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.
Mark Brown Jan. 7, 2016, 3:40 p.m. UTC | #4
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?
Måns Rullgård Jan. 7, 2016, 4:14 p.m. UTC | #5
Mark Brown <broonie@kernel.org> writes:

> 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?

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.
Nicolas Ferre Jan. 27, 2016, 3:46 p.m. UTC | #6
Le 05/01/2016 22:50, Måns Rullgård a écrit :
> Cyrille Pitchen <cyrille.pitchen@atmel.com> 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@atmel.com>
>> ---
>>  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,
Måns Rullgård Jan. 27, 2016, 3:53 p.m. UTC | #7
Nicolas Ferre <nicolas.ferre@atmel.com> writes:

> Le 05/01/2016 22:50, Måns Rullgård a écrit :
>> Cyrille Pitchen <cyrille.pitchen@atmel.com> 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@atmel.com>
>>> ---
>>>  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.
Nicolas Ferre Jan. 27, 2016, 4:55 p.m. UTC | #8
Le 27/01/2016 16:53, Måns Rullgård a écrit :
> Nicolas Ferre <nicolas.ferre@atmel.com> writes:
> 
>> Le 05/01/2016 22:50, Måns Rullgård a écrit :
>>> Cyrille Pitchen <cyrille.pitchen@atmel.com> 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@atmel.com>
>>>> ---
>>>>  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.
Måns Rullgård Jan. 27, 2016, 4:57 p.m. UTC | #9
Nicolas Ferre <nicolas.ferre@atmel.com> writes:

> Le 27/01/2016 16:53, Måns Rullgård a écrit :
>> Nicolas Ferre <nicolas.ferre@atmel.com> writes:
>> 
>>> Le 05/01/2016 22:50, Måns Rullgård a écrit :
>>>> Cyrille Pitchen <cyrille.pitchen@atmel.com> 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@atmel.com>
>>>>> ---
>>>>>  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.
diff mbox

Patch

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) {