From: Arnd Bergmann <[email protected]>
Creating sysfs files for all Cells caused a boot failure for linux-6.8-rc1 on
Apple M1, which (in downstream dts files) has multiple nvmem cells that use the
same byte address. This causes the device probe to fail with
[ 0.605336] sysfs: cannot create duplicate filename '/devices/platform/soc@200000000/2922bc000.efuse/apple_efuses_nvmem0/cells/efuse@a10'
[ 0.605347] CPU: 7 PID: 1 Comm: swapper/0 Tainted: G S 6.8.0-rc1-arnd-5+ #133
[ 0.605355] Hardware name: Apple Mac Studio (M1 Ultra, 2022) (DT)
[ 0.605362] Call trace:
[ 0.605365] show_stack+0x18/0x2c
[ 0.605374] dump_stack_lvl+0x60/0x80
[ 0.605383] dump_stack+0x18/0x24
[ 0.605388] sysfs_warn_dup+0x64/0x80
[ 0.605395] sysfs_add_bin_file_mode_ns+0xb0/0xd4
[ 0.605402] internal_create_group+0x268/0x404
[ 0.605409] sysfs_create_groups+0x38/0x94
[ 0.605415] devm_device_add_groups+0x50/0x94
[ 0.605572] nvmem_populate_sysfs_cells+0x180/0x1b0
[ 0.605682] nvmem_register+0x38c/0x470
[ 0.605789] devm_nvmem_register+0x1c/0x6c
[ 0.605895] apple_efuses_probe+0xe4/0x120
[ 0.606000] platform_probe+0xa8/0xd0
As far as I can tell, this is a problem for any device with multiple cells on
different bits of the same address. Avoid the issue by changing the file name
to include the first bit number.
Fixes: 0088cbc19276 ("nvmem: core: Expose cells through sysfs")
Link: https://github.com/AsahiLinux/linux/blob/bd0a1a7d4/arch/arm64/boot/dts/apple/t600x-dieX.dtsi#L156
Cc: [email protected]
Cc: Miquel Raynal <[email protected]>
Cc: Rafał Miłecki <[email protected]>
Cc: Chen-Yu Tsai <[email protected]>
Cc: Srinivas Kandagatla <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Cc: Sven Peter <[email protected]>
Signed-off-by: Arnd Bergmann <[email protected]>
---
This might not be the fix we want upstream, but it illustrates the issue and
I have successfully tested that this lets me boot v6.8-rc1 on my workstation
again.
Feel free to just treat this as a bug report and suggest a different fix.
As the ABI was only introduced in 6.8-rc1, it can still be changed without
causing other regressions for users, but time is running out for that.
---
Documentation/ABI/testing/sysfs-nvmem-cells | 16 ++++++++--------
drivers/nvmem/core.c | 5 +++--
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-nvmem-cells b/Documentation/ABI/testing/sysfs-nvmem-cells
index 7af70adf3690..c7c9444f92a8 100644
--- a/Documentation/ABI/testing/sysfs-nvmem-cells
+++ b/Documentation/ABI/testing/sysfs-nvmem-cells
@@ -4,18 +4,18 @@ KernelVersion: 6.5
Contact: Miquel Raynal <[email protected]>
Description:
The "cells" folder contains one file per cell exposed by the
- NVMEM device. The name of the file is: <name>@<where>, with
- <name> being the cell name and <where> its location in the NVMEM
- device, in hexadecimal (without the '0x' prefix, to mimic device
- tree node names). The length of the file is the size of the cell
- (when known). The content of the file is the binary content of
- the cell (may sometimes be ASCII, likely without trailing
- character).
+ NVMEM device. The name of the file is: "<name>@<byte>,<bit>",
+ with <name> being the cell name and <where> its location in
+ the NVMEM device, in hexadecimal bytes and bits (without the
+ '0x' prefix, to mimic device tree node names). The length of
+ the file is the size of the cell (when known). The content of
+ the file is the binary content of the cell (may sometimes be
+ ASCII, likely without trailing character).
Note: This file is only present if CONFIG_NVMEM_SYSFS
is enabled.
Example::
- hexdump -C /sys/bus/nvmem/devices/1-00563/cells/product-name@d
+ hexdump -C /sys/bus/nvmem/devices/1-00563/cells/product-name@d,0
00000000 54 4e 34 38 4d 2d 50 2d 44 4e |TN48M-P-DN|
0000000a
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 1d77724f367d..9616c6001b3c 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -460,8 +460,9 @@ static int nvmem_populate_sysfs_cells(struct nvmem_device *nvmem)
list_for_each_entry(entry, &nvmem->cells, node) {
sysfs_bin_attr_init(&attrs[i]);
attrs[i].attr.name = devm_kasprintf(&nvmem->dev, GFP_KERNEL,
- "%s@%x", entry->name,
- entry->offset);
+ "%s@%x,%x", entry->name,
+ entry->offset,
+ entry->bit_offset);
attrs[i].attr.mode = 0444;
attrs[i].size = entry->bytes;
attrs[i].read = &nvmem_cell_attr_read;
--
2.39.2
On Mon, 22 Jan 2024 16:34:10 +0100, Arnd Bergmann wrote:
> Creating sysfs files for all Cells caused a boot failure for linux-6.8-rc1 on
> Apple M1, which (in downstream dts files) has multiple nvmem cells that use the
> same byte address. This causes the device probe to fail with
>
> [ 0.605336] sysfs: cannot create duplicate filename '/devices/platform/soc@200000000/2922bc000.efuse/apple_efuses_nvmem0/cells/efuse@a10'
> [ 0.605347] CPU: 7 PID: 1 Comm: swapper/0 Tainted: G S 6.8.0-rc1-arnd-5+ #133
> [ 0.605355] Hardware name: Apple Mac Studio (M1 Ultra, 2022) (DT)
> [ 0.605362] Call trace:
> [ 0.605365] show_stack+0x18/0x2c
> [ 0.605374] dump_stack_lvl+0x60/0x80
> [ 0.605383] dump_stack+0x18/0x24
> [ 0.605388] sysfs_warn_dup+0x64/0x80
> [ 0.605395] sysfs_add_bin_file_mode_ns+0xb0/0xd4
> [ 0.605402] internal_create_group+0x268/0x404
> [ 0.605409] sysfs_create_groups+0x38/0x94
> [ 0.605415] devm_device_add_groups+0x50/0x94
> [ 0.605572] nvmem_populate_sysfs_cells+0x180/0x1b0
> [ 0.605682] nvmem_register+0x38c/0x470
> [ 0.605789] devm_nvmem_register+0x1c/0x6c
> [ 0.605895] apple_efuses_probe+0xe4/0x120
> [ 0.606000] platform_probe+0xa8/0xd0
>
> [...]
Applied, thanks!
[1/1] nvmem: include bit index in cell sysfs file name
commit: b40fed13870045731e374e6bb48800cde0feb4e2
Best regards,
--
Srinivas Kandagatla <[email protected]>
Hi Arnd,
[email protected] wrote on Mon, 22 Jan 2024 16:34:10 +0100:
> From: Arnd Bergmann <[email protected]>
>
> Creating sysfs files for all Cells caused a boot failure for linux-6.8-rc1 on
> Apple M1, which (in downstream dts files) has multiple nvmem cells that use the
> same byte address. This causes the device probe to fail with
o_O I didn't even know this was allowed...
> [ 0.605336] sysfs: cannot create duplicate filename '/devices/platform/soc@200000000/2922bc000.efuse/apple_efuses_nvmem0/cells/efuse@a10'
> [ 0.605347] CPU: 7 PID: 1 Comm: swapper/0 Tainted: G S 6.8.0-rc1-arnd-5+ #133
> [ 0.605355] Hardware name: Apple Mac Studio (M1 Ultra, 2022) (DT)
> [ 0.605362] Call trace:
> [ 0.605365] show_stack+0x18/0x2c
> [ 0.605374] dump_stack_lvl+0x60/0x80
> [ 0.605383] dump_stack+0x18/0x24
> [ 0.605388] sysfs_warn_dup+0x64/0x80
> [ 0.605395] sysfs_add_bin_file_mode_ns+0xb0/0xd4
> [ 0.605402] internal_create_group+0x268/0x404
> [ 0.605409] sysfs_create_groups+0x38/0x94
> [ 0.605415] devm_device_add_groups+0x50/0x94
> [ 0.605572] nvmem_populate_sysfs_cells+0x180/0x1b0
> [ 0.605682] nvmem_register+0x38c/0x470
> [ 0.605789] devm_nvmem_register+0x1c/0x6c
> [ 0.605895] apple_efuses_probe+0xe4/0x120
> [ 0.606000] platform_probe+0xa8/0xd0
>
> As far as I can tell, this is a problem for any device with multiple cells on
> different bits of the same address. Avoid the issue by changing the file name
> to include the first bit number.
There is only one bit number right? We are talking about byte offsets
so this value can only range from 0 to 7? If we understand each other
correctly then why not, I'm fine with the extra ",0" thing.
> Fixes: 0088cbc19276 ("nvmem: core: Expose cells through sysfs")
> Link: https://github.com/AsahiLinux/linux/blob/bd0a1a7d4/arch/arm64/boot/dts/apple/t600x-dieX.dtsi#L156
> Cc: [email protected]
> Cc: Miquel Raynal <[email protected]>
> Cc: Rafał Miłecki <[email protected]>
> Cc: Chen-Yu Tsai <[email protected]>
> Cc: Srinivas Kandagatla <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: [email protected]
> Cc: Sven Peter <[email protected]>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
Thanks,
Miquèl
On Wed, Jan 24, 2024, at 18:22, Miquel Raynal wrote:
> [email protected] wrote on Mon, 22 Jan 2024 16:34:10 +0100:
>
>> From: Arnd Bergmann <[email protected]>
>>
>>
>> As far as I can tell, this is a problem for any device with multiple cells on
>> different bits of the same address. Avoid the issue by changing the file name
>> to include the first bit number.
>
> There is only one bit number right? We are talking about byte offsets
> so this value can only range from 0 to 7? If we understand each other
> correctly then why not, I'm fine with the extra ",0" thing.
On the Apple M1, the nvmem registers are 32 bit wide, so the
bit numbers can go up to 31. I can imagine some system using
64-bit registers, but it's unlikely to be higher than that.
Arnd
Hi Arnd,
[email protected] wrote on Wed, 24 Jan 2024 20:49:53 +0100:
> On Wed, Jan 24, 2024, at 18:22, Miquel Raynal wrote:
> > [email protected] wrote on Mon, 22 Jan 2024 16:34:10 +0100:
> >
> >> From: Arnd Bergmann <[email protected]>
> >>
> >>
> >> As far as I can tell, this is a problem for any device with multiple cells on
> >> different bits of the same address. Avoid the issue by changing the file name
> >> to include the first bit number.
> >
> > There is only one bit number right? We are talking about byte offsets
> > so this value can only range from 0 to 7? If we understand each other
> > correctly then why not, I'm fine with the extra ",0" thing.
>
> On the Apple M1, the nvmem registers are 32 bit wide, so the
> bit numbers can go up to 31. I can imagine some system using
> 64-bit registers, but it's unlikely to be higher than that.
In this case we will soon or later have a problem again. Can we include
the full offset of the bit and not just the first digit?
Thanks,
Miquèl
On Wed, Jan 24, 2024, at 23:11, Miquel Raynal wrote:
> Hi Arnd,
>
> [email protected] wrote on Wed, 24 Jan 2024 20:49:53 +0100:
>
>> On Wed, Jan 24, 2024, at 18:22, Miquel Raynal wrote:
>> > [email protected] wrote on Mon, 22 Jan 2024 16:34:10 +0100:
>> >
>> >> From: Arnd Bergmann <[email protected]>
>> >>
>> >>
>> >> As far as I can tell, this is a problem for any device with multiple cells on
>> >> different bits of the same address. Avoid the issue by changing the file name
>> >> to include the first bit number.
>> >
>> > There is only one bit number right? We are talking about byte offsets
>> > so this value can only range from 0 to 7? If we understand each other
>> > correctly then why not, I'm fine with the extra ",0" thing.
>>
>> On the Apple M1, the nvmem registers are 32 bit wide, so the
>> bit numbers can go up to 31. I can imagine some system using
>> 64-bit registers, but it's unlikely to be higher than that.
>
> In this case we will soon or later have a problem again. Can we include
> the full offset of the bit and not just the first digit?
I thought that is what my patch does, maybe I don't
undestand the problem you are referring to. This is what
I see on my system with the patch applied:
$ cd /sys/devices/platform/soc@200000000/2922bc000.efuse
$ find . -name efuse\*
/apple_efuses_nvmem0/cells/efuse@a24,11
/apple_efuses_nvmem0/cells/efuse@a24,9
/apple_efuses_nvmem0/cells/efuse@a1c,f
/apple_efuses_nvmem0/cells/efuse@a20,17
/apple_efuses_nvmem0/cells/efuse@a20,1e
/apple_efuses_nvmem0/cells/efuse@a18,0
/apple_efuses_nvmem0/cells/efuse@a14,b
/apple_efuses_nvmem0/cells/efuse@a1c,1f
/apple_efuses_nvmem0/cells/efuse@a1c,d
/apple_efuses_nvmem0/cells/efuse@a20,1c
/apple_efuses_nvmem0/cells/efuse@a18,15
/apple_efuses_nvmem0/cells/efuse@a14,0
/apple_efuses_nvmem0/cells/efuse@a1c,14
/apple_efuses_nvmem0/cells/efuse@a24,3
/apple_efuses_nvmem0/cells/efuse@a20,7
/apple_efuses_nvmem0/cells/efuse@a18,5
/apple_efuses_nvmem0/cells/efuse@a10,16
/apple_efuses_nvmem0/cells/efuse@a1c,12
/apple_efuses_nvmem0/cells/efuse@a20,5
/apple_efuses_nvmem0/cells/efuse@a18,3
/apple_efuses_nvmem0/cells/efuse@a18,a
/apple_efuses_nvmem0/cells/efuse@a10,1b
/apple_efuses_nvmem0/cells/efuse@a14,5
/apple_efuses_nvmem0/cells/efuse@a1c,19
/apple_efuses_nvmem0/cells/efuse@a24,f
/apple_efuses_nvmem0/cells/efuse@a18,1d
/apple_efuses_nvmem0/cells/efuse@a14,13
/apple_efuses_nvmem0/cells/efuse@a18,8
/apple_efuses_nvmem0/cells/efuse@a18,f
/apple_efuses_nvmem0/cells/efuse@a20,14
/apple_efuses_nvmem0/cells/efuse@a10,19
/apple_efuses_nvmem0/cells/efuse@a18,1b
/apple_efuses_nvmem0/cells/efuse@a14,11
/apple_efuses_nvmem0/cells/efuse@a1c,a
/apple_efuses_nvmem0/cells/efuse@a10,1e
/apple_efuses_nvmem0/cells/efuse@a20,19
Arnd
Hi Arnd,
[email protected] wrote on Thu, 25 Jan 2024 13:15:26 +0100:
> On Wed, Jan 24, 2024, at 23:11, Miquel Raynal wrote:
> > Hi Arnd,
> >
> > [email protected] wrote on Wed, 24 Jan 2024 20:49:53 +0100:
> >
> >> On Wed, Jan 24, 2024, at 18:22, Miquel Raynal wrote:
> >> > [email protected] wrote on Mon, 22 Jan 2024 16:34:10 +0100:
> >> >
> >> >> From: Arnd Bergmann <[email protected]>
> >> >>
> >> >>
> >> >> As far as I can tell, this is a problem for any device with multiple cells on
> >> >> different bits of the same address. Avoid the issue by changing the file name
> >> >> to include the first bit number.
> >> >
> >> > There is only one bit number right? We are talking about byte offsets
> >> > so this value can only range from 0 to 7? If we understand each other
> >> > correctly then why not, I'm fine with the extra ",0" thing.
> >>
> >> On the Apple M1, the nvmem registers are 32 bit wide, so the
> >> bit numbers can go up to 31. I can imagine some system using
> >> 64-bit registers, but it's unlikely to be higher than that.
> >
> > In this case we will soon or later have a problem again. Can we include
> > the full offset of the bit and not just the first digit?
>
> I thought that is what my patch does, maybe I don't
> undestand the problem you are referring to. This is what
> I see on my system with the patch applied:
>
> $ cd /sys/devices/platform/soc@200000000/2922bc000.efuse
> $ find . -name efuse\*
> ./apple_efuses_nvmem0/cells/efuse@a24,11
Sorry for the misunderstanding, I thought the above situation would
be:
./apple_efuses_nvmem0/cells/efuse@a24,1
But the below output is actually fine.
Reviewed-by: Miquel Raynal <[email protected]>
> ./apple_efuses_nvmem0/cells/efuse@a24,9
> ./apple_efuses_nvmem0/cells/efuse@a1c,f
> ./apple_efuses_nvmem0/cells/efuse@a20,17
> ./apple_efuses_nvmem0/cells/efuse@a20,1e
> ./apple_efuses_nvmem0/cells/efuse@a18,0
> ./apple_efuses_nvmem0/cells/efuse@a14,b
> ./apple_efuses_nvmem0/cells/efuse@a1c,1f
> ./apple_efuses_nvmem0/cells/efuse@a1c,d
> ./apple_efuses_nvmem0/cells/efuse@a20,1c
> ./apple_efuses_nvmem0/cells/efuse@a18,15
> ./apple_efuses_nvmem0/cells/efuse@a14,0
> ./apple_efuses_nvmem0/cells/efuse@a1c,14
> ./apple_efuses_nvmem0/cells/efuse@a24,3
> ./apple_efuses_nvmem0/cells/efuse@a20,7
> ./apple_efuses_nvmem0/cells/efuse@a18,5
> ./apple_efuses_nvmem0/cells/efuse@a10,16
> ./apple_efuses_nvmem0/cells/efuse@a1c,12
> ./apple_efuses_nvmem0/cells/efuse@a20,5
> ./apple_efuses_nvmem0/cells/efuse@a18,3
> ./apple_efuses_nvmem0/cells/efuse@a18,a
> ./apple_efuses_nvmem0/cells/efuse@a10,1b
> ./apple_efuses_nvmem0/cells/efuse@a14,5
> ./apple_efuses_nvmem0/cells/efuse@a1c,19
> ./apple_efuses_nvmem0/cells/efuse@a24,f
> ./apple_efuses_nvmem0/cells/efuse@a18,1d
> ./apple_efuses_nvmem0/cells/efuse@a14,13
> ./apple_efuses_nvmem0/cells/efuse@a18,8
> ./apple_efuses_nvmem0/cells/efuse@a18,f
> ./apple_efuses_nvmem0/cells/efuse@a20,14
> ./apple_efuses_nvmem0/cells/efuse@a10,19
> ./apple_efuses_nvmem0/cells/efuse@a18,1b
> ./apple_efuses_nvmem0/cells/efuse@a14,11
> ./apple_efuses_nvmem0/cells/efuse@a1c,a
> ./apple_efuses_nvmem0/cells/efuse@a10,1e
> ./apple_efuses_nvmem0/cells/efuse@a20,19
>
> Arnd
Thanks,
Miquèl
On 22.01.24 17:55, Srinivas Kandagatla wrote:
> On Mon, 22 Jan 2024 16:34:10 +0100, Arnd Bergmann wrote:
>> Creating sysfs files for all Cells caused a boot failure for linux-6.8-rc1 on
>> Apple M1, which (in downstream dts files) has multiple nvmem cells that use the
>> same byte address. This causes the device probe to fail with
>>
>> [ 0.605336] sysfs: cannot create duplicate filename '/devices/platform/soc@200000000/2922bc000.efuse/apple_efuses_nvmem0/cells/efuse@a10'
>> [ 0.605347] CPU: 7 PID: 1 Comm: swapper/0 Tainted: G S 6.8.0-rc1-arnd-5+ #133
>> [ 0.605355] Hardware name: Apple Mac Studio (M1 Ultra, 2022) (DT)
>> [ 0.605362] Call trace:
>> [...]
>
> Applied, thanks!
>
> [1/1] nvmem: include bit index in cell sysfs file name
> commit: b40fed13870045731e374e6bb48800cde0feb4e2
The problem description from Arnd to an outsider like me sounded like
this is something that should be fixed rather sooner than later in
mainline. Am I wrong with that? If not: will this be heading to Linus
soon? Just wondering, as the fix seems to be a in "for-next" branch[1]
of the nvmem repo and not in a "fixes" branch.
Or am I missing something here?
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
[1] https://git.kernel.org/pub/scm/linux/kernel/git/srini/nvmem.git
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
#regzbot poke
On Fri, Feb 9, 2024, at 10:09, Linux regression tracking (Thorsten Leemhuis) wrote:
> On 22.01.24 17:55, Srinivas Kandagatla wrote:
>> On Mon, 22 Jan 2024 16:34:10 +0100, Arnd Bergmann wrote:
>>> Creating sysfs files for all Cells caused a boot failure for linux-6.8-rc1 on
>>> Apple M1, which (in downstream dts files) has multiple nvmem cells that use the
>>> same byte address. This causes the device probe to fail with
>>>
>>> [ 0.605336] sysfs: cannot create duplicate filename '/devices/platform/soc@200000000/2922bc000.efuse/apple_efuses_nvmem0/cells/efuse@a10'
>>> [ 0.605347] CPU: 7 PID: 1 Comm: swapper/0 Tainted: G S 6.8.0-rc1-arnd-5+ #133
>>> [ 0.605355] Hardware name: Apple Mac Studio (M1 Ultra, 2022) (DT)
>>> [ 0.605362] Call trace:
>>> [...]
>>
>> Applied, thanks!
>>
>> [1/1] nvmem: include bit index in cell sysfs file name
>> commit: b40fed13870045731e374e6bb48800cde0feb4e2
>
> The problem description from Arnd to an outsider like me sounded like
> this is something that should be fixed rather sooner than later in
> mainline. Am I wrong with that? If not: will this be heading to Linus
> soon? Just wondering, as the fix seems to be a in "for-next" branch[1]
> of the nvmem repo and not in a "fixes" branch.
Yes, this that this needs to be fixed before v6.8 is
out. I assumed it had gone upstream already.
If anyone is still unsure about the ABI, we could also revert
the original commit 0088cbc19276 ("nvmem: core: Expose cells
through sysfs") for v6.8 and try again for v6.9 with the
fixed ABI, but I think we already had an agreement on the
changes that I sent.
Arnd