[PATCH v2 0/3] Add control driver for ST M24LR RFID/NFC EEPROM chips

[PATCH v2 0/3] Add control driver for ST M24LR RFID/NFC EEPROM chips

[Abd-Alrhman Masalkhi]

This patch series adds support for the control interface of STMicroelectronics 
M24LR RFID/NFC EEPROM devices, such as the M24LR04E-R. The chip exposes two 
I2C addresses and two distinct address spaces: one for the system parameter 
sector and another for the EEPROM memory. These address spaces overlap (both
start at address 0). Due to this overlap, the chip cannot be represented by a
single Device Tree node and handled by the standard at24 driver.

While it might be possible to model the system parameter sector and EEPROM
as two separate DT nodes, each handled by the standard at24 driver, this
leads to synchronization problems. For example, if a multi-page EEPROM write
is in progress and a control command (e.g., a i2c write lock) is issued
concurrently, it can result in incomplete or inconsistent writeing.

To overcome these issues, while maintaining a clean architecture and ensuring
synchronization, the chip is managed by two drivers:

1- The first driver exposes the system parameter sector and acts as an I2C mux
   (gate) for EEPROM access. Using an I2C mux solves the synchronization issue.
   This patch series implements this driver.
2- The second is the standard at24 driver, which handles the EEPROM.

Tested on: M24LR04E-R using Yocto on Raspberry Pi 4

Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>

Abd-Alrhman Masalkhi (3):
  dt-bindings: eeprom: Add ST M24LR control interface
  misc: add sysfs control driver for ST M24LR series RFID/NFC chips
  ABI: sysfs: document control attributes for ST M24LR

 .../testing/sysfs-bus-i2c-devices-m24lr_ctl   |  96 +++
 .../devicetree/bindings/eeprom/st,m24lr.yaml  |  72 ++
 drivers/misc/Kconfig                          |  15 +
 drivers/misc/Makefile                         |   1 +
 drivers/misc/m24lr_ctl.c                      | 677 ++++++++++++++++++
 5 files changed, 861 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c-devices-m24lr_ctl
 create mode 100644 Documentation/devicetree/bindings/eeprom/st,m24lr.yaml
 create mode 100644 drivers/misc/m24lr_ctl.c

-- 
2.43.0

[PATCH v2 1/3] dt-bindings: eeprom: Add ST M24LR control interface

[Abd-Alrhman Masalkhi]

Add support for the control interface of STMicroelectronics M24LR
RFID/NFC EEPROM chips.

Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
 .../devicetree/bindings/eeprom/st,m24lr.yaml  | 72 +++++++++++++++++++
 1 file changed, 72 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/eeprom/st,m24lr.yaml

diff --git a/Documentation/devicetree/bindings/eeprom/st,m24lr.yaml b/Documentation/devicetree/bindings/eeprom/st,m24lr.yaml
new file mode 100644
index 000000000000..6d72325865d6
--- /dev/null
+++ b/Documentation/devicetree/bindings/eeprom/st,m24lr.yaml
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/eeprom/st,m24lr.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STMicroelectronics M24LR Series NFC/RFID EEPROM Control Interface
+
+maintainers:
+  - Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
+
+description: |
+  STMicroelectronics M24LR series are dual-interface (RF + I2C)
+  EEPROM chips. These devices support I2C-based access to both
+  memory and a system area that controls authentication and configuration.
+  They expose two I2C addresses: one for EEPROM memory and one for the
+  system control area (e.g., UID, password management).
+
+allOf:
+  - $ref: ../i2c/i2c-mux.yaml#
+
+properties:
+  compatible:
+    enum:
+      - st,m24lr04e-r
+      - st,m24lr16e-r
+      - st,m24lr64e-r
+
+  reg:
+    maxItems: 1
+
+  pagesize:
+    enum: [1, 4, 8, 16, 32, 64, 128, 256]
+    default: 1
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: >
+      Maximum number of bytes that can be written in one I2C transaction.
+      the default is 1.
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      i2cmux@57 {
+        compatible = "st,m24lr04e-r";
+        reg = <0x57>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        i2c@0 {
+          reg = <0x0>;
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          eeprom@53 {
+            compatible = "atmel,24c04";
+            reg = <0x53>;
+            address-width = <16>;
+            pagesize = <4>;
+          };
+        };
+      };
+    };
+...
-- 
2.43.0

[PATCH v2 2/3] misc: add sysfs control driver for ST M24LR series RFID/NFC chips

[Abd-Alrhman Masalkhi]

Add a sysfs-based control driver for the STMicroelectronics M24LR
series RFID/NFC EEPROM chips, such as the M24LR04E-R. It enables
access to control registers for features such as password
authentication, memory access, and device configuration. It also
synchronize access to the device. (The EEPROM uses a separate driver;
see the note below for details.)

This driver provides only the control interface for M24LR chips. It
also acts as an I2C mux (gate) for the EEPROM. Therefore, the EEPROM
is represented as a child node in the Device Tree and is accessed
through a separate driver (the standard 'at24' driver). This setup
is possible because M24LR chips use two I2C addresses: one for
accessing the system parameter sector, and another for accessing
the EEPROM.

Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
 drivers/misc/Kconfig     |  15 +
 drivers/misc/Makefile    |   1 +
 drivers/misc/m24lr_ctl.c | 677 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 693 insertions(+)
 create mode 100644 drivers/misc/m24lr_ctl.c

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index c161546d728f..c4152f03695f 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -644,6 +644,21 @@ config MCHP_LAN966X_PCI
 	    - lan966x-miim (MDIO_MSCC_MIIM)
 	    - lan966x-switch (LAN966X_SWITCH)
 
+config M24LR_CTL
+	tristate "M24LR I2C RFID/NFC Control Interface driver"
+	depends on I2C_MUX && SYSFS
+	select REGMAP_I2C
+	help
+	  This driver provides support for the control interface of M24LR I2C
+	  RFID/NFC chips.
+
+	  Note This driver does not handle the EEPROM on the device. For EEPROM
+	  access, use the standard 'at24' driver (drivers/misc/eeprom/at24.c),
+	  which supports I2C-based EEPROMs.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called m24lr_ctl.
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 054cee9b08a4..5ae54112ad7e 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -75,3 +75,4 @@ lan966x-pci-objs		:= lan966x_pci.o
 lan966x-pci-objs		+= lan966x_pci.dtbo.o
 obj-$(CONFIG_MCHP_LAN966X_PCI)	+= lan966x-pci.o
 obj-y				+= keba/
