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

[Krzysztof Kozlowski <krzk@kernel.org>]

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