Re: [PATCH v3 3/3] ABI: sysfs: add documentation for ST M24LR EEPROM and control interface

[Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>]

Hi greg,

Thanks for the feedback.

>> +                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
>
> Why is the kernel in the business of adding passwords to devices?  That
> feels wrong, and a way to just flood the device with a "try all the
> values" attempt if needed.

You're absolutely right, implementing password-based access in kernel
space isn't ideal. However, this behavior is defined by the hardware
itself. The M24LR chips require the user to "unlock" the device by writing
a password before certain registers become writable (such as the Sector
Security Status registors) and unfortunately, the chip does not provide
any status or feedback to indicate whether the unlock was successful,
which limits what the driver can safely report or validate.

>> +What:           /sys/bus/i2c/devices/<busnum>-<primary-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
>
> How "deep" does this sysfs tree get here?  This feels like the wrong api
> for read/write to the device, just do it with a single binary file if
> you really want a "passthrough" way to get to the hardware.

The depth of the sysfs tree depends on the M24LR variant. For example,
the M24LR04E-R has 4 sectors, resulting in 4 entries: sss0 through sss3.

I understand the concern about exposing multiple sysfs entries. The
reason for this design is that each sector has its own SSS byte, and
separating them helps reflect the per-sector nature of the access
control. That said, I'm open to refactoring this to expose the SSS
area via a single binary file if that's more in line with expected
kernel interfaces.

Best regards,
Abd-Alrhman Masalkhi