linux-kernel - Re: [PATCH] mtd: spi-nor: winbond: add support for W25Q512NW-IQ/IN

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1ce1ac2fabf7e46e303da01782469355@walle.cc>
Date:   Sat, 16 Jul 2022 00:52:06 +0200
From:   Michael Walle <michael@...le.cc>
To:     Jae Hyun Yoo <quic_jaehyoo@...cinc.com>
Cc:     clg@...d.org, linux-kernel@...r.kernel.org,
        linux-mtd@...ts.infradead.org, p.yadav@...com,
        quic_ggregory@...cinc.com, quic_jiles@...cinc.com,
        tudor.ambarus@...rochip.com
Subject: Re: [PATCH] mtd: spi-nor: winbond: add support for W25Q512NW-IQ/IN

Hi,

Am 2022-07-15 22:15, schrieb Jae Hyun Yoo:
> On 7/14/2022 7:30 AM, Jae Hyun Yoo wrote:
>> On 7/14/2022 7:21 AM, Michael Walle wrote:
>>> Am 2022-07-14 16:16, schrieb Michael Walle:
>>>> Am 2022-07-14 15:47, schrieb Jae Hyun Yoo:
>>>>> On 7/14/2022 12:41 AM, Michael Walle wrote:
>>>>>> What does "doesn't boot at all" mean? Are there any kernel startup
>>>>>> messages?
>>>>> 
>>>>> I'm sharing the error messages below.
>>>> 
>>>> Thanks.
>>>> 
>>>>> [    0.748594] spi-nor spi0.0: w25q512nwq (65536 Kbytes)
>>>>> [    0.865216] spi-aspeed-smc 1e620000.spi: CE0 read buswidth:4 
>>>>> [0x406c0741]
>>>>> [    0.872833] ------------[ cut here ]------------
>>>>> [    0.877984] WARNING: CPU: 1 PID: 1 at drivers/mtd/mtdcore.c:583
>>>>> add_mtd_device+0x28c/0x53c
>>>>> [    0.887237] CPU: 1 PID: 1 Comm: swapper/0 Not tainted
>>>>> 5.15.43-AUTOINC-dirty-23801a6 #1
>>>> 
>>>> Could you please try it on the latest (vanilla) linux-next?
>>> 
>>> or spi-nor/next [1] as there are quite a lot of changes. The
>>> patches shall be based on that.
>> 
>> Okay. Let me try that. I tested it using 5.15.43 with back-ported
>> spi-nor patches from the latest. I'll back-port more changes from
>> the spi-nor/next and will test the INFO(0xef6020, 0, 0, 0) setting
>> again.
> 
> I tested the setting again after cherry picking all SPI relating 
> changes
> from the 'for-next' branch of
> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi repository.

That is not the tree I mentioned in my previous mail. Why do you
cherry-pick the changes instead of just trying the spi-nor/next
tree?

> No luck! It's still making the same warning dump at 'add_mtd_device'
> since 'mtd->erasesize' is checked as 0.
> 
> I traced it further to check if the erasesize is properly parsed from
> the sfdp and checked that erase map seems parsed and initialized
> correctly in 'spi_nor_parse_bfpt' but problem is, a target
> mtd->erasesize is not properly selected in 'spi_nor_select_erase' since
> the 'wanted_size' variable is initialized as sector size of info table
> so a selected target mtd->erasesize is also 0 so looks like it's the
> reason why it can't initialize mtd device if we use
> INFO(0xef6020, 0, 0, 0).

Have a look at
https://lore.kernel.org/linux-mtd/20220510140232.3519184-2-michael@walle.cc/
wanted_size can be 0. In this case spi_nor_select_uniform_erase()
should return the biggest valid erase type. Could you please check that
(1) spi_nor_select_uniform_erase() return non-NULL
(2) check what value erase->size has (I guess it should be 64k in your 
case)

 From what you tell me erase->size should be zero. If that is the
case look at spi_nor_parse_bfpt() where the erase sizes are parsed.
Also take a look at spi_nor_parse_4bait() where the erase types might
be cleared again.

I've checked your SFDP data and it contains three valid erase sizes
and opcodes for 3byte addressing and two valid erase opcodes for 4
byte addressing. So that looks all good. After all the SFDP parsing
I expect that you have two valid erase types:
  - erase size 4096 with erase opcode 21h
  - erase size 65536 with erase opcode DCh

> Also, checked that the mtd->erasesize is set to 4096 if I enable
> CONFIG_MTD_SPI_NOR_USE_4K_SECTORS so the SPI flash can be initialized
> with the INFO(0xef6020, 0, 0, 0) setting but, it should cover even
> when
> the configuration is not enabled. I think, this patch should go as it
> is. The erasesize selecting issue could be fixed using a separate
> patch.
> Are you still sure that the INFO(0xef6020, 0, 0, 0) works in the
> latest spi-next?

I've got two tested-by's with two different flashes, so yes, I'm
pretty sure it works in principle. It might still be something
wrong with your flash though.

-michael