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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <56A8F66E.6070105@atmel.com>
Date:	Wed, 27 Jan 2016 17:55:10 +0100
From:	Nicolas Ferre <nicolas.ferre@...el.com>
To:	Måns Rullgård <mans@...sr.com>
CC:	Cyrille Pitchen <cyrille.pitchen@...el.com>,
	Mark Browk <broonie@...nel.org>, <linux-spi@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>,
	<devicetree@...r.kernel.org>, Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	"Yang, Wenyou" <Wenyou.Yang@...el.com>
Subject: Re: [PATCH v3 1/3] spi: atmel: add support for the internal
 chip-select of the spi controller

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