devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Add control driver for ST M24LR RFID/NFC EEPROM chips
@ 2025-05-31  8:11 Abd-Alrhman Masalkhi
  2025-05-31  8:11 ` [PATCH 1/3] dt-bindings: misc: Add binding for ST M24LR control interface Abd-Alrhman Masalkhi
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Abd-Alrhman Masalkhi @ 2025-05-31  8:11 UTC (permalink / raw)
  To: linux-kernel, devicetree
  Cc: robh, krzk+dt, conor+dt, arnd, gregkh, abd.masalkhi

This patch series adds support for the control interface of
STMicroelectronics M24LR RFID/NFC EEPROM devices, such as M24LR04E-R.

The device exposes two I2C addresses: one for the control interface
and another for EEPROM memory. To support this design, the driver
acts as an I2C mux (gate), exposing the EEPROM as a child node
handled by the standard at24 driver. Using the mux not only enables
clean separation of functions but also allows synchronize access to
the device.

Patches:
  - Patch 1: Adds Device Tree binding for the control interface.
  - Patch 2: Adds the sysfs-based control driver.
  - Patch 3: Adds a MAINTAINERS entry for the driver.

Tested on: m24lr04e-r 

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

Abd-Alrhman Masalkhi (3):
  dt-bindings: misc: Add binding for ST M24LR control interface
  misc: add sysfs control driver for ST M24LR series RFID/NFC chips
  MAINTAINERS: Add entry for ST M24LR control driver

 .../devicetree/bindings/misc/st,m24lr.yaml    |  70 ++
 MAINTAINERS                                   |   8 +
 drivers/misc/Kconfig                          |  15 +
 drivers/misc/Makefile                         |   1 +
 drivers/misc/m24lr_ctl.c                      | 677 ++++++++++++++++++
 5 files changed, 771 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/st,m24lr.yaml
 create mode 100644 drivers/misc/m24lr_ctl.c

-- 
2.43.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] dt-bindings: misc: Add binding for ST M24LR control interface
  2025-05-31  8:11 [PATCH 0/3] Add control driver for ST M24LR RFID/NFC EEPROM chips Abd-Alrhman Masalkhi
@ 2025-05-31  8:11 ` Abd-Alrhman Masalkhi
  2025-05-31  9:25   ` Rob Herring (Arm)
  2025-05-31 13:37   ` Krzysztof Kozlowski
  2025-05-31  8:11 ` [PATCH 2/3] misc: add sysfs control driver for ST M24LR series RFID/NFC chips Abd-Alrhman Masalkhi
  2025-05-31  8:11 ` [PATCH 3/3] MAINTAINERS: Add entry for ST M24LR control driver Abd-Alrhman Masalkhi
  2 siblings, 2 replies; 12+ messages in thread
From: Abd-Alrhman Masalkhi @ 2025-05-31  8:11 UTC (permalink / raw)
  To: linux-kernel, devicetree
  Cc: robh, krzk+dt, conor+dt, arnd, gregkh, abd.masalkhi

Add a Device Tree binding for the STMicroelectronics M24LR series
RFID/NFC EEPROM chips (e.g., M24LR04E-R), which support a separate
I2C interface for control and configuration.

This binding documents the control interface that is managed by
a dedicated driver exposing sysfs attributes. The EEPROM memory
interface is handled by the standard 'at24' driver and is
represented as a child node in the Device Tree.

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

