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

[Greg KH <gregkh@linuxfoundation.org>]

On Fri, Jun 06, 2025 at 02:46:57PM +0000, Abd-Alrhman Masalkhi wrote:
> 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.

Who and what is going to be talking to this device through this
interface?  Is this unique and special to ONLY this one chip/device or
does it fit in with all other types of this device (i.e. eeproms)?  You
can't create a userspace api without actually having a user at all, so
if there is no userspace code using this, why even have this?

thanks,

greg k-h