Messages in this thread | | | Subject | Re: [RFC 2/3] mtd: spi-nor: core: compare JEDEC bytes to already found flash_info | From | Rasmus Villemoes <> | Date | Fri, 2 Jul 2021 15:17:48 +0200 |
| |
On 23/06/2021 08.46, Tudor.Ambarus@microchip.com wrote: > 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.
It seems that we have established that by now, right?
Rasmus
| |