diff --git a/Documentation/devicetree/bindings/misc/st,m24lr.yaml b/Documentation/devicetree/bindings/misc/st,m24lr.yaml
new file mode 100644
index 000000000000..5a8f5aef13ec
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/st,m24lr.yaml
@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/misc/st,m24lr.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STMicroelectronics M24LR Series NFC/RFID EEPROM Control Interface
+
+maintainers:
+  - name: Abd-Alrhman Masalkhi
+    email: abd.masalkhi@gmail.com
+
+description: |
+  This binding describes the control interface for STMicroelectronics
+  M24LR series RFID/NFC EEPROM chips (e.g., M24LR04E-R, M24LR16E-R).
+  This driver provides sysfs access to device-specific control registers
+  (authentication, UID, etc.) over the I2C interface. It act as a
+  I2C gate for the EEPROM. Therefore, The EEPROM is represented as a
+  child node under a port and is accessed through a separate driver
+  (the standard 'at24' driver). This implementation is possible because
+  the M24LR chips uses two I2C addresses: one for accessing the
+  system parameter sector and another for the EEPROM.
+
+allOf:
+  - $ref: "i2c-mux.yaml#"
+
+properties:
+  compatible:
+    enum:
+    - st,m24lr04e-r
+    - st,m24lr16e-r
+    - st,m24lr64e-r
+
+  reg:
+    maxItems: 1
+    description: I2C address of the device.
+
+  pagesize:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: >
+      Maximum number of bytes that can be written in a single I2C
+      transaction. the default is 1.
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      m24lr@57 {
+        compatible = "st,m24lr04e-r";
+        reg = <0x57>;
+
+        i2c-gate {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          m24lr_eeprom@53 {
+            compatible = "atmel,24c04";
+            reg = <0x53>;
+            address-width = <16>;
+            pagesize = <4>;
+          };
+        };
+      };
+    };
+...
\ No newline at end of file
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] misc: add sysfs control driver for ST M24LR series RFID/NFC chips
  2025-05-31  8:11 [PATCH 0/3] Add control driver for ST M24LR RFID/NFC EEPROM chips Abd-Alrhman Masalkhi
  2025-05-31  8:11 ` [PATCH 1/3] dt-bindings: misc: Add binding for ST M24LR control interface Abd-Alrhman Masalkhi
@ 2025-05-31  8:11 ` Abd-Alrhman Masalkhi
  2025-05-31  8:24   ` Greg KH
                     ` (3 more replies)
  2025-05-31  8:11 ` [PATCH 3/3] MAINTAINERS: Add entry for ST M24LR control driver Abd-Alrhman Masalkhi
  2 siblings, 4 replies; 12+ messages in thread
From: Abd-Alrhman Masalkhi @ 2025-05-31  8:11 UTC (permalink / raw)
  To: linux-kernel, devicetree
  Cc: robh, krzk+dt, conor+dt, arnd, gregkh, abd.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..c854ccc49811
--- /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 m24lr04e_r_ctl_unlock_store(struct device *dev,
+					   struct device_attribute *attr,
+					   const char *buf, size_t count);
+static ssize_t m24lr04e_r_ctl_newpass_store(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, m24lr04e_r_ctl_unlock_store)},
+	{.attr = __ATTR(new_pass, 0200, NULL, m24lr04e_r_ctl_newpass_store)},
+	{.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;
+	int 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;
+}
+
+/**
+ * m24lr04e_r_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 m24lr04e_r_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 m24lr04e_r_ctl_newpass_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));
+
+	return m24lr04e_r_ctl_write_pass(ctl, buf, count, 7);
+}
+
+static ssize_t m24lr04e_r_ctl_unlock_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));
+
+	return m24lr04e_r_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_err(dev,
+			"Invalid register size: must be a power of 2 and <= 8 bytes (%u)\n",
+			reg_size);
+		return -EIO;
+	}
+
+	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)
+{
+	int 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 scnprintf(buf, PAGE_SIZE, "%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,
+			   const struct i2c_device_id *id)
+{
+	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;
+	int i, err;
+	u8 test;
+	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 lenngth %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, err);
+	}
+
+	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 = kasprintf(GFP_KERNEL, "sss%02d", 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, err);
+		}
+	}
+
+	regmap = devm_regmap_init_i2c(client, &m24lr_ctl_regmap_conf);
+	if (IS_ERR(regmap)) {
+		err = PTR_ERR(regmap);
+		dev_err(dev, "Failed to init regmap (error: %d)\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;
+
+	err = i2c_mux_add_adapter(muxc, 0, 0, 0);
+	if (err)
+		return err;
+
+	dev_info(&client->dev, "control interface initialized for %s\n",
+		 client->name);
+
+	return 0;
+}
+
+static int remove(struct i2c_client *client)
+{
+	struct m24lr_ctl *ctl = i2c_get_clientdata(client);
+
+	i2c_mux_del_adapters(ctl->muxc);
+
+	return 0;
+}
+
+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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] MAINTAINERS: Add entry for ST M24LR control driver
  2025-05-31  8:11 [PATCH 0/3] Add control driver for ST M24LR RFID/NFC EEPROM chips Abd-Alrhman Masalkhi
  2025-05-31  8:11 ` [PATCH 1/3] dt-bindings: misc: Add binding for ST M24LR control interface Abd-Alrhman Masalkhi
  2025-05-31  8:11 ` [PATCH 2/3] misc: add sysfs control driver for ST M24LR series RFID/NFC chips Abd-Alrhman Masalkhi
@ 2025-05-31  8:11 ` Abd-Alrhman Masalkhi
  2025-05-31 13:37   ` Krzysztof Kozlowski
  2 siblings, 1 reply; 12+ messages in thread
From: Abd-Alrhman Masalkhi @ 2025-05-31  8:11 UTC (permalink / raw)
  To: linux-kernel, devicetree
  Cc: robh, krzk+dt, conor+dt, arnd, gregkh, abd.masalkhi

Add a MAINTAINERS entry for the newly introduced sysfs control driver
supporting STMicroelectronics M24LR series RFID/NFC EEPROM chips.

This entry includes the driver source, Device Tree binding, and assigns
maintainership to the original contributor.

Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index eb11c6f57500..f08975ac4d9f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23017,6 +23017,14 @@ W:	http://www.st.com/
 F:	Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml
 F:	drivers/iio/imu/st_lsm6dsx/
 
+ST M24LR CONTROL DRIVER
+M:	Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
+L:	linux-kernel@vger.kernel.org
+L:	devicetree@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/misc/st,m24lr.yaml
+F:	drivers/misc/m24lr_ctl.c
+
 ST MIPID02 CSI-2 TO PARALLEL BRIDGE DRIVER
 M:	Benjamin Mugnier <benjamin.mugnier@foss.st.com>
 M:	Sylvain Petinot <sylvain.petinot@foss.st.com>
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] misc: add sysfs control driver for ST M24LR series RFID/NFC chips
  2025-05-31  8:11 ` [PATCH 2/3] misc: add sysfs control driver for ST M24LR series RFID/NFC chips Abd-Alrhman Masalkhi
@ 2025-05-31  8:24   ` Greg KH
  2025-05-31  8:25   ` Greg KH
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Greg KH @ 2025-05-31  8:24 UTC (permalink / raw)
  To: Abd-Alrhman Masalkhi
  Cc: linux-kernel, devicetree, robh, krzk+dt, conor+dt, arnd

On Sat, May 31, 2025 at 08:11:58AM +0000, Abd-Alrhman Masalkhi wrote:
> 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

