* [PATCH 0/3] Add support for Cypress/Infineon cy15b102q
@ 2024-07-29 10:20 A. Zini
2024-07-29 10:20 ` [PATCH 1/3] mtd: spi-nor: handle JEDEC manufacturer bank A. Zini
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: A. Zini @ 2024-07-29 10:20 UTC (permalink / raw)
To: linux-mtd
Cc: Tudor Ambarus, Pratyush Yadav, Michael Walle, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Alessandro Zini
From: Alessandro Zini <alessandro.zini@siemens.com>
This patch series introduces support for Cypress/Infineon cy15b102q, a 2Mbit
serial SPI F-RAM device.
As the JEDEC-assigned manufacturer ID places the Cypress (Ramtron)
identifier in bank 7, this series also introduces a mechanism to handle eventual
continuation codes (0x7f) that may be prefixed to the actual ID.
Alessandro Zini (3):
mtd: spi-nor: handle JEDEC manufacturer bank
mtd: spi-nor: convert existing devices to use mfr_bank
mtd: spi-nor: add support for Cypress cy15b102q
drivers/mtd/spi-nor/core.c | 71 +++++++++++++++++++++++++++++-----
drivers/mtd/spi-nor/core.h | 10 ++++-
drivers/mtd/spi-nor/debugfs.c | 2 +
drivers/mtd/spi-nor/issi.c | 6 ++-
drivers/mtd/spi-nor/spansion.c | 7 ++++
drivers/mtd/spi-nor/sysfs.c | 13 +++++++
6 files changed, 97 insertions(+), 12 deletions(-)
--
2.45.2
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] mtd: spi-nor: handle JEDEC manufacturer bank
2024-07-29 10:20 [PATCH 0/3] Add support for Cypress/Infineon cy15b102q A. Zini
@ 2024-07-29 10:20 ` A. Zini
2024-08-05 9:18 ` Michael Walle
2024-07-29 10:20 ` [PATCH 2/3] mtd: spi-nor: convert existing devices to use mfr_bank A. Zini
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: A. Zini @ 2024-07-29 10:20 UTC (permalink / raw)
To: linux-mtd
Cc: Tudor Ambarus, Pratyush Yadav, Michael Walle, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Alessandro Zini
From: Alessandro Zini <alessandro.zini@siemens.com>
JEDEC JEP106 maintains a list of manufacturers IDs, consisting in 7
bit of information plus 1 parity bit, for a total of 1 Byte. Since the
number of manufacturers is way larger than this, JEDEC additionally
defines the continuation code 0x7f to be used as a prefix to the
actual ID, subdividing IDs in different banks.
This commit handles such manufacturer bank code by introducing a new
mfr_bank field in flash_info struct. This field is intended to be
populated when specifying a manufacturer part, and for retro
compatibility it is assumed to be 1 if omitted.
Note that this assumption was already implicitly taken, as only a
couple of the already supported manufacturer parts have the
continuation code prefixed to the actual ID.
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.
Signed-off-by: Alessandro Zini <alessandro.zini@siemens.com>
---
drivers/mtd/spi-nor/core.c | 71 ++++++++++++++++++++++++++++++-----
drivers/mtd/spi-nor/core.h | 10 ++++-
drivers/mtd/spi-nor/debugfs.c | 2 +
drivers/mtd/spi-nor/sysfs.c | 13 +++++++
4 files changed, 86 insertions(+), 10 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index e0c4efc424f4..07d2627c5850 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -407,6 +407,32 @@ int spi_nor_write_disable(struct spi_nor *nor)
return ret;
}
+/**
+ * spi_nor_get_mfr_bank() - Get the JEDEC manufacturer bank number.
+ * @id: pointer to the buffer where the JEDEC ID (incl. eventual
+ * continuation codes) is stored.
+ * @bank: pointer to a buffer where the bank number will be written.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+int spi_nor_get_mfr_bank(u8 *id, u8 *bank)
+{
+ u8 tmp_bank;
+
+ tmp_bank = 0;
+ while (tmp_bank < SPI_NOR_MAX_ID_READ_LEN &&
+ id[tmp_bank] == SPI_NOR_CONTINUATION_CODE)
+ tmp_bank++;
+
+ /* Incomplete read of JEDEC id */
+ if (tmp_bank == SPI_NOR_MAX_ID_READ_LEN)
+ return -EIO;
+
+ *bank = tmp_bank;
+
+ return 0;
+}
+
/**
* spi_nor_read_id() - Read the JEDEC ID.
* @nor: pointer to 'struct spi_nor'.
@@ -415,7 +441,7 @@ int spi_nor_write_disable(struct spi_nor *nor)
* @ndummy: number of dummy bytes to send after an opcode or address. Can
* be zero if the operation does not require dummy bytes.
* @id: pointer to a DMA-able buffer where the value of the JEDEC ID
- * will be written.
+ * (incl. eventual continuation codes) will be written.
* @proto: the SPI protocol for register operation.
*
* Return: 0 on success, -errno otherwise.
@@ -427,13 +453,14 @@ int spi_nor_read_id(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
if (nor->spimem) {
struct spi_mem_op op =
- SPI_NOR_READID_OP(naddr, ndummy, id, SPI_NOR_MAX_ID_LEN);
+ SPI_NOR_READID_OP(naddr, ndummy, id,
+ SPI_NOR_MAX_ID_READ_LEN);
spi_nor_spimem_setup_op(nor, &op, proto);
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
ret = nor->controller_ops->read_reg(nor, SPINOR_OP_RDID, id,
- SPI_NOR_MAX_ID_LEN);
+ SPI_NOR_MAX_ID_READ_LEN);
}
return ret;
}
@@ -1984,7 +2011,8 @@ static const struct flash_info spi_nor_generic_flash = {
};
static const struct flash_info *spi_nor_match_id(struct spi_nor *nor,
- const u8 *id)
+ const u8 *id,
+ u8 bank)
{
const struct flash_info *part;
unsigned int i, j;
@@ -1992,8 +2020,22 @@ static const struct flash_info *spi_nor_match_id(struct spi_nor *nor,
for (i = 0; i < ARRAY_SIZE(manufacturers); i++) {
for (j = 0; j < manufacturers[i]->nparts; j++) {
part = &manufacturers[i]->parts[j];
+
if (part->id &&
- !memcmp(part->id->bytes, id, part->id->len)) {
+ !memcmp(part->id->bytes, id + bank, part->id->len)) {
+ /*
+ * Subtract default bank number since
+ * JEDEC does not count banks from 0
+ */
+ if (part->mfr_bank && part->mfr_bank -
+ bank != SPI_NOR_DEFAULT_MFR_BANK)
+ continue;
+
+ /*
+ * Either we found a matching bank or we
+ * implicitly assume the default bank is
+ * intended if not specified otherwise
+ */
nor->manufacturer = manufacturers[i];
return part;
}
@@ -2007,6 +2049,7 @@ static const struct flash_info *spi_nor_detect(struct spi_nor *nor)
{
const struct flash_info *info;
u8 *id = nor->bouncebuf;
+ u8 bank;
int ret;
ret = spi_nor_read_id(nor, 0, 0, id, nor->reg_proto);
@@ -2015,13 +2058,21 @@ static const struct flash_info *spi_nor_detect(struct spi_nor *nor)
return ERR_PTR(ret);
}
+ ret = spi_nor_get_mfr_bank(id, &bank);
+ if (ret) {
+ dev_dbg(nor->dev,
+ "error %d getting JEDEC manufacturer bank\n", ret);
+ return ERR_PTR(ret);
+ }
+
+ info = spi_nor_match_id(nor, id, bank);
+
/* Cache the complete flash ID. */
- nor->id = devm_kmemdup(nor->dev, id, SPI_NOR_MAX_ID_LEN, GFP_KERNEL);
+ nor->id = devm_kmemdup(nor->dev, id + bank,
+ SPI_NOR_MAX_ID_LEN, GFP_KERNEL);
if (!nor->id)
return ERR_PTR(-ENOMEM);
- info = spi_nor_match_id(nor, id);
-
/* Fallback to a generic flash described only by its SFDP data. */
if (!info) {
ret = spi_nor_check_sfdp_signature(nor);
@@ -3502,7 +3553,9 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
if (ret)
return ret;
- dev_dbg(dev, "Manufacturer and device ID: %*phN\n",
+ dev_dbg(dev, "Manufacturer bank: %u, device ID: %*phN\n",
+ nor->info->mfr_bank ?
+ nor->info->mfr_bank : SPI_NOR_DEFAULT_MFR_BANK,
SPI_NOR_MAX_ID_LEN, nor->id);
return 0;
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 1516b6d0dc37..5091d720eba6 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -9,7 +9,11 @@
#include "sfdp.h"
-#define SPI_NOR_MAX_ID_LEN 6
+#define SPI_NOR_MAX_ID_LEN 6
+/* Account for up to 128 banks */
+#define SPI_NOR_MAX_ID_READ_LEN (SPI_NOR_MAX_ID_LEN + 128)
+#define SPI_NOR_DEFAULT_MFR_BANK 1
+#define SPI_NOR_CONTINUATION_CODE 0x7f
/*
* 256 bytes is a sane default for most older flashes. Newer flashes will
* have the page size defined within their SFDP tables.
@@ -498,6 +502,8 @@ struct spi_nor_id {
* @mfr_flags: manufacturer private flags. Used in the manufacturer fixup
* hooks to differentiate support between flashes of the same
* manufacturer.
+ * @mfr_bank: (optional) manufacturer ID bank as per JEDEC JEP106.
+ * Defaults to 1.
* @otp_org: flash's OTP organization.
* @fixups: part specific fixup hooks.
*/
@@ -535,6 +541,7 @@ struct flash_info {
#define SPI_NOR_IO_MODE_EN_VOLATILE BIT(1)
u8 mfr_flags;
+ u8 mfr_bank;
const struct spi_nor_otp_organization *otp;
const struct spi_nor_fixups *fixups;
@@ -613,6 +620,7 @@ void spi_nor_unlock_and_unprep(struct spi_nor *nor);
int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor);
int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor);
int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor);
+int spi_nor_get_mfr_bank(u8 *id, u8 *bank);
int spi_nor_read_id(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
enum spi_nor_protocol reg_proto);
int spi_nor_read_sr(struct spi_nor *nor, u8 *sr);
diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c
index fa6956144d2e..035f0cf97111 100644
--- a/drivers/mtd/spi-nor/debugfs.c
+++ b/drivers/mtd/spi-nor/debugfs.c
@@ -85,6 +85,8 @@ static int spi_nor_params_show(struct seq_file *s, void *data)
seq_printf(s, "name\t\t%s\n", info->name);
seq_printf(s, "id\t\t%*ph\n", SPI_NOR_MAX_ID_LEN, nor->id);
+ seq_printf(s, "mfr bank\t%u\n",
+ info->mfr_bank ? info->mfr_bank : SPI_NOR_DEFAULT_MFR_BANK);
string_get_size(params->size, 1, STRING_UNITS_2, buf, sizeof(buf));
seq_printf(s, "size\t\t%s\n", buf);
seq_printf(s, "write size\t%u\n", params->writesize);
diff --git a/drivers/mtd/spi-nor/sysfs.c b/drivers/mtd/spi-nor/sysfs.c
index 96064e4babf0..4b8bda191c73 100644
--- a/drivers/mtd/spi-nor/sysfs.c
+++ b/drivers/mtd/spi-nor/sysfs.c
@@ -42,10 +42,23 @@ static ssize_t jedec_id_show(struct device *dev,
}
static DEVICE_ATTR_RO(jedec_id);
+static ssize_t jedec_mfr_bank_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct spi_device *spi = to_spi_device(dev);
+ struct spi_mem *spimem = spi_get_drvdata(spi);
+ struct spi_nor *nor = spi_mem_get_drvdata(spimem);
+
+ return sysfs_emit(buf, "%u\n", nor->info->mfr_bank ?
+ nor->info->mfr_bank : SPI_NOR_DEFAULT_MFR_BANK);
+}
+static DEVICE_ATTR_RO(jedec_mfr_bank);
+
static struct attribute *spi_nor_sysfs_entries[] = {
&dev_attr_manufacturer.attr,
&dev_attr_partname.attr,
&dev_attr_jedec_id.attr,
+ &dev_attr_jedec_mfr_bank.attr,
NULL
};
--
2.45.2
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] mtd: spi-nor: convert existing devices to use mfr_bank
2024-07-29 10:20 [PATCH 0/3] Add support for Cypress/Infineon cy15b102q A. Zini
2024-07-29 10:20 ` [PATCH 1/3] mtd: spi-nor: handle JEDEC manufacturer bank A. Zini
@ 2024-07-29 10:20 ` A. Zini
2024-07-29 10:20 ` [PATCH 3/3] mtd: spi-nor: add support for Cypress cy15b102q A. Zini
2024-08-05 9:08 ` [PATCH 0/3] Add support for Cypress/Infineon cy15b102q Michael Walle
3 siblings, 0 replies; 9+ messages in thread
From: A. Zini @ 2024-07-29 10:20 UTC (permalink / raw)
To: linux-mtd
Cc: Tudor Ambarus, Pratyush Yadav, Michael Walle, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Alessandro Zini
From: Alessandro Zini <alessandro.zini@siemens.com>
Convert existing devices to use the newly introduced manufacturer bank
information, instead of wrongly counting continuation codes as part of
the device ID.
This patch depends on patch #1.
Signed-off-by: Alessandro Zini <alessandro.zini@siemens.com>
---
drivers/mtd/spi-nor/issi.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
index 18d9a00aa22e..4966580fc0dd 100644
--- a/drivers/mtd/spi-nor/issi.c
+++ b/drivers/mtd/spi-nor/issi.c
@@ -60,16 +60,18 @@ static const struct flash_info issi_nor_parts[] = {
.no_sfdp_flags = SECT_4K,
.fixups = &pm25lv_nor_fixups
}, {
- .id = SNOR_ID(0x7f, 0x9d, 0x20),
+ .id = SNOR_ID(0x9d, 0x20),
.name = "is25cd512",
.sector_size = SZ_32K,
.size = SZ_64K,
.no_sfdp_flags = SECT_4K,
+ .mfr_bank = 2,
}, {
- .id = SNOR_ID(0x7f, 0x9d, 0x46),
+ .id = SNOR_ID(0x9d, 0x46),
.name = "pm25lq032",
.size = SZ_4M,
.no_sfdp_flags = SECT_4K,
+ .mfr_bank = 2,
}, {
.id = SNOR_ID(0x9d, 0x40, 0x13),
.name = "is25lq040b",
--
2.45.2
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] mtd: spi-nor: add support for Cypress cy15b102q
2024-07-29 10:20 [PATCH 0/3] Add support for Cypress/Infineon cy15b102q A. Zini
2024-07-29 10:20 ` [PATCH 1/3] mtd: spi-nor: handle JEDEC manufacturer bank A. Zini
2024-07-29 10:20 ` [PATCH 2/3] mtd: spi-nor: convert existing devices to use mfr_bank A. Zini
@ 2024-07-29 10:20 ` A. Zini
2024-08-05 9:08 ` [PATCH 0/3] Add support for Cypress/Infineon cy15b102q Michael Walle
3 siblings, 0 replies; 9+ messages in thread
From: A. Zini @ 2024-07-29 10:20 UTC (permalink / raw)
To: linux-mtd
Cc: Tudor Ambarus, Pratyush Yadav, Michael Walle, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Alessandro Zini
From: Alessandro Zini <alessandro.zini@siemens.com>
The Cypress cy15b102q is a 2Mbit serial SPI F-RAM device.
Add support for it to the spi-nor driver.
As per datasheeet, manufacturer id is 0xC2, which from the official
JEDEC JEP106 corresponds to Ramtron. Ramtron has been later acquired by
Cypress, which in turn merged with Infineon. Prior to this acquisition,
Cypress merged with Spansion. Given this history, add the device to
spansion.c, as it already includes other products part of the Infineon
family.
The JEDEC-assigned manufacturer ID places the Cypress (Ramtron)
identifier in bank 7.
This patch depends on patch #1.
Signed-off-by: Alessandro Zini <alessandro.zini@siemens.com>
---
drivers/mtd/spi-nor/spansion.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index 6cc237c24e07..9687dfb62a9b 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -981,6 +981,13 @@ static const struct flash_info spansion_nor_parts[] = {
.name = "s28hs02gt",
.mfr_flags = USE_CLPEF,
.fixups = &s28hx_t_fixups,
+ }, {
+ .id = SNOR_ID(0xc2, 0x25, 0xc8),
+ .name = "cy15b102q",
+ .size = SZ_256K,
+ .sector_size = SZ_256K,
+ .flags = SPI_NOR_NO_ERASE,
+ .mfr_bank = 7,
}, {
.id = SNOR_ID(0xef, 0x40, 0x13),
.name = "s25fl004k",
--
2.45.2
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] Add support for Cypress/Infineon cy15b102q
2024-07-29 10:20 [PATCH 0/3] Add support for Cypress/Infineon cy15b102q A. Zini
` (2 preceding siblings ...)
2024-07-29 10:20 ` [PATCH 3/3] mtd: spi-nor: add support for Cypress cy15b102q A. Zini
@ 2024-08-05 9:08 ` Michael Walle
2024-08-07 12:31 ` A. Zini
3 siblings, 1 reply; 9+ messages in thread
From: Michael Walle @ 2024-08-05 9:08 UTC (permalink / raw)
To: A. Zini, linux-mtd
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra
[-- Attachment #1.1: Type: text/plain, Size: 229 bytes --]
Hi,
> This patch series introduces support for Cypress/Infineon cy15b102q, a 2Mbit
> serial SPI F-RAM device.
Have you had a look at drivers/misc/eeprom/at25.c? There is already
support for the cypress,fm25.
-michael
[-- 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/
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] mtd: spi-nor: handle JEDEC manufacturer bank
2024-07-29 10:20 ` [PATCH 1/3] mtd: spi-nor: handle JEDEC manufacturer bank A. Zini
@ 2024-08-05 9:18 ` Michael Walle
2024-08-07 13:37 ` A. Zini
0 siblings, 1 reply; 9+ messages in thread
From: Michael Walle @ 2024-08-05 9:18 UTC (permalink / raw)
To: A. Zini, linux-mtd
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra
[-- Attachment #1.1: Type: text/plain, Size: 1660 bytes --]
Hi,
> JEDEC JEP106 maintains a list of manufacturers IDs, consisting in 7
> bit of information plus 1 parity bit, for a total of 1 Byte. Since the
> number of manufacturers is way larger than this, JEDEC additionally
> defines the continuation code 0x7f to be used as a prefix to the
> actual ID, subdividing IDs in different banks.
>
> This commit handles such manufacturer bank code by introducing a new
> mfr_bank field in flash_info struct. This field is intended to be
> populated when specifying a manufacturer part, and for retro
> compatibility it is assumed to be 1 if omitted.
> Note that this assumption was already implicitly taken, as only a
> couple of the already supported manufacturer parts have the
> continuation code prefixed to the actual ID.
>
> 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.
Quick remarks without having looked deeper into this patch.
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'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.
-michael
[-- 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/
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] Add support for Cypress/Infineon cy15b102q
2024-08-05 9:08 ` [PATCH 0/3] Add support for Cypress/Infineon cy15b102q Michael Walle
@ 2024-08-07 12:31 ` A. Zini
0 siblings, 0 replies; 9+ messages in thread
From: A. Zini @ 2024-08-07 12:31 UTC (permalink / raw)
To: mwalle, linux-mtd
Cc: alessandro.zini, miquel.raynal, pratyush, richard, tudor.ambarus,
vigneshr
> Have you had a look at drivers/misc/eeprom/at25.c? There is already
> support for the cypress,fm25.
This is a very good suggestion, thanks. I missed it.
This patch series can be dropped, although I'd continue the first
patch's discussion on how to handle the manufacturer bank.
--
Alessandro
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] mtd: spi-nor: handle JEDEC manufacturer bank
2024-08-05 9:18 ` Michael Walle
@ 2024-08-07 13:37 ` A. Zini
2024-08-08 8:38 ` Michael Walle
0 siblings, 1 reply; 9+ messages in thread
From: A. Zini @ 2024-08-07 13:37 UTC (permalink / raw)
To: mwalle, linux-mtd
Cc: alessandro.zini, miquel.raynal, pratyush, richard, tudor.ambarus,
vigneshr
> > 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"?
> 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.
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
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] mtd: spi-nor: handle JEDEC manufacturer bank
2024-08-07 13:37 ` A. Zini
@ 2024-08-08 8:38 ` Michael Walle
0 siblings, 0 replies; 9+ messages in thread
From: Michael Walle @ 2024-08-08 8:38 UTC (permalink / raw)
To: A. Zini, linux-mtd
Cc: miquel.raynal, pratyush, richard, tudor.ambarus, vigneshr
[-- 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/
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-08-08 8:38 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-29 10:20 [PATCH 0/3] Add support for Cypress/Infineon cy15b102q A. Zini
2024-07-29 10:20 ` [PATCH 1/3] mtd: spi-nor: handle JEDEC manufacturer bank A. Zini
2024-08-05 9:18 ` Michael Walle
2024-08-07 13:37 ` A. Zini
2024-08-08 8:38 ` Michael Walle
2024-07-29 10:20 ` [PATCH 2/3] mtd: spi-nor: convert existing devices to use mfr_bank A. Zini
2024-07-29 10:20 ` [PATCH 3/3] mtd: spi-nor: add support for Cypress cy15b102q A. Zini
2024-08-05 9:08 ` [PATCH 0/3] Add support for Cypress/Infineon cy15b102q Michael Walle
2024-08-07 12:31 ` A. Zini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).