linux-kernel - Re: [PATCH RFT/RFC] mfd: cs42l43: setup true links with software nodes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMRc=MciO0WYejOYZduqE73U4OVTxcaMfe6Sv1VXWJWL2FFNmw@mail.gmail.com>
Date: Wed, 19 Nov 2025 03:58:08 -0800
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Charles Keepax <ckeepax@...nsource.cirrus.com>
Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, 
	David Rhodes <david.rhodes@...rus.com>, Richard Fitzgerald <rf@...nsource.cirrus.com>, 
	Lee Jones <lee@...nel.org>, Mark Brown <broonie@...nel.org>, 
	Philipp Zabel <p.zabel@...gutronix.de>, Linus Walleij <linus.walleij@...aro.org>, 
	Maciej Strozek <mstrozek@...nsource.cirrus.com>, Andy Shevchenko <andy@...nel.org>, 
	linux-sound@...r.kernel.org, patches@...nsource.cirrus.com, 
	linux-kernel@...r.kernel.org, linux-spi@...r.kernel.org, 
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>, Bartosz Golaszewski <brgl@...ev.pl>
Subject: Re: [PATCH RFT/RFC] mfd: cs42l43: setup true links with software nodes

On Wed, 19 Nov 2025 12:24:09 +0100, Charles Keepax
<ckeepax@...nsource.cirrus.com> said:
> On Wed, Nov 19, 2025 at 12:06:57PM +0100, Bartosz Golaszewski wrote:
>> On Wed, Nov 19, 2025 at 11:58 AM Andy Shevchenko
>> <andriy.shevchenko@...ux.intel.com> wrote:
>> >
>> > On Wed, Nov 19, 2025 at 11:50:09AM +0100, Bartosz Golaszewski wrote:
>> > > On Wed, Nov 19, 2025 at 11:47 AM Charles Keepax
>> > > <ckeepax@...nsource.cirrus.com> wrote:
>> > > > On Wed, Nov 19, 2025 at 11:38:35AM +0100, Bartosz Golaszewski wrote:
>> >
>> > ...
>> >
>> > > > Cool, thanks for all your help here. Might take me a couple hours
>> > > > but I will get to the bottom of the EBUSY thing, and report back.
>> > >
>> > > Sure. Could you just post the stack trace down to where the EBUSY is
>> > > returned in drivers/base/swnode.c? I'd like to analyze the logic
>> > > myself too if that's not too much work for you.
>> >
>> >
>> > FWIW, there is also an interesting debug technique. Put in your driver where
>> > you get this error code something like this (after all #include directives):
>> >
>> > #undef EBUSY
>> > #define EBUSY __LINE__
>> >
>> > And then you get some error code which is line number in some C file. You can
>> > narrow down grepping of the EBUSY in the suspected files and get the one.
>> >
>> >         git grep -n -w EBUSY -- ...files of interest...
>> >
>> > Hope this saves some minutes for you.
>> >
>>
>> Charles confirmed he gets it from device_add_software_node(). I want
>> to know how we're getting there and how we could end up already having
>> a software node.
>
> Ok I think I see what is happening now, the swnode is created on
> the first cell (the pinctrl). Then it moves onto the second cell,
> but mfd_acpi_add_device() copies the firmware node into both
> devices, the device_set_node() call at the bottom. So it inherits
> the swnode node through that primary fwnode.
>

You probably mean this line:

     device_set_node(&pdev->dev, acpi_fwnode_handle(adev ?: parent));

What is the actual device whose node we copy here? Would doing the following
help?

diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
index 7d14a1e7631ee..f7843f179e129 100644
--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -55,6 +55,7 @@ static void mfd_acpi_add_device(const struct mfd_cell *cell,
 				struct platform_device *pdev)
 {
 	const struct mfd_cell_acpi_match *match = cell->acpi_match;
+	struct fwnode_handle *acpi_fwnode;
 	struct acpi_device *adev = NULL;
 	struct acpi_device *parent;

@@ -87,7 +88,10 @@ static void mfd_acpi_add_device(const struct mfd_cell *cell,
 		}
 	}

-	device_set_node(&pdev->dev, acpi_fwnode_handle(adev ?: parent));
+	acpi_fwnode = acpi_fwnode_handle(adev ?: parent);
+
+	if (!is_software_node(acpi_fwnode) || !cell->swnode)
+		device_set_node(&pdev->dev, acpi_fwnode_handle(adev ?: parent));
 }
 #else
 static inline void mfd_acpi_add_device(const struct mfd_cell *cell,

> I am guessing this code has perhaps been more heavily tested on
> device tree where it is more common to have nodes for each cell,
> whereas ACPI is far more likely to have a single firmware node for
> the whole device.
>

If my logic above is right, we should not set the node here unless it's
an actual node coming from firmware OR the cell doesn't define its own
software node.

Bart

> Stack dump as requested:
>
> [    8.130022] Tainted: [S]=CPU_OUT_OF_SPEC, [E]=UNSIGNED_MODULE
> [    8.130023] Hardware name: AAEON UPX-TGL01/UPX-TGL01, BIOS UXTGBM16 03/31/2022
> [    8.130025] Workqueue: events_long cs42l43_boot_work [cs42l43]
> [    8.130032] Call Trace:
> [    8.130034]  <TASK>
> [    8.130037]  dump_stack_lvl+0x5d/0x80
> [    8.130048]  device_add_software_node+0x5c/0xb2
> [    8.130054]  mfd_add_device+0x248/0x447 [mfd_core]
> [    8.130061]  ? _printk+0x6b/0x90
> [    8.130069]  devm_mfd_add_devices.cold+0x3b/0x70a [mfd_core]
> [    8.130077]  cs42l43_boot_work.cold+0x1d3/0x7f7 [cs42l43]
> [    8.130084]  process_one_work+0x237/0x5c0
> [    8.130096]  worker_thread+0x1e1/0x3d0
> [    8.130103]  ? __pfx_worker_thread+0x10/0x10
> [    8.130107]  kthread+0x10d/0x250
> [    8.130112]  ? __pfx_kthread+0x10/0x10
> [    8.130117]  ret_from_fork+0x182/0x1d0
> [    8.130120]  ? __pfx_kthread+0x10/0x10
> [    8.130123]  ret_from_fork_asm+0x1a/0x30
> [    8.130134]  </TASK>
>
> Thanks,
> Charles
>