If you are adding sysfs files, you must have Documentation/ABI/ entries
to describe this new user/kernel api that you are creating and must
support for the next 20+ years, and to give us a hint as to how to
review this driver properly.

One minor comment below:

> +	err = i2c_mux_add_adapter(muxc, 0, 0, 0);
> +	if (err)
> +		return err;
> +
> +	dev_info(&client->dev, "control interface initialized for %s\n",
> +		 client->name);

When drivers work properly, they are quiet, no need to spam the kernel
log for a correctly working system.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] misc: add sysfs control driver for ST M24LR series RFID/NFC chips
  2025-05-31  8:11 ` [PATCH 2/3] misc: add sysfs control driver for ST M24LR series RFID/NFC chips Abd-Alrhman Masalkhi
  2025-05-31  8:24   ` Greg KH
@ 2025-05-31  8:25   ` Greg KH
  2025-06-01  2:19   ` kernel test robot
  2025-06-01  3:21   ` kernel test robot
  3 siblings, 0 replies; 12+ messages in thread
From: Greg KH @ 2025-05-31  8:25 UTC (permalink / raw)
  To: Abd-Alrhman Masalkhi
  Cc: linux-kernel, devicetree, robh, krzk+dt, conor+dt, arnd

On Sat, May 31, 2025 at 08:11:58AM +0000, Abd-Alrhman Masalkhi wrote:
> +	return scnprintf(buf, PAGE_SIZE, "%llx\n", val);

Please use sysfs_emit() for writing data out in sysfs show callbacks.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] dt-bindings: misc: Add binding for ST M24LR control interface
  2025-05-31  8:11 ` [PATCH 1/3] dt-bindings: misc: Add binding for ST M24LR control interface Abd-Alrhman Masalkhi
