[RFC PATCH 0/2] eeprom: at25: support Cypress FRAMs without device ID

[RFC PATCH 0/2] eeprom: at25: support Cypress FRAMs without device ID

[Markus Heidelberg]

Hello,

this patch series is marked as RFC because I'm unsure if it
should rather be implemented with an adaption in this binding:

  Documentation/devicetree/bindings/eeprom/at25.yaml

Currently supported FRAMs use compatible="cypress,fm25","atmel,at25" in
Devicetree, the memory size is read from its device ID.
For FRAMs without device ID this is not possible, so the "size" property
has to be set manually as it is done for EEPROMs.

I had a few solutions for implementation in mind, but opted for the
simplest one as base for discussion:

- Use the existing "compatible" string and additionally set "size". Only
  read the device ID if "size" is not set.

  But this way there is already the problem that "size" is required for
  FRAMs without device ID, but I cannot specify that in the binding
  because of the reused "compatible" string.

Other ideas that came to mind:

- Add "cypress,fm25l16b" (chip is named FM25L16B) and define "size" as
  required property. Use that instead of "cypress,fm25".

  According to Documentation/devicetree/bindings/writing-bindings.rst
  this might even be necessary regarding this statement:

    "DO add new compatibles in case there are new features or bugs."

  The existing "cypress,fm25" ("FM25" is not the real name of a chip,
  but the common prefix) also doesn't seem chosen right regarding this
  statement:

    "DO make ‘compatible’ properties specific. DON'T use wildcards in
    compatible strings."

- Add a boolean property "no-device-id" to the existing "compatible"
  string and in case this boolean is set, define "size" as required.

  This seems a bit awkward at first sight. Also, would this really solve
  the above mentioned problem with specification of the binding?

Bye!

Markus Heidelberg (2):
  eeprom: at25: support Cypress FRAMs without device ID
  eeprom: at25: make FRAM device ID error message more precise

 drivers/misc/eeprom/at25.c | 42 ++++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 18 deletions(-)

-- 
2.43.0

[RFC PATCH 1/2] eeprom: at25: support Cypress FRAMs without device ID

[Markus Heidelberg]

Not all FRAM chips have a device ID and implement the corresponding read
command. For such chips this led to the following error on module
loading:

    at25 spi2.0: Error: no Cypress FRAM (id 00)

The device ID contains the memory size, so devices without this ID are
supported now by setting the size manually in Devicetree using the
"size" property.

Tested with FM25L16B.

According to Infineon/Cypress datasheets, these FRAMs have a device ID:

    FM25V01A
    FM25V02A
    FM25V05
    FM25V10
    FM25V20A
    FM25VN10

but these do not:

    FM25040B
    FM25640B
    FM25C160B
    FM25CL64B
    FM25L04B
    FM25L16B
    FM25W256

So all "FM25V*" FRAMs and only these have a device ID. The letter after
"FM25" (V/C/L/W) only describes the voltage range, though.

Signed-off-by: Markus Heidelberg <m.heidelberg@cab.de>
---
 drivers/misc/eeprom/at25.c | 42 ++++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 18 deletions(-)

diff --git a/drivers/misc/eeprom/at25.c b/drivers/misc/eeprom/at25.c
index 595ceb9a7126..16ce36c02ed4 100644
--- a/drivers/misc/eeprom/at25.c
+++ b/drivers/misc/eeprom/at25.c
@@ -382,35 +382,41 @@ static int at25_fram_to_chip(struct device *dev, struct spi_eeprom *chip)
 	struct at25_data *at25 = container_of(chip, struct at25_data, chip);
 	u8 sernum[FM25_SN_LEN];
 	u8 id[FM25_ID_LEN];
+	u32 val;
 	int i;
 
 	strscpy(chip->name, "fm25", sizeof(chip->name));
 
