lkml.org 
[lkml]   [2019]   [Mar]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2] mtd: spi-nor: Return error when nor->addr_width does not match the device size
From
Date
On 26/03/19 8:50 PM, Liu Xiang wrote:
> At 2019-03-19 13:22:15, "Vignesh Raghavendra" <vigneshr@ti.com> wrote:
>> Hi,
>>
>> On 13/03/19 7:15 PM, Liu Xiang wrote:
>>> In some is25lp256, the DWORD1 of JEDEC Basic Flash Parameter Header
>>> is 0xfff920e5. So the DWORD1[18:17] Address Bytes bits are 0b00,
>>> means that 3-Byte only addressing. But the device size is larger
>>> than 16MB, nor->addr_width must be 4 to access the whole address.
>>> An error should be returned when nor->addr_width does not match
>>> the device size in spi_nor_parse_bfpt(). Then it can go back to
>>> use spi_nor_ids[] for setting the right addr_width.
>>>
>>> Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com>
>>> Signed-off-by: Liu Xiang <liu.xiang6@zte.com.cn>
>>> ---
>>> drivers/mtd/spi-nor/spi-nor.c | 8 ++++++++
>>> 1 file changed, 8 insertions(+)
>>>
>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>> index 6e13bbd..63933c7 100644
>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>> @@ -2811,6 +2811,14 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>>> }
>>> params->size >>= 3; /* Convert to bytes. */
>>>
>>> + /*
>>> + * If the device exceeds 16MiB, addr_width must be 4.
>>> + * addr_width == 3 means the Address Bytes we are
>>> + * reading from BFPT is wrong.
>>> + */
>>
>> JESD216 standard does not mandate flash devices >16MiB to always support
>> 4 byte addressing opcode. So, its okay for flash vendor to support
>>> 16MiB flash with 3 byte addressing and Bank/extended address register.
>>
>>> + if (params->size > 0x1000000 && nor->addr_width == 3)
>>> + return -EINVAL;
>>> +
>>
>> Assuming only DWORD1[18:17] bits are wrong, then returning from here
>> would mean we miss parsing Sector Erase settings, Quad Enable
>> Requirements etc from BFPT which is kind of bad.
>> I suggest to move the fix to[1], addr_width indicated in flash_info
>> struct of the device can take precedence over SFDP.
>>
>> [1]https://elixir.bootlin.com/linux/latest/source/drivers/mtd/spi-nor/spi-nor.c#L4106
>
> Boris has added a fixup function, do you think this is more better:
>
> static int
> is25lp256_post_bfpt_fixups(struct spi_nor *nor,
> const struct sfdp_parameter_header *bfpt_header,
> const struct sfdp_bfpt *bfpt,
> struct spi_nor_flash_parameter *params)
> {
> /*
> * IS25LP256 supports 4B opcodes.
> * Unfortunately, some devices get BFPT_DWORD1_ADDRESS_BYTES_3_ONLY
> * from BFPT table for address width. We should fix it.
> */
> if (bfpt.dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK ==
> BFPT_DWORD1_ADDRESS_BYTES_3_ONLY)
> nor->addr_width = 4;
>
> return 0;
> }
>
> static struct spi_nor_fixups is25lp256_fixups = {
> .post_bfpt = is25lp256_post_bfpt_fixups,
> };
>
>

Sounds fine to me as is25lp256 is the only part that needs this quirk at
the moment.


--
Regards
Vignesh

\
 
 \ /
  Last update: 2019-03-27 17:04    [W:0.052 / U:3.296 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site