From: Michael Walle <michael@walle.cc> To: Rasmus Villemoes <linux@rasmusvillemoes.dk> Cc: linux-mtd@lists.infradead.org, Frieder Schrempf <frieder.schrempf@kontron.de>, Boris Brezillon <bbrezillon@kernel.org>, Tudor Ambarus <tudor.ambarus@microchip.com>, Pratyush Yadav <p.yadav@ti.com>, linux-kernel@vger.kernel.org, Esben Haabendal <esben@geanix.com>, Zhengxun Li <zhengxunli@mxic.com.tw>, Jaime Liao <jaimeliao@mxic.com.tw>, masonccyang@mxic.com.tw, ycllin@mxic.com.tw Subject: Re: [RFC 2/3] mtd: spi-nor: core: compare JEDEC bytes to already found flash_info Date: Tue, 22 Jun 2021 13:57:00 +0200 [thread overview] Message-ID: <223232ecd0865e819477f17cd464ab08@walle.cc> (raw) In-Reply-To: <20210621152320.3811194-3-linux@rasmusvillemoes.dk> [+ 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? This is also interesting because we are discussing reading the SFDP without reading the ID first. Of course this will not save us from two different devices sharing the same ID and both having no RDSFDP support. > 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. 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. > This also makes initialization slightly faster in the common case > where the flash_info was found from the name and the JEDEC bytes do > match - it avoids a second linear search over all known chips. > > Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> -michael [1] https://lore.kernel.org/linux-mtd/20200103223423.14025-1-michael@walle.cc/
WARNING: multiple messages have this Message-ID (diff)
From: Michael Walle <michael@walle.cc> To: Rasmus Villemoes <linux@rasmusvillemoes.dk> Cc: linux-mtd@lists.infradead.org, Frieder Schrempf <frieder.schrempf@kontron.de>, Boris Brezillon <bbrezillon@kernel.org>, Tudor Ambarus <tudor.ambarus@microchip.com>, Pratyush Yadav <p.yadav@ti.com>, linux-kernel@vger.kernel.org, Esben Haabendal <esben@geanix.com>, Zhengxun Li <zhengxunli@mxic.com.tw>, Jaime Liao <jaimeliao@mxic.com.tw>, masonccyang@mxic.com.tw, ycllin@mxic.com.tw Subject: Re: [RFC 2/3] mtd: spi-nor: core: compare JEDEC bytes to already found flash_info Date: Tue, 22 Jun 2021 13:57:00 +0200 [thread overview] Message-ID: <223232ecd0865e819477f17cd464ab08@walle.cc> (raw) In-Reply-To: <20210621152320.3811194-3-linux@rasmusvillemoes.dk> [+ 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? This is also interesting because we are discussing reading the SFDP without reading the ID first. Of course this will not save us from two different devices sharing the same ID and both having no RDSFDP support. > 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. 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. > This also makes initialization slightly faster in the common case > where the flash_info was found from the name and the JEDEC bytes do > match - it avoids a second linear search over all known chips. > > Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> -michael [1] https://lore.kernel.org/linux-mtd/20200103223423.14025-1-michael@walle.cc/ ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2021-06-22 11:57 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-06-21 15:23 [RFC 0/3] mtd: spi-nor: dealing with reused JEDEC id c22016 Rasmus Villemoes 2021-06-21 15:23 ` Rasmus Villemoes 2021-06-21 15:23 ` [RFC 1/3] mtd: spi-nor: core: create helper to compare JEDEC id to struct flash_info Rasmus Villemoes 2021-06-21 15:23 ` Rasmus Villemoes 2021-06-21 15:23 ` [RFC 2/3] mtd: spi-nor: core: compare JEDEC bytes to already found flash_info Rasmus Villemoes 2021-06-21 15:23 ` Rasmus Villemoes 2021-06-22 11:57 ` Michael Walle [this message] 2021-06-22 11:57 ` Michael Walle 2021-06-22 20:58 ` Rasmus Villemoes 2021-06-22 20:58 ` Rasmus Villemoes 2021-06-23 6:46 ` Tudor.Ambarus 2021-06-23 6:46 ` Tudor.Ambarus 2021-07-02 13:17 ` Rasmus Villemoes 2021-07-02 13:17 ` Rasmus Villemoes 2021-07-02 13:34 ` Tudor.Ambarus 2021-07-02 13:34 ` Tudor.Ambarus [not found] ` <OF7CD5328D.D33B2BE2-ON482586FD.0027BCF2-482586FD.00280249@mxic.com.tw> 2021-06-23 8:33 ` 回信: " Tudor.Ambarus 2021-06-23 8:33 ` Tudor.Ambarus 2021-06-29 7:42 ` Rasmus Villemoes 2021-06-29 7:42 ` Rasmus Villemoes 2021-06-21 15:23 ` [RFC 3/3] mtd: spi-nor: macronix: add entry for mx25l3233f Rasmus Villemoes 2021-06-21 15:23 ` Rasmus Villemoes 2021-06-22 8:55 ` [RFC 0/3] mtd: spi-nor: dealing with reused JEDEC id c22016 Rasmus Villemoes 2021-06-22 8:55 ` Rasmus Villemoes 2021-06-22 9:26 ` Esben Haabendal 2021-06-22 9:26 ` Esben Haabendal
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=223232ecd0865e819477f17cd464ab08@walle.cc \ --to=michael@walle.cc \ --cc=bbrezillon@kernel.org \ --cc=esben@geanix.com \ --cc=frieder.schrempf@kontron.de \ --cc=jaimeliao@mxic.com.tw \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mtd@lists.infradead.org \ --cc=linux@rasmusvillemoes.dk \ --cc=masonccyang@mxic.com.tw \ --cc=p.yadav@ti.com \ --cc=tudor.ambarus@microchip.com \ --cc=ycllin@mxic.com.tw \ --cc=zhengxunli@mxic.com.tw \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.