-	/* Get ID of chip */
-	fm25_aux_read(at25, id, FM25_RDID, FM25_ID_LEN);
-	if (id[6] != 0xc2) {
-		dev_err(dev, "Error: no Cypress FRAM (id %02x)\n", id[6]);
-		return -ENODEV;
-	}
-	/* Set size found in ID */
-	if (id[7] < 0x21 || id[7] > 0x26) {
-		dev_err(dev, "Error: unsupported size (id %02x)\n", id[7]);
-		return -ENODEV;
+	if (!device_property_read_u32(dev, "size", &val)) {
+		chip->byte_len = val;
+	} else {
+		/* Get ID of chip */
+		fm25_aux_read(at25, id, FM25_RDID, FM25_ID_LEN);
+		if (id[6] != 0xc2) {
+			dev_err(dev, "Error: no Cypress FRAM (id %02x)\n", id[6]);
+			return -ENODEV;
+		}
+		/* Set size found in ID */
+		if (id[7] < 0x21 || id[7] > 0x26) {
+			dev_err(dev, "Error: unsupported size (id %02x)\n", id[7]);
+			return -ENODEV;
+		}
+
+		chip->byte_len = BIT(id[7] - 0x21 + 4) * 1024;
+
+		if (id[8]) {
+			fm25_aux_read(at25, sernum, FM25_RDSN, FM25_SN_LEN);
+			/* Swap byte order */
+			for (i = 0; i < FM25_SN_LEN; i++)
+				at25->sernum[i] = sernum[FM25_SN_LEN - 1 - i];
+		}
 	}
 
-	chip->byte_len = BIT(id[7] - 0x21 + 4) * 1024;
 	if (chip->byte_len > 64 * 1024)
 		chip->flags |= EE_ADDR3;
 	else
 		chip->flags |= EE_ADDR2;
 
-	if (id[8]) {
-		fm25_aux_read(at25, sernum, FM25_RDSN, FM25_SN_LEN);
-		/* Swap byte order */
-		for (i = 0; i < FM25_SN_LEN; i++)
-			at25->sernum[i] = sernum[FM25_SN_LEN - 1 - i];
-	}
-
 	chip->page_size = PAGE_SIZE;
 	return 0;
 }
-- 
2.43.0

[RFC PATCH 2/2] eeprom: at25: make FRAM device ID error message more precise

[Markus Heidelberg]

The error description would be wrong in case the "size" Devicetree
property is missing for an FRAM without device ID.

Signed-off-by: Markus Heidelberg <m.heidelberg@cab.de>
---
 drivers/misc/eeprom/at25.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/eeprom/at25.c b/drivers/misc/eeprom/at25.c
index 16ce36c02ed4..c45413e06e9c 100644
--- a/drivers/misc/eeprom/at25.c
+++ b/drivers/misc/eeprom/at25.c
@@ -393,7 +393,7 @@ static int at25_fram_to_chip(struct device *dev, struct spi_eeprom *chip)
 		/* Get ID of chip */
 		fm25_aux_read(at25, id, FM25_RDID, FM25_ID_LEN);
 		if (id[6] != 0xc2) {
-			dev_err(dev, "Error: no Cypress FRAM (id %02x)\n", id[6]);
+			dev_err(dev, "Error: no Cypress FRAM with device ID (manufacturer ID bank 7: %02x)\n", id[6]);
 			return -ENODEV;
 		}
 		/* Set size found in ID */
-- 
2.43.0

Re: [RFC PATCH 0/2] eeprom: at25: support Cypress FRAMs without device ID

[Christian Eggers]

Hi Markus,

I use the following FRAM device: Fujitsu mb85rs1mt. 

This FRAM is also not able to report its size (at least I didn't
try). I can use this FRAM with the following (Eeeprom) settings:

		compatible = "fujitsu,mb85rs1mt", "atmel,at25";
		reg = <0>;
		spi-max-frequency = <30000000>;
		/* mode0, uncomment for mode3 */
		/*spi-cpha;
		spi-cpol;*/

		/* from the datasheet it seems that there is no page size for FRAM */
		pagesize = <131072>;
		size = <131072>;
		address-width = <24>;