@ 2025-05-31  9:25   ` Rob Herring (Arm)
  2025-05-31 13:37   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 12+ messages in thread
From: Rob Herring (Arm) @ 2025-05-31  9:25 UTC (permalink / raw)
  To: Abd-Alrhman Masalkhi
  Cc: devicetree, krzk+dt, conor+dt, linux-kernel, gregkh, arnd


On Sat, 31 May 2025 08:11:57 +0000, Abd-Alrhman Masalkhi wrote:
> Add a Device Tree binding for the STMicroelectronics M24LR series
> RFID/NFC EEPROM chips (e.g., M24LR04E-R), which support a separate
> I2C interface for control and configuration.
> 
> This binding documents the control interface that is managed by
> a dedicated driver exposing sysfs attributes. The EEPROM memory
> interface is handled by the standard 'at24' driver and is
> represented as a child node in the Device Tree.
> 
> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
> ---
>  .../devicetree/bindings/misc/st,m24lr.yaml    | 70 +++++++++++++++++++
>  1 file changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/st,m24lr.yaml
> 

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

yamllint warnings/errors:
./Documentation/devicetree/bindings/misc/st,m24lr.yaml:25:11: [error] string value is redundantly quoted with any quotes (quoted-strings)
./Documentation/devicetree/bindings/misc/st,m24lr.yaml:30:5: [warning] wrong indentation: expected 6 but found 4 (indentation)
./Documentation/devicetree/bindings/misc/st,m24lr.yaml:70:4: [error] no new line character at the end of file (new-line-at-end-of-file)

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/misc/st,m24lr.yaml: maintainers:0: {'name': 'Abd-Alrhman Masalkhi', 'email': 'abd.masalkhi@gmail.com'} is not of type 'string'
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/misc/st,m24lr.yaml:
	Error in referenced schema matching $id: http://devicetree.org/schemas/misc/i2c-mux.yaml
	Tried these paths (check schema $id if path is wrong):
	/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/misc/i2c-mux.yaml
	/usr/local/lib/python3.11/dist-packages/dtschema/schemas/misc/i2c-mux.yaml
Documentation/devicetree/bindings/misc/st,m24lr.example.dts:21.13-26: Warning (reg_format): /example-0/i2c/m24lr@57:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
Documentation/devicetree/bindings/misc/st,m24lr.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/misc/st,m24lr.example.dtb: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/misc/st,m24lr.example.dtb: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/misc/st,m24lr.example.dts:18.13-35.11: Warning (i2c_bus_bridge): /example-0/i2c: incorrect #address-cells for I2C bus
Documentation/devicetree/bindings/misc/st,m24lr.example.dts:18.13-35.11: Warning (i2c_bus_bridge): /example-0/i2c: incorrect #size-cells for I2C bus
Documentation/devicetree/bindings/misc/st,m24lr.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/misc/st,m24lr.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'i2c_bus_bridge'
Documentation/devicetree/bindings/misc/st,m24lr.example.dtb: Warning (spi_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/misc/st,m24lr.example.dts:19.20-34.13: Warning (avoid_default_addr_size): /example-0/i2c/m24lr@57: Relying on default #address-cells value
Documentation/devicetree/bindings/misc/st,m24lr.example.dts:19.20-34.13: Warning (avoid_default_addr_size): /example-0/i2c/m24lr@57: Relying on default #size-cells value
Documentation/devicetree/bindings/misc/st,m24lr.example.dtb: Warning (unique_unit_address_if_enabled): Failed prerequisite 'avoid_default_addr_size'
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/misc/st,m24lr.example.dtb: m24lr@57 (st,m24lr04e-r): 'i2c-gate' does not match any of the regexes: '^pinctrl-[0-9]+$'
	from schema $id: http://devicetree.org/schemas/misc/st,m24lr.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/misc/st,m24lr.example.dtb: m24lr@57 (st,m24lr04e-r): {'compatible': ['st,m24lr04e-r'], 'reg': [[87]], 'i2c-gate': {'#address-cells': 1, '#size-cells': 0, 'm24lr_eeprom@53': {'compatible': ['atmel,24c04'], 'reg': [[83]], 'address-width': 16, 'pagesize': 4}}, '$nodename': ['m24lr@57']} should not be valid under {'description': "Can't find referenced schema: http://devicetree.org/schemas/misc/i2c-mux.yaml#"}
	from schema $id: http://devicetree.org/schemas/misc/st,m24lr.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/misc/st,m24lr.example.dtb: m24lr_eeprom@53 (atmel,24c04): $nodename:0: 'm24lr_eeprom@53' does not match '^eeprom@[0-9a-f]{1,2}$'
	from schema $id: http://devicetree.org/schemas/eeprom/at24.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/misc/st,m24lr.example.dtb: m24lr_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/misc/st,m24lr.example.dtb: m24lr_eeprom@53 (atmel,24c04): Unevaluated properties are not allowed ('$nodename', 'pagesize' were 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/20250531081159.2007319-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.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] dt-bindings: misc: Add binding for ST M24LR control interface
  2025-05-31  8:11 ` [PATCH 1/3] dt-bindings: misc: Add binding for ST M24LR control interface Abd-Alrhman Masalkhi
  2025-05-31  9:25   ` Rob Herring (Arm)
@ 2025-05-31 13:37   ` Krzysztof Kozlowski
  2025-06-01  7:31     ` [PATCH v2 1/3] dt-bindings: Add Device Tree " Abd-Alrhman Masalkhi
  1 sibling, 1 reply; 12+ messages in thread
From: Krzysztof Kozlowski @ 2025-05-31 13:37 UTC (permalink / raw)
  To: Abd-Alrhman Masalkhi, linux-kernel, devicetree
  Cc: robh, krzk+dt, conor+dt, arnd, gregkh

On 31/05/2025 10:11, Abd-Alrhman Masalkhi wrote:
> Add a Device Tree binding for the STMicroelectronics M24LR series
> RFID/NFC EEPROM chips (e.g., M24LR04E-R), which support a separate
> I2C interface for control and configuration.
> 
> This binding documents the control interface that is managed by
> a dedicated driver exposing sysfs attributes. The EEPROM memory
> interface is handled by the standard 'at24' driver and is
> represented as a child node in the Device Tree.
> 
> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
> ---
>  .../devicetree/bindings/misc/st,m24lr.yaml    | 70 +++++++++++++++++++

That's not a misc device, but eeprom. Place it in appropriate directory.

A nit, subject: drop second/last, redundant "binding for". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18


>  1 file changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/st,m24lr.yaml
> 
> diff --git a/Documentation/devicetree/bindings/misc/st,m24lr.yaml b/Documentation/devicetree/bindings/misc/st,m24lr.yaml
> new file mode 100644
> index 000000000000..5a8f5aef13ec
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/st,m24lr.yaml
> @@ -0,0 +1,70 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/misc/st,m24lr.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: STMicroelectronics M24LR Series NFC/RFID EEPROM Control Interface
> +
> +maintainers:
> +  - name: Abd-Alrhman Masalkhi
> +    email: abd.masalkhi@gmail.com
> +
> +description: |
> +  This binding describes the control interface for STMicroelectronics

Describe the hardware, not the binding.

> +  M24LR series RFID/NFC EEPROM chips (e.g., M24LR04E-R, M24LR16E-R).
> +  This driver provides sysfs access to device-specific control registers
> +  (authentication, UID, etc.) over the I2C interface. It act as a

Describe hardware, not drivers.

> +  I2C gate for the EEPROM. Therefore, The EEPROM is represented as a
> +  child node under a port and is accessed through a separate driver
> +  (the standard 'at24' driver). This implementation is possible because

again, describe hardware not driver

> +  the M24LR chips uses two I2C addresses: one for accessing the
> +  system parameter sector and another for the EEPROM.

This suggests you have two I2C addresses for one device, not two devices
with parent child relationship.

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

Drop quotes. So this is I2C mux or EEPROM?

> +
> +properties:
> +  compatible:
> +    enum:
> +    - st,m24lr04e-r
> +    - st,m24lr16e-r
> +    - st,m24lr64e-r
> +
> +  reg:
> +    maxItems: 1
> +    description: I2C address of the device.

Drop description, redundant.


This device is not compatible with AT24?

> +
> +  pagesize:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: >
> +      Maximum number of bytes that can be written in a single I2C
> +      transaction. the default is 1.

enum:
default:

and drop redundant free form text.

> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false

unevaluatedProperties instead

> +
> +examples:
> +  - |
> +    i2c {
> +      m24lr@57 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


> +        compatible = "st,m24lr04e-r";
> +        reg = <0x57>;
> +
> +        i2c-gate {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +
> +          m24lr_eeprom@53 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


> +            compatible = "atmel,24c04";
> +            reg = <0x53>;
> +            address-width = <16>;
> +            pagesize = <4>;
> +          };
> +        };
> +      };
> +    };
> +...
> \ No newline at end of file


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] MAINTAINERS: Add entry for ST M24LR control driver
  2025-05-31  8:11 ` [PATCH 3/3] MAINTAINERS: Add entry for ST M24LR control driver Abd-Alrhman Masalkhi
