Re: [PATCH v5 1/3] mtd: spi-nor: add support for is25wp256

[Vignesh Raghavendra]

On 24/06/19 6:10 PM, Sagar Kadam wrote:
> Hello Vignesh,
> 
> On Mon, Jun 24, 2019 at 3:04 PM Vignesh Raghavendra <vigneshr@xxxxxx> wrote:
>>
>> Hi,
>>
>> On 21/06/19 3:58 PM, Sagar Kadam wrote:
>>> Hello Vignesh,
>>>
>>> On Fri, Jun 21, 2019 at 11:33 AM Vignesh Raghavendra <vigneshr@xxxxxx> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 17/06/19 8:48 PM, Sagar Kadam wrote:
>>>>> Hello Vignesh,
>>>>>
>>>>> Thanks for your review comments.
>>>>>
>>>>> On Sun, Jun 16, 2019 at 6:14 PM Vignesh Raghavendra <vigneshr@xxxxxx> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On 12-Jun-19 4:17 PM, Sagar Shrikant Kadam wrote:
>>>>>> [...]
>>>>>>
>>>>>>> @@ -4129,7 +4137,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>>>>>>>       if (ret)
>>>>>>>               return ret;
>>>>>>>
>>>>>>> -     if (nor->addr_width) {
>>>>>>> +     if (nor->addr_width && JEDEC_MFR(info) != SNOR_MFR_ISSI) {
>>>>>>>               /* already configured from SFDP */
>>>>>>
>>>>>> Hmm, why would you want to ignore addr_width that's read from SFDP table?
>>>>>
>>>>> The SFDP table for ISSI device considered here, has addr_width set to
>>>>> 3 byte, and the flash considered
>>>>> here is 32MB. With 3 byte address width we won't be able to access
>>>>> flash memories higher address range.
>>>>
>>>> Is it specific to a particular ISSI part as indicated here[1]? If so,
>>>> please submit solution agreed there i.e. use spi_nor_fixups callback
>>>>
>>>> [1]https://patchwork.ozlabs.org/patch/1056049/
>>>>
>>>
>>> Thanks for sharing the link.
>>> From what I understand here, it seems that "Address Bytes" of SFDP
>>> table for the device under
>>> consideration (is25lp256) supports 3 byte only Addressing mode
>>> (DWORD1[18:17] = 0b00.
>>> where as that of ISSI device (is25LP/WP 256Mb/512/Mb/1Gb) support 3 or
>>> 4 byte Addressing mode DWORD1[18:17] = 0b01.
>>>
>>
>> Okay, so that SFDP table entry is correct. SPI NOR framework should
>> using 4 byte addressing if WORD1[18:17] = 0b01. Could you see if below
>> diff helps:
>>
> Thank-you for the suggestion.
> I applied it, and observed, that data in SFDP table mentioned in
> document received
> from ISSI support doesn't match with what is actually present on the
> device (I have raised a query with issi support for the same)
> The WP device also has the same SFDP entry as the LP device (the one
> which you shared).
> So, will submit V7 with the solution agreed in the link you shared above.
>      https://patchwork.ozlabs.org/patch/1056049/
> Apologies for the confusion, so please excuse the v6 which I submitted earlier.
> 

There is an updated version of the patch:
https://patchwork.ozlabs.org/patch/1071453/

You may want to align with Liu Xiang to avoid duplication of effort

> Thanks & BR,
> Sagar Kadam
> 
>> --->8---
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index c0a8837c0575..ebf32aebe5e9 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -2808,6 +2808,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>>                 break;
>>
>>         case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
>> +       case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
>>                 nor->addr_width = 4;
>>                 break;
> 
> 
>>
>>
>>
>>
>> --
>> Regards
>> Vignesh

-- 
Regards
Vignesh