Re: [RESEND PATCH v2 00/13] eeprom: support for at24cs and at24mac

[Wolfram Sang <wsa@the-dreams.de>]

[-- Attachment #1: Type: text/plain, Size: 1859 bytes --]

Hi,

On Mon, Apr 11, 2016 at 11:57:16AM -0700, Bartosz Golaszewski wrote:
> Chips from the at24cs EEPROM series have an additional read-only
> memory area containing a factory pre-programmed serial number. In
> order to access it, a dummy write must be executed before reading
> the serial number bytes.
> 
> Chips from the at24mac familiy, apart from the serial number, have
> a second special memory area containing a factory programmed
> EUI-48/EUI64 mac address.
> 
> The read-only serial/mac memory area is accessible on a different i2c
> slave address (addr + 0x08). This patchset makes it possible to
> instantiate a separate at24 device on this second address and access
> the read-only area through the regular eeprom sysfs attribute or the
> new nvmem subsystem.
> 
> This series also contains several patches intoducing some minor tweaks
> and refactoring.
> 
> Tested with at24cs32 and at24cs02 chips (for both 16 and 8 bit address
> pointers). I have no means of testing the support for at24mac chips, I
> relied solely on the datasheet.

Thanks for adding at24mac nonetheless!

I still don't understand why you want to add this to at24 instead of
doing a seperate driver for it? You basically tie a read_function to an
nvmem device. You don't need the special smbus handling, cutting large
requests into chunks, write support, size flags and all this from at24.
And with the function pointers, it makes the already complex code a bit
more complex.

IMO, this could be solved by a simple, seperate driver. You could start
with at24 and remove unneeded stuff. I think there should be mainly
nvmem-init code and your read routines left. I also think this new
driver should be allowed to set the "compat" option of the nvmem
framework, so you'll get the "eeprom" file, too.

Or am I overlooking something?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]