linux-kernel - Re: [PATCH v4 3/3] mtd: spi-nor: Add support for IS25LX256 operating in 1S-8S-8S octal read mode

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]

Date:   Tue, 27 Dec 2022 13:37:43 +0100
From:   Michael Walle <michael@...le.cc>
To:     Tudor Ambarus <tudor.ambarus@...aro.org>
Cc:     Nathan Barrett-Morrison <nathan.morrison@...esys.com>,
        greg.malysa@...esys.com,
        Tudor Ambarus <tudor.ambarus@...rochip.com>,
        Pratyush Yadav <pratyush@...nel.org>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        Richard Weinberger <richard@....at>,
        Vignesh Raghavendra <vigneshr@...com>,
        "open list:SPI NOR SUBSYSTEM" <linux-mtd@...ts.infradead.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 3/3] mtd: spi-nor: Add support for IS25LX256 operating
 in 1S-8S-8S octal read mode

Am 2022-12-26 09:17, schrieb Tudor Ambarus:
> On 26.12.2022 10:04, Tudor Ambarus wrote:
>> Hi, Nathan,
>> 
>> The series is starting to look good, but I'll need another version,
>> please.
>> 
>> On 02.12.2022 15:55, Nathan Barrett-Morrison wrote:
>>> This adds the IS25LX256 chip into the ISSI flash_info parts table
>> 
>> Describe your changes in imperative mood, e.g. "Add support for
>> S25LX256" instead of "This adds ..."
>> 
>> It may worth to re-read
>> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>> once in a while.
>> 
>>> 
>>> Signed-off-by: Nathan Barrett-Morrison <nathan.morrison@...esys.com>
>>> ---
>>>   drivers/mtd/spi-nor/issi.c | 32 ++++++++++++++++++++++++++++++++
>>>   1 file changed, 32 insertions(+)
>>> 
>>> diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
>>> index 89a66a19d754..362bc3603d8f 100644
>>> --- a/drivers/mtd/spi-nor/issi.c
>>> +++ b/drivers/mtd/spi-nor/issi.c
>>> @@ -29,6 +29,35 @@ static const struct spi_nor_fixups 
>>> is25lp256_fixups = {
>>>       .post_bfpt = is25lp256_post_bfpt_fixups,
>>>   };
>>> +static int
>>> +is25lx256_post_bfpt_fixups(struct spi_nor *nor,
>>> +               const struct sfdp_parameter_header *bfpt_header,
>>> +               const struct sfdp_bfpt *bfpt)
>>> +{
>>> +    /*
>>> +     * IS25LX256 supports both 1S-1S-8S and 1S-8S-8S.
>>> +     * However, the BFPT does not contain any information denoting 
>>> this
>>> +     * functionality, so the proper fast read opcodes are never 
>>> setup.
>>> +     * We're correcting this issue via the fixup below.  Page 
>>> program
>>> +     * commands are detected and setup properly via the 4BAIT 
>>> lookup.
>>> +     */
> 
> Why don't you set the READ support when parsing the 4bait table?

That would deviate from the read handling of all the other modes.

> We need to see the SFDP dump to determine how we treat this. I'm not
> sure a post_bfpt hook is the right thing to do for this flash.

See:
https://lore.kernel.org/linux-mtd/e07105d41ec62a6ee47ca0295ca347dc@walle.cc/

But yes, I missed that you should include the sfdp dump and some other
commands in the comment section of the patch which adds support for that
flash (see Tudors reply).

Tudor, what is the status of your documentation patch? I'd really like
to refer to the kernel docs instead of having write the same over and
over again ;)

-michael