@ 2025-05-31 13:37   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2025-05-31 13:37 UTC (permalink / raw)
  To: Abd-Alrhman Masalkhi, linux-kernel, devicetree
  Cc: robh, krzk+dt, conor+dt, arnd, gregkh

On 31/05/2025 10:11, Abd-Alrhman Masalkhi wrote:
> Add a MAINTAINERS entry for the newly introduced sysfs control driver
> supporting STMicroelectronics M24LR series RFID/NFC EEPROM chips.
> 
> This entry includes the driver source, Device Tree binding, and assigns
> maintainership to the original contributor.
> 
> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
> ---
>  MAINTAINERS | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index eb11c6f57500..f08975ac4d9f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -23017,6 +23017,14 @@ W:	http://www.st.com/
>  F:	Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml
>  F:	drivers/iio/imu/st_lsm6dsx/
>  
> +ST M24LR CONTROL DRIVER
> +M:	Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
> +L:	linux-kernel@vger.kernel.org
> +L:	devicetree@vger.kernel.org

No, drop. There are no entries like that.


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] misc: add sysfs control driver for ST M24LR series RFID/NFC chips
  2025-05-31  8:11 ` [PATCH 2/3] misc: add sysfs control driver for ST M24LR series RFID/NFC chips Abd-Alrhman Masalkhi
  2025-05-31  8:24   ` Greg KH
  2025-05-31  8:25   ` Greg KH
@ 2025-06-01  2:19   ` kernel test robot
  2025-06-01  3:21   ` kernel test robot
  3 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2025-06-01  2:19 UTC (permalink / raw)
  To: Abd-Alrhman Masalkhi, linux-kernel, devicetree
  Cc: oe-kbuild-all, robh, krzk+dt, conor+dt, arnd, gregkh,
	abd.masalkhi

Hi Abd-Alrhman,

kernel test robot noticed the following build warnings:

[auto build test WARNING on char-misc/char-misc-linus]
[also build test WARNING on robh/for-next soc/for-next linus/master v6.15]
[cannot apply to char-misc/char-misc-testing char-misc/char-misc-next next-20250530]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Abd-Alrhman-Masalkhi/dt-bindings-misc-Add-binding-for-ST-M24LR-control-interface/20250531-161342
base:   char-misc/char-misc-linus
patch link:    https://lore.kernel.org/r/20250531081159.2007319-3-abd.masalkhi%40gmail.com
patch subject: [PATCH 2/3] misc: add sysfs control driver for ST M24LR series RFID/NFC chips
config: s390-allyesconfig (https://download.01.org/0day-ci/archive/20250601/202506011056.lwz5lTUN-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250601/202506011056.lwz5lTUN-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506011056.lwz5lTUN-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/linux/build_bug.h:5,
                    from include/linux/bits.h:22,
                    from include/linux/ioport.h:13,
                    from include/linux/acpi.h:12,
                    from include/linux/i2c.h:13,
                    from drivers/misc/m24lr_ctl.c:22:
   drivers/misc/m24lr_ctl.c: In function 'm24lr_ctl_write':
>> include/linux/err.h:28:49: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
      28 | #define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned long)-MAX_ERRNO)
         |                                                 ^
   include/linux/compiler.h:77:45: note: in definition of macro 'unlikely'
      77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
         |                                             ^
   drivers/misc/m24lr_ctl.c:378:21: note: in expansion of macro 'IS_ERR_VALUE'
     378 |                 if (IS_ERR_VALUE(err)) {
         |                     ^~~~~~~~~~~~
   drivers/misc/m24lr_ctl.c: In function 'm24lr_ctl_show':
>> include/linux/err.h:28:49: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
      28 | #define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned long)-MAX_ERRNO)
         |                                                 ^
   include/linux/compiler.h:77:45: note: in definition of macro 'unlikely'
      77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
         |                                             ^
   drivers/misc/m24lr_ctl.c:499:13: note: in expansion of macro 'IS_ERR_VALUE'
     499 |         if (IS_ERR_VALUE(ret))
         |             ^~~~~~~~~~~~
   In file included from include/linux/device.h:15,
                    from include/linux/acpi.h:14:
   drivers/misc/m24lr_ctl.c: In function 'm24lr_ctl_probe':
   drivers/misc/m24lr_ctl.c:589:34: warning: too many arguments for format [-Wformat-extra-args]
     589 |                                  "Failed to create sysfs entry '%s'\n",
         |                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
         |                              ^~~
   include/linux/dev_printk.h:156:61: note: in expansion of macro 'dev_fmt'
     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                             ^~~~~~~
   drivers/misc/m24lr_ctl.c:588:25: note: in expansion of macro 'dev_warn'
     588 |                         dev_warn(dev,
         |                         ^~~~~~~~
   drivers/misc/m24lr_ctl.c:613:42: warning: too many arguments for format [-Wformat-extra-args]
     613 |                                          "Failed to create sysfs entry '%s'\n",
         |                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
         |                              ^~~
   include/linux/dev_printk.h:156:61: note: in expansion of macro 'dev_fmt'
     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                             ^~~~~~~
   drivers/misc/m24lr_ctl.c:612:33: note: in expansion of macro 'dev_warn'
     612 |                                 dev_warn(dev,
         |                                 ^~~~~~~~
>> include/linux/err.h:28:49: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
      28 | #define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned long)-MAX_ERRNO)
         |                                                 ^
   include/linux/compiler.h:77:45: note: in definition of macro 'unlikely'
      77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
         |                                             ^
   drivers/misc/m24lr_ctl.c:642:13: note: in expansion of macro 'IS_ERR_VALUE'
     642 |         if (IS_ERR_VALUE(err))
         |             ^~~~~~~~~~~~
   drivers/misc/m24lr_ctl.c:645:15: error: too many arguments to function 'i2c_mux_add_adapter'; expected 3, have 4
     645 |         err = i2c_mux_add_adapter(muxc, 0, 0, 0);
         |               ^~~~~~~~~~~~~~~~~~~             ~
   In file included from drivers/misc/m24lr_ctl.c:25:
   include/linux/i2c-mux.h:58:5: note: declared here
      58 | int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
         |     ^~~~~~~~~~~~~~~~~~~
   drivers/misc/m24lr_ctl.c: At top level:
   drivers/misc/m24lr_ctl.c:669:21: error: initialization of 'int (*)(struct i2c_client *)' from incompatible pointer type 'int (*)(struct i2c_client *, const struct i2c_device_id *)' [-Wincompatible-pointer-types]
     669 |         .probe    = m24lr_ctl_probe,
         |                     ^~~~~~~~~~~~~~~
   drivers/misc/m24lr_ctl.c:669:21: note: (near initialization for 'm24lr_ctl_driver.probe')
   drivers/misc/m24lr_ctl.c:540:12: note: 'm24lr_ctl_probe' declared here
     540 | static int m24lr_ctl_probe(struct i2c_client *client,
         |            ^~~~~~~~~~~~~~~
   drivers/misc/m24lr_ctl.c:670:21: error: initialization of 'void (*)(struct i2c_client *)' from incompatible pointer type 'int (*)(struct i2c_client *)' [-Wincompatible-pointer-types]
     670 |         .remove   = remove,
         |                     ^~~~~~
   drivers/misc/m24lr_ctl.c:670:21: note: (near initialization for 'm24lr_ctl_driver.remove')
   drivers/misc/m24lr_ctl.c:655:12: note: 'remove' declared here
     655 | static int remove(struct i2c_client *client)
         |            ^~~~~~


vim +28 include/linux/err.h

ebba5f9fcb8823 Randy Dunlap   2006-09-27  21  
4d744ce9d5d7cf James Seo      2023-05-09  22  /**
4d744ce9d5d7cf James Seo      2023-05-09  23   * IS_ERR_VALUE - Detect an error pointer.
4d744ce9d5d7cf James Seo      2023-05-09  24   * @x: The pointer to check.
4d744ce9d5d7cf James Seo      2023-05-09  25   *
4d744ce9d5d7cf James Seo      2023-05-09  26   * Like IS_ERR(), but does not generate a compiler warning if result is unused.
4d744ce9d5d7cf James Seo      2023-05-09  27   */
aa00edc1287a69 Linus Torvalds 2016-05-27 @28  #define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned long)-MAX_ERRNO)
07ab67c8d0d7c1 Linus Torvalds 2005-05-19  29  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] misc: add sysfs control driver for ST M24LR series RFID/NFC chips
  2025-05-31  8:11 ` [PATCH 2/3] misc: add sysfs control driver for ST M24LR series RFID/NFC chips Abd-Alrhman Masalkhi
                     ` (2 preceding siblings ...)
  2025-06-01  2:19   ` kernel test robot
