Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753434AbbG1SPQ (ORCPT ); Tue, 28 Jul 2015 14:15:16 -0400 Received: from mail-out.m-online.net ([212.18.0.9]:38902 "EHLO mail-out.m-online.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751159AbbG1SPO (ORCPT ); Tue, 28 Jul 2015 14:15:14 -0400 X-Auth-Info: yZORMfa4elhphJyI5tSwGji39ceQiVmL4XhD8R3j7N8= From: Marek Vasut To: Michal Suchanek Subject: Re: [PATCH 1/2] mtd: spi-nor: rework spi nor read and write. Date: Tue, 28 Jul 2015 20:15:10 +0200 User-Agent: KMail/1.13.7 (Linux/3.14-2-amd64; KDE/4.13.1; x86_64; ; ) Cc: David Woodhouse , Brian Norris , Han Xu , " =?utf-8?q?Rafa=C5=82?= =?utf-8?q?_Mi=C5=82ecki?=" , Alison Chaiken , Huang Shijie , Ben Hutchings , Gabor Juhos , "Bean Huo =?utf-8?q?=E9=9C=8D=E6=96=8C=E6=96=8C?= \(beanhuo\)" , linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org References: <79b4c3521cdb13872c7cf939ea25219399c1a345.1438074682.git.hramrach@gmail.com> In-Reply-To: <79b4c3521cdb13872c7cf939ea25219399c1a345.1438074682.git.hramrach@gmail.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Message-Id: <201507282015.10775.marex@denx.de> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2696 Lines: 73 On Tuesday, July 28, 2015 at 11:23:02 AM, Michal Suchanek wrote: > The spi_nor read and write functions pass thru the mtd retlen to the > chip-specific read and write function. This makes it difficult to check > for errors in read and write functions and these errors are not checked. > This leads to silent data corruption. > > This patch styles the chip-specific read and write function as unix > read(2) and write(2) and updates the retlen in the spi-nor generic driver. > > This also makes it possible to check for write errors. > When pl330 fails to transfer the flash data over SPI I get I/O error > instead of 4M of zeroes. > > I do not have sst and fsl hardware to test with. > > Signed-off-by: Michal Suchanek > --- > drivers/mtd/devices/m25p80.c | 33 ++++++++++++++--------- > drivers/mtd/spi-nor/fsl-quadspi.c | 29 ++++++++++---------- > drivers/mtd/spi-nor/spi-nor.c | 57 > ++++++++++++++++++++++++++++----------- include/linux/mtd/spi-nor.h > | 8 +++--- > 4 files changed, 81 insertions(+), 46 deletions(-) > > diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c > index d313f948b..d8f064b 100644 > --- a/drivers/mtd/devices/m25p80.c > +++ b/drivers/mtd/devices/m25p80.c > @@ -75,14 +75,15 @@ static int m25p80_write_reg(struct spi_nor *nor, u8 > opcode, u8 *buf, int len, return spi_write(spi, flash->command, len + 1); > } > > -static void m25p80_write(struct spi_nor *nor, loff_t to, size_t len, > - size_t *retlen, const u_char *buf) > +static ssize_t m25p80_write(struct spi_nor *nor, loff_t to, size_t len, > + const u_char *buf) > { > struct m25p *flash = nor->priv; > struct spi_device *spi = flash->spi; > struct spi_transfer t[2] = {}; > struct spi_message m; > int cmd_sz = m25p_cmdsz(nor); > + ssize_t ret; > > spi_message_init(&m); > > @@ -100,9 +101,14 @@ static void m25p80_write(struct spi_nor *nor, loff_t > to, size_t len, t[1].len = len; > spi_message_add_tail(&t[1], &m); > > - spi_sync(spi, &m); > + ret = spi_sync(spi, &m); > + if (ret) > + return ret; > > - *retlen += m.actual_length - cmd_sz; > + ret = m.actual_length - cmd_sz; > + if (ret < 0) > + return -EIO; > + return ret; I'd prefer to just add the return value and keep the retlen to keep error codes and transfer length separate. btw. you change the transfer length from unsigned to signed type -- long transfer might get interpreted as an error. [...] -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/