linux-kernel - Re: [PATCH 2/3] misc: add sysfs control driver for ST M24LR series RFID/NFC chips

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2025053131-starship-radiantly-2cd8@gregkh>
Date: Sat, 31 May 2025 10:24:07 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: Abd-Alrhman Masalkhi <abd.masalkhi@...il.com>
Cc: linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
	robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
	arnd@...db.de
Subject: Re: [PATCH 2/3] misc: add sysfs control driver for ST M24LR series
 RFID/NFC chips

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@...il.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