+obj-$(CONFIG_M24LR_CTL)		+= m24lr_ctl.o
diff --git a/drivers/misc/m24lr_ctl.c b/drivers/misc/m24lr_ctl.c
new file mode 100644
index 000000000000..5c8fbabbaac7
--- /dev/null
+++ b/drivers/misc/m24lr_ctl.c
@@ -0,0 +1,677 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * m24lr_ctl.c - Sysfs control interface for ST M24LR series RFID/NFC chips
+ *
+ * Copyright (c) 2025 Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
+ *
+ * This driver implements a sysfs-based control interface for interacting with
+ * STMicroelectronics M24LR series chips (e.g., M24LR04E-R). It enables access
+ * to control registers for features such as password authentication, memory
+ * access, and device configuration. It also synchronize access to the device
+ * (the EEPROM uses a separate driver, see the note below for details)
+ *
+ * NOTE:
+ * This driver provides only the control interface for M24LR chips. It acts
+ * as an I2C mux (gate) for the EEPROM. Therefore, the EEPROM is represented
+ * as a child node in the Device Tree and is accessed through a separate driver
+ * (the standard 'at24' driver). This setup is possible because M24LR chips use
+ * two I2C addresses: one for accessing the system parameter sector, and another
+ * for accessing the EEPROM.
+ */
+
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <linux/module.h>
+#include <linux/i2c-mux.h>
+#include <linux/device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+
+#define M24LR_CTL_PAGESIZE_DEFAULT    1u
+
+/*
+ * Limits the number of I/O control bytes to 64 to prevent holding the
+ * I2C bus for too long, especially important when operating at low I2C
+ * frequencies
+ */
+#define M24LR_CTL_PAGESIZE_LIMIT      64u
+#define M24LR_CTL_WRITE_TIMEOUT	      25u
+#define M24LR_CTL_READ_TIMEOUT	      (M24LR_CTL_WRITE_TIMEOUT)
+
+#define to_sys_entry(attrp)   container_of(attrp, struct m24lr_sys_entry, attr)
+
+/**
+ * struct m24lr_sys_entry - describes a sysfs entry for M24LR device access
+ * @reg_addr:  register address in the M24LR device
+ * @reg_size:  size of the register in bytes
+ * @attr:      sysfs attribute used for exposing register access to userspace
+ *
+ * Used to define readable/writable register entries through sysfs.
+ */
+struct m24lr_sys_entry {
+	unsigned int reg_addr;
+	unsigned int reg_size;
+	struct device_attribute attr;
+};
+
+/**
+ * struct m24lr_ctl_chip - describes chip-specific sysfs layout
+ * @entries:       array of sysfs entries specific to the chip variant
+ * @n_entries:     number of entries in the array
+ * @n_sss_entries: number of sss entries required for the chip
+ *
+ * Supports multiple M24LR chip variants (e.g., M24LRxx) by allowing each
+ * to define its own set of sysfs attributes, depending on its available
+ * registers and features.
+ */
+struct m24lr_ctl_chip {
+	const struct m24lr_sys_entry *entries;
+	unsigned int n_entries;
+	unsigned int n_sss_entries;
+};
+
+/**
+ * struct m24lr_ctl - core driver data for M24LR chip control
+ * @page_size:     chip-specific limit on the maximum number of bytes allowed
+ *                 in a single write operation.
+ * @muxc:          mux core struct as the driver act as a gate for the eeprom
+ * @regmap:        regmap interface for accessing chip registers
+ * @gate_lock:     mutex to synchronize operations to the device from this
+ *                 driver and the eeprom driver
+ * @sss_entries:   array of sssc sysfs entries specific to the chip variant
+ * @n_sss_entries: number of entries in the sss entries array
+ *
+ * Central data structure holding the state and resources used by the
+ * M24LR device driver.
+ */
+struct m24lr_ctl {
+	unsigned int page_size;
+	struct regmap *regmap;
+	struct i2c_mux_core *muxc;
+	struct mutex gate_lock;  /* synchronize operations to the device */
+	struct m24lr_sys_entry *sss_entries;
+	unsigned int n_sss_entries;
+};
+
+static ssize_t m24lr_ctl_show(struct device *dev,
+			      struct device_attribute *attr, char *buf);
+static ssize_t m24lr_ctl_store(struct device *dev, struct device_attribute *attr,
+			       const char *buf, size_t count);
+static ssize_t m24lr_ctl_unlock_command(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t count);
+static ssize_t m24lr_ctl_newpass_command(struct device *dev,
+					 struct device_attribute *attr,
+					 const char *buf, size_t count);
+
+static const struct regmap_range m24lr_ctl_vo_ranges[] = {
+	regmap_reg_range(0, 63),
+};
+
+static const struct regmap_access_table m24lr_ctl_vo_table = {
+	.yes_ranges = m24lr_ctl_vo_ranges,
+	.n_yes_ranges = ARRAY_SIZE(m24lr_ctl_vo_ranges),
+};
+
+static const struct regmap_config m24lr_ctl_regmap_conf = {
+	.name = "m24lr_ctl",
+	.reg_stride = 1,
+	.reg_bits = 16,
+	.val_bits = 8,
+	.disable_locking = false,
+	.cache_type = REGCACHE_RBTREE,/* Flat can't be used, there's huge gap */
+	.volatile_table = &m24lr_ctl_vo_table,
+};
+
+/* define the default sysfs entries specific to the M24LR */
+static const struct m24lr_sys_entry m24lr_ctl_sys_entry_default_table[] = {
+	{.attr = __ATTR(unlock, 0200, NULL, m24lr_ctl_unlock_command)},
+	{.attr = __ATTR(new_pass, 0200, NULL, m24lr_ctl_newpass_command)},
+	{.reg_addr = 2324, .reg_size = 8,
+	 .attr = __ATTR(uid, 0444, m24lr_ctl_show, NULL)},
+	{.reg_addr = 2334, .reg_size = 1,
+	 .attr = __ATTR(mem_size, 0400, m24lr_ctl_show, NULL)},
+};
+
+/* Chip descriptor for M24LR04E-R variant */
+static const struct m24lr_ctl_chip m24lr04e_r_chip = {
+	.entries = m24lr_ctl_sys_entry_default_table,
+	.n_entries = ARRAY_SIZE(m24lr_ctl_sys_entry_default_table),
+	.n_sss_entries = 4,
+};
+
+/* Chip descriptor for M24LR16E-R variant */
+static const struct m24lr_ctl_chip m24lr16e_r_chip = {
+	.entries = m24lr_ctl_sys_entry_default_table,
+	.n_entries = ARRAY_SIZE(m24lr_ctl_sys_entry_default_table),
+	.n_sss_entries = 16,
+};
+
+/* Chip descriptor for M24LR64E-R variant */
+static const struct m24lr_ctl_chip m24lr64e_r_chip = {
+	.entries = m24lr_ctl_sys_entry_default_table,
+	.n_entries = ARRAY_SIZE(m24lr_ctl_sys_entry_default_table),
+	.n_sss_entries = 64,
+};
+
+static const struct i2c_device_id m24lr_ctl_ids[] = {
+	{ "m24lr04e-r", (kernel_ulong_t)&m24lr04e_r_chip},
+	{ "m24lr16e-r", (kernel_ulong_t)&m24lr16e_r_chip},
+	{ "m24lr64e-r", (kernel_ulong_t)&m24lr64e_r_chip},
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, m24lr_ctl_ids);
+
+static const struct of_device_id m24lr_ctl_of_match[] = {
+	{ .compatible = "st,m24lr04e-r", .data = &m24lr04e_r_chip},
+	{ .compatible = "st,m24lr16e-r", .data = &m24lr16e_r_chip},
+	{ .compatible = "st,m24lr64e-r", .data = &m24lr64e_r_chip},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, m24lr_ctl_of_match);
+
+static int m24lr_ctl_gate_select(struct i2c_mux_core *muxc,
+				 unsigned int chan_id)
+{
+	struct m24lr_ctl *ctl = i2c_mux_priv(muxc);
+
+	mutex_lock(&ctl->gate_lock);
+
+	return 0;
+}
+
+static int m24lr_ctl_gate_deselect(struct i2c_mux_core *muxc,
+				   unsigned int chan_id)
+{
+	struct m24lr_ctl *ctl = i2c_mux_priv(muxc);
+
+	mutex_unlock(&ctl->gate_lock);
+
+	return 0;
+}
+
+/**
+ * m24lr_parse_le_value - Parse hex string and convert to little-endian binary
+ * @buf:        Input string buffer (hex format)
+ * @reg_size:   Size of the register in bytes (must be 1, 2, 4, or 8)
+ * @output:     Output buffer to store the value in little-endian format
+ *
+ * Converts a hexadecimal string to a numeric value of the given register size
+ * and writes it in little-endian byte order into the provided buffer.
+ *
+ * Return: 0 on success, or negative error code on failure
+ */
+static int m24lr_parse_le_value(const char *buf, u32 reg_size, u8 *output)
+{
+	int err;
+
+	switch (reg_size) {
+	case 1: {
+		u8 tmp;
+
+		err = kstrtou8(buf, 16, &tmp);
+		if (!err)
+			*output = tmp;
+		break;
+	}
+	case 2: {
+		u16 tmp;
+
+		err = kstrtou16(buf, 16, &tmp);
+		if (!err)
+			*(__le16 *)output = cpu_to_le16(tmp);
+		break;
+	}
+	case 4: {
+		u32 tmp;
+
+		err = kstrtou32(buf, 16, &tmp);
+		if (!err)
+			*(__le32 *)output = cpu_to_le32(tmp);
+		break;
+	}
+	case 8: {
+		u64 tmp;
+
+		err = kstrtou64(buf, 16, &tmp);
+		if (!err)
+			*(__le64 *)output = cpu_to_le64(tmp);
+		break;
+	}
+	default:
+		err = -EINVAL;
+	}
+
+	return err;
+}
+
+/**
+ * m24lr_ctl_regmap_read - read data using regmap with retry on failure
+ * @regmap:  regmap instance for the device
+ * @buf:     buffer to store the read data
+ * @size:    number of bytes to read
+ * @offset:  starting register address
+ *
+ * Attempts to read a block of data from the device with retries and timeout.
+ * Some M24LR chips may transiently NACK reads (e.g., during internal write
+ * cycles), so this function retries with a short sleep until the timeout
+ * expires.
+ *
+ * Returns:
+ *       Number of bytes read on success,
+ *       -ETIMEDOUT if the read fails within the timeout window.
+ */
+static ssize_t m24lr_ctl_regmap_read(struct regmap *regmap, u8 *buf,
+				     size_t size, unsigned int offset)
+{
+	int err;
+	unsigned long timeout, read_time;
+	ssize_t ret = -ETIMEDOUT;
+
+	timeout = jiffies + msecs_to_jiffies(M24LR_CTL_READ_TIMEOUT);
+	do {
+		read_time = jiffies;
+
+		err = regmap_bulk_read(regmap, offset, buf, size);
+		if (!err) {
+			ret = size;
+			break;
+		}
+
+		usleep_range(1000, 2000);
+	} while (time_before(read_time, timeout));
+
+	return ret;
+}
+
+/**
+ * m24lr_ctl_regmap_write - write data using regmap with retry on failure
+ * @regmap: regmap instance for the device
+ * @buf:    buffer containing the data to write
+ * @size:   number of bytes to write
+ * @offset: starting register address
+ *
+ * Attempts to write a block of data to the device with retries and a timeout.
+ * Some M24LR devices may NACK I2C writes while an internal write operation
+ * is in progress. This function retries the write operation with a short delay
+ * until it succeeds or the timeout is reached.
+ *
+ * Returns:
+ *       Number of bytes written on success,
+ *       -ETIMEDOUT if the write fails within the timeout window.
+ */
+static ssize_t m24lr_ctl_regmap_write(struct regmap *regmap, const u8 *buf,
+				      size_t size, unsigned int offset)
+{
+	int err;
+	unsigned long timeout, write_time;
+	ssize_t ret = -ETIMEDOUT;
+
+	timeout = jiffies + msecs_to_jiffies(M24LR_CTL_WRITE_TIMEOUT);
+
+	do {
+		write_time = jiffies;
+
+		err = regmap_bulk_write(regmap, offset, buf, size);
+		if (!err) {
+			ret = size;
+			break;
+		}
+
+		usleep_range(1000, 2000);
+	} while (time_before(write_time, timeout));
+
+	return ret;
+}
+
+static ssize_t m24lr_ctl_read(struct m24lr_ctl *ctl, u8 *buf,
+			      size_t size, unsigned int offset)
+{
+	int ret = 0;
+	struct regmap *regmap = ctl->regmap;
+
+	if (unlikely(!size))
+		return ret;
+
+	m24lr_ctl_gate_select(ctl->muxc, 0);
+	ret = m24lr_ctl_regmap_read(regmap, buf, size, offset);
+	m24lr_ctl_gate_deselect(ctl->muxc, 0);
+
+	return ret;
+}
+
+/**
+ * m24lr_ctl_write - write buffer to M24LR device with page alignment handling
+ * @ctl: pointer to driver context
+ * @buf: data buffer to write
+ * @size: number of bytes to write
+ * @offset: target register address in the device
+ *
+ * Writes data to the M24LR device using regmap, split into chunks no larger
+ * than page_size to respect device-specific write limitations (e.g., page
+ * size or I2C hold-time concerns). Each chunk is aligned to the page boundary
+ * defined by page_size.
+ *
+ * Returns:
+ *       Total number of bytes written on success,
+ *       A negative error code if any write fails.
+ */
+static ssize_t m24lr_ctl_write(struct m24lr_ctl *ctl, const u8 *buf,
+			       size_t size, unsigned int offset)
+{
+	unsigned int n, next_sector;
+	struct regmap *regmap = ctl->regmap;
+	long err;
+	ssize_t ret = 0;
+
+	if (unlikely(!size))
+		return ret;
+
+	n = min(size, ctl->page_size);
+	next_sector = roundup(offset + 1, ctl->page_size);
+	if (offset + n > next_sector)
+		n = next_sector - offset;
+
+	m24lr_ctl_gate_select(ctl->muxc, 0);
+	while (n) {
+		err = m24lr_ctl_regmap_write(regmap, buf, n, offset);
+		if (IS_ERR_VALUE(err)) {
+			m24lr_ctl_gate_deselect(ctl->muxc, 0);
+			if (ret)
+				return ret;
+			else
+				return err;
+		}
+
+		offset += n;
+		size -= n;
+		ret += n;
+		n = min(size, ctl->page_size);
+	}
+	m24lr_ctl_gate_deselect(ctl->muxc, 0);
+
+	return ret;
+}
+
+/**
+ * m24lr_ctl_write_pass - Write password to M24LR043-R using secure format
+ * @ctl:   Pointer to device control structure
+ * @buf:   Input buffer containing hex-encoded password
+ * @count: Number of bytes in @buf
+ * @code:  Operation code to embed between password copies
+ *
+ * This function parses a 4-byte password, encodes it in  big-endian format,
+ * and constructs a 9-byte sequence of the form:
+ *
+ *        [BE(password), code, BE(password)]
+ *
+ * The result is written to register 0x0900 (2304), which is the password
+ * register in M24LR04E-R chip.
+ *
+ * Return: Number of bytes written on success, or negative error code on failure
+ */
+static ssize_t m24lr_ctl_write_pass(struct m24lr_ctl *ctl, const char *buf,
+				    size_t count, u8 code)
+{
+	int ret;
+	u32 pass;
+	__be32 be_pass;
+	u8 output[9];
+
+	ret = kstrtou32(buf, 16, &pass);
+	if (ret)
+		return ret;
+
+	be_pass = cpu_to_be32(pass);
+
+	memcpy(output, &be_pass, sizeof(be_pass));
+	output[4] = code;
+	memcpy(output + 5, &be_pass, sizeof(be_pass));
+
+	m24lr_ctl_gate_select(ctl->muxc, 0);
+	ret = m24lr_ctl_regmap_write(ctl->regmap, output, 9, 2304);
+	m24lr_ctl_gate_deselect(ctl->muxc, 0);
+
+	return ret;
+}
+
+static ssize_t m24lr_ctl_newpass_command(struct device *dev,
+					 struct device_attribute *attr,
+					 const char *buf, size_t count)
+{
+	struct m24lr_ctl *ctl = i2c_get_clientdata(to_i2c_client(dev));
+
+	if (count > 8)
+		return -EINVAL;
+
+	return m24lr_ctl_write_pass(ctl, buf, count, 7);
+}
+
+static ssize_t m24lr_ctl_unlock_command(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t count)
+{
+	struct m24lr_ctl *ctl = i2c_get_clientdata(to_i2c_client(dev));
+
+	if (count > 8)
+		return -EINVAL;
+
+	return m24lr_ctl_write_pass(ctl, buf, count, 9);
+}
+
+static ssize_t m24lr_ctl_store(struct device *dev, struct device_attribute *attr,
+			       const char *buf, size_t count)
+{
+	struct m24lr_ctl *ctl = i2c_get_clientdata(to_i2c_client(dev));
+	struct m24lr_sys_entry *entry = to_sys_entry(attr);
+	unsigned int reg_size = entry->reg_size;
+	unsigned int reg_addr = entry->reg_addr;
+	u8 output[8];
+	int err = 0;
+
+	if (unlikely(!is_power_of_2(reg_size) || reg_size > 8)) {
+		dev_dbg(dev,
+			"Invalid register size: must be a power of 2 and <= 8 bytes (%u)\n",
+			reg_size);
+		return -EIO;
+	}
+
+	if (count > (reg_size << 1))
+		return -EINVAL;
+
+	err = m24lr_parse_le_value(buf, reg_size, output);
+	if (err)
+		return err;
+
+	return m24lr_ctl_write(ctl, (u8 *)&output, reg_size, reg_addr);
+}
+
+static ssize_t m24lr_ctl_show(struct device *dev, struct device_attribute *attr,
+			      char *buf)
+{
+	long ret;
+	u64 val;
+	__le64 input = 0;
+	struct m24lr_ctl *ctl = i2c_get_clientdata(to_i2c_client(dev));
+	struct m24lr_sys_entry *entry = to_sys_entry(attr);
+	unsigned int reg_addr = entry->reg_addr;
+	unsigned int reg_size = entry->reg_size;
+
+	if (unlikely(!is_power_of_2(reg_size) || reg_size > 8)) {
+		dev_dbg(dev,
+			"Invalid register size: must be a power of 2 and <= 8 bytes (%u)\n",
+			reg_size);
+		return -EIO;
+	}
+
+	ret = m24lr_ctl_read(ctl, (u8 *)&input, reg_size, reg_addr);
+	if (IS_ERR_VALUE(ret))
+		return ret;
+
+	if (ret != reg_size)
+		return -EIO;
+
+	switch (reg_size) {
+	case 1:
+		val = *(u8 *)&input;
+		break;
+	case 2:
+		val = le16_to_cpu((__le16)input);
+		break;
+	case 4:
+		val = le32_to_cpu((__le32)input);
+		break;
+	case 8:
+		val = le64_to_cpu((__le64)input);
+		break;
+	};
+
+	return sysfs_emit(buf, "%llx\n", val);
+}
+
+static const struct m24lr_ctl_chip *m24lr_ctl_get_chip(struct device *dev)
+{
+	const struct m24lr_ctl_chip *ret;
+	const struct i2c_device_id *id;
+
+	id = i2c_match_id(m24lr_ctl_ids, to_i2c_client(dev));
+
+	if (dev->of_node && of_match_device(m24lr_ctl_of_match, dev))
+		ret = of_device_get_match_data(dev);
+	else if (id)
+		ret = (void *)id->driver_data;
+	else
+		ret = acpi_device_get_match_data(dev);
+
+	return ret;
+}
+
+static int m24lr_ctl_probe(struct i2c_client *client)
+{
+	struct regmap *regmap;
+	struct m24lr_ctl *ctl;
+	struct i2c_mux_core *muxc;
+	const struct m24lr_ctl_chip *chip;
+	struct m24lr_sys_entry *sss = NULL;
+	unsigned int page_size;
+	unsigned int n_sss;
+	long err;
+	u8 test;
+	int i;
+	struct device *dev = &client->dev;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		return -EOPNOTSUPP;
+
+	chip = m24lr_ctl_get_chip(dev);
+	if (!chip)
+		return -ENODEV;
+
+	ctl = devm_kzalloc(dev, sizeof(struct m24lr_ctl), GFP_KERNEL);
+	if (!ctl)
+		return -ENOMEM;
+
+	err = device_property_read_u32(dev, "pagesize", &page_size);
+	if (!err) {
+		if (!is_power_of_2(page_size)) {
+			dev_warn(dev,
+				 "Invalid pagesize length %d (not power of 2); using default %d byte\n",
+				 page_size, M24LR_CTL_PAGESIZE_DEFAULT);
+			page_size = M24LR_CTL_PAGESIZE_DEFAULT;
+		}
+		if (page_size > M24LR_CTL_PAGESIZE_LIMIT) {
+			dev_info(dev,
+				 "pagesize %d exceeds limit; rounded down to %d\n",
+				 page_size, M24LR_CTL_PAGESIZE_LIMIT);
+			page_size = M24LR_CTL_PAGESIZE_LIMIT;
+		}
+	} else { /* use the default */
+		page_size = M24LR_CTL_PAGESIZE_DEFAULT;
+	}
+
+	for (i = 0; i < chip->n_entries; i++) {
+		const struct device_attribute *attr = &chip->entries[i].attr;
+
+		err = device_create_file(dev, attr);
+		if (err)
+			dev_warn(dev,
+				 "Failed to create sysfs entry '%s'\n",
+				 attr->attr.name);
+	}
+
+	n_sss = chip->n_sss_entries;
+	if (n_sss) {
+		sss = devm_kzalloc(dev, n_sss * sizeof(struct m24lr_sys_entry),
+				   GFP_KERNEL);
+		if (!sss)
+			return -ENOMEM;
+
+		for (i = 0; i < n_sss; i++) {
+			char *name = devm_kasprintf(dev, GFP_KERNEL, "sss%d", i);
+
+			sss[i].reg_size = 1;
+			sss[i].reg_addr = i;
+			sss[i].attr.attr.name = name;
+			sss[i].attr.attr.mode = 0600;
+			sss[i].attr.show = m24lr_ctl_show;
+			sss[i].attr.store = m24lr_ctl_store;
+
+			err = device_create_file(dev, &sss[i].attr);
+			if (err)
+				dev_warn(dev,
+					 "Failed to create sysfs entry '%s'\n",
+					 name);
+		}
+	}
+
+	regmap = devm_regmap_init_i2c(client, &m24lr_ctl_regmap_conf);
+	if (IS_ERR(regmap)) {
+		err = PTR_ERR(regmap);
+		dev_dbg(dev, "Failed to init regmap (error: %ld)\n", err);
+		return err;
+	}
+
+	muxc = i2c_mux_alloc(client->adapter, &client->dev, 1, 0, I2C_MUX_GATE,
+			     m24lr_ctl_gate_select, m24lr_ctl_gate_deselect);
+	if (!muxc)
+		return -ENOMEM;
+
+	muxc->priv = ctl;
+
+	mutex_init(&ctl->gate_lock);
+	ctl->page_size = page_size;
+	ctl->regmap = regmap;
+	ctl->muxc = muxc;
+	ctl->n_sss_entries = n_sss;
+	ctl->sss_entries = sss;
+
+	i2c_set_clientdata(client, ctl);
+
+	err = m24lr_ctl_read(ctl, &test, 1, 0);
+	if (IS_ERR_VALUE(err))
+		return -ENODEV;
+
+	return i2c_mux_add_adapter(muxc, 0, 0);
+}
+
+static void remove(struct i2c_client *client)
+{
+	struct m24lr_ctl *ctl = i2c_get_clientdata(client);
+
+	i2c_mux_del_adapters(ctl->muxc);
+}
+
+static struct i2c_driver m24lr_ctl_driver = {
+	.driver = {
+		.name = "m24lr_ctl",
+		.of_match_table = m24lr_ctl_of_match,
+	},
+	.probe    = m24lr_ctl_probe,
+	.remove   = remove,
+	.id_table = m24lr_ctl_ids,
+};
+module_i2c_driver(m24lr_ctl_driver);
+
+MODULE_AUTHOR("Abd-Alrhman Masalkhi");
+MODULE_DESCRIPTION("st m24lr control driver");
+MODULE_LICENSE("GPL");
-- 
2.43.0

