Re: [PATCH] eeprom: at24: Support custom device names for AT24 EEPROMs

[Jon Hunter <jonathanh@nvidia.com>]

On 04/06/2021 11:00, Bartosz Golaszewski wrote:
> On Wed, Jun 2, 2021 at 7:08 PM Alexander Fomichev <fomichev.ru@gmail.com> wrote:

...

>> This change has a serious defect, as it doesn't guarantee a name
>> uniqueness. For my case there are a bunch of NVMEM devices with
>> 'dimm-spd' name. So the module initialization fails with several error
>> dumps in dmesg, like following:
>>
>> [    4.784679] at24 3-0051: supply vcc not found, using dummy regulator
>> [    4.784781] sysfs: cannot create duplicate filename '/bus/nvmem/devices/dimm-spd'
>> [    4.784783] CPU: 24 PID: 1354 Comm: systemd-udevd Not tainted 5.13.0-rc4-at24-catch+ #25
>> [    4.784787] Call Trace:
>> [    4.784789] [c00000003f3eb010] [c000000000914700] dump_stack+0xc4/0x114 (unreliable)
>> [    4.784797] [c00000003f3eb060] [c00000000061c5c8] sysfs_warn_dup+0x88/0xc0
>> [    4.784803] [c00000003f3eb0e0] [c00000000061ccec] sysfs_do_create_link_sd+0x17c/0x190
>> [    4.784809] [c00000003f3eb130] [c000000000ac3014] bus_add_device+0x94/0x1d0
>> [    4.784817] [c00000003f3eb1b0] [c000000000abe7b8] device_add+0x428/0xb90
>> [    4.784822] [c00000003f3eb2a0] [c000000000debbd0] nvmem_register+0x220/0xe00
>> [    4.784829] [c00000003f3eb390] [c000000000dec80c] devm_nvmem_register+0x5c/0xc0
>> [    4.784835] [c00000003f3eb3d0] [c008000016f40c20] at24_probe+0x668/0x940 [at24]
>> [    4.784845] [c00000003f3eb650] [c000000000cfecd4] i2c_device_probe+0x194/0x650
>> [    4.784850] [c00000003f3eb6f0] [c000000000ac4d3c] really_probe+0x1cc/0x790
>> [    4.784855] [c00000003f3eb790] [c000000000ac545c] driver_probe_device+0x15c/0x200
>> [    4.784861] [c00000003f3eb810] [c000000000ac5ecc] device_driver_attach+0x11c/0x130
>> [    4.784866] [c00000003f3eb850] [c000000000ac5fd0] __driver_attach+0xf0/0x200
>> [    4.784873] [c00000003f3eb8d0] [c000000000ac1158] bus_for_each_dev+0xa8/0x130
>> [    4.784879] [c00000003f3eb930] [c000000000ac4104] driver_attach+0x34/0x50
>> [    4.784885] [c00000003f3eb950] [c000000000ac35f0] bus_add_driver+0x1b0/0x2f0
>> [    4.784893] [c00000003f3eb9e0] [c000000000ac70b4] driver_register+0xb4/0x1c0
>> [    4.784900] [c00000003f3eba50] [c000000000cfe498] i2c_register_driver+0x78/0x120
>> [    4.784905] [c00000003f3ebad0] [c008000016f41260] at24_init+0x6c/0x88 [at24]
>> [    4.784914] [c00000003f3ebb30] [c0000000000122c0] do_one_initcall+0x60/0x2c0
>> [    4.784920] [c00000003f3ebc00] [c0000000002537bc] do_init_module+0x7c/0x350
>> [    4.784926] [c00000003f3ebc90] [c000000000257904] __do_sys_finit_module+0xd4/0x160
>> [    4.784932] [c00000003f3ebdb0] [c00000000002c104] system_call_exception+0xf4/0x200
>> [    4.784938] [c00000003f3ebe10] [c00000000000cf70] system_call_vectored_common+0xf0/0x268
>> [    4.784944] --- interrupt: 3000 at 0x7c1adac4b4c4
>> [    4.784948] NIP:  00007c1adac4b4c4 LR: 0000000000000000 CTR: 0000000000000000
>> [    4.784951] REGS: c00000003f3ebe80 TRAP: 3000   Not tainted  (5.13.0-rc4-at24-catch+)
>> [    4.784955] MSR:  900000000280f033 <SF,HV,VEC,VSX,EE,PR,FP,ME,IR,DR,RI,LE>  CR: 48222844  XER: 00000000
>> [    4.784976] IRQMASK: 0
>>                GPR00: 0000000000000161 00007fffefc78b90 00007c1adad37000 0000000000000006
>>                GPR04: 00000f6614d56be0 0000000000000000 0000000000000006 0000000000000000
>>                GPR08: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>                GPR12: 0000000000000000 00007c1adafde680 0000000020000000 0000000000000000
>>                GPR16: 0000000000000000 00000f66118b1980 00000f66118b1a18 00000f66118b1948
>>                GPR20: 0000000000000000 00000f6614d60500 00007fffefc78df0 00000f6614d535c0
>>                GPR24: 00000f6614d56be0 00000f6614d60500 000000000000000c 00000f6614d49cb0
>>                GPR28: 00000f6614d56be0 0000000000020000 0000000000000000 00000f6614d60500
>> [    4.785033] NIP [00007c1adac4b4c4] 0x7c1adac4b4c4
>> [    4.785036] LR [0000000000000000] 0x0
>> [    4.785040] --- interrupt: 3000
>> [    4.785146] at24: probe of 3-0051 failed with error -17
>>
>>
>> It needs either to use NVMEM_DEVID_AUTO flag irrespective of the 'label'
>> property or to add a sort of counter suffix to the name field.
>>
>>
>> Reported-by: Alexander Fomichev <fomichev.ru@gmail.com>
>> CC: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>
>> --
>> Regards,
>>   Alexander
> 
> Alexander: Thanks for your bug report. The counter suffix you suggest
> is precisely what NVMEM_DEVID_AUTO would do so I think we'll need to
> use it. On the other hand, a non-unique label is bad design but
> obviously we can't break working setups.

Yes the intention was that the label itself would be unique. In our case
we wanted to have something specifically named 'system' or 'module' to
identify a specific eeprom.

BTW I did a quick grep from 'dimm-spd' and did not find it in the
kernel. Where is the device-tree you are using?

> Jon: As the author of this patch - do you have any objections/better ideas?

I would need to check if appending a suffix then has ramifications for
what we were trying to achieve.

Jon

-- 
nvpublic