Messages in this thread | | | From | <> | Subject | Re: [RFC 2/3] mtd: spi-nor: core: compare JEDEC bytes to already found flash_info | Date | Wed, 23 Jun 2021 06:46:08 +0000 |
| |
Hi,
On 6/22/21 11:58 PM, Rasmus Villemoes wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 22/06/2021 13.57, Michael Walle wrote: >> [+ some people from MXIC as they are ones who posted to the ML >> lately. Feel free to forward this mail to the corresponding people.] >> >> Am 2021-06-21 17:23, schrieb Rasmus Villemoes: >>> Macronix engineers, in their infinite wisdom, have a habit of reusing >>> JEDEC ids for different chips. There's already one >>> workaround (MX25L25635F v MX25L25635E), but the same problem exists >>> for MX25L3205D v MX25L3233F, the latter of which is not currently >>> supported by linux. >>> >>> AFAICT, that case cannot really be handled with any of the ->fixup >>> machinery: The correct entry for the MX25L3233F would read >>> >>> { "mx25l3233f", INFO(0xc22016, 0, 64 * 1024, 64, SECT_4K | >>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ ) }, >>> >>> while the existing one is >>> >>> { "mx25l3205d", INFO(0xc22016, 0, 64 * 1024, 64, SECT_4K) }, >>> >>> So in spi_nor_init_params(), we won't even try reading the sfdp >>> info (i.e. call spi_nor_sfdp_init_params), and hence >>> spi_nor_post_sfdp_fixups() has no way of distinguishing the >>> chips. >>> >>> Replacing the existing entry with the mx25l3233f one to coerce the >>> core into issuing the SPINOR_OP_RDSFDP is also not really an option, >>> because the data sheet for the mx25l3205d explicitly says not to issue >>> any commands not listed ("It is not recommended to adopt any other >>> code not in the command definition table, which will potentially enter >>> the hidden mode.", whatever that means). >> >> Maybe we should ask Macronix if it is safe to send the RDSFDP command. >> Can anyone from MXIC comment this? > > Yeah, that would be useful to know, but I don't have any hopes > whatsoever of Macronix engineers being able to help sort out the mess > they've created by reusing IDs in the first place. They don't seem to > understand how that can possibly be a problem. > > I, and my client, have contacted them on several occasions to ask how > we're supposed to deal with that. At one point, the answer was > "MX25L3233F support Serial Flash Discoverable Parameters (SFDP) mode, > MX25L3205D does not support.", but when I asked the obvious follow-up > ("but the MX25L3205D datasheet warns against doing RDSFDP or any other > not explicitly allowed command"), I got no response. > > Another response was > > "I can only comment on Linux 4.4, as that is the only version that I > have supporting material for. Basically we have a patch for MTD/SPI-NOR > (see attached). This is to allow allow the MTD subsystem to cope with > devices that have the same ID (see below first paragraph of application > note attached). Please note that the MX25L3205D had an EOL notification > on 14th May 2010." > > and that attached patch is a 173KB .patch file that made me taste my > breakfast again. > > And they keep repeating the argument that when a chip is EOL, it's OK to > reuse its ID (because obviously nobody have used that chip in a product > that would receive OS updates, so any OS released later than that EOL > date can just include support for the newer chip and drop the old one...). > >>> In order to support such cases, extend the logic in spi_nor_read_id() >>> a little so that if we already have a struct flash_info* from the name >>> in device tree, check the JEDEC bytes against that, and if it is a >>> match, accept that (device tree compatible + matching JEDEC bytes) is >>> stronger than merely matching JEDEC bytes. >> >> This won't help much without a proper dt schema. No in-tree devicetree >> could use is because the DT validation would complain. > > I can certainly extend the regexp in jedec,spi-nor.yaml to match this > new one. DT is supposed to describe the hardware, so I can't see how > that could possibly be controversial.
No, please don't go that path yet.
> > So if this will >> go in (and the maintainers are rather hesitant to add it, I tried >> it myself [1]), you'd also need to add it to jedec,spi-nor.yaml and >> get an ack from Rob.
I'm not hesitant, I'm keeping my NACK until we're sure there isn't any other way to differentiate at run-time. I've contacted a macronix representative, asking if RDSFDP is harmful for MX25L3205D, let's wait for a few days. Maybe Zhengxun or ycllin in cc can help too. Is there anyone here having a MX25L3205D?
Cheers, ta
> > Thanks for that link. So it seems this isn't the first time recycled IDs > have come up, and not just for Macronix. > > Yes, vendors shouldn't recycle IDs. They do. They should be punished by > people not using those chips in new designs. Doesn't work, hardware > designers do use them. Auto-detection is preferred over using hard-coded > values from DT. Sure, absolutely, and when the ID is known to be > ambiguous, by all means throw in all the heuristics and chip-specific > quirks one can think of to disambiguate. But at the end of the day, > there are chips out there which cannot be distinguished without help > from DT, and as DT is supposed to describe the hardware, why is that > such a big problem? > > And I'm not suggesting any change whatsoever as to how a compatible > string of merely "jedec,spi-nor" would be handled - it would just take > the first chip with the read JEDEC id, then apply any appropriate quirks > and fixups. > > Rasmus >
| |