Is this what you are looking for? Of course, the "type" attribute 
reports "EEPROM" with this configuration, but my application don't care
about this.

regards,
Christian

On Tuesday, 1 April 2025, 15:30:46 CEST, Markus Heidelberg wrote:
> Hello,
> 
> this patch series is marked as RFC because I'm unsure if it
> should rather be implemented with an adaption in this binding:
> 
>   Documentation/devicetree/bindings/eeprom/at25.yaml
> 
> Currently supported FRAMs use compatible="cypress,fm25","atmel,at25" in
> Devicetree, the memory size is read from its device ID.
> For FRAMs without device ID this is not possible, so the "size" property
> has to be set manually as it is done for EEPROMs.
> 
> I had a few solutions for implementation in mind, but opted for the
> simplest one as base for discussion:
> 
> - Use the existing "compatible" string and additionally set "size". Only
>   read the device ID if "size" is not set.
> 
>   But this way there is already the problem that "size" is required for
>   FRAMs without device ID, but I cannot specify that in the binding
>   because of the reused "compatible" string.
> 
> Other ideas that came to mind:
> 
> - Add "cypress,fm25l16b" (chip is named FM25L16B) and define "size" as
>   required property. Use that instead of "cypress,fm25".
> 
>   According to Documentation/devicetree/bindings/writing-bindings.rst
>   this might even be necessary regarding this statement:
> 
>     "DO add new compatibles in case there are new features or bugs."
> 
>   The existing "cypress,fm25" ("FM25" is not the real name of a chip,
>   but the common prefix) also doesn't seem chosen right regarding this
>   statement:
> 
>     "DO make ‘compatible’ properties specific. DON'T use wildcards in
>     compatible strings."
> 
> - Add a boolean property "no-device-id" to the existing "compatible"
>   string and in case this boolean is set, define "size" as required.
> 
>   This seems a bit awkward at first sight. Also, would this really solve
>   the above mentioned problem with specification of the binding?
> 
> Bye!
> 
> Markus Heidelberg (2):
>   eeprom: at25: support Cypress FRAMs without device ID
>   eeprom: at25: make FRAM device ID error message more precise
> 
>  drivers/misc/eeprom/at25.c | 42 ++++++++++++++++++++++----------------
>  1 file changed, 24 insertions(+), 18 deletions(-)
> 
> 

Re: [RFC PATCH 0/2] eeprom: at25: support Cypress FRAMs without device ID

[Markus Heidelberg]

Hi Christian,

On Tue, Apr 01, 2025 at 03:45:14PM +0200, Christian Eggers wrote:
> 
> I use the following FRAM device: Fujitsu mb85rs1mt. 
> 
> This FRAM is also not able to report its size (at least I didn't
> try).

According to the datasheet there is a device ID, but with a different
response compared to Cypress FRAMs. It wouldn't work without modifying
the current implementation.

> I can use this FRAM with the following (Eeeprom) settings:
> 
> 		compatible = "fujitsu,mb85rs1mt", "atmel,at25";
> 		reg = <0>;
> 		spi-max-frequency = <30000000>;
> 		/* mode0, uncomment for mode3 */
> 		/*spi-cpha;
> 		spi-cpol;*/
> 
> 		/* from the datasheet it seems that there is no page size for FRAM */
> 		pagesize = <131072>;
> 		size = <131072>;
> 		address-width = <24>;
> 
> Is this what you are looking for? Of course, the "type" attribute 
> reports "EEPROM" with this configuration, but my application don't care
> about this.

This is what I started with, but I thought there has to be a reason that
EEPROM and FRAM are distinguished in the driver (at25 and nvmem core)
and I wanted to do it right. If not relevant now, maybe in the future.

Markus