@ 2025-06-01  3:21   ` kernel test robot
  3 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2025-06-01  3:21 UTC (permalink / raw)
  To: Abd-Alrhman Masalkhi, linux-kernel, devicetree
  Cc: llvm, oe-kbuild-all, robh, krzk+dt, conor+dt, arnd, gregkh,
	abd.masalkhi

Hi Abd-Alrhman,

kernel test robot noticed the following build errors:

[auto build test ERROR on char-misc/char-misc-linus]
[also build test ERROR on robh/for-next soc/for-next linus/master v6.15]
[cannot apply to char-misc/char-misc-testing char-misc/char-misc-next next-20250530]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Abd-Alrhman-Masalkhi/dt-bindings-misc-Add-binding-for-ST-M24LR-control-interface/20250531-161342
base:   char-misc/char-misc-linus
patch link:    https://lore.kernel.org/r/20250531081159.2007319-3-abd.masalkhi%40gmail.com
patch subject: [PATCH 2/3] misc: add sysfs control driver for ST M24LR series RFID/NFC chips
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20250601/202506011126.RpYXQiPu-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250601/202506011126.RpYXQiPu-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506011126.RpYXQiPu-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

>> drivers/misc/m24lr_ctl.c:378:7: warning: cast to 'void *' from smaller integer type 'int' [-Wint-to-void-pointer-cast]
     378 |                 if (IS_ERR_VALUE(err)) {
         |                     ^~~~~~~~~~~~~~~~~
   include/linux/err.h:28:49: note: expanded from macro 'IS_ERR_VALUE'
      28 | #define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned long)-MAX_ERRNO)
         |                         ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:77:42: note: expanded from macro 'unlikely'
      77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
         |                                             ^
   drivers/misc/m24lr_ctl.c:499:6: warning: cast to 'void *' from smaller integer type 'int' [-Wint-to-void-pointer-cast]
     499 |         if (IS_ERR_VALUE(ret))
         |             ^~~~~~~~~~~~~~~~~
   include/linux/err.h:28:49: note: expanded from macro 'IS_ERR_VALUE'
      28 | #define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned long)-MAX_ERRNO)
         |                         ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:77:42: note: expanded from macro 'unlikely'
      77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
         |                                             ^
>> drivers/misc/m24lr_ctl.c:590:23: warning: data argument not used by format string [-Wformat-extra-args]
     589 |                                  "Failed to create sysfs entry '%s'\n",
         |                                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     590 |                                  attr->attr.name, err);
         |                                                   ^
   include/linux/dev_printk.h:156:70: note: expanded from macro 'dev_warn'
     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                                     ~~~     ^
   include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
         |                              ~~~    ^
   drivers/misc/m24lr_ctl.c:614:13: warning: data argument not used by format string [-Wformat-extra-args]
     613 |                                          "Failed to create sysfs entry '%s'\n",
         |                                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     614 |                                          name, err);
         |                                                ^
   include/linux/dev_printk.h:156:70: note: expanded from macro 'dev_warn'
     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                                     ~~~     ^
   include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
         |                              ~~~    ^
   drivers/misc/m24lr_ctl.c:642:6: warning: cast to 'void *' from smaller integer type 'int' [-Wint-to-void-pointer-cast]
     642 |         if (IS_ERR_VALUE(err))
         |             ^~~~~~~~~~~~~~~~~
   include/linux/err.h:28:49: note: expanded from macro 'IS_ERR_VALUE'
      28 | #define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned long)-MAX_ERRNO)
         |                         ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:77:42: note: expanded from macro 'unlikely'
      77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
         |                                             ^
>> drivers/misc/m24lr_ctl.c:645:40: error: too many arguments to function call, expected 3, have 4
     645 |         err = i2c_mux_add_adapter(muxc, 0, 0, 0);
         |               ~~~~~~~~~~~~~~~~~~~             ^
   include/linux/i2c-mux.h:58:5: note: 'i2c_mux_add_adapter' declared here
      58 | int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
         |     ^                   ~~~~~~~~~~~~~~~~~~~~~~~~~~
      59 |                         u32 force_nr, u32 chan_id);
         |                         ~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/misc/m24lr_ctl.c:669:14: error: incompatible function pointer types initializing 'int (*)(struct i2c_client *)' with an expression of type 'int (struct i2c_client *, const struct i2c_device_id *)' [-Wincompatible-function-pointer-types]
     669 |         .probe    = m24lr_ctl_probe,
         |                     ^~~~~~~~~~~~~~~
>> drivers/misc/m24lr_ctl.c:670:14: error: incompatible function pointer types initializing 'void (*)(struct i2c_client *)' with an expression of type 'int (struct i2c_client *)' [-Wincompatible-function-pointer-types]
     670 |         .remove   = remove,
         |                     ^~~~~~
   5 warnings and 3 errors generated.


vim +645 drivers/misc/m24lr_ctl.c

   539	
   540	static int m24lr_ctl_probe(struct i2c_client *client,
   541				   const struct i2c_device_id *id)
   542	{
   543		struct regmap *regmap;
   544		struct m24lr_ctl *ctl;
   545		struct i2c_mux_core *muxc;
   546		const struct m24lr_ctl_chip *chip;
   547		struct m24lr_sys_entry *sss = NULL;
   548		unsigned int page_size;
   549		unsigned int n_sss;
   550		int i, err;
   551		u8 test;
   552		struct device *dev = &client->dev;
   553	
   554		if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
   555			return -EOPNOTSUPP;
   556	
   557		chip = m24lr_ctl_get_chip(dev);
   558		if (!chip)
   559			return -ENODEV;
   560	
   561		ctl = devm_kzalloc(dev, sizeof(struct m24lr_ctl), GFP_KERNEL);
   562		if (!ctl)
   563			return -ENOMEM;
   564	
   565		err = device_property_read_u32(dev, "pagesize", &page_size);
   566		if (!err) {
   567			if (!is_power_of_2(page_size)) {
   568				dev_warn(dev,
   569					 "Invalid pagesize lenngth %d (not power of 2); using default %d byte\n",
   570					 page_size, M24LR_CTL_PAGESIZE_DEFAULT);
   571				page_size = M24LR_CTL_PAGESIZE_DEFAULT;
   572			}
   573			if (page_size > M24LR_CTL_PAGESIZE_LIMIT) {
   574				dev_info(dev,
   575					 "pagesize %d exceeds limit; rounded down to %d\n",
   576					 page_size, M24LR_CTL_PAGESIZE_LIMIT);
   577				page_size = M24LR_CTL_PAGESIZE_LIMIT;
   578			}
   579		} else { /* use the default */
   580			page_size = M24LR_CTL_PAGESIZE_DEFAULT;
   581		}
   582	
   583		for (i = 0; i < chip->n_entries; i++) {
   584			const struct device_attribute *attr = &chip->entries[i].attr;
   585	
   586			err = device_create_file(dev, attr);
   587			if (err)
   588				dev_warn(dev,
   589					 "Failed to create sysfs entry '%s'\n",
 > 590					 attr->attr.name, err);
   591		}
   592	
   593		n_sss = chip->n_sss_entries;
   594		if (n_sss) {
   595			sss = devm_kzalloc(dev, n_sss * sizeof(struct m24lr_sys_entry),
   596					   GFP_KERNEL);
   597			if (!sss)
   598				return -ENOMEM;
   599	
   600			for (i = 0; i < n_sss; i++) {
   601				char *name = kasprintf(GFP_KERNEL, "sss%02d", i);
   602	
   603				sss[i].reg_size = 1;
   604				sss[i].reg_addr = i;
   605				sss[i].attr.attr.name = name;
   606				sss[i].attr.attr.mode = 0600;
   607				sss[i].attr.show = m24lr_ctl_show;
   608				sss[i].attr.store = m24lr_ctl_store;
   609	
   610				err = device_create_file(dev, &sss[i].attr);
   611				if (err)
   612					dev_warn(dev,
   613						 "Failed to create sysfs entry '%s'\n",
   614						 name, err);
   615			}
   616		}
   617	
   618		regmap = devm_regmap_init_i2c(client, &m24lr_ctl_regmap_conf);
   619		if (IS_ERR(regmap)) {
   620			err = PTR_ERR(regmap);
   621			dev_err(dev, "Failed to init regmap (error: %d)\n", err);
   622			return err;
   623		}
   624	
   625		muxc = i2c_mux_alloc(client->adapter, &client->dev, 1, 0, I2C_MUX_GATE,
   626				     m24lr_ctl_gate_select, m24lr_ctl_gate_deselect);
   627		if (!muxc)
   628			return -ENOMEM;
   629	
   630		muxc->priv = ctl;
   631	
   632		mutex_init(&ctl->gate_lock);
   633		ctl->page_size = page_size;
   634		ctl->regmap = regmap;
   635		ctl->muxc = muxc;
   636		ctl->n_sss_entries = n_sss;
   637		ctl->sss_entries = sss;
   638	
   639		i2c_set_clientdata(client, ctl);
   640	
   641		err = m24lr_ctl_read(ctl, &test, 1, 0);
   642		if (IS_ERR_VALUE(err))
   643			return -ENODEV;
   644	
 > 645		err = i2c_mux_add_adapter(muxc, 0, 0, 0);
   646		if (err)
   647			return err;
   648	
   649		dev_info(&client->dev, "control interface initialized for %s\n",
   650			 client->name);
   651	
   652		return 0;
   653	}
   654	
   655	static int remove(struct i2c_client *client)
   656	{
   657		struct m24lr_ctl *ctl = i2c_get_clientdata(client);
   658	
   659		i2c_mux_del_adapters(ctl->muxc);
   660	
   661		return 0;
   662	}
   663	
   664	static struct i2c_driver m24lr_ctl_driver = {
   665		.driver = {
   666			.name = "m24lr_ctl",
   667			.of_match_table = m24lr_ctl_of_match,
   668		},
 > 669		.probe    = m24lr_ctl_probe,
 > 670		.remove   = remove,
   671		.id_table = m24lr_ctl_ids,
   672	};
   673	module_i2c_driver(m24lr_ctl_driver);
   674	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] dt-bindings: Add Device Tree binding for ST M24LR control interface
  2025-05-31 13:37   ` Krzysztof Kozlowski