[PATCH v2 3/3] ABI: sysfs: document control attributes for ST M24LR

[Abd-Alrhman Masalkhi]

Add documentation for sysfs attributes used by the M24LR control
interface, including unlock, password update, UID, memory size,
and sector security status.

Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
 .../testing/sysfs-bus-i2c-devices-m24lr_ctl   | 96 +++++++++++++++++++
 1 file changed, 96 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c-devices-m24lr_ctl

diff --git a/Documentation/ABI/testing/sysfs-bus-i2c-devices-m24lr_ctl b/Documentation/ABI/testing/sysfs-bus-i2c-devices-m24lr_ctl
new file mode 100644
index 000000000000..50228f9f43f6
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-i2c-devices-m24lr_ctl
@@ -0,0 +1,96 @@
+What:           /sys/bus/i2c/devices/<busnum>-<addr>/unlock
+Date:           2025-05-31
+KernelVersion:  6.16
+Contact:        Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
+Description:
+                Write-only attribute used to present a password and unlock
+                access to protected areas of the M24LR chip, including
+                configuration registers such as the Sector Security Status
+                (SSS) bytes. A valid password must be written to enable write
+                access to these regions via the I2C interface.
+
+                Format:
+                  - Hexadecimal string representing a 32-bit (4-byte) password
+                  - Accepts 1 to 8 hex digits (e.g., "c", "1F", "a1b2c3d4")
+                  - No "0x" prefix, whitespace, or trailing newline
+                  - Case-insensitive
+
+                Behavior:
+                  - If the password matches the internal stored value,
+                    access to protected memory/configuration is granted
+                  - If the password does not match the internally stored value,
+                    it will fail silently
+
+What:           /sys/bus/i2c/devices/<busnum>-<addr>/new_pass
+Date:           2025-05-31
+KernelVersion:  6.16
+Contact:        Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
+Description:
+                Write-only attribute used to update the password required to
+                unlock the M24LR chip.
+
+                Format:
+                  - Hexadecimal string representing a new 32-bit password
+                  - Accepts 1 to 8 hex digits (e.g., "1A", "ffff", "c0ffee00")
+                  - No "0x" prefix, whitespace, or trailing newline
+                  - Case-insensitive
+
+                Behavior:
+                  - Overwrites the current password stored in the I2C password
+                    register
+                  - Requires the device to be unlocked before changing the
+                    password
+                  - If the device is locked, the write silently fails
+
+What:           /sys/bus/i2c/devices/<busnum>-<addr>/uid
+Date:           2025-05-31
+KernelVersion:  6.16
+Contact:        Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
+Description:
+                Read-only attribute that exposes the 8-byte unique identifier
+                programmed into the M24LR chip at the factory.
+
+                Format:
+                  - Lowercase hexadecimal string representing a 64-bit value
+                  - 1 to 16 hex digits (e.g., "e00204f12345678")
+                  - No "0x" prefix
+                  - Includes a trailing newline
+
+What:           /sys/bus/i2c/devices/<busnum>-<addr>/mem_size
+Date:           2025-05-31
+KernelVersion:  6.16
+Contact:        Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
+Description:
+                Read-only attribute that exposes the internal memory size code
+                of the M24LR device, as stored in the system register area.
+
+                Format:
+                  - Unsigned 8-bit integer
+                  - Includes a trailing newline
+
+                Notes:
+                  - Value is encoded by the chip and corresponds to the EEPROM
+                    size (e.g., 3 = 4 kbit for M24LR04E-R)
+
+What:           /sys/bus/i2c/devices/<busnum>-<addr>/sss<N>
+Date:           2025-05-31
+KernelVersion:  6.16
+Contact:        Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
+Description:
+                Read/write attribute representing the Sector Security Status
+                (SSS) byte for EEPROM sector <N> in the M24LR chips. Each sector
+                has one SSS byte, which defines I2c and RF access control via a
+                combination of protection and password settings.
+
+                Format:
+                  - Read: returns a 8-bit hexadecimal value followed by a
+                          newline
+                  - Write: requires exactly one or two hexadecimal digits
+                      - No "0x" prefix, whitespace, or trailing newline
+                      - Case-insensitive
+
+                Notes:
+                  - Refer to the M24LR chip datasheet for full bit definitions
+                    and usage
+                  - Write access requires prior password authentication in I2C
+                    mode
-- 
2.43.0

