[PATCH RESEND 0/4] m25p80 enhance patches

[PATCH RESEND 0/4] m25p80 enhance patches

[Chen Gong]

Re: [PATCH 3/3] [MTD] m25p80.c

[David Woodhouse]

On Thu, 2008-08-07 at 11:41 +0800, Chen Gong wrote:
> Signed-off-by: Chen Gong <g.chen@freescale.com>

Changelog entry for this one?

-- 
dwmw2

Re: [PATCH 3/3] [MTD] m25p80.c

[David Woodhouse]

On Thu, 2008-08-07 at 11:41 +0800, Chen Gong wrote:
> @@ -547,6 +550,7 @@ static struct flash_info *__devinit jedec_probe(struct spi_device *spi)
>         u8                      code = OPCODE_RDID;
>         u8                      id[3];
>         u32                     jedec;
> +       u16                     ext_jedec;
>         struct flash_info       *info;
>  
>         /* JEDEC also defines an optional "extended device information"
	 * string for after vendor-specific data, after the three bytes
	 * we use here.  Supporting some chips might require using it.
	 */
	tmp = spi_write_then_read(spi, &code, 1, id, 3);
	if (tmp < 0) {
		DEBUG(MTD_DEBUG_LEVEL0, "%s: error %d reading JEDEC ID\n",
			spi->dev.bus_id, tmp);
		return NULL;
	}
	jedec = id[0];
	jedec = jedec << 8;
	jedec |= id[1];
>         jedec = jedec << 8;
>         jedec |= id[2];
>  
> +       ext_jedec = id[3] << 8 | id[4];
> +

You're trying to read off the end of the id[] array, which has only
three elements. And you didn't ask spi_write_then_read() to read that
many bytes _anyway_. And if you did... would it fail on chips which
don't have the extended device information?

Please test and send a fix, since this is already in the git tree.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: [PATCH 3/3] [MTD] m25p80.c

[chen gong]

2008/9/15 David Woodhouse <dwmw2@infradead.org>:
> On Thu, 2008-08-07 at 11:41 +0800, Chen Gong wrote:
>> @@ -547,6 +550,7 @@ static struct flash_info *__devinit jedec_probe(struct spi_device *spi)
>>         u8                      code = OPCODE_RDID;
>>         u8                      id[3];
>>         u32                     jedec;
>> +       u16                     ext_jedec;
>>         struct flash_info       *info;
>>
>>         /* JEDEC also defines an optional "extended device information"
>         * string for after vendor-specific data, after the three bytes
>         * we use here.  Supporting some chips might require using it.
>         */
>        tmp = spi_write_then_read(spi, &code, 1, id, 3);
>        if (tmp < 0) {
>                DEBUG(MTD_DEBUG_LEVEL0, "%s: error %d reading JEDEC ID\n",
>                        spi->dev.bus_id, tmp);
>                return NULL;
>        }
>        jedec = id[0];
>        jedec = jedec << 8;
>        jedec |= id[1];
>>         jedec = jedec << 8;
>>         jedec |= id[2];
>>
>> +       ext_jedec = id[3] << 8 | id[4];
>> +
>
> You're trying to read off the end of the id[] array, which has only
> three elements. And you didn't ask spi_write_then_read() to read that
> many bytes _anyway_. And if you did... would it fail on chips which
> don't have the extended device information?
>

Sorry for late. Last day is Mid-autumn Festival in our Country. We have a
nice holiday.
It is a stupid error that I forget to fix it from my branch. I will
send a new patch
to fix it soon.

> Please test and send a fix, since this is already in the git tree.
>
> --
> David Woodhouse                            Open Source Technology Centre
> David.Woodhouse@intel.com                              Intel Corporation
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>