@ 2025-06-01  7:31     ` Abd-Alrhman Masalkhi
  0 siblings, 0 replies; 12+ messages in thread
From: Abd-Alrhman Masalkhi @ 2025-06-01  7:31 UTC (permalink / raw)
  To: krzk
  Cc: abd.masalkhi, arnd, conor+dt, devicetree, gregkh, krzk+dt,
	linux-kernel, robh

Hi Krzysztof,

Thank you for your review.

> Drop quotes. So this is I2C mux or EEPROM?

The system parameter sector and the EEPROM do not share a continuous address
space, each starts at address 0 and spans its own internal region. This
overlapping addressing creates ambiguity if treated as a single memory space.

Additionally, there's a synchronization issue: during multi-page writes to
the EEPROM, if a control command (e.g., a lock) is issued mid-operation,
it may result in partial or inconsistent writes.

To address both challenges, the driver uses a mux-based design:

1- The m24lr_ctl driver acts as a gate for EEPROM access.

2- It provides exclusive, serialized access and handles the control interface.

3- The EEPROM itself is exposed as a child node using the standard at24 driver.

This separation ensures reliable operation and a clean, maintainable
architecture.

> That's not a misc device, but eeprom. Place it in appropriate directory.

Given the above, I'm unsure if placing it under eeprom/ is the best choice.
Would you suggest still placing it eeprom or under somewhere else maybe i2c/
(e.g., i2c/i2c-mux-stm24lr.yaml)?

Best regards,
Abd-Alrhman Masalkhi

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-06-01  7:31 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-31  8:11 [PATCH 0/3] Add control driver for ST M24LR RFID/NFC EEPROM chips Abd-Alrhman Masalkhi
2025-05-31  8:11 ` [PATCH 1/3] dt-bindings: misc: Add binding for ST M24LR control interface Abd-Alrhman Masalkhi
2025-05-31  9:25   ` Rob Herring (Arm)
2025-05-31 13:37   ` Krzysztof Kozlowski
2025-06-01  7:31     ` [PATCH v2 1/3] dt-bindings: Add Device Tree " Abd-Alrhman Masalkhi
2025-05-31  8:11 ` [PATCH 2/3] misc: add sysfs control driver for ST M24LR series RFID/NFC chips Abd-Alrhman Masalkhi
2025-05-31  8:24   ` Greg KH
2025-05-31  8:25   ` Greg KH
2025-06-01  2:19   ` kernel test robot
2025-06-01  3:21   ` kernel test robot
2025-05-31  8:11 ` [PATCH 3/3] MAINTAINERS: Add entry for ST M24LR control driver Abd-Alrhman Masalkhi
2025-05-31 13:37   ` Krzysztof Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).