linux-kernel - Re: [RESEND] spi/tegra114: Correct support for cs

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]

Date:	Mon, 23 Sep 2013 13:58:27 -0600
From:	Stephen Warren <swarren@...dotorg.org>
To:	Rhyland Klein <rklein@...dia.com>
CC:	Grant Likely <grant.likely@...retlab.ca>,
	Laxman Dewangan <ldewangan@...dia.com>,
	"spi-devel-general@...ts.sourceforge.net" 
	<spi-devel-general@...ts.sourceforge.net>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
	Simon Glass <sjg@...omium.org>, Olof Johansson <olof@...om.net>
Subject: Re: [RESEND] spi/tegra114: Correct support for cs_change

On 09/23/2013 01:48 PM, Rhyland Klein wrote:
> On 9/23/2013 2:51 PM, Stephen Warren wrote:
>> On 09/18/2013 12:17 PM, Rhyland Klein wrote:
>>> The tegra114 driver wasn't currently handling the cs_change functionality.
>>> It is meant to invert normal behavior, and we were only using it to possibly
>>> delay at the end of a transfer.
...
>>> diff --git a/drivers/spi/spi-tegra114.c b/drivers/spi/spi-tegra114.c
...
>>> @@ -717,7 +718,12 @@ static int tegra_spi_start_transfer_one(struct spi_device *spi,
>>>  		else if (req_mode == SPI_MODE_3)
>>>  			command1 |= SPI_CONTROL_MODE_3;
>>>  
>>> -		tegra_spi_writel(tspi, command1, SPI_COMMAND1);
>>> +		if (tspi->cs_control) {
>>> +			if (tspi->cs_control != spi)
>>> +				tegra_spi_writel(tspi, command1, SPI_COMMAND1);
>>
>> Do you really need a separate write call there? The value of command1
>> isn't fully set up there (the CS bits are all set up immediately after),
>> so won't that glitch the CS lines in some cases?
> 
> On our hardware (as far as I've seen), the CS line is normally low. We

I assume you meant "normally *active* low", not "normally low"?

> need to generate a falling-edge to trigger the beginning of a SPI
> transaction. Doing this write with the default value of SPI_COMMAND1
> causes a brief rise and fall of CS, giving us our falling-edge.

That sounds like exactly the glitch I was talking about.

Assuming CS isn't held constantly asserted (low), isn't CS de-asserted
(rises) at the end of transaction n-1, and re-asserted (falls) at the
start of transaction n? If so, I'm not sure why the setup for
transaction n needs to both de-assert and then re-assert it? It seems
like cs_control should be handled at the end of a transaction, not at
the start of the next one.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/