Re: [PATCH v2 1/3] dt-bindings: eeprom: Add ST M24LR control interface

[Krzysztof Kozlowski]

On 01/06/2025 17:30, Abd-Alrhman Masalkhi wrote:
> Add support for the control interface of STMicroelectronics M24LR
> RFID/NFC EEPROM chips.
> 
> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
> ---
>  .../devicetree/bindings/eeprom/st,m24lr.yaml  | 72 +++++++++++++++++++

Do not send next version while the discussion is still happening.

>  1 file changed, 72 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/eeprom/st,m24lr.yaml

I still do not understand what is this device exactly. You said this is
not EEPROM, so my advice of putting this in eeprom seems not correct.

> 
> diff --git a/Documentation/devicetree/bindings/eeprom/st,m24lr.yaml b/Documentation/devicetree/bindings/eeprom/st,m24lr.yaml
> new file mode 100644
> index 000000000000..6d72325865d6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/eeprom/st,m24lr.yaml
> @@ -0,0 +1,72 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/eeprom/st,m24lr.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: STMicroelectronics M24LR Series NFC/RFID EEPROM Control Interface
> +
> +maintainers:
> +  - Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  STMicroelectronics M24LR series are dual-interface (RF + I2C)
> +  EEPROM chips. These devices support I2C-based access to both
> +  memory and a system area that controls authentication and configuration.
> +  They expose two I2C addresses: one for EEPROM memory and one for the
> +  system control area (e.g., UID, password management).

