linux-kernel - Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read

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:	Thu, 6 Aug 2015 17:46:56 +0100
From:	Mark Brown <broonie@...nel.org>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
Cc:	Vignesh R <vigneshr@...com>, Michal Suchanek <hramrach@...il.com>,
	devicetree <devicetree@...r.kernel.org>,
	Brian Norris <computersforpeace@...il.com>,
	Tony Lindgren <tony@...mide.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	linux-spi <linux-spi@...r.kernel.org>,
	Huang Shijie <b32955@...escale.com>,
	MTD Maling List <linux-mtd@...ts.infradead.org>,
	linux-omap@...r.kernel.org, David Woodhouse <dwmw2@...radead.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read

On Thu, Aug 06, 2015 at 02:51:29PM +0100, Russell King - ARM Linux wrote:

> The M25P80 driver just appends additional bytes to the message to
> achieve this:
> 
>         struct m25p *flash = nor->priv;
>         unsigned int dummy = nor->read_dummy;
> 
>         /* convert the dummy cycles to the number of bytes */
>         dummy /= 8;
> 
>         flash->command[0] = nor->read_opcode;
>         m25p_addr2cmd(nor, from, flash->command);
> 
>         t[0].tx_buf = flash->command;
>         t[0].len = m25p_cmdsz(nor) + dummy;
>         spi_message_add_tail(&t[0], &m);

> The reason that the number of dummy bytes can't be detected is because
> it's all hidden in the first transaction as the total number of bytes to
> be transmitted - and the dummy bytes are uninitialised, so you can't
> make any assumptions what value they are.  There is no way for the SPI
> driver to know whether these dummy bytes are dummy bytes or whether they
> have an effect on the targetted device.

We *could* (as you suggest below) indicate dummy transfers by having a
separate transfer which omits the transmit buffers though I'd expect
that normally that is going to be a small performance hit if interpreted
directly so we need to think what to do there.  We do get other devices
sending dummy bytes, it's sometimes a requirement for high speed
register access to give settling time for the device, so other things
would get some milage from it.

> What may make more sense from the SPI point of view is to communicate to
> all SPI drivers how many dummy bytes are to be transferred.  I'm not fully
> up on SPI, but maybe something like this:

> 	t[0].tx_buf = flash->command;
> 	t[0].len = m25p_cmdsz(nor);
> 	spi_message_add_tail(&t[0], &m);
> 	t[1].tx_buf = dummy_buffer;
> 	t[1].len = dummy;
> 	t[1].dummy = 1;
> 	spi_message_add_tail(&t[1], &m);

> This way, we're describing the transfer to the SPI core, and explicitly
> indicating that there are some dummy bytes.  The SPI driver can then
> tell that these are dummy bytes, and if the SPI message consists of:

That'd work as well, my first thought would to use NULL as a dummy
buffer pointer and let the core substitute in data for the drivers.  We
currently insist on having at least one buffer but that's fixable.

> This would not be a hack to the SPI code: we're describing to the SPI
> code what we want to achieve in terms of the activity on the bus, and
> providing that level of description then allows the SPI driver to make
> informed decisions on whether it can handle the transfer using some
> non-standard feature.

Yup.

Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)