Re: [RFC PATCH 0/2] eeprom: at25: support Cypress FRAMs without device ID

[Christian Eggers]

Hi Markus,

<disclaimer>I'm not maintaining the AT25 driver</disclaimer>

On Wednesday, 2 April 2025, 09:48:52 CEST, Markus Heidelberg wrote:
> Hi Christian,
> 
> On Tue, Apr 01, 2025 at 03:45:14PM +0200, Christian Eggers wrote:
> > 
> > I use the following FRAM device: Fujitsu mb85rs1mt. 
> > 
> > This FRAM is also not able to report its size (at least I didn't
> > try).
> 
> According to the datasheet there is a device ID, but with a different
> response compared to Cypress FRAMs. It wouldn't work without modifying
> the current implementation.
> 
> > I can use this FRAM with the following (Eeeprom) settings:
> > 
> > 		compatible = "fujitsu,mb85rs1mt", "atmel,at25";
> > 		reg = <0>;
> > 		spi-max-frequency = <30000000>;
> > 		/* mode0, uncomment for mode3 */
> > 		/*spi-cpha;
> > 		spi-cpol;*/
> > 
> > 		/* from the datasheet it seems that there is no page size for FRAM */
> > 		pagesize = <131072>;
> > 		size = <131072>;
> > 		address-width = <24>;
> > 
> > Is this what you are looking for? Of course, the "type" attribute 
> > reports "EEPROM" with this configuration, but my application don't care
> > about this.
> 
> This is what I started with, but I thought there has to be a reason that
> EEPROM and FRAM are distinguished in the driver (at25 and nvmem core)
> and I wanted to do it right. If not relevant now, maybe in the future.

maybe the "EEPROM" protocol used by at24 (I2C) and at25 (SPI) EEPROMs is
not smart enough to provide really useful detection of device capabilities.
At least I remember that I2C eeproms of different sizes require a different
number of bytes for addressing. AFAIK, using a wrong number of addressing bytes
may accidentally overwrite data on the device. If this is the same for SPI
eeproms / FRAMs, reliable auto-detection may be impossible or require
at least knowing the vendor in advance.

Flash (MTD) devices provide much more powerful methods for enumerating the
device's geometry/capabilities than eeprom/fram. But even for ONFI there are
extra tables for vendor/device specific workarounds. I am not sure whether
adding such stuff for at24/at25 devices is really worth the trouble...

> 
> Markus
> 

regards,
Christian

Re: [RFC PATCH 0/2] eeprom: at25: support Cypress FRAMs without device ID

[Markus Heidelberg]

On Wed, Apr 02, 2025 at 12:49:24PM +0200, Christian Eggers wrote:
> maybe the "EEPROM" protocol used by at24 (I2C) and at25 (SPI) EEPROMs is
> not smart enough to provide really useful detection of device capabilities.
> At least I remember that I2C eeproms of different sizes require a different
> number of bytes for addressing. AFAIK, using a wrong number of addressing bytes
> may accidentally overwrite data on the device. If this is the same for SPI
> eeproms / FRAMs, reliable auto-detection may be impossible or require
> at least knowing the vendor in advance.

The "read device ID" command works without address, so it can be used to
determine the memory size and thus the address length.

If the response to this command is similar for various devices/vendors
(in consideration of the variable length manufacturer ID using the 0x7F
continuation code), auto-detection should be possible without having to
know the manufacturer in advance and without having to interpret it from
the read ID.

But the maximum possible response length increases by one byte with each
new manufacturer bank of up to 126 manufacturers added to the JEDEC ID
list.

> Flash (MTD) devices provide much more powerful methods for enumerating the
> device's geometry/capabilities than eeprom/fram. But even for ONFI there are
> extra tables for vendor/device specific workarounds. I am not sure whether
> adding such stuff for at24/at25 devices is really worth the trouble...

I feel the same that this wouldn't be worth it, but I guess it's
avoidable if further auto-detection should needed by someone.

Markus