From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pavel Machek Date: Fri, 1 May 2015 16:49:37 +0200 Subject: [U-Boot] [PATCH] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset In-Reply-To: <201505011624.45335.marex@denx.de> References: <1412176391-24475-1-git-send-email-sr@denx.de> <201504271835.30446.marex@denx.de> <20150501090108.GA7205@amd> <201505011624.45335.marex@denx.de> Message-ID: <20150501144937.GA16957@amd> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Fri 2015-05-01 16:24:45, Marek Vasut wrote: > On Friday, May 01, 2015 at 11:01:09 AM, Pavel Machek wrote: > > diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c > > index 201471c..f7cfbd9 100644 > > --- a/drivers/mtd/spi/sf_probe.c > > +++ b/drivers/mtd/spi/sf_probe.c > > @@ -347,6 +348,36 @@ int spi_flash_probe_slave(struct spi_slave *spi, > > struct spi_flash *flash) } > > } > > > > +#ifdef CONFIG_SPI_N25Q256A_RESET > > Should be CONFIG_SPI_MICRON_RESET, since other parts which can also be > used would have similar issue. I'm pretty sure some Micron parts use different interface. > It'd be nice if you > added diffstat into your patches as it makes things easier during > review. Yes, it also makes patch harder to create (as it is tricky to hand-edit the patches), and having diffstat for a patch that fits on a screen is just stupid. > > + * { "n25q256a", INFO(0x20ba19, 0, 64 * 1024, 512, > > + * SECT_4K | SHUTDOWN_3BYTE) }, > > + * > > + * Add SHUTDOWN_3BYTE here. > > + */ > > + ret = spi_flash_cmd(spi, CMD_RESET_ENABLE, NULL, 0); > > + if (ret) { > > + printf("SF: Failed issue reset command\n"); > > I thought this was just a reset-enable command. If this command > fails, user won't be able to tell which of these two failed, so > it's a bad idea to use the same error message for both. Ok. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html