2024-02-09 16:46:31

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH] nvmem: include bit index in cell sysfs file name

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: 0331c611949f ("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]>
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
Hi Greg,

Here is a fix in nvmem for 6.8, could you queue these for next possible rc.

Did not cc Stable as this is only targeted for 6.8 and no backporting is
required.

Thanks,
Srini

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 980123fb4dde..eb357ac2e54a 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.25.1



2024-02-09 17:01:56

by Eric Curtin

[permalink] [raw]
Subject: Re: [PATCH] nvmem: include bit index in cell sysfs file name

On Fri, 9 Feb 2024 at 16:43, <[email protected]> wrote:
>
> 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: 0331c611949f ("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]>
> Signed-off-by: Srinivas Kandagatla <[email protected]>

Seems reasonable to me.

Reviewed-by: Eric Curtin <[email protected]>

Is mise le meas/Regards,

Eric Curtin

> ---
> Hi Greg,
>
> Here is a fix in nvmem for 6.8, could you queue these for next possible rc.
>
> Did not cc Stable as this is only targeted for 6.8 and no backporting is
> required.
>
> Thanks,
> Srini
>
> 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 980123fb4dde..eb357ac2e54a 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.25.1
>
>


2024-02-13 19:21:29

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH] nvmem: include bit index in cell sysfs file name

Hi,

[email protected] wrote on Fri, 9 Feb 2024 16:49:22 +0000:

> On Fri, 9 Feb 2024 at 16:43, <[email protected]> wrote:
> >
> > 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: 0331c611949f ("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]>
> > Signed-off-by: Srinivas Kandagatla <[email protected]>
>

My R-by must have been lost, here it is again:

Reviewed-by: Miquel Raynal <[email protected]>

Thanks,
Miquèl

2024-02-14 15:29:01

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] nvmem: include bit index in cell sysfs file name

On Fri, Feb 09, 2024 at 04:34:54PM +0000, [email protected] wrote:
> 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: 0331c611949f ("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]>
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
> Hi Greg,
>
> Here is a fix in nvmem for 6.8, could you queue these for next possible rc.
>
> Did not cc Stable as this is only targeted for 6.8 and no backporting is
> required.

Sorry for the delay, now queued up.

greg k-h