[U-Boot] [EXT] Re: [Patch V3] drivers: mtd :spi: Enable 4B opcodes for SPANSION s25fl512s

[Vignesh Raghavendra]

On 18/07/19 4:29 PM, Jagan Teki wrote:
> On Thu, Jul 18, 2019 at 4:15 PM Ashish Kumar <ashish.kumar at nxp.com> wrote:
>>
>>
>>
>>> -----Original Message-----
>>> From: Jagan Teki <jagan at amarulasolutions.com>
>>> Sent: Thursday, July 18, 2019 3:59 PM
>>> To: Ashish Kumar <ashish.kumar at nxp.com>; Vignesh R <vigneshr at ti.com>
>>> Cc: U-Boot-Denx <u-boot at lists.denx.de>
>>> Subject: [EXT] Re: [Patch V3] drivers: mtd :spi: Enable 4B opcodes for
>>> SPANSION s25fl512s
>>>
>>> Caution: EXT Email
>>>
>>> + Vignesh
>>>
>>> On Wed, Jul 17, 2019 at 11:46 AM Ashish Kumar <Ashish.Kumar at nxp.com>
>>> wrote:
>>>>
>>>> s25fs512s and s25fl512s which has same JEDEC ID but only varies in
>>>> operating volatge so s25fs512s shares same command set as mentioned
>>>> below:
>>>> – Serial Command subset and footprint compatible with S25FL-A,
>>>> S25FL-K, S25FL-P, and S25FL-S SPI families – Multi I/O Command subset
>>>> and footprint compatible with S25FL-P, and S25FL-S SPI families
>>>>
>>>> Signed-off-by: Ashish Kumar <Ashish.Kumar at nxp.com>
>>>> ---
>>>> v3:
>>>> 1. Add version info, rebase to top.
>>>> 2. Re-word commit message.
>>>> v2:
>>>> 1. Adding more description in commit msg.
>>>> 2. consolidating "" and  "" in single patch.
>>>>
>>>>  drivers/mtd/spi/spi-nor-ids.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mtd/spi/spi-nor-ids.c
>>>> b/drivers/mtd/spi/spi-nor-ids.c index 3217fc6..a992966 100644
>>>> --- a/drivers/mtd/spi/spi-nor-ids.c
>>>> +++ b/drivers/mtd/spi/spi-nor-ids.c
>>>> @@ -181,7 +181,7 @@ const struct flash_info spi_nor_ids[] = {
>>>>         { INFO("s25sl064p",  0x010216, 0x4d00,  64 * 1024, 128,
>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>>>>         { INFO("s25fl256s0", 0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR) },
>>>>         { INFO("s25fl256s1", 0x010219, 0x4d01,  64 * 1024, 512,
>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>>>> -       { INFO6("s25fl512s",  0x010220, 0x4d0081, 256 * 1024, 256,
>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>>>> +       { INFO6("s25fl512s",  0x010220, 0x4d0081, 256 * 1024, 256,
>>>> + SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR |
>>>> + SPI_NOR_4B_OPCODES) },
>>>
>>> I didn't find any diff b/w this with respect to v1 patch, seems like Vignesh
>>> commented some issue? any update on that.
>>
>> Hi Jagan, Vignesh,
>>
>> I had updated commit message.
>>
>> Wrt Vignesh's comment:
>> To me it seems not valid. This Flash can be used in extended 3-byte addressing mode as well as 4-byte addressing mode.
>> Adding SPI_NOR_4B_OPCODE, will make use of 4-byte addressing mode with CONFIG_SPI_FLASH_BAR being __not__ set.
>> By adding SPI_NOR_4B_OPCODES code flow will via spi_nor_set_4byte_opcodes() in place of set_4byte().
>>

This does not seem right. Here is the code snippet from spi-nor-core.c::spi_nor_scan():

#ifndef CONFIG_SPI_FLASH_BAR
                /* enable 4-byte addressing if the device exceeds 16MiB */
                nor->addr_width = 4;
                if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
                    info->flags & SPI_NOR_4B_OPCODES)
                        spi_nor_set_4byte_opcodes(nor, info);
#else
        /* Configure the BAR - discover bank cmds and read current bank */
        nor->addr_width = 3;
        ret = read_bar(nor, info);
        if (ret < 0)
                return ret;
#endif


So as long as CONFIG_SPI_FLASH_BAR is not defined and flash is Spansion flash
spi_nor_set_4byte_opcodes() is always called irrespective of SPI_NOR_4B_OPCODES

Also from spi_nor_init():,


        if (nor->addr_width == 4 &&
            (JEDEC_MFR(nor->info) != SNOR_MFR_SPANSION) &&
            !(nor->info->flags & SPI_NOR_4B_OPCODES)) {
                /*
                 * If the RESET# pin isn't hooked up properly, or the system
                 * otherwise doesn't perform a reset command in the boot
                 * sequence, it's impossible to 100% protect against unexpected
                 * reboots (e.g., crashes). Warn the user (or hopefully, system
                 * designer) that this is bad.
                 */
                if (nor->flags & SNOR_F_BROKEN_RESET)
                        printf("enabling reset hack; may not recover from unexpected reboots\n");
                set_4byte(nor, nor->info, 1);
        }

So set_4byte() is not called for Spansion flashes. So I don't see need for this patch.


>> There is patch to unset CONFIG_SPI_FLASH_BAR, for boards that are using 4-byte addressing mode with this flash
>> http://patchwork.ozlabs.org/patch/1108287/
> 
> Thanks for the information.
> 
> Applied to u-boot-spi/master
> 

-- 
Regards
Vignesh