[PATCH v8 0/5] add support for FRAM

[PATCH v8 0/5] add support for FRAM

[Jiri Prchal]

Adds support for Cypress FRAMs.

Jiri Prchal (5):
  nvmem: prepare basics for FRAM support
  nvmem: eeprom: at25: add support for FRAM
  dt-bindings: nvmem: at25: add for FRAM support
  nvmem: eeprom: at25: export FRAM serial num
  nvmem: eeprom: add documentation of sysfs fram and sernum file

 .../ABI/testing/sysfs-class-spi-eeprom        |  13 ++
 .../devicetree/bindings/eeprom/at25.yaml      |  31 +++-
 drivers/misc/eeprom/Kconfig                   |   5 +-
 drivers/misc/eeprom/at25.c                    | 160 ++++++++++++++----
 drivers/nvmem/core.c                          |   4 +
 include/linux/nvmem-provider.h                |   1 +
 6 files changed, 176 insertions(+), 38 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-spi-eeprom

-- 
2.25.1

[PATCH v8 1/5] nvmem: prepare basics for FRAM support

[Jiri Prchal]

Added enum and string for FRAM to expose it as "fram".

Signed-off-by: Jiri Prchal <jiri.prchal@aksignal.cz>
---
v2: no change here
v3: resend and added more recipients
v4: resend
v5: no change here
v6: changed commit subject
v7: no change here
v8: changed subject
---
 drivers/nvmem/core.c           | 4 ++++
 include/linux/nvmem-provider.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 177f5bf27c6d..01ef9a934b0a 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -180,6 +180,7 @@ static const char * const nvmem_type_str[] = {
 	[NVMEM_TYPE_EEPROM] = "EEPROM",
 	[NVMEM_TYPE_OTP] = "OTP",
 	[NVMEM_TYPE_BATTERY_BACKED] = "Battery backed",
+	[NVMEM_TYPE_FRAM] = "FRAM",
 };
 
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
@@ -359,6 +360,9 @@ static int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
 	if (!config->base_dev)
 		return -EINVAL;
 
+	if (config->type == NVMEM_TYPE_FRAM)
+		bin_attr_nvmem_eeprom_compat.attr.name = "fram";
+
 	nvmem->eeprom = bin_attr_nvmem_eeprom_compat;
 	nvmem->eeprom.attr.mode = nvmem_bin_attr_get_umode(nvmem);
 	nvmem->eeprom.size = nvmem->size;
diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
index e162b757b6d5..890003565761 100644
--- a/include/linux/nvmem-provider.h
+++ b/include/linux/nvmem-provider.h
@@ -25,6 +25,7 @@ enum nvmem_type {
 	NVMEM_TYPE_EEPROM,
 	NVMEM_TYPE_OTP,
 	NVMEM_TYPE_BATTERY_BACKED,
+	NVMEM_TYPE_FRAM,
 };
 
 #define NVMEM_DEVID_NONE	(-1)
-- 
2.25.1

[PATCH v8 2/5] nvmem: eeprom: at25: add support for FRAM

[Jiri Prchal]

Added support for Cypress FRAMs.
These frams have ID and some of them have serial number too.
Size of them is recognized by ID. They don't have pages, it could
be read or written at once, but it's limited in this driver to
io limit 4096.

Signed-off-by: Jiri Prchal <jiri.prchal@aksignal.cz>
---
v2: fixed warning at %zd at int
Reported-by: kernel test robot <lkp@intel.com>
v3: resend and added more recipients
v4: resend
v5: used in-kernel function int_pow
v6: no change here
v7: moved definition of sernum size to patch 4
v8: changed buffer type to u8
---
 drivers/misc/eeprom/Kconfig |   5 +-
 drivers/misc/eeprom/at25.c  | 140 ++++++++++++++++++++++++++++--------
 2 files changed, 113 insertions(+), 32 deletions(-)

diff --git a/drivers/misc/eeprom/Kconfig b/drivers/misc/eeprom/Kconfig
index 0f791bfdc1f5..f0a7531f354c 100644
--- a/drivers/misc/eeprom/Kconfig
+++ b/drivers/misc/eeprom/Kconfig
@@ -32,12 +32,13 @@ config EEPROM_AT24
 	  will be called at24.
 
 config EEPROM_AT25
-	tristate "SPI EEPROMs from most vendors"
+	tristate "SPI EEPROMs (FRAMs) from most vendors"
 	depends on SPI && SYSFS
 	select NVMEM
 	select NVMEM_SYSFS
 	help
-	  Enable this driver to get read/write support to most SPI EEPROMs,
+	  Enable this driver to get read/write support to most SPI EEPROMs
+	  and Cypress FRAMs,
 	  after you configure the board init code to know about each eeprom
 	  on your target board.
 
diff --git a/drivers/misc/eeprom/at25.c b/drivers/misc/eeprom/at25.c
index b76e4901b4a4..e25cec7c474b 100644
--- a/drivers/misc/eeprom/at25.c
+++ b/drivers/misc/eeprom/at25.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * at25.c -- support most SPI EEPROMs, such as Atmel AT25 models
+ *	     and Cypress FRAMs FM25 models
  *
  * Copyright (C) 2006 David Brownell
  */
