[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAOgh=FwFMaYvTwmqeOQrqHq2XANxghZbTuY3+SgFx_ozpysBOg@mail.gmail.com>
Date: Fri, 9 Feb 2024 16:49:22 +0000
From: Eric Curtin <ecurtin@...hat.com>
To: srinivas.kandagatla@...aro.org
Cc: gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
Arnd Bergmann <arnd@...db.de>, regressions@...ts.linux.dev,
Miquel Raynal <miquel.raynal@...tlin.com>, Rafał Miłecki <rafal@...ecki.pl>,
Chen-Yu Tsai <wenst@...omium.org>, asahi@...ts.linux.dev, Sven Peter <sven@...npeter.dev>
Subject: Re: [PATCH] nvmem: include bit index in cell sysfs file name
On Fri, 9 Feb 2024 at 16:43, <srinivas.kandagatla@...aro.org> wrote:
>
> From: Arnd Bergmann <arnd@...db.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@...000000/2922bc000.efuse/apple_efuses_nvmem0/cells/efuse@...'
> [ 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@...ts.linux.dev
> Cc: Miquel Raynal <miquel.raynal@...tlin.com>
> Cc: Rafał Miłecki <rafal@...ecki.pl>
> Cc: Chen-Yu Tsai <wenst@...omium.org>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Cc: asahi@...ts.linux.dev
> Cc: Sven Peter <sven@...npeter.dev>
> Signed-off-by: Arnd Bergmann <arnd@...db.de>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
Seems reasonable to me.
Reviewed-by: Eric Curtin <ecurtin@...hat.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@...tlin.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
>
>
Powered by blists - more mailing lists