e.g., -> e.g.

> +
> +allOf:
> +  - $ref: ../i2c/i2c-mux.yaml#

Full path, so /schemas/i2c/i2c-mux.... but this is not an i2c mux, at
least not in your description, so something feels incomplete or incorrect.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - st,m24lr04e-r
> +      - st,m24lr16e-r
> +      - st,m24lr64e-r

What does "r" stand for?

> +
> +  reg:
> +    maxItems: 1
> +
> +  pagesize:
> +    enum: [1, 4, 8, 16, 32, 64, 128, 256]
> +    default: 1
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: >

Do not need '>' unless you need to preserve formatting.

> +      Maximum number of bytes that can be written in one I2C transaction.
> +      the default is 1.

Don't repeat constraints in free form text. I already asked for this.


> +
> +required:
> +  - compatible
> +  - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      i2cmux@57 {

Nothing in commit msg or binding description explained why this is
i2c-mux.  i2c-mux is not an eeprom. Exposing two I2C addresses also does
not mean it is mux - we already have such devices and they were never
called mux.

> +        compatible = "st,m24lr04e-r";
> +        reg = <0x57>;

Where is the second address? It is supposed to be here.

> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        i2c@0 {
> +          reg = <0x0>;
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +
> +          eeprom@53 {
> +            compatible = "atmel,24c04";


And even if there was a mux here, then where is the second device?

> +            reg = <0x53>;
> +            address-width = <16>;
> +            pagesize = <4>;
> +          };
> +        };
> +      };
> +    };
> +...


