Message-ID: <3c49628c-00b0-488d-a660-4b904febca1e@kernel.org>
Date: Mon, 2 Jun 2025 08:28:32 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Abd-Alrhman Masalkhi <abd.masalkhi@...il.com>
Cc: arnd@...db.de, conor+dt@...nel.org, devicetree@...r.kernel.org,
 gregkh@...uxfoundation.org, krzk+dt@...nel.org,
 linux-kernel@...r.kernel.org, robh@...nel.org
Subject: Re: [PATCH v2 1/3] dt-bindings: Add Device Tree binding for ST M24LR
 control interface

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

Powered by blists - more mailing lists