On Friday, May 01, 2015 at 04:49:37 PM, Pavel Machek wrote:
> 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.
Which ones ?
> > 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)
git format-patch automatically inserts the diffstat for you.
> , and having diffstat for a patch that fits on a
> screen is just stupid.
It's a 2-line diffstat for this patch. If you get a 1-screen big
diffstat, then your patch is most likely wrong.
> > > + * { "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
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
