diff mbox series

nvmem: include bit index in cell sysfs file name

Message ID	20240209163454.98051-1-srinivas.kandagatla@linaro.org
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel+bounces-59611-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1; From: srinivas.kandagatla@linaro.org To: gregkh@linuxfoundation.org Cc: linux-kernel@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>, regressions@lists.linux.dev, Miquel Raynal <miquel.raynal@bootlin.com>, =?utf-8?b?UmFmYcWCIE1pxYJlY2tp?= <rafal@milecki.pl>, Chen-Yu Tsai <wenst@chromium.org>, Srinivas Kandagatla <srinivas.kandagatla@linaro.org>, asahi@lists.linux.dev, Sven Peter <sven@svenpeter.dev> Subject: [PATCH] nvmem: include bit index in cell sysfs file name Date: Fri, 9 Feb 2024 16:34:54 +0000 Message-Id: <20240209163454.98051-1-srinivas.kandagatla@linaro.org> Precedence: bulk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-getmail-retrieved-from-mailbox: INBOX
Series	nvmem: include bit index in cell sysfs file name \| nvmem: include bit index in cell sysfs file name

Commit Message

Srinivas Kandagatla Feb. 9, 2024, 4:34 p.m. UTC

  From: Arnd Bergmann <arnd@arndb.de>

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: regressions@lists.linux.dev
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Rafał Miłecki <rafal@milecki.pl>
Cc: Chen-Yu Tsai <wenst@chromium.org>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: asahi@lists.linux.dev
Cc: Sven Peter <sven@svenpeter.dev>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
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(-)

Comments

Eric Curtin Feb. 9, 2024, 4:49 p.m. UTC | #1

On Fri, 9 Feb 2024 at 16:43, <srinivas.kandagatla@linaro.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> 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: regressions@lists.linux.dev
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Rafał Miłecki <rafal@milecki.pl>
> Cc: Chen-Yu Tsai <wenst@chromium.org>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: asahi@lists.linux.dev
> Cc: Sven Peter <sven@svenpeter.dev>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Seems reasonable to me.

Reviewed-by: Eric Curtin <ecurtin@redhat.com>

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 <miquel.raynal@bootlin.com>
>  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
>
>

Miquel Raynal Feb. 13, 2024, 7:20 p.m. UTC | #2

Hi,

ecurtin@redhat.com wrote on Fri, 9 Feb 2024 16:49:22 +0000:

> On Fri, 9 Feb 2024 at 16:43, <srinivas.kandagatla@linaro.org> wrote:
> >
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > 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: regressions@lists.linux.dev
> > Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> > Cc: Rafał Miłecki <rafal@milecki.pl>
> > Cc: Chen-Yu Tsai <wenst@chromium.org>
> > Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: asahi@lists.linux.dev
> > Cc: Sven Peter <sven@svenpeter.dev>
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>  
> 

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

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

Greg KH Feb. 14, 2024, 3:28 p.m. UTC | #3

On Fri, Feb 09, 2024 at 04:34:54PM +0000, srinivas.kandagatla@linaro.org wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> 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: regressions@lists.linux.dev
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Rafał Miłecki <rafal@milecki.pl>
> Cc: Chen-Yu Tsai <wenst@chromium.org>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: asahi@lists.linux.dev
> Cc: Sven Peter <sven@svenpeter.dev>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
> 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

diff mbox series

Patch

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 <miquel.raynal@bootlin.com>
 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;