@@ -16,6 +17,9 @@
 #include <linux/spi/spi.h>
 #include <linux/spi/eeprom.h>
 #include <linux/property.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/math.h>
 
 /*
  * NOTE: this is an *EEPROM* driver.  The vagaries of product naming
@@ -34,6 +38,7 @@ struct at25_data {
 	unsigned		addrlen;
 	struct nvmem_config	nvmem_config;
 	struct nvmem_device	*nvmem;
+	int has_sernum;
 };
 
 #define	AT25_WREN	0x06		/* latch the write enable */
@@ -42,6 +47,9 @@ struct at25_data {
 #define	AT25_WRSR	0x01		/* write status register */
 #define	AT25_READ	0x03		/* read byte(s) */
 #define	AT25_WRITE	0x02		/* write byte(s)/sector */
+#define	FM25_SLEEP	0xb9		/* enter sleep mode */
+#define	FM25_RDID	0x9f		/* read device ID */
+#define	FM25_RDSN	0xc3		/* read S/N */
 
 #define	AT25_SR_nRDY	0x01		/* nRDY = write-in-progress */
 #define	AT25_SR_WEN	0x02		/* write enable (latched) */
@@ -51,6 +59,8 @@ struct at25_data {
 
 #define	AT25_INSTR_BIT3	0x08		/* Additional address bit in instr */
 
+#define	FM25_ID_LEN	9		/* ID length */
+
 #define EE_MAXADDRLEN	3		/* 24 bit addresses, up to 2 MBytes */
 
 /* Specs often allow 5 msec for a page write, sometimes 20 msec;
@@ -58,6 +68,9 @@ struct at25_data {
  */
 #define	EE_TIMEOUT	25
 
+#define	IS_EEPROM	0
+#define	IS_FRAM		1
+
 /*-------------------------------------------------------------------------*/
 
 #define	io_limit	PAGE_SIZE	/* bytes */
@@ -129,6 +142,36 @@ static int at25_ee_read(void *priv, unsigned int offset,
 	return status;
 }
 
+/*
+ * read extra registers as ID or serial number
+ */
+static int fm25_aux_read(struct at25_data *at25, u8 *buf, uint8_t command,
+			 int len)
+{
+	int status;
+	struct spi_transfer t[2];
+	struct spi_message m;
+
+	spi_message_init(&m);
+	memset(t, 0, sizeof(t));
+
+	t[0].tx_buf = &command;
+	t[0].len = 1;
+	spi_message_add_tail(&t[0], &m);
+
+	t[1].rx_buf = buf;
+	t[1].len = len;
+	spi_message_add_tail(&t[1], &m);
+
+	mutex_lock(&at25->lock);
+
+	status = spi_sync(at25->spi, &m);
+	dev_dbg(&at25->spi->dev, "read %d aux bytes --> %d\n", len, status);
+
+	mutex_unlock(&at25->lock);
+	return status;
+}
+
 static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
 {
 	struct at25_data *at25 = priv;
@@ -303,34 +346,37 @@ static int at25_fw_to_chip(struct device *dev, struct spi_eeprom *chip)
 	return 0;
 }
 
+static const struct of_device_id at25_of_match[] = {
+	{ .compatible = "atmel,at25", .data = (const void *)IS_EEPROM },
+	{ .compatible = "cypress,fm25", .data = (const void *)IS_FRAM },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, at25_of_match);
+
 static int at25_probe(struct spi_device *spi)
 {
 	struct at25_data	*at25 = NULL;
 	struct spi_eeprom	chip;
 	int			err;
 	int			sr;
-	int			addrlen;
+	u8 id[FM25_ID_LEN];
+	const struct of_device_id *match;
+	int is_fram = 0;
+
+	match = of_match_device(of_match_ptr(at25_of_match), &spi->dev);
+	if (match)
+		is_fram = (int)(uintptr_t)match->data;
 
 	/* Chip description */
 	if (!spi->dev.platform_data) {
-		err = at25_fw_to_chip(&spi->dev, &chip);
-		if (err)
-			return err;
+		if (!is_fram) {
+			err = at25_fw_to_chip(&spi->dev, &chip);
+			if (err)
+				return err;
+		}
 	} else
 		chip = *(struct spi_eeprom *)spi->dev.platform_data;
 
-	/* For now we only support 8/16/24 bit addressing */
-	if (chip.flags & EE_ADDR1)
-		addrlen = 1;
-	else if (chip.flags & EE_ADDR2)
-		addrlen = 2;
-	else if (chip.flags & EE_ADDR3)
-		addrlen = 3;
-	else {
-		dev_dbg(&spi->dev, "unsupported address type\n");
-		return -EINVAL;
-	}
-
 	/* Ping the chip ... the status register is pretty portable,
 	 * unlike probing manufacturer IDs.  We do expect that system
 	 * firmware didn't write it in the past few milliseconds!
@@ -349,9 +395,49 @@ static int at25_probe(struct spi_device *spi)
 	at25->chip = chip;
 	at25->spi = spi;
 	spi_set_drvdata(spi, at25);
-	at25->addrlen = addrlen;
 
-	at25->nvmem_config.type = NVMEM_TYPE_EEPROM;
+	if (is_fram) {
+		/* Get ID of chip */
+		fm25_aux_read(at25, id, FM25_RDID, FM25_ID_LEN);
+		if (id[6] != 0xc2) {
+			dev_err(&spi->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(&spi->dev, "Error: unsupported size (id %02x)\n", id[7]);
+			return -ENODEV;
+		}
+		chip.byte_len = int_pow(2, id[7] - 0x21 + 4) * 1024;
+
+		if (at25->chip.byte_len > 64 * 1024)
+			at25->chip.flags |= EE_ADDR3;
+		else
+			at25->chip.flags |= EE_ADDR2;
+
+		if (id[8])
+			at25->has_sernum = 1;
+		else
+			at25->has_sernum = 0;
+
+		at25->chip.page_size = PAGE_SIZE;
+		strncpy(at25->chip.name, "fm25", sizeof(at25->chip.name));
+	}
+
+	/* For now we only support 8/16/24 bit addressing */
+	if (at25->chip.flags & EE_ADDR1)
+		at25->addrlen = 1;
+	else if (at25->chip.flags & EE_ADDR2)
+		at25->addrlen = 2;
+	else if (at25->chip.flags & EE_ADDR3)
+		at25->addrlen = 3;
+	else {
+		dev_dbg(&spi->dev, "unsupported address type\n");
+		return -EINVAL;
+	}
+
+	at25->nvmem_config.type = is_fram ? NVMEM_TYPE_FRAM : NVMEM_TYPE_EEPROM;
 	at25->nvmem_config.name = dev_name(&spi->dev);
 	at25->nvmem_config.dev = &spi->dev;
 	at25->nvmem_config.read_only = chip.flags & EE_READONLY;
@@ -370,23 +456,17 @@ static int at25_probe(struct spi_device *spi)
 	if (IS_ERR(at25->nvmem))
 		return PTR_ERR(at25->nvmem);
 
-	dev_info(&spi->dev, "%d %s %s eeprom%s, pagesize %u\n",
-		(chip.byte_len < 1024) ? chip.byte_len : (chip.byte_len / 1024),
-		(chip.byte_len < 1024) ? "Byte" : "KByte",
-		at25->chip.name,
-		(chip.flags & EE_READONLY) ? " (readonly)" : "",
-		at25->chip.page_size);
+	dev_info(&spi->dev, "%d %s %s %s%s, pagesize %u\n",
+		 (chip.byte_len < 1024) ? chip.byte_len : (chip.byte_len / 1024),
+		 (chip.byte_len < 1024) ? "Byte" : "KByte",
+		 at25->chip.name, is_fram ? "fram" : "eeprom",
+		 (chip.flags & EE_READONLY) ? " (readonly)" : "",
+		 at25->chip.page_size);
 	return 0;
 }
 
 /*-------------------------------------------------------------------------*/
 
-static const struct of_device_id at25_of_match[] = {
-	{ .compatible = "atmel,at25", },
-	{ }
-};
-MODULE_DEVICE_TABLE(of, at25_of_match);
-
 static struct spi_driver at25_driver = {
 	.driver = {
 		.name		= "at25",
-- 
2.25.1

Re: [PATCH v8 2/5] nvmem: eeprom: at25: add support for FRAM

[Greg Kroah-Hartman]

On Mon, Jun 07, 2021 at 06:11:58PM +0200, Jiri Prchal wrote:
> Added support for Cypress FRAMs.
> These frams have ID and some of them have serial number too.
> Size of them is recognized by ID. They don't have pages, it could
> be read or written at once, but it's limited in this driver to
> io limit 4096.
> 
> Signed-off-by: Jiri Prchal <jiri.prchal@aksignal.cz>
> ---
> v2: fixed warning at %zd at int
> Reported-by: kernel test robot <lkp@intel.com>
> v3: resend and added more recipients
> v4: resend
> v5: used in-kernel function int_pow
> v6: no change here
> v7: moved definition of sernum size to patch 4
> v8: changed buffer type to u8
> ---
>  drivers/misc/eeprom/Kconfig |   5 +-
>  drivers/misc/eeprom/at25.c  | 140 ++++++++++++++++++++++++++++--------
>  2 files changed, 113 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/misc/eeprom/Kconfig b/drivers/misc/eeprom/Kconfig
> index 0f791bfdc1f5..f0a7531f354c 100644
> --- a/drivers/misc/eeprom/Kconfig
> +++ b/drivers/misc/eeprom/Kconfig
> @@ -32,12 +32,13 @@ config EEPROM_AT24
>  	  will be called at24.
>  
>  config EEPROM_AT25
> -	tristate "SPI EEPROMs from most vendors"
> +	tristate "SPI EEPROMs (FRAMs) from most vendors"
>  	depends on SPI && SYSFS
>  	select NVMEM
>  	select NVMEM_SYSFS
>  	help
> -	  Enable this driver to get read/write support to most SPI EEPROMs,
> +	  Enable this driver to get read/write support to most SPI EEPROMs
> +	  and Cypress FRAMs,
>  	  after you configure the board init code to know about each eeprom
>  	  on your target board.
>  
> diff --git a/drivers/misc/eeprom/at25.c b/drivers/misc/eeprom/at25.c
> index b76e4901b4a4..e25cec7c474b 100644
> --- a/drivers/misc/eeprom/at25.c
> +++ b/drivers/misc/eeprom/at25.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /*
>   * at25.c -- support most SPI EEPROMs, such as Atmel AT25 models
> + *	     and Cypress FRAMs FM25 models
>   *
>   * Copyright (C) 2006 David Brownell
>   */
> @@ -16,6 +17,9 @@
>  #include <linux/spi/spi.h>
>  #include <linux/spi/eeprom.h>
>  #include <linux/property.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/math.h>
>  
>  /*
>   * NOTE: this is an *EEPROM* driver.  The vagaries of product naming
> @@ -34,6 +38,7 @@ struct at25_data {
>  	unsigned		addrlen;
>  	struct nvmem_config	nvmem_config;
>  	struct nvmem_device	*nvmem;
> +	int has_sernum;

bool?


>  };
>  
>  #define	AT25_WREN	0x06		/* latch the write enable */
> @@ -42,6 +47,9 @@ struct at25_data {
>  #define	AT25_WRSR	0x01		/* write status register */
>  #define	AT25_READ	0x03		/* read byte(s) */
>  #define	AT25_WRITE	0x02		/* write byte(s)/sector */
> +#define	FM25_SLEEP	0xb9		/* enter sleep mode */
> +#define	FM25_RDID	0x9f		/* read device ID */
> +#define	FM25_RDSN	0xc3		/* read S/N */
>  
>  #define	AT25_SR_nRDY	0x01		/* nRDY = write-in-progress */
>  #define	AT25_SR_WEN	0x02		/* write enable (latched) */
> @@ -51,6 +59,8 @@ struct at25_data {
>  
>  #define	AT25_INSTR_BIT3	0x08		/* Additional address bit in instr */
>  
> +#define	FM25_ID_LEN	9		/* ID length */
> +
>  #define EE_MAXADDRLEN	3		/* 24 bit addresses, up to 2 MBytes */
>  
>  /* Specs often allow 5 msec for a page write, sometimes 20 msec;
> @@ -58,6 +68,9 @@ struct at25_data {
>   */
>  #define	EE_TIMEOUT	25
>  
> +#define	IS_EEPROM	0
> +#define	IS_FRAM		1
> +
>  /*-------------------------------------------------------------------------*/
>  
>  #define	io_limit	PAGE_SIZE	/* bytes */
> @@ -129,6 +142,36 @@ static int at25_ee_read(void *priv, unsigned int offset,
>  	return status;
>  }
>  
> +/*
> + * read extra registers as ID or serial number
> + */
> +static int fm25_aux_read(struct at25_data *at25, u8 *buf, uint8_t command,
> +			 int len)
> +{
> +	int status;
> +	struct spi_transfer t[2];
> +	struct spi_message m;
> +
> +	spi_message_init(&m);
> +	memset(t, 0, sizeof(t));

Are you allowed to send spi messages off of the stack?




> +
> +	t[0].tx_buf = &command;
> +	t[0].len = 1;
> +	spi_message_add_tail(&t[0], &m);
> +
> +	t[1].rx_buf = buf;
> +	t[1].len = len;
> +	spi_message_add_tail(&t[1], &m);
> +
> +	mutex_lock(&at25->lock);
> +
> +	status = spi_sync(at25->spi, &m);
> +	dev_dbg(&at25->spi->dev, "read %d aux bytes --> %d\n", len, status);
> +
> +	mutex_unlock(&at25->lock);
> +	return status;
> +}
> +
>  static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
>  {
>  	struct at25_data *at25 = priv;
> @@ -303,34 +346,37 @@ static int at25_fw_to_chip(struct device *dev, struct spi_eeprom *chip)
>  	return 0;
>  }
>  
> +static const struct of_device_id at25_of_match[] = {
> +	{ .compatible = "atmel,at25", .data = (const void *)IS_EEPROM },
> +	{ .compatible = "cypress,fm25", .data = (const void *)IS_FRAM },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, at25_of_match);
> +
>  static int at25_probe(struct spi_device *spi)
>  {
>  	struct at25_data	*at25 = NULL;
>  	struct spi_eeprom	chip;
>  	int			err;
>  	int			sr;
> -	int			addrlen;
> +	u8 id[FM25_ID_LEN];
> +	const struct of_device_id *match;
> +	int is_fram = 0;
> +
> +	match = of_match_device(of_match_ptr(at25_of_match), &spi->dev);
> +	if (match)
> +		is_fram = (int)(uintptr_t)match->data;
>  
>  	/* Chip description */
>  	if (!spi->dev.platform_data) {
> -		err = at25_fw_to_chip(&spi->dev, &chip);
> -		if (err)
> -			return err;
> +		if (!is_fram) {
> +			err = at25_fw_to_chip(&spi->dev, &chip);
> +			if (err)
> +				return err;
> +		}
>  	} else
>  		chip = *(struct spi_eeprom *)spi->dev.platform_data;
>  
> -	/* For now we only support 8/16/24 bit addressing */
> -	if (chip.flags & EE_ADDR1)
> -		addrlen = 1;
> -	else if (chip.flags & EE_ADDR2)
> -		addrlen = 2;
> -	else if (chip.flags & EE_ADDR3)
> -		addrlen = 3;
> -	else {
> -		dev_dbg(&spi->dev, "unsupported address type\n");
> -		return -EINVAL;
> -	}
> -
>  	/* Ping the chip ... the status register is pretty portable,
>  	 * unlike probing manufacturer IDs.  We do expect that system
>  	 * firmware didn't write it in the past few milliseconds!
> @@ -349,9 +395,49 @@ static int at25_probe(struct spi_device *spi)
>  	at25->chip = chip;
>  	at25->spi = spi;
>  	spi_set_drvdata(spi, at25);
> -	at25->addrlen = addrlen;
>  
> -	at25->nvmem_config.type = NVMEM_TYPE_EEPROM;
> +	if (is_fram) {
> +		/* Get ID of chip */
> +		fm25_aux_read(at25, id, FM25_RDID, FM25_ID_LEN);
> +		if (id[6] != 0xc2) {
> +			dev_err(&spi->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(&spi->dev, "Error: unsupported size (id %02x)\n", id[7]);
> +			return -ENODEV;
> +		}
> +		chip.byte_len = int_pow(2, id[7] - 0x21 + 4) * 1024;
> +
> +		if (at25->chip.byte_len > 64 * 1024)
> +			at25->chip.flags |= EE_ADDR3;
> +		else
> +			at25->chip.flags |= EE_ADDR2;
> +
> +		if (id[8])
> +			at25->has_sernum = 1;
> +		else
> +			at25->has_sernum = 0;
> +
> +		at25->chip.page_size = PAGE_SIZE;
> +		strncpy(at25->chip.name, "fm25", sizeof(at25->chip.name));
> +	}
> +
> +	/* For now we only support 8/16/24 bit addressing */
> +	if (at25->chip.flags & EE_ADDR1)
> +		at25->addrlen = 1;
> +	else if (at25->chip.flags & EE_ADDR2)
> +		at25->addrlen = 2;
> +	else if (at25->chip.flags & EE_ADDR3)
> +		at25->addrlen = 3;
> +	else {
> +		dev_dbg(&spi->dev, "unsupported address type\n");
> +		return -EINVAL;
> +	}
> +
> +	at25->nvmem_config.type = is_fram ? NVMEM_TYPE_FRAM : NVMEM_TYPE_EEPROM;
>  	at25->nvmem_config.name = dev_name(&spi->dev);
>  	at25->nvmem_config.dev = &spi->dev;
>  	at25->nvmem_config.read_only = chip.flags & EE_READONLY;
> @@ -370,23 +456,17 @@ static int at25_probe(struct spi_device *spi)
>  	if (IS_ERR(at25->nvmem))
>  		return PTR_ERR(at25->nvmem);
>  
> -	dev_info(&spi->dev, "%d %s %s eeprom%s, pagesize %u\n",
> -		(chip.byte_len < 1024) ? chip.byte_len : (chip.byte_len / 1024),
> -		(chip.byte_len < 1024) ? "Byte" : "KByte",
> -		at25->chip.name,
> -		(chip.flags & EE_READONLY) ? " (readonly)" : "",
> -		at25->chip.page_size);
> +	dev_info(&spi->dev, "%d %s %s %s%s, pagesize %u\n",
> +		 (chip.byte_len < 1024) ? chip.byte_len : (chip.byte_len / 1024),
> +		 (chip.byte_len < 1024) ? "Byte" : "KByte",
> +		 at25->chip.name, is_fram ? "fram" : "eeprom",
> +		 (chip.flags & EE_READONLY) ? " (readonly)" : "",
> +		 at25->chip.page_size);

When drivers work properly, they should be quiet.  This whole dev_info()
should be removed in a later patch.

thanks,

greg k-h

Re: [PATCH v8 2/5] nvmem: eeprom: at25: add support for FRAM

[Jiří Prchal]

On 08. 06. 21 11:07, Greg Kroah-Hartman wrote:
>> +	int has_sernum;
> 
> bool?

OK.
> 
>> +	spi_message_init(&m);
>> +	memset(t, 0, sizeof(t));
> 
> Are you allowed to send spi messages off of the stack?

I don't know, but it's functional. Copied from read function.
> 
> 
>> -	dev_info(&spi->dev, "%d %s %s eeprom%s, pagesize %u\n",
>> -		(chip.byte_len < 1024) ? chip.byte_len : (chip.byte_len / 1024),
>> -		(chip.byte_len < 1024) ? "Byte" : "KByte",
>> -		at25->chip.name,
>> -		(chip.flags & EE_READONLY) ? " (readonly)" : "",
>> -		at25->chip.page_size);
>> +	dev_info(&spi->dev, "%d %s %s %s%s, pagesize %u\n",
>> +		 (chip.byte_len < 1024) ? chip.byte_len : (chip.byte_len / 1024),
>> +		 (chip.byte_len < 1024) ? "Byte" : "KByte",
>> +		 at25->chip.name, is_fram ? "fram" : "eeprom",
>> +		 (chip.flags & EE_READONLY) ? " (readonly)" : "",
>> +		 at25->chip.page_size);
> 
> When drivers work properly, they should be quiet.  This whole dev_info()
> should be removed in a later patch.

OK, didn't know, originally there is such info output. And keeping 
simplest smallest patch changes.

[PATCH v8 3/5] dt-bindings: nvmem: at25: add for FRAM support

[Jiri Prchal]

Added dt binding documentation.

Signed-off-by: Jiri Prchal <jiri.prchal@aksignal.cz>
Reviewed-by: Rob Herring <robh@kernel.org>
---
v2: fixed dt_binding_check warnings thanks to Rob Herring
v3: resend and added more recipients
v4: resend
v5: no change here
v6: no change here
v7: no change here
v8: changed subject
---
 .../devicetree/bindings/eeprom/at25.yaml      | 31 +++++++++++++++----
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/eeprom/at25.yaml b/Documentation/devicetree/bindings/eeprom/at25.yaml
index 121a601db22e..840ee7a83a14 100644
--- a/Documentation/devicetree/bindings/eeprom/at25.yaml
+++ b/Documentation/devicetree/bindings/eeprom/at25.yaml
@@ -4,14 +4,16 @@
 $id: "http://devicetree.org/schemas/eeprom/at25.yaml#"
 $schema: "http://devicetree.org/meta-schemas/core.yaml#"
 
-title: SPI EEPROMs compatible with Atmel's AT25
+title: SPI EEPROMs or FRAMs compatible with Atmel's AT25
 
 maintainers:
   - Christian Eggers <ceggers@arri.de>
 
 properties:
   $nodename:
-    pattern: "^eeprom@[0-9a-f]{1,2}$"
+    anyOf:
+      - pattern: "^eeprom@[0-9a-f]{1,2}$"
+      - pattern: "^fram@[0-9a-f]{1,2}$"
 
   # There are multiple known vendors who manufacture EEPROM chips compatible
   # with Atmel's AT25. The compatible string requires two items where the
@@ -31,6 +33,7 @@ properties:
               - microchip,25lc040
               - st,m95m02
               - st,m95256
+              - cypress,fm25
 
           - const: atmel,at25
 
@@ -48,7 +51,7 @@ properties:
     $ref: /schemas/types.yaml#/definitions/uint32
     enum: [1, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, 8192, 16384, 32768, 65536, 131072]
     description:
-      Size of the eeprom page.
+      Size of the eeprom page. FRAMs don't have pages.
 
   size:
     $ref: /schemas/types.yaml#/definitions/uint32
@@ -101,9 +104,19 @@ required:
   - compatible
   - reg
   - spi-max-frequency
-  - pagesize
-  - size
-  - address-width
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          not:
+            contains:
+              const: cypress,fm25
+    then:
+      required:
+        - pagesize
+        - size
+        - address-width
 
 additionalProperties: false
 
@@ -126,4 +139,10 @@ examples:
             size = <32768>;
             address-width = <16>;
         };
+
+        fram@1 {
+            compatible = "cypress,fm25", "atmel,at25";
+            reg = <1>;
+            spi-max-frequency = <40000000>;
+        };
     };
-- 
2.25.1

[PATCH v8 4/5] nvmem: eeprom: at25: export FRAM serial num

[Jiri Prchal]

This exports serial number of FRAM in sysfs file named "sernum".
Formatted in hex, each byte separated by space.
Example:
$ cat /sys/class/spi_master/spi0/spi0.0/sernum
0000a43644f2ae6c

Signed-off-by: Jiri Prchal <jiri.prchal@aksignal.cz>
---
v2: no change here
v3: resend and added more recipients
v4: resend
v5: reworked up on Greg comments: no spaces in string, sysfs done correctly
v6: no change here
v7: moved FM25_SN_LEN, static array, used sysfs_emit, DEVICE_ATTR_RO
v8: clarify sysfs_emit format
---
 drivers/misc/eeprom/at25.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/eeprom/at25.c b/drivers/misc/eeprom/at25.c
index e25cec7c474b..4a47a7e2d90d 100644
--- a/drivers/misc/eeprom/at25.c
+++ b/drivers/misc/eeprom/at25.c
@@ -31,6 +31,7 @@
  *   AT25M02, AT25128B
  */
 
+#define	FM25_SN_LEN	8		/* serial number length */
 struct at25_data {
 	struct spi_device	*spi;
 	struct mutex		lock;
@@ -39,6 +40,7 @@ struct at25_data {
 	struct nvmem_config	nvmem_config;
 	struct nvmem_device	*nvmem;
 	int has_sernum;
+	u8 sernum[FM25_SN_LEN];
 };
 
 #define	AT25_WREN	0x06		/* latch the write enable */
@@ -172,6 +174,21 @@ static int fm25_aux_read(struct at25_data *at25, u8 *buf, uint8_t command,
 	return status;
 }
 
+static ssize_t sernum_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct at25_data *at25;
+
+	at25 = dev_get_drvdata(dev);
+	return sysfs_emit(buf, "%*phN\n", sizeof at25->sernum, at25->sernum);
+}
+static DEVICE_ATTR_RO(sernum);
+
+static struct attribute *sernum_attrs[] = {
+	&dev_attr_sernum.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(sernum);
+
 static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
 {
 	struct at25_data *at25 = priv;
@@ -416,8 +433,10 @@ static int at25_probe(struct spi_device *spi)
 		else
 			at25->chip.flags |= EE_ADDR2;
 
-		if (id[8])
+		if (id[8]) {
 			at25->has_sernum = 1;
+			fm25_aux_read(at25, at25->sernum, FM25_RDSN, FM25_SN_LEN);
+		}
 		else
 			at25->has_sernum = 0;
 
@@ -471,6 +490,7 @@ static struct spi_driver at25_driver = {
 	.driver = {
 		.name		= "at25",
 		.of_match_table = at25_of_match,
+		.dev_groups	= sernum_groups,
 	},
 	.probe		= at25_probe,
 };
-- 
2.25.1

Re: [PATCH v8 4/5] nvmem: eeprom: at25: export FRAM serial num

[Greg Kroah-Hartman]

On Mon, Jun 07, 2021 at 06:12:00PM +0200, Jiri Prchal wrote:
> This exports serial number of FRAM in sysfs file named "sernum".
> Formatted in hex, each byte separated by space.
> Example:
> $ cat /sys/class/spi_master/spi0/spi0.0/sernum
> 0000a43644f2ae6c
> 
> Signed-off-by: Jiri Prchal <jiri.prchal@aksignal.cz>
> ---
> v2: no change here
> v3: resend and added more recipients
> v4: resend
> v5: reworked up on Greg comments: no spaces in string, sysfs done correctly
> v6: no change here
> v7: moved FM25_SN_LEN, static array, used sysfs_emit, DEVICE_ATTR_RO
> v8: clarify sysfs_emit format
> ---
>  drivers/misc/eeprom/at25.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/eeprom/at25.c b/drivers/misc/eeprom/at25.c
> index e25cec7c474b..4a47a7e2d90d 100644
> --- a/drivers/misc/eeprom/at25.c
> +++ b/drivers/misc/eeprom/at25.c
> @@ -31,6 +31,7 @@
>   *   AT25M02, AT25128B
>   */
>  
> +#define	FM25_SN_LEN	8		/* serial number length */
>  struct at25_data {
>  	struct spi_device	*spi;
>  	struct mutex		lock;
> @@ -39,6 +40,7 @@ struct at25_data {
>  	struct nvmem_config	nvmem_config;
>  	struct nvmem_device	*nvmem;
>  	int has_sernum;
> +	u8 sernum[FM25_SN_LEN];
>  };
>  
>  #define	AT25_WREN	0x06		/* latch the write enable */
> @@ -172,6 +174,21 @@ static int fm25_aux_read(struct at25_data *at25, u8 *buf, uint8_t command,
>  	return status;
>  }
>  
> +static ssize_t sernum_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct at25_data *at25;
> +
> +	at25 = dev_get_drvdata(dev);
> +	return sysfs_emit(buf, "%*phN\n", sizeof at25->sernum, at25->sernum);

sizeof(at25->sernum) is the normal way to write this.

thanks,

greg k-h

[PATCH v8 5/5] nvmem: eeprom: add documentation of sysfs fram and sernum file

[Jiri Prchal]

Added sysfs fram and sernum file documentation.

Signed-off-by: Jiri Prchal <jiri.prchal@aksignal.cz>
---
v5: new
v6: no change here
v7: no change here
v8: added fram file doc
---
 Documentation/ABI/testing/sysfs-class-spi-eeprom | 13 +++++++++++++
 1 file changed, 13 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-class-spi-eeprom

diff --git a/Documentation/ABI/testing/sysfs-class-spi-eeprom b/Documentation/ABI/testing/sysfs-class-spi-eeprom
new file mode 100644
index 000000000000..b41420fe1329
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-spi-eeprom
@@ -0,0 +1,13 @@
+What:		/sys/class/spi_master/spi<bus>/spi<bus>.<dev>/sernum
+Date:		May 2021
+KernelVersion:	5.13
+Contact:	Jiri Prchal <jiri.prchal@aksignal.cz>
+Description:
+		(RO) Exports serial number of Cypress FRAM (FM25VN). 8 bytes as is in chip in hex string.
+
+What:		/sys/class/spi_master/spi<bus>/spi<bus>.<dev>/fram
+Date:		June 2021
+KernelVersion:	5.13
+Contact:	Jiri Prchal <jiri.prchal@aksignal.cz>
+Description:
+		(RW) FRAM data.
-- 
2.25.1

Re: [PATCH v8 5/5] nvmem: eeprom: add documentation of sysfs fram and sernum file

[Greg Kroah-Hartman]

On Mon, Jun 07, 2021 at 06:12:01PM +0200, Jiri Prchal wrote:
> Added sysfs fram and sernum file documentation.
> 
> Signed-off-by: Jiri Prchal <jiri.prchal@aksignal.cz>
> ---
> v5: new
> v6: no change here
> v7: no change here
> v8: added fram file doc
> ---
>  Documentation/ABI/testing/sysfs-class-spi-eeprom | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-spi-eeprom
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-spi-eeprom b/Documentation/ABI/testing/sysfs-class-spi-eeprom
> new file mode 100644
> index 000000000000..b41420fe1329
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-spi-eeprom
> @@ -0,0 +1,13 @@
> +What:		/sys/class/spi_master/spi<bus>/spi<bus>.<dev>/sernum
> +Date:		May 2021
> +KernelVersion:	5.13
> +Contact:	Jiri Prchal <jiri.prchal@aksignal.cz>
> +Description:
> +		(RO) Exports serial number of Cypress FRAM (FM25VN). 8 bytes as is in chip in hex string.

Please properly wrap your lines.

What is "(RO)" here?

And the grammer is a bit odd, what is the second sentence supposed to
mean?

> +
> +What:		/sys/class/spi_master/spi<bus>/spi<bus>.<dev>/fram
> +Date:		June 2021
> +KernelVersion:	5.13

Obviously it can not make 5.13, right?

> +Contact:	Jiri Prchal <jiri.prchal@aksignal.cz>
> +Description:
> +		(RW) FRAM data.

I have no idea what that is, do you?

Please provide real documentation for these entries.

thannks,

greg k-h

Re: [PATCH v8 5/5] nvmem: eeprom: add documentation of sysfs fram and sernum file

[Jiří Prchal]

On 08. 06. 21 11:05, Greg Kroah-Hartman wrote:
> On Mon, Jun 07, 2021 at 06:12:01PM +0200, Jiri Prchal wrote:
>> Added sysfs fram and sernum file documentation.
>>
>> Signed-off-by: Jiri Prchal <jiri.prchal@aksignal.cz>
>> ---
>> v5: new
>> v6: no change here
>> v7: no change here
>> v8: added fram file doc
>> ---
>>   Documentation/ABI/testing/sysfs-class-spi-eeprom | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>   create mode 100644 Documentation/ABI/testing/sysfs-class-spi-eeprom
>>
>> diff --git a/Documentation/ABI/testing/sysfs-class-spi-eeprom b/Documentation/ABI/testing/sysfs-class-spi-eeprom
>> new file mode 100644
>> index 000000000000..b41420fe1329
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-class-spi-eeprom
>> @@ -0,0 +1,13 @@
>> +What:		/sys/class/spi_master/spi<bus>/spi<bus>.<dev>/sernum
>> +Date:		May 2021
>> +KernelVersion:	5.13
>> +Contact:	Jiri Prchal <jiri.prchal@aksignal.cz>
>> +Description:
>> +		(RO) Exports serial number of Cypress FRAM (FM25VN). 8 bytes as is in chip in hex string.
> 
> Please properly wrap your lines.
> 
> What is "(RO)" here?

Read Only, as seen in another doc.

> 
> And the grammer is a bit odd, what is the second sentence supposed to
> mean?
> 
>> +
>> +What:		/sys/class/spi_master/spi<bus>/spi<bus>.<dev>/fram
>> +Date:		June 2021
>> +KernelVersion:	5.13
> 
> Obviously it can not make 5.13, right?

Sorry for missunderstanding, what number should be here?

Re: [PATCH v8 5/5] nvmem: eeprom: add documentation of sysfs fram and sernum file

[Greg Kroah-Hartman]

On Tue, Jun 08, 2021 at 11:50:03AM +0200, Jiří Prchal wrote:
> 
> 
> On 08. 06. 21 11:05, Greg Kroah-Hartman wrote:
> > On Mon, Jun 07, 2021 at 06:12:01PM +0200, Jiri Prchal wrote:
> > > Added sysfs fram and sernum file documentation.
> > > 
> > > Signed-off-by: Jiri Prchal <jiri.prchal@aksignal.cz>
> > > ---
> > > v5: new
> > > v6: no change here
> > > v7: no change here
> > > v8: added fram file doc
> > > ---
> > >   Documentation/ABI/testing/sysfs-class-spi-eeprom | 13 +++++++++++++
> > >   1 file changed, 13 insertions(+)
> > >   create mode 100644 Documentation/ABI/testing/sysfs-class-spi-eeprom
> > > 
> > > diff --git a/Documentation/ABI/testing/sysfs-class-spi-eeprom b/Documentation/ABI/testing/sysfs-class-spi-eeprom
> > > new file mode 100644
> > > index 000000000000..b41420fe1329
> > > --- /dev/null
> > > +++ b/Documentation/ABI/testing/sysfs-class-spi-eeprom
> > > @@ -0,0 +1,13 @@
> > > +What:		/sys/class/spi_master/spi<bus>/spi<bus>.<dev>/sernum
> > > +Date:		May 2021
> > > +KernelVersion:	5.13
> > > +Contact:	Jiri Prchal <jiri.prchal@aksignal.cz>
> > > +Description:
> > > +		(RO) Exports serial number of Cypress FRAM (FM25VN). 8 bytes as is in chip in hex string.
> > 
> > Please properly wrap your lines.
> > 
> > What is "(RO)" here?
> 
> Read Only, as seen in another doc.

Perhaps this should say something like:
	Contains the serial number of the Cypress FRAM (FM25VN) if it is
	present.  It will be displayed as a 8 byte hex string, as read
	from the device.

	This is a read-only attribute.

> > And the grammer is a bit odd, what is the second sentence supposed to
> > mean?
> > 
> > > +
> > > +What:		/sys/class/spi_master/spi<bus>/spi<bus>.<dev>/fram
> > > +Date:		June 2021
> > > +KernelVersion:	5.13
> > 
> > Obviously it can not make 5.13, right?
> 
> Sorry for missunderstanding, what number should be here?

5.14 if all goes well, right?

thanks,

greg k-h