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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160416211733.GD1522@katana>
Date:	Sat, 16 Apr 2016 23:17:33 +0200
From:	Wolfram Sang <wsa@...-dreams.de>
To:	Bartosz Golaszewski <bgolaszewski@...libre.com>
Cc:	linux-i2c <linux-i2c@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	srinivas.kandagatla@...aro.org, maxime.ripard@...e-electrons.com,
	Andrew Lunn <andrew@...n.ch>
Subject: Re: [RESEND PATCH v2 00/13] eeprom: support for at24cs and at24mac

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?

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)