Best regards,
Krzysztof

Re: [PATCH v2 0/3] Add control driver for ST M24LR RFID/NFC EEPROM chips

[Greg KH]

On Sun, Jun 01, 2025 at 03:30:18PM +0000, Abd-Alrhman Masalkhi wrote:
> This patch series adds support for the control interface of STMicroelectronics 
> M24LR RFID/NFC EEPROM devices, such as the M24LR04E-R. The chip exposes two 
> I2C addresses and two distinct address spaces: one for the system parameter 
> sector and another for the EEPROM memory. These address spaces overlap (both
> start at address 0). Due to this overlap, the chip cannot be represented by a
> single Device Tree node and handled by the standard at24 driver.
> 
> While it might be possible to model the system parameter sector and EEPROM
> as two separate DT nodes, each handled by the standard at24 driver, this
> leads to synchronization problems. For example, if a multi-page EEPROM write
> is in progress and a control command (e.g., a i2c write lock) is issued
> concurrently, it can result in incomplete or inconsistent writeing.
> 
> To overcome these issues, while maintaining a clean architecture and ensuring
> synchronization, the chip is managed by two drivers:
> 
> 1- The first driver exposes the system parameter sector and acts as an I2C mux
>    (gate) for EEPROM access. Using an I2C mux solves the synchronization issue.
>    This patch series implements this driver.
> 2- The second is the standard at24 driver, which handles the EEPROM.
> 
> Tested on: M24LR04E-R using Yocto on Raspberry Pi 4
> 
> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
> 
> Abd-Alrhman Masalkhi (3):
>   dt-bindings: eeprom: Add ST M24LR control interface
>   misc: add sysfs control driver for ST M24LR series RFID/NFC chips
>   ABI: sysfs: document control attributes for ST M24LR
> 
>  .../testing/sysfs-bus-i2c-devices-m24lr_ctl   |  96 +++
>  .../devicetree/bindings/eeprom/st,m24lr.yaml  |  72 ++
>  drivers/misc/Kconfig                          |  15 +
>  drivers/misc/Makefile                         |   1 +
>  drivers/misc/m24lr_ctl.c                      | 677 ++++++++++++++++++
>  5 files changed, 861 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-i2c-devices-m24lr_ctl
>  create mode 100644 Documentation/devicetree/bindings/eeprom/st,m24lr.yaml
>  create mode 100644 drivers/misc/m24lr_ctl.c
> 
> -- 
> 2.43.0
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- This looks like a new version of a previously submitted patch, but you
  did not list below the --- line any changes from the previous version.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/process/submitting-patches.rst for what
  needs to be done here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

Re: [PATCH v2 1/3] dt-bindings: eeprom: Add ST M24LR control interface

[Rob Herring (Arm)]

On Sun, 01 Jun 2025 15:30:19 +0000, Abd-Alrhman Masalkhi wrote:
> Add support for the control interface of STMicroelectronics M24LR
> RFID/NFC EEPROM chips.
> 
> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
> ---
>  .../devicetree/bindings/eeprom/st,m24lr.yaml  | 72 +++++++++++++++++++
>  1 file changed, 72 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/eeprom/st,m24lr.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/eeprom/st,m24lr.example.dtb: eeprom@53 (atmel,24c04): pagesize: 4 is not one of [1, 8, 16, 32, 64, 128, 256]
	from schema $id: http://devicetree.org/schemas/eeprom/at24.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/eeprom/st,m24lr.example.dtb: eeprom@53 (atmel,24c04): Unevaluated properties are not allowed ('pagesize' was unexpected)
	from schema $id: http://devicetree.org/schemas/eeprom/at24.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250601153022.2027919-2-abd.masalkhi@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v2 1/3] dt-bindings: Add Device Tree binding for ST M24LR control interface

[Abd-Alrhman Masalkhi]

Hi Krzysztof,

Thank you for the detailed feedback.

> Do not send next version while the discussion is still happening.

You're right, I sent the updated version too early while discussion was
still ongoing. I'll hold off on sending further revisions until the current
points are fully resolved.

> Full path, so /schemas/i2c/i2c-mux.... but this is not an i2c mux, at
> least not in your description, so something feels incomplete or incorrect.

A Brief Overview of the Device:

The M24LR series is a dual-interface EEPROM with both I2C and ISO/IEC 15693
RF support. While it is technically an EEPROM, it also exposes a control
interface over I2C via a second address, which is used to manage features
such as password protection, energy harvesting configuration, and UID access.
This secondary interface is not memory-mapped like traditional EEPROMs, which
is why I initially considered separating the control aspect in the software.

