linux-kernel - Re: [LINUX PATCH 1/2] mtd: Added dummy entry in the spi

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: <20160321130734.GS2566@sirena.org.uk>
Date:	Mon, 21 Mar 2016 13:07:34 +0000
From:	Mark Brown <broonie@...nel.org>
To:	P L Sai Krishna <lakshmi.sai.krishna.potthuri@...inx.com>
Cc:	Michal Simek <michal.simek@...inx.com>,
	Soren Brinkmann <soren.brinkmann@...inx.com>,
	David Woodhouse <dwmw2@...radead.org>,
	Brian Norris <computersforpeace@...il.com>,
	Javier Martinez Canillas <javier@....samsung.com>,
	Boris Brezillon <boris.brezillon@...e-electrons.com>,
	Stephen Warren <swarren@...dia.com>,
	Geert Uytterhoeven <geert+renesas@...der.be>,
	"Andrew F. Davis" <afd@...com>, Marek Vasut <marex@...x.de>,
	Jagan Teki <jteki@...nedev.com>,
	Rafał Miłecki <zajec5@...il.com>,
	linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
	linux-spi@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	Harini Katakam <harinik@...inx.com>,
	Punnaiah Choudary Kalluri <punnaia@...inx.com>,
	Anirudha Sarangi <anirudh@...inx.com>,
	P L Sai Krishna <lakshmis@...inx.com>
Subject: Re: [LINUX PATCH 1/2] mtd: Added dummy entry in the spi_transfer
 structure.

On Mon, Mar 21, 2016 at 05:50:08PM +0530, P L Sai Krishna wrote:
> This patch does following things.
> 1. Added dummy entry in the spi_transfer structure.
> 2. Assigned dummy cycles to dummy member in the transfer
> structure during read operation.

Please try to follow the patch submission process covered in
SubmittingPatches, in particular please use subject lines reflecting the
style for the subsystem (which helps people identify relevant changes to
review) and...

>  drivers/mtd/devices/m25p80.c | 1 +
>  include/linux/spi/spi.h      | 2 ++
>  2 files changed, 3 insertions(+)

...split things up into individual patches, for example here you're
both adding a new feature and adding a user of that feature in a single
patch.

> + * @dummy: number of dummy cycles.

This needs to be clearer about what a dummy cycle is and where it gets
inserted.  We probably also want a better name, just "dummy" makes it
look like a padding field in the structure.  How about dummy_cycles?

> @@ -752,6 +753,7 @@ struct spi_transfer {
>  	u8		bits_per_word;
>  	u16		delay_usecs;
>  	u32		speed_hz;
> +	u32		dummy;
>  
>  	struct list_head transfer_list;
>  };

This isn't enough to add the feature - a client driver trying to make
use of this needs to be able to tell if the cycles are actually going to
be inserted.  I'd expect to see a capability flag that can be checked
and some error checking so that if we try to do a transfer with dummy
cycles and can't support it we don't silently ignore the dummy cycles,
ideally also something that'll handle multiples of 8 bits with SPI
controllers that don't otherwise support this feature.

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