linux-kernel - Re: [PATCH RFT v3 4/4] hwmon: (spd5118) Add support for reading SPD data

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]

Date: Sat, 1 Jun 2024 16:08:22 +0200
From: Thomas Weißschuh <linux@...ssschuh.net>
To: Guenter Roeck <linux@...ck-us.net>
Cc: linux-hwmon@...r.kernel.org, devicetree@...r.kernel.org, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>, linux-kernel@...r.kernel.org, Armin Wolf <W_Armin@....de>, 
	René Rebe <rene@...ctcode.de>, Wolfram Sang <wsa+renesas@...g-engineering.com>
Subject: Re: [PATCH RFT v3 4/4] hwmon: (spd5118) Add support for reading SPD
 data

On 2024-06-01 06:48:29+0000, Guenter Roeck wrote:

<snip>

> Makes sense. Another question:
> 
> This:
> 
> +        struct nvmem_config nvmem_config = {
> +               .type = NVMEM_TYPE_EEPROM,
> +               .name = dev_name(dev),
> +               .id = NVMEM_DEVID_AUTO,
> 
> results in:
> 
> $ ls /sys/bus/nvmem/devices
> 0-00501  0-00512  0-00523  0-00534  cmos_nvram0
> ^^^^^^^  ^^^^^^^  ^^^^^^^  ^^^^^^^
> 
> which really doesn't look good. My current plan is to go with NVMEM_DEVID_NONE,
> which results in
> 
> $ ls /sys/bus/nvmem/devices
> 0-0050	0-0051	0-0052	0-0053	cmos_nvram0
> 
> We could also used fixed strings, but "spd" results in "spd[1-4]" which
> I think would be a bit misleading since the DDR3/4 SPD data format is
> different, and "spd5118" would result in "spd5118[1-4]" which again would
> look odd. Any suggestions ?

In order of descending, personal preference:

* spd-ddr5-[0-3] (.id = client->address - 0x50)
* spd-ddr5-[0-3] (NVMEM_DEVID_AUTO)
* Same with only "ddr5-"
* spd5118-[0-3]
* Your proposal from above
* nvmem[0-3] (default handling)
* 0-0050-[0-3]

Also can't a user of the eeprom already figure out which kind of module
it is by looking at the eeprom contents?
The first few bytes used for that seem to be compatible between at least
DDR4 and DDR5.

So using plain spd[1-4] could be enough.