Re: [PATCH v2] mtd: micron-st: enable lock/unlock for mt25qu512a

[SHUKLA Mamta Ramendra]

Hello,

> On 10/6/23 17:09, SHUKLA Mamta Ramendra wrote:
>>
>>
>> Hello Tudor,
>>
> Hi!
> 
>>
>>> On 10/6/23 14:33, Tudor Ambarus wrote:
>>>>
>>>> Hi,
>>>>
>>>> Thanks for the debugging info.
>>>>
>>>> On 10/6/23 11:30, SHUKLA Mamta Ramendra wrote:
>>>>
>>>> cut
>>>>
>>>>> --------------------------------------------------------
>>>>>
>>>>> IMO, HAS_16BIT_SR flag is causing lock/unlock failure,
>>>>> since BP bits are calculated wrong then.
>>>>>
>>>>> I tested also for a case where I don't parse SFDP and
>>>>> reverted the condition in  micron_st_nor_default_init()
>>>>> for 16BIT Status Register Flag. And lock/unlock fails with
>>>>> same log as Non-working case.
>>>>>
>>>>> And this mt25qu512 has 8-BIT SR as typical micron-st flash.
>>>>>
>>>>
>>>> Indeed, the problem is that HAS_16BIT_SR gets set when it shouldn't have
>>>> to. This means that the BFPT table of the flash is wrong and we should
>>>> fix the parsed settings via a post_bfpt hook.
>>>>
>>>> Does the following fix your problem?
>>>>
>>>
>>> here it is again, this time compile tested:
>>>
>>> diff --git a/drivers/mtd/spi-nor/micron-st.c
>>> b/drivers/mtd/spi-nor/micron-st.c
>>> index 4afcfc57c896..20f76e278095 100644
>>> --- a/drivers/mtd/spi-nor/micron-st.c
>>> +++ b/drivers/mtd/spi-nor/micron-st.c
>>> @@ -180,6 +180,18 @@ static const struct flash_info micron_nor_parts[] = {
>>>           },
>>>    };
>>>
>>> +static int mt25qu512a_post_bfpt_fixup(struct spi_nor *nor,
>>> +                               const struct sfdp_parameter_header
>>> *bfpt_header,
>>> +                               const struct sfdp_bfpt *bfpt)
>>> +{
>>> +       nor->flags &= ~SNOR_F_HAS_16BIT_SR;
>>> +       return 0;
>>> +}
>>> +
>>> +static struct spi_nor_fixups mt25qu512a_fixups = {
>>> +       .post_bfpt = mt25qu512a_post_bfpt_fixup,
>>> +};
>>> +
>>>    static const struct flash_info st_nor_parts[] = {
>>>           {
>>>                   .name = "m25p05-nonjedec",
>>> @@ -405,10 +417,10 @@ static const struct flash_info st_nor_parts[] = {
>>>           }, {
>>>                   .id = SNOR_ID(0x20, 0xbb, 0x20, 0x10, 0x44, 0x00),
>>>                   .name = "mt25qu512a",
>>> -               .size = SZ_64M,
>>> -               .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ |
>>> SPI_NOR_QUAD_READ,
>>> -               .fixup_flags = SPI_NOR_4B_OPCODES,
>>> +               .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
>>> SPI_NOR_4BIT_BP |
>>> +                        SPI_NOR_BP3_SR_BIT6,
>>>                   .mfr_flags = USE_FSR,
>>> +               .fixups = &mt25qu512a_fixups,
>>>           }, {
>>>                   .id = SNOR_ID(0x20, 0xbb, 0x20),
>>>                   .name = "n25q512a",
>>>
>>> cut
>>
>>
>> Thanks. Yes, this post  BFPT fixup resolves lock/unlock issue and
>> I don't see HAS_16BIT_SR flag in params.
> 
> Good. Please send some patches addressing all that we discussed until now.

v3: 
https://patchwork.ozlabs.org/project/linux-mtd/patch/6b89ae4e4d7a381050746458cb000cd3c60f7a42.1696849423.git.mamta.shukla@xxxxxxxxxxxxxxxxxxxx/ 


>>
>> Datasheet is public:
>> https://media-www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_512_abb_0.pdf?rev=b259aadc3bea49ea8210a41c9ad58211 
>>
> I'll take a look later today.
> 
> Cheers,
> ta
>>
>>>> If yes, please add some prints in sfdp.c to determine where it's set,
>>>> either in BFPT_DWORD15_QER_SR2_BIT1 or BFPT_DWORD15_QER_SR2_BIT1_NO_RD
>>>> Is the datasheet for this flash public? Would you send me a link to it
>>>> please?
>>>>
>>>> Cheers,
>>>> ta