How to Access the EEPROM and the System Parameter Sector?

According to the datasheet for the M24LR04E-R, the E2 bit must be set
appropriately in the I2C device select code to distinguish between EEPROM
access and control access.

What is E2?
E2 is a bit in the I2C device select code. It determines which internal
function of the chip is being accessed.

Device Select Code Format:
Bit:                b7  b6  b5  b4  b3  b2  b1  b0
Value:              1   0   1   0   E2  1   1   R/W

To access the EEPROM memory, E2 (b3) should be 0:

Device Select Code Format:
Bit:                b7  b6  b5  b4  b3  b2  b1  b0
Value:              1   0   1   0   0  1   1   R/W

To access the system control interface, E2 (b3) should be 1:

Device Select Code Format:
Bit:                b7  b6  b5  b4  b3  b2  b1  b0
Value:              1   0   1   0   0  1   1   R/W

Is This a Gate?
Correct me if I'm wrong, but to me this behavior resembles a form of gate,
instead of using a separate hardware pin to access the EEPROM, the chip
encodes this selection in the I2C device address. However, the datasheet
does not explicitly mention anything about a "gate" or "mux," which is
why I've been careful to not label it as an I2C gate in the binding.

That said, I see it as a kind of implicit I2C mux (of type gate), where
the chip use the 0x57 address as in the example and to select the internal
EEPROM we just reset the b3 in the device select code.

Why This View Matters in my driver design:

Looking at the device from this perspective has helped me keep the driver
design cleaner while keeping the synchronization issue in mind:

1- Avoiding code duplication (such as rewriting parts of the at24 driver).
2- Ensuring concurrent access to EEPROM and control areas is properly
   handled and isolated.
3- Representing the dual-role nature of the chip more explicitly.

> What does "r" stand for?

The r in st,m24lr04e-r stands for RF, these are the RF-enabled variants
of the M24LR series, as specified by STMicroelectronics.

> Do not need '>' unless you need to preserve formatting.

I'll remove the | and > where formatting preservation is not needed.

> Don't repeat constraints in free form text. I already asked for this.

I'll avoid repeating constraints in free-form text.

> Where is the second address? It is supposed to be here.

