Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752720Ab0FLG1f (ORCPT ); Sat, 12 Jun 2010 02:27:35 -0400 Received: from mail-gw0-f46.google.com ([74.125.83.46]:38875 "EHLO mail-gw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751964Ab0FLG1d (ORCPT ); Sat, 12 Jun 2010 02:27:33 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-type:content-transfer-encoding; b=lo7N1fwW/kFTHaCHE8vMxvYSpplZdZLvn4rfUEj43y3/vlMFK701RGSJEIUMlZAi64 G+9//uE/gYwWjOnmbDT5BWDHhtg6D2SwsH9X3IvXQomf4JYxbO4la9T5qQjELjKcZf2j 0Qrsf492Uekzz0bREkBIiVoD30GGDKg10Aiak= MIME-Version: 1.0 In-Reply-To: <20090818214622.GA22651@oksana.dev.rtsoft.ru> References: <20090818214449.GA12848@oksana.dev.rtsoft.ru> <20090818214622.GA22651@oksana.dev.rtsoft.ru> From: Barry Song <21cnbao@gmail.com> Date: Sat, 12 Jun 2010 14:27:12 +0800 Message-ID: Subject: Re: [PATCH 1/2] mtd: m25p80: Rework probing/JEDEC code To: Anton Vorontsov Cc: Andrew Morton , David Brownell , David Woodhouse , Artem Bityutskiy , linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org, linuxppc-dev@ozlabs.org Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by alpha.home.local id o5C6Rqim019391 Content-Length: 6542 Lines: 3 On Wed, Aug 19, 2009 at 5:46 AM, Anton Vorontsov 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. InThis patch caused a problem:even though the external flash doesn't exist, it will still pass theprobe() 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 > --->  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/????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?