All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
To: linux-mtd@lists.infradead.org
Cc: Frieder Schrempf <frieder.schrempf@kontron.de>,
	Boris Brezillon <bbrezillon@kernel.org>,
	Tudor Ambarus <tudor.ambarus@microchip.com>,
	Michael Walle <michael@walle.cc>, Pratyush Yadav <p.yadav@ti.com>,
	linux-kernel@vger.kernel.org, Esben Haabendal <esben@geanix.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>
Subject: [RFC 2/3] mtd: spi-nor: core: compare JEDEC bytes to already found flash_info
Date: Mon, 21 Jun 2021 17:23:19 +0200	[thread overview]
Message-ID: <20210621152320.3811194-3-linux@rasmusvillemoes.dk> (raw)
In-Reply-To: <20210621152320.3811194-1-linux@rasmusvillemoes.dk>

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).

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 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>
---
 drivers/mtd/spi-nor/core.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 6a1adef0fe9f..21ae655d6a6f 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1869,7 +1869,8 @@ spi_nor_search_part_by_id(const struct flash_info *parts, unsigned int nparts,
 	return NULL;
 }
 
-static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
+static const struct flash_info *
+spi_nor_read_id(struct spi_nor *nor, const struct flash_info *guess)
 {
 	const struct flash_info *info;
 	u8 *id = nor->bouncebuf;
@@ -1893,6 +1894,9 @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
 		return ERR_PTR(ret);
 	}
 
+	if (guess && spi_nor_match_part(guess, id))
+		return guess;
+
 	for (i = 0; i < ARRAY_SIZE(manufacturers); i++) {
 		info = spi_nor_search_part_by_id(manufacturers[i]->parts,
 						 manufacturers[i]->nparts,
@@ -3031,7 +3035,7 @@ static const struct flash_info *spi_nor_get_flash_info(struct spi_nor *nor,
 		info = spi_nor_match_id(nor, name);
 	/* Try to auto-detect if chip name wasn't specified or not found */
 	if (!info)
-		info = spi_nor_read_id(nor);
+		info = spi_nor_read_id(nor, NULL);
 	if (IS_ERR_OR_NULL(info))
 		return ERR_PTR(-ENOENT);
 
@@ -3042,7 +3046,7 @@ static const struct flash_info *spi_nor_get_flash_info(struct spi_nor *nor,
 	if (name && info->id_len) {
 		const struct flash_info *jinfo;
 
-		jinfo = spi_nor_read_id(nor);
+		jinfo = spi_nor_read_id(nor, info);
 		if (IS_ERR(jinfo)) {
 			return jinfo;
 		} else if (jinfo != info) {
-- 
2.31.1


WARNING: multiple messages have this Message-ID (diff)
From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
To: linux-mtd@lists.infradead.org
Cc: Frieder Schrempf <frieder.schrempf@kontron.de>,
	Boris Brezillon <bbrezillon@kernel.org>,
	Tudor Ambarus <tudor.ambarus@microchip.com>,
	Michael Walle <michael@walle.cc>, Pratyush Yadav <p.yadav@ti.com>,
	linux-kernel@vger.kernel.org, Esben Haabendal <esben@geanix.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>
Subject: [RFC 2/3] mtd: spi-nor: core: compare JEDEC bytes to already found flash_info
Date: Mon, 21 Jun 2021 17:23:19 +0200	[thread overview]
Message-ID: <20210621152320.3811194-3-linux@rasmusvillemoes.dk> (raw)
In-Reply-To: <20210621152320.3811194-1-linux@rasmusvillemoes.dk>

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).

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 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>
---
 drivers/mtd/spi-nor/core.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 6a1adef0fe9f..21ae655d6a6f 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1869,7 +1869,8 @@ spi_nor_search_part_by_id(const struct flash_info *parts, unsigned int nparts,
 	return NULL;
 }
 
-static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
+static const struct flash_info *
+spi_nor_read_id(struct spi_nor *nor, const struct flash_info *guess)
 {
 	const struct flash_info *info;
 	u8 *id = nor->bouncebuf;
@@ -1893,6 +1894,9 @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
 		return ERR_PTR(ret);
 	}
 
+	if (guess && spi_nor_match_part(guess, id))
+		return guess;
+
 	for (i = 0; i < ARRAY_SIZE(manufacturers); i++) {
 		info = spi_nor_search_part_by_id(manufacturers[i]->parts,
 						 manufacturers[i]->nparts,
@@ -3031,7 +3035,7 @@ static const struct flash_info *spi_nor_get_flash_info(struct spi_nor *nor,
 		info = spi_nor_match_id(nor, name);
 	/* Try to auto-detect if chip name wasn't specified or not found */
 	if (!info)
-		info = spi_nor_read_id(nor);
+		info = spi_nor_read_id(nor, NULL);
 	if (IS_ERR_OR_NULL(info))
 		return ERR_PTR(-ENOENT);
 
@@ -3042,7 +3046,7 @@ static const struct flash_info *spi_nor_get_flash_info(struct spi_nor *nor,
 	if (name && info->id_len) {
 		const struct flash_info *jinfo;
 
-		jinfo = spi_nor_read_id(nor);
+		jinfo = spi_nor_read_id(nor, info);
 		if (IS_ERR(jinfo)) {
 			return jinfo;
 		} else if (jinfo != info) {
-- 
2.31.1


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

  parent reply	other threads:[~2021-06-21 15:23 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 ` Rasmus Villemoes [this message]
2021-06-21 15:23   ` [RFC 2/3] mtd: spi-nor: core: compare JEDEC bytes to already found flash_info Rasmus Villemoes
2021-06-22 11:57   ` Michael Walle
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=20210621152320.3811194-3-linux@rasmusvillemoes.dk \
    --to=linux@rasmusvillemoes.dk \
    --cc=bbrezillon@kernel.org \
    --cc=esben@geanix.com \
    --cc=frieder.schrempf@kontron.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=michael@walle.cc \
    --cc=p.yadav@ti.com \
    --cc=tudor.ambarus@microchip.com \
    /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: link
Be 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.