>> +          #size-cells = <0>;
>> +
>> +          eeprom@53 {
>> +            compatible = "atmel,24c04";
>> +            reg = <0x53>;

Here is the second address is 0x53, to select the EEPROM 

I appreciate your feedback.

Best regards,
Abd-Alrhman Masalkhi

Re: [PATCH v2 1/3] dt-bindings: Add Device Tree binding for ST M24LR control interface

[Krzysztof Kozlowski]

On 02/06/2025 05:48, Abd-Alrhman Masalkhi wrote:
> Hi Krzysztof,
> 
> Thank you for the detailed feedback.
> 
>> Do not send next version while the discussion is still happening.
> 
> You're right, I sent the updated version too early while discussion was
> still ongoing. I'll hold off on sending further revisions until the current
> points are fully resolved.
> 
>> Full path, so /schemas/i2c/i2c-mux.... but this is not an i2c mux, at
>> least not in your description, so something feels incomplete or incorrect.
> 
> A Brief Overview of the Device:
> 
> The M24LR series is a dual-interface EEPROM with both I2C and ISO/IEC 15693
> RF support. While it is technically an EEPROM, it also exposes a control
> interface over I2C via a second address, which is used to manage features
> such as password protection, energy harvesting configuration, and UID access.
> This secondary interface is not memory-mapped like traditional EEPROMs, which
> is why I initially considered separating the control aspect in the software.
> 
> How to Access the EEPROM and the System Parameter Sector?
> 
> According to the datasheet for the M24LR04E-R, the E2 bit must be set
> appropriately in the I2C device select code to distinguish between EEPROM
> access and control access.
> 
> What is E2?
> E2 is a bit in the I2C device select code. It determines which internal
> function of the chip is being accessed.
> 
> Device Select Code Format:
> Bit:                b7  b6  b5  b4  b3  b2  b1  b0
> Value:              1   0   1   0   E2  1   1   R/W
> 
> To access the EEPROM memory, E2 (b3) should be 0:
> 
> Device Select Code Format:
> Bit:                b7  b6  b5  b4  b3  b2  b1  b0
> Value:              1   0   1   0   0  1   1   R/W
> 
> To access the system control interface, E2 (b3) should be 1:

So these are just two different addresses. I already commented on this.
This is not I2C mux but a device with two addresses.

> 
> Device Select Code Format:
> Bit:                b7  b6  b5  b4  b3  b2  b1  b0
> Value:              1   0   1   0   0  1   1   R/W
> 
> Is This a Gate?
> Correct me if I'm wrong, but to me this behavior resembles a form of gate,
> instead of using a separate hardware pin to access the EEPROM, the chip
> encodes this selection in the I2C device address. However, the datasheet
> does not explicitly mention anything about a "gate" or "mux," which is
> why I've been careful to not label it as an I2C gate in the binding.
> 
> That said, I see it as a kind of implicit I2C mux (of type gate), where
> the chip use the 0x57 address as in the example and to select the internal
> EEPROM we just reset the b3 in the device select code.
> 
> Why This View Matters in my driver design:

We do not talk here about driver design but bindings.

> 
> Looking at the device from this perspective has helped me keep the driver
> design cleaner while keeping the synchronization issue in mind:
> 
> 1- Avoiding code duplication (such as rewriting parts of the at24 driver).
> 2- Ensuring concurrent access to EEPROM and control areas is properly
>    handled and isolated.
> 3- Representing the dual-role nature of the chip more explicitly.
> 
>> What does "r" stand for?
> 
> The r in st,m24lr04e-r stands for RF, these are the RF-enabled variants
> of the M24LR series, as specified by STMicroelectronics.

It's fine.

> 
>> Do not need '>' unless you need to preserve formatting.
> 
> I'll remove the | and > where formatting preservation is not needed.

Respond inline, not by removing entire context.

Best regards,
Krzysztof

Re: [PATCH v2 1/3] dt-bindings: Add Device Tree binding for ST M24LR control interface

[Abd-Alrhman Masalkhi]

Hi Krzysztof,

Thank you again for yourfeedback, it's been very helpful in
understanding how best to write the binding file represent
this device.

>> A Brief Overview of the Device:
>> 
>> The M24LR series is a dual-interface EEPROM with both I2C and ISO/IEC 15693
>> RF support. While it is technically an EEPROM, it also exposes a control
>> interface over I2C via a second address, which is used to manage features
>> such as password protection, energy harvesting configuration, and UID access.
>> This secondary interface is not memory-mapped like traditional EEPROMs, which
>> is why I initially considered separating the control aspect in the software.
>> 
>> How to Access the EEPROM and the System Parameter Sector?
>> 
>> According to the datasheet for the M24LR04E-R, the E2 bit must be set
>> appropriately in the I2C device select code to distinguish between EEPROM
>> access and control access.
>> 
>> What is E2?
>> E2 is a bit in the I2C device select code. It determines which internal
>> function of the chip is being accessed.
>> 
>> Device Select Code Format:
>> Bit:                b7  b6  b5  b4  b3  b2  b1  b0
>> Value:              1   0   1   0   E2  1   1   R/W
>> 
>> To access the EEPROM memory, E2 (b3) should be 0:
>> 
>> Device Select Code Format:
>> Bit:                b7  b6  b5  b4  b3  b2  b1  b0
>> Value:              1   0   1   0   0  1   1   R/W
>> 
>> To access the system control interface, E2 (b3) should be 1:
>
> So these are just two different addresses. I already commented on this.
> This is not I2C mux but a device with two addresses.

I'd like to clarify one point before proceeding further, If I remove
the i2c-mux usage from the binding file, as your recommendation, does
this also imply that I should stop using the i2c-mux API in my driver
implementation itself? and treat this as a multi-function device, or
Is it acceptable to keep the current driver logic as-is, while limiting
the change to just fixing the binding and device tree representation?

and if only the binding file, should i represent it as the following:

  eeprom-ctl@57 {
  #address-cells = <1>;
  #size-cells = <0>;
  compatible = "st,m24lr04e-r";
  reg = <0x57>;

  }

Best regards,
Abd-Alrhman Masalkhi

Re: [PATCH v2 1/3] dt-bindings: Add Device Tree binding for ST M24LR control interface

[Abd-Alrhman Masalkhi]

Hi Krzysztof,

>> Is it acceptable to keep the current driver logic as-is, while limiting
>> the change to just fixing the binding and device tree representation?
>
> Dunno, did not look at your driver. Again, I am here talking about
> bindings. How you implement it in the driver is up to you. You have
> however one device with two addresses, so you should most likely have
> only one device node in DTS with two addresses. In certain cases, like
> separate resources and re-usability, children are common, but that was
> not really the case here.
>
> Look at ti,pcm6240.yaml, ti,tas2781.yaml and few others (you can git
> grep for them for reg with coma).

Okay, I'll rework the pathcs to use a single device node with two
addresses instead of modeling it as an I2C mux. Thanks also for the
hint about ti,pcm6240.yaml and ti,tas2781.yaml, I'll review those
to make sure the structure aligns properly.

I appreciate your guidance.

Best regards,
Abd-Alrhman Masalkhi

Re: [PATCH v2 1/3] dt-bindings: Add Device Tree binding for ST M24LR control interface

[Krzysztof Kozlowski]

On 02/06/2025 19:23, Abd-Alrhman Masalkhi wrote:
>>>
>>> Device Select Code Format:
>>> Bit:                b7  b6  b5  b4  b3  b2  b1  b0
>>> Value:              1   0   1   0   E2  1   1   R/W
>>>
>>> To access the EEPROM memory, E2 (b3) should be 0:
>>>
>>> Device Select Code Format:
>>> Bit:                b7  b6  b5  b4  b3  b2  b1  b0
>>> Value:              1   0   1   0   0  1   1   R/W
>>>
>>> To access the system control interface, E2 (b3) should be 1:
>>
>> So these are just two different addresses. I already commented on this.
>> This is not I2C mux but a device with two addresses.
> 
> I'd like to clarify one point before proceeding further, If I remove
> the i2c-mux usage from the binding file, as your recommendation, does
> this also imply that I should stop using the i2c-mux API in my driver
> implementation itself? and treat this as a multi-function device, or


Not using i2c-mux schema means only that you cannot use i2c-mux
properties. Maybe this implies that you cannot use i2c-mux API, feels
likely, but not really sure and I don't want to take responsibility for
that decision.

I am commenting here about bindings, not the drivers. Drivers can
implement binding in many different ways, thus discussion about bindings
is independent, except narrowing/restricting the ABI.


> Is it acceptable to keep the current driver logic as-is, while limiting
> the change to just fixing the binding and device tree representation?

Dunno, did not look at your driver. Again, I am here talking about
bindings. How you implement it in the driver is up to you. You have
however one device with two addresses, so you should most likely have
only one device node in DTS with two addresses. In certain cases, like
separate resources and re-usability, children are common, but that was
not really the case here.

Look at ti,pcm6240.yaml, ti,tas2781.yaml and few others (you can git
grep for them for reg with coma).

> 
> and if only the binding file, should i represent it as the following:
> 
>   eeprom-ctl@57 {
>   #address-cells = <1>;
>   #size-cells = <0>;

These two are obviously not used.

>   compatible = "st,m24lr04e-r";
>   reg = <0x57>;

That's only one address, you need two.


> 
>   }
> 
> Best regards,
> Abd-Alrhman Masalkhi


Best regards,
Krzysztof

Re: [PATCH v2 1/3] dt-bindings: eeprom: Add ST M24LR control interface

[Abd-Alrhman Masalkhi]

Hi Rob,

Thanks for the feedback.

> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/eeprom/st,m24lr.example.dtb: eeprom@53 > (atmel,24c04): pagesize: 4 is not one of [1, 8, 16, 32, 64, 128, 256]
> 	from schema $id: http://devicetree.org/schemas/eeprom/at24.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/eeprom/st,m24lr.example.dtb: eeprom@53 > (atmel,24c04): Unevaluated properties are not allowed ('pagesize' was unexpected)
> 	from schema $id: http://devicetree.org/schemas/eeprom/at24.yaml#
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250601153022.2027919-2-abd.masalkhi@gmail.com
> 
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade

I had already updated dtschema to the latest version and ensured that
yamllint was installed, as advised in the previous version. i have
executed 'make dt_binding_check' as you guided me, but still it did not
emit any errors.

Best regards,
Abd-Alrhman Masalkhi

Re: [PATCH v2 1/3] dt-bindings: eeprom: Add ST M24LR control interface

[Krzysztof Kozlowski]

On 03/06/2025 07:40, Abd-Alrhman Masalkhi wrote:
> Hi Rob,
> 
> Thanks for the feedback.
> 
>> yamllint warnings/errors:
>>
>> dtschema/dtc warnings/errors:
>> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/eeprom/st,m24lr.example.dtb: eeprom@53 > (atmel,24c04): pagesize: 4 is not one of [1, 8, 16, 32, 64, 128, 256]
>> 	from schema $id: http://devicetree.org/schemas/eeprom/at24.yaml#
>> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/eeprom/st,m24lr.example.dtb: eeprom@53 > (atmel,24c04): Unevaluated properties are not allowed ('pagesize' was unexpected)
>> 	from schema $id: http://devicetree.org/schemas/eeprom/at24.yaml#
>>
>> doc reference errors (make refcheckdocs):
>>
>> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250601153022.2027919-2-abd.masalkhi@gmail.com
>>
>> The base for the series is generally the latest rc1. A different dependency
>> should be noted in *this* patch.
>>
>> If you already ran 'make dt_binding_check' and didn't see the above
>> error(s), then make sure 'yamllint' is installed and dt-schema is up to
>> date:
>>
>> pip3 install dtschema --upgrade
> 
> I had already updated dtschema to the latest version and ensured that
> yamllint was installed, as advised in the previous version. i have
> executed 'make dt_binding_check' as you guided me, but still it did not
> emit any errors.

I doubt that you run it exactly the way as describe here, because you
have obvious error in binding which is nicely reported here - you put
pagesize to wrong node.

Best regards,
Krzysztof