lkml.org 
[lkml]   [2010]   [Jun]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 1/2] mtd: m25p80: Rework probing/JEDEC code
    On Wed, Aug 19, 2009 at 5:46 AM, Anton Vorontsov
    <avorontsov@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/?action=TrackerItemEdit&tracker_item_id=5975&start=0

    > that case we should not trust partitions any longer, but they might be
    > still useful (i.e. they could protect some parts of the chip).
    >
    > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
    > ---
    >  drivers/mtd/devices/m25p80.c |   69 ++++++++++++++++++++++++-----------------
    >  1 files changed, 40 insertions(+), 29 deletions(-)
    >
    > diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
    > index 0d74b38..b75e319 100644
    > --- a/drivers/mtd/devices/m25p80.c
    > +++ b/drivers/mtd/devices/m25p80.c
    > @@ -581,6 +581,14 @@ static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi)
    >        jedec = jedec << 8;
    >        jedec |= id[2];
    >
    > +       /*
    > +        * Some chips (like Numonyx M25P80) have JEDEC and non-JEDEC variants,
    > +        * which depend on technology process. Officially RDID command doesn't
    > +        * exist for non-JEDEC chips, but for compatibility they return ID 0.
    > +        */
    > +       if (jedec == 0)
    > +               return NULL;
    > +
    >        ext_jedec = id[3] << 8 | id[4];
    >
    >        for (tmp = 0; tmp < ARRAY_SIZE(m25p_ids) - 1; tmp++) {
    > @@ -602,7 +610,7 @@ static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi)
    >  */
    >  static int __devinit m25p_probe(struct spi_device *spi)
    >  {
    > -       const struct spi_device_id      *id;
    > +       const struct spi_device_id      *id = spi_get_device_id(spi);
    >        struct flash_platform_data      *data;
    >        struct m25p                     *flash;
    >        struct flash_info               *info;
    > @@ -615,41 +623,44 @@ static int __devinit m25p_probe(struct spi_device *spi)
    >         */
    >        data = spi->dev.platform_data;
    >        if (data && data->type) {
    > +               const struct spi_device_id *plat_id;
    > +
    >                for (i = 0; i < ARRAY_SIZE(m25p_ids) - 1; i++) {
    > -                       id = &m25p_ids[i];
    > -                       info = (void *)m25p_ids[i].driver_data;
    > -                       if (strcmp(data->type, id->name))
    > +                       plat_id = &m25p_ids[i];
    > +                       if (strcmp(data->type, plat_id->name))
    >                                continue;
    >                        break;
    >                }
    >
    > -               /* unrecognized chip? */
    > -               if (i == ARRAY_SIZE(m25p_ids) - 1) {
    > -                       DEBUG(MTD_DEBUG_LEVEL0, "%s: unrecognized id %s\n",
    > -                                       dev_name(&spi->dev), data->type);
    > -                       info = NULL;
    > -
    > -               /* recognized; is that chip really what's there? */
    > -               } else if (info->jedec_id) {
    > -                       id = jedec_probe(spi);
    > -
    > -                       if (id != &m25p_ids[i]) {
    > -                               dev_warn(&spi->dev, "found %s, expected %s\n",
    > -                                               id ? id->name : "UNKNOWN",
    > -                                               m25p_ids[i].name);
    > -                               info = NULL;
    > -                       }
    > -               }
    > -       } else {
    > -               id = jedec_probe(spi);
    > -               if (!id)
    > -                       id = spi_get_device_id(spi);
    > -
    > -               info = (void *)id->driver_data;
    > +               if (plat_id)
    > +                       id = plat_id;
    > +               else
    > +                       dev_warn(&spi->dev, "unrecognized id %s\n", data->type);
    >        }
    >
    > -       if (!info)
    > -               return -ENODEV;
    > +       info = (void *)id->driver_data;
    > +
    > +       if (info->jedec_id) {
    > +               const struct spi_device_id *jid;
    > +
    > +               jid = jedec_probe(spi);
    > +               if (!jid) {
    > +                       dev_info(&spi->dev, "non-JEDEC variant of %s\n",
    > +                                id->name);
    > +               } else if (jid != id) {
    > +                       /*
    > +                        * JEDEC knows better, so overwrite platform ID. We
    > +                        * can't trust partitions any longer, but we'll let
    > +                        * mtd apply them anyway, since some partitions may be
    > +                        * marked read-only, and we don't want to lose that
    > +                        * information, even if it's not 100% accurate.
    > +                        */
    > +                       dev_warn(&spi->dev, "found %s, expected %s\n",
    > +                                jid->name, id->name);
    > +                       id = jid;
    > +                       info = (void *)jid->driver_data;
    > +               }
    > +       }
    >
    >        flash = kzalloc(sizeof *flash, GFP_KERNEL);
    >        if (!flash)
    > --
    > 1.6.3.3
    >
    > --
    > 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/
    \
     
     \ /
      Last update: 2010-06-12 08:31    [W:0.031 / U:30.972 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site