[U-Boot] Regressions in MTD / SPI FLASH

Tue Sep 10 12:27:55 UTC 2019

Hi Eugeniy,

One more request:

I am trying to find a better way to identify parts that don't support
4byte addressing.

Could you enable CONFIG_SPI_FLASH_SFDP_SUPPORT and also enable debug
prints in spi_mem_exec_op() (in drivers/spi/spi-mem.c like before) and
provide logs?

Just logs of "sf probe" should be sufficient.

Regards
Vignesh

On 10/09/19 5:24 PM, Vignesh Raghavendra wrote:
> 
> 
> On 10/09/19 5:11 PM, Eugeniy Paltsev wrote:
>> Hi Vignesh,
>>
>> that patch helps - both erase and  write works fine.
>>
> 
> Thanks for testing! I will cleanup the patches and send formal patches
> to the list with your tested by.
> 
> Regards
> Vignesh
> 
>> For n25q512ax3:
>> Tested-by: "Eugeniy Paltsev <paltsev at synopsys.com>"
>>
>> ---
>>  Eugeniy Paltsev
>>
>>
>> ________________________________________
>> From: Vignesh Raghavendra <vigneshr at ti.com>
>> Sent: Tuesday, September 10, 2019 08:07
>> To: Eugeniy Paltsev; Jagan Teki
>> Cc: u-boot at lists.denx.de; uboot-snps-arc at synopsys.com; Alexey Brodkin; trini at konsulko.com
>> Subject: Re: Regressions in MTD / SPI FLASH
>>
>> Hi,
>>
>> On 10/09/19 12:18 AM, Eugeniy Paltsev wrote:
>>> Hi!
>>> Comments are inlined:
>>>
>>>> On 04/09/19 11:37 PM, Eugeniy Paltsev wrote:
>>>>> We faced with regressions caused by
>>>>> commit c4e8862308d4 (mtd: spi: Switch to new SPI NOR framework)
>>>>> This switch was performed by removing entire u-boot spi-flash
>>>>> core implementation and copying it from another project.
>>>>> However the switch is performed without proper testing and
>>>>> investigations about fixes/improvements were made in u-boot
>>>>> spi-flash core. This results in regressions.
>>>>>
>>>>
>>>> Apologies for the trouble...
>>>> As stated in cover letter, this change was necessary as U-Boot SPI flash
>>>> stack at that time did not features such as 4 byte addressing, SFDP
>>>> parsing, SPI controllers with MMIO interfaces etc. Also there was need
>>>> to move to SPI MEM framework to support both SPI NAND and SPI NOR
>>>> flashes using a single SPI controller drivers.
>>>> I have to disagree on the part that there was no proper testing... As
>>>> evident from mailing list archives, patch series was reviewed by
>>>> multiple reviewers and tested on their platforms as well...
>>>> Unfortunately its impossible to get all boards owners to test it.
>>>
>>> I'm not talking about getting all customers board and testing changes on them.
>>> It could be done another way - i.e. like it is done for u-boot driver-model migration:
>>> by introducing the option for choosing which stack will be used and allow customers
>>> to switch the flash IC they use to new stack until some deadline.
>>>
>>
>> I did start off with this. But maintaining two stacks is too cumbersome
>> and adds to code size (which is a big factor for SPL stage)
>>
>>
>>>>> One of regression we faced with is related to SST26 flash series which
>>>>> is used on HSDK board. The cause is that SST26 protection ops
>>>>> implementation was dropped. The fix of this regression is send
>>>>> as a patch in this series.
>>>>>
>>>>
>>>> I retained most U-Boot specific code as is (like support for BANK
>>>> address registers, restriction in transfer sizes) but I somehow
>>>> overlooked this part. Sorry for that
>>>>
>>>>> However there are another regressions. I.E: we also faced with broken
>>>>> SPI flash on another SNPS boards - AXS101 and AXS103. They use different
>>>>> flash IC (n25q512ax3) and I didn't investigate the cause yet.
>>>>>
>>>>
>>>> Could you provide more details here:
>>>> What exactly fails? Is the detected correctly? Does reads work fine? Is
>>>> Erase or Write broken?
>>>
>>> It seems to me that something is wrong with protection ops.
>>> The erase and write finish without errors however nothing actually happens.
>>>
>>
>> I doubt so, because if the blocks were protected, erase/write would have failed
>> and Read status/Read flag status register should have reported error values.
>> Anyways, I guess I found a wrt how 4 Byte addressing is handled wrt n25q512* series.
>>
>> Could you try with below patch helps[1]?
>> If not please provide logs similar what you have provide now.
>>
>> If below patch does not help, then please try enabling CONFIG_SPI_FLASH_BAR and see if that helps.
>>
>> [1]
>>
>> ---8<-----
>>
>> From 1de4c447cd4b2590c98f9ceccf8ed32029b42d36 Mon Sep 17 00:00:00 2001
>> From: Vignesh Raghavendra <vigneshr at ti.com>
>> Date: Tue, 10 Sep 2019 10:25:17 +0530
>> Subject: [TST PATCH] mtd: spi: spi-nor-ids: Disable SPI_NOR_4B_OPCODES
>>
>> Not all variants of n25q256* and n25q512* support 4 Byte stateless
>> addressing and there is no easy way to discover this at runtime.
>> Therefore don't set SPI_NOR_4B_OPCODES for these flashes
>>
>> Signed-off-by: Vignesh Raghavendra <vigneshr at ti.com>
>> ---
>>  drivers/mtd/spi/spi-nor-ids.c | 10 ++++------
>>  1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
>> index a3920ba520e0..66ac3256e8f5 100644
>> --- a/drivers/mtd/spi/spi-nor-ids.c
>> +++ b/drivers/mtd/spi/spi-nor-ids.c
>> @@ -161,12 +161,10 @@ const struct flash_info spi_nor_ids[] = {
>>         { INFO("n25q064a",    0x20bb17, 0, 64 * 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) },
>>         { INFO("n25q128a11",  0x20bb18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
>>         { INFO("n25q128a13",  0x20ba18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
>> -       { INFO("n25q256a",    0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
>> -       { INFO("n25q256ax1",  0x20bb19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
>> -       { INFO6("mt25qu512a",  0x20bb20, 0x104400, 64 * 1024, 1024,
>> -                SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
>> -       { INFO("n25q512a",    0x20bb20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
>> -       { INFO("n25q512ax3",  0x20ba20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
>> +       { INFO("n25q256a",    0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>> +       { INFO("n25q256ax1",  0x20bb19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
>> +       { INFO("n25q512a",    0x20bb20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
>> +       { INFO("n25q512ax3",  0x20ba20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
>>         { INFO("n25q00",      0x20ba21, 0, 64 * 1024, 2048, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ | NO_CHIP_ERASE) },
>>         { INFO("n25q00a",     0x20bb21, 0, 64 * 1024, 2048, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ | NO_CHIP_ERASE) },
>>         { INFO("mt25qu02g",   0x20bb22, 0, 64 * 1024, 4096, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ | NO_CHIP_ERASE) },
>> --
>> 2.23.0
>>
> 

-- 
Regards
Vignesh