Re: [PATCH 1/3] mtd: spi-nor: handle JEDEC manufacturer bank

["Michael Walle" <mwalle@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2943 bytes --]

Hi,

On Wed Aug 7, 2024 at 3:37 PM CEST, A. Zini wrote:
> > > Given the fast expanding pace of JEP106, the read ID operation has
> > > been expanded to 128 Bytes plus the pre-existing 6 Bytes for the ID
> > > code, thus supporting up to 128 banks.
> >
> > I really don't like issuing a 128byte command for older flashes. So
> > maybe we can just stick to the 6 bytes and if that's not enough we
> > can use the extended format.
>
> I agree that there may be better solutions than this. The idea for this
> patch was indeed to gather some of them and trigger a discussion.
>
> The question I have here is how can we determine when it's "enough"?

Given that most IDs are three bytes long (ignoring any extended IDs
for now) and that the former read length was 6, I'd say it's enough
up until bank 2. IOW. if the IDs are starting with more than 3
continuation codes, resend an extended RDID.

> > I'd like to keep the .id as the primary index. This will now
> > introduce a mfr_bank, so the unique key will be (mfr_bank,id). Can
> > we somehow encode the continuation codes into the id itself? E.g.
> > we know the manufacturer ID is always < 127. Honestly, I'm not sure
> > this is the way to go as we know flash manufacturers sometimes don't
> > care. So right now, we just compare the .id with whats returned by
> > the RDID command without interpreting it.
>
> Even though the manufacturer bank is technically not part of the ID as
> per JEDEC standard, it's still a piece of information needed to
> correctly identify a chip and avoid collisions.
> Therefore, my opinion is that it should be part of the unique key,
> whether encoded in the id itself or in a different field.

Yes of course. I'm just wondering about the advantages of encoding
this as "mfr_bank" instead of just keep it dead simple and use
SNOR_ID(0x7f, 0x7f...). Yes, they might get long and take up a bit
of code space, OTOH we know that vendors f*ck up and get things
wrong. E.g. have a look at the cy15x104q entry.. It wouldn't be
possible to use mfr_bank with that entry.

FWIW, I'm still envisioning a .match() callback, where an SPI-NOR
flash driver could just register its own match function and the core
provides a default "match_snor_id()" one. We could get rid of all
the fixup functions, where we correct the flash parameters in case a
different flash is found which happens to have the same ID.

-michael

> One other possibility could be, when needed, to add two bytes to the id
> field, them being one continuation code and one multiplier factor
> indicating how many continuation codes are present for this
> manufacturer. The absence of the leading continuation code would
> (obviously) be assumed as bank 1.
> The spi_nor_match_id() function should of course be adapted to handle
> this additional information, but we would not have to add an additional
> mfr_bank field.
>
> --
> Alessandro


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/