On Mon, Jun 21, 2010 at 10:42 AM, Song, Barry <barry.s...@analog.com> wrote: > > >>-----Original Message----- >>From: uclinux-dist-devel-boun...@blackfin.uclinux.org >>[mailto:uclinux-dist-devel-boun...@blackfin.uclinux.org] On >>Behalf Of Anton Vorontsov >>Sent: Friday, June 18, 2010 9:32 PM >>To: Barry Song >>Cc: David Brownell; Artem Bityutskiy; >>linux-ker...@vger.kernel.org; linuxppc-...@ozlabs.org; >>linux-...@lists.infradead.org; >>uclinux-dist-de...@blackfin.uclinux.org; Andrew Morton >>Subject: Re: [Uclinux-dist-devel] [PATCH 1/2] mtd: m25p80: >>Reworkprobing/JEDEC code >> >>On Sat, Jun 12, 2010 at 02:27:12PM +0800, Barry Song wrote: >>> On Wed, Aug 19, 2009 at 5:46 AM, Anton Vorontsov >>> <avoront...@ru.mvista.com> wrote: >>> > >>> > Previosly the driver always tried JEDEC probing, assuming >>that non-JEDEC >>> > chips will return '0'. But truly non-JEDEC chips (like >>CAT25) won't do >>> > that, their behaviour on RDID command is undefined, so the >>driver should >>> > not call jedec_probe() for these chips. >>> > >>> > Also, be less strict on error conditions, don't fail to >>probe if JEDEC >>> > found a chip that is different from what platform code >>told, instead >>> > just print some warnings and use an information obtained >>via JEDEC. In >>> This patch caused a problem: >>> even though the external flash doesn't exist, it will still pass the >>> probe() and be registerred into kernel and given the partition table. >>> You may refer to this bug report: >>> >>http://blackfin.uclinux.org/gf/project/uclinux-dist/tracker/?ac >>tion=TrackerItemEdit&tracker_item_id=5975&start=0 >> >>Thanks for the report. >> >>There's little we can do about it. Platform code asked us >>to register the device, and JEDEC probing of M25Pxx chips isn't >>reliable (thanks to various vendors that make these JEDEC and >>non-JEDEC variants), so the best thing we can do is to register >>the chip anyway. >> >>OTOH, if the board pulls MISO line up, then the following patch >>should help. > Make sense with pullup to keep the value high while external device > doesn't exist. >> >>If this won't work, we'll have to add some flag to the platform >>data, i.e. to force JEDEC probing, and not trust platform data. > > How about we add a non_jedec flag in platform_data, if the flag is 1, we > let the detection pass even though the ID is 0? Otherwise, we need a > valid ID? Here i mean: Index: drivers/mtd/devices/m25p80.c =================================================================== --- drivers/mtd/devices/m25p80.c (revision 8927) +++ drivers/mtd/devices/m25p80.c (revision 8929) @@ -795,8 +795,13 @@
jid = jedec_probe(spi); if (!jid) { - dev_info(&spi->dev, "non-JEDEC variant of %s\n", - id->name); + if (!data->non_jedec) { + dev_err(&spi->dev, "fail to detect%s\n", + id->name); + return -ENODEV; + } else + dev_info(&spi->dev, "non-JEDEC variant of %s\n", + id->name); } else if (jid != id) { /* * JEDEC knows better, so overwrite platform ID. We Index: include/linux/spi/flash.h =================================================================== --- include/linux/spi/flash.h (revision 8927) +++ include/linux/spi/flash.h (revision 8929) @@ -25,6 +25,11 @@ char *type; + /* + * For non-JEDEC, id will be 0. In this case, we can't be sure + * whether the flash exists with runtime probing. + */ + int non_jedec; /* we'll likely add more ... use JEDEC IDs, etc */ }; > >> >>Not-yet-Signed-off-by: Anton Vorontsov <cbouatmai...@gmail.com> >>--- >> >>diff --git a/drivers/mtd/devices/m25p80.c >>b/drivers/mtd/devices/m25p80.c >>index 81e49a9..a307929 100644 >>--- a/drivers/mtd/devices/m25p80.c >>+++ b/drivers/mtd/devices/m25p80.c >>@@ -16,6 +16,7 @@ >> */ >> >> #include <linux/init.h> >>+#include <linux/errno.h> >> #include <linux/module.h> >> #include <linux/device.h> >> #include <linux/interrupt.h> >>@@ -723,7 +724,7 @@ static const struct spi_device_id >>*__devinit jedec_probe(struct spi_device *spi) >> if (tmp < 0) { >> DEBUG(MTD_DEBUG_LEVEL0, "%s: error %d reading >>JEDEC ID\n", >> dev_name(&spi->dev), tmp); >>- return NULL; >>+ return ERR_PTR(tmp); >> } >> jedec = id[0]; >> jedec = jedec << 8; >>@@ -737,7 +738,7 @@ static const struct spi_device_id >>*__devinit jedec_probe(struct spi_device *spi) >> * exist for non-JEDEC chips, but for compatibility >>they return ID 0. >> */ >> if (jedec == 0) >>- return NULL; >>+ return ERR_PTR(-EEXIST); >> >> ext_jedec = id[3] << 8 | id[4]; >> >>@@ -749,7 +750,7 @@ static const struct spi_device_id >>*__devinit jedec_probe(struct spi_device *spi) >> return &m25p_ids[tmp]; >> } >> } >>- return NULL; >>+ return ERR_PTR(-ENODEV); >> } >> >> >>@@ -794,9 +795,11 @@ static int __devinit m25p_probe(struct >>spi_device *spi) >> const struct spi_device_id *jid; >> >> jid = jedec_probe(spi); >>- if (!jid) { >>+ if (IS_ERR(jid) && PTR_ERR(jid) == -EEXIST) { >> dev_info(&spi->dev, "non-JEDEC variant of %s\n", >> id->name); >>+ } else if (IS_ERR(jid)) { >>+ return PTR_ERR(jid); >> } else if (jid != id) { >> /* >> * JEDEC knows better, so overwrite >>platform ID. We >>_______________________________________________ >>Uclinux-dist-devel mailing list >>uclinux-dist-de...@blackfin.uclinux.org >>https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel >> > _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev