Hi

On 24/09/19 9:45 PM, Eugeniy Paltsev wrote:
> Hi Vignesh,
> sorry for delay, I was pretty busy with another project :(
> 
> I've tried to test SPI with CONFIG_SPI_FLASH_SFDP_SUPPORT enabled, however it 
> doesn't seem to work.
> 
> CONFIG_SPI_FLASH_SFDP_SUPPORT enabled:
> ---------------------------------->8--------------------------------------------
> AXS# sf probe && echo OK
> 9f | [6B in] 20 ba 20 10 00 00 [ret 0]
> 5a 00 00 00 | [16B in] 53 46 44 50 00 01 00 ff 00 00 01 09 30 00 00 ff [ret 0]
> 5a 00 00 30 | [36B in] e5 20 fb ff ff ff ff 1f 29 eb 27 6b 27 3b 27 bb ff ff 
> ff ff ff ff 27 bb ff ff 29 eb 0c 20 10 d8 00 00 00 00 [ret 0]
> 06 | [0B -] [ret 0]
> b7 | [0B -] [ret 0]
> 04 | [0B -] [ret 0]
> SF: Detected n25q512ax3 with page size 256 Bytes, erase size 64 KiB, total 64 
> MiB

Oops, the sector size is being set 64K here instead of 4K in this
case... Will send a fix for that

> OK,
> AXS# sf read 0x81000000 0x180000 0x10 && echo OK
> device 0 offset 0x180000, size 0x10
> 0b 00 18 00 00 | [16B in] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
> [ret 0]
> SF: 16 bytes @ 0x180000 Read: OK
> OK
> AXS# md.b 0x81000000 0x10
> 81000000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
> AXS# sf protect unlock 0x0 0x4000000 && echo OK
> 05 | [1B in] 00 [ret 0]
> OK
> AXS# sf erase 0x180000 0x1000 && echo OK
> SF: Erase offset/length not multiple of erase size
> SF: 4096 bytes @ 0x180000 Erased: ERROR
> ---------------------------------->8--------------------------------------------
> 

And therefore this failure... Thanks for the debug logs!

Regards
Vignesh


> 
> 
> CONFIG_SPI_FLASH_SFDP_SUPPORT disabled (everything OK):
> ---------------------------------->8--------------------------------------------
> AXS# sf probe && echo OK
> 9f | [6B in] 20 ba 20 10 00 00 [ret 0]
> 06 | [0B -] [ret 0]
> b7 | [0B -] [ret 0]
> 04 | [0B -] [ret 0]
> SF: Detected n25q512ax3 with page size 256 Bytes, erase size 4 KiB, total 64 
> MiB
> OK
> AXS# sf read 0x81000000 0x180000 0x10 && echo OK
> device 0 offset 0x180000, size 0x10
> 0b 00 18 00 00 | [16B in] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
> [ret 0]
> SF: 16 bytes @ 0x180000 Read: OK
> OK
> AXS# md.b 0x81000000 0x10
> 81000000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
> AXS# sf protect unlock 0x0 0x4000000 && echo OK
> 05 | [1B in] 00 [ret 0]
> OK
> AXS# sf erase 0x180000 0x1000 && echo OK
> 06 | [0B -] [ret 0]
> 20 00 18 00 00 | [0B -] [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 03 [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 00 [ret 0]
> 70 | [1B in] 81 [ret 0]
> 04 | [0B -] [ret 0]
> SF: 4096 bytes @ 0x180000 Erased: OK
> OK
> AXS# sf read 0x81000000 0x180000 0x10 && echo OK
> device 0 offset 0x180000, size 0x10
> 0b 00 18 00 00 | [16B in] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff 
> [ret 0]
> SF: 16 bytes @ 0x180000 Read: OK
> OK
> AXS# md.b 0x81000000 0x10
> 81000000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
> AXS# mw 0x81000000 0 5
> AXS# sf write 0x81000000 0x180000 0x10 && echo OK
> device 0 offset 0x180000, size 0x10
> 06 | [0B -] [ret 0]
> 02 00 18 00 00 | [16B out] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
> [ret 0]
> 05 | [1B in] 00 [ret 0]
> 70 | [1B in] 81 [ret 0]
> SF: 16 bytes @ 0x180000 Written: OK
> OK
> AXS# sf read 0x81000000 0x180000 0x10 && echo OK
> device 0 offset 0x180000, size 0x10
> 0b 00 18 00 00 | [16B in] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
> [ret 0]
> SF: 16 bytes @ 0x180000 Read: OK
> OK
> AXS# md.b 0x81000000 0x10
> 81000000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
> AXS# 
> ---------------------------------->8--------------------------------------------
> 
> All tests were performed on 4b95daf5dca with your commit (for disabling 4B 
> opcodes) applied.
> 
> ---
>  Eugeniy Paltsev
> 
> 
> ________________________________________
> From: Vignesh Raghavendra <vigne...@ti.com>
> Sent: Monday, September 23, 2019 08:31
> To: Eugeniy Paltsev
> Cc: uboot-snps-...@synopsys.com; Alexey Brodkin
> Subject: Re: [U-Boot] Regressions in MTD / SPI FLASH
> 
> Hi Eugeniy
> 
> On 12/09/19 5:59 PM, Eugeniy Paltsev wrote:
>> Hi Vignesh,
>>
>> I doesn't have access to board with n25q512ax3 currently, however I can test 
>> this on Monday (16.09)
> 
> Could you provide please provide logs that I requested below?
> 
> "
>> 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?
> "
> 
> There are some objections to the fixes[1] that I provided to you. Above
> logs will helps me in justifying the fix or provide better patch. I
> would like to close this as soon as possible before 2019.10 is out.
> Appreciate you help!
> 
> [1] 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__patchwork.ozlabs.org_patch_1160501_&d=DwICaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=ZlJN1MriPUTkBKCrPSx67GmaplEUGcAEk9yPtCLdUXI&m=Yb1WwCtRxov5WTaRZMp5FA3-x1hLIH-G-192fad-opU&s=w3B4OJ4ml06qS7q7BjbvhzZXkLQswsLfXxuPt7McUU4&e=
> 
>>
>> ---
>>  Eugeniy Paltsev
>>
>>
>> ________________________________________
>> From: Vignesh Raghavendra <vigne...@ti.com>
>> Sent: Tuesday, September 10, 2019 15:27
>> To: Eugeniy Paltsev; Jagan Teki
>> Cc: u-boot@lists.denx.de; Alexey Brodkin; tr...@konsulko.com; 
>> uboot-snps-...@synopsys.com
>> Subject: Re: [U-Boot] Regressions in MTD / SPI FLASH
>>
>> 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 <palt...@synopsys.com>"
>>>>
>>>> ---
>>>>  Eugeniy Paltsev
>>>>
>>>>
>>>> ________________________________________
>>>> From: Vignesh Raghavendra <vigne...@ti.com>
>>>> Sent: Tuesday, September 10, 2019 08:07
>>>> To: Eugeniy Paltsev; Jagan Teki
>>>> Cc: u-boot@lists.denx.de; uboot-snps-...@synopsys.com; Alexey Brodkin; 
>>>> tr...@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 <vigne...@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 <vigne...@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
>>
> 
> --
> Regards
> Vignesh
> 

-- 
Regards
Vignesh
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to