Re: [PATCH 16/21] soundwire: amd: Initialise and cancel IRQ work around manager lifetime

[Mukunda,Vijendar]

On 11/2/26 15:01, Lee Jones wrote:
> [Some people who received this message don't often get email from lee@xxxxxxxxxx. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> On Fri, 30 Oct 2026, Mario Limonciello wrote:
> > On 10/30/26 07:13, Mukunda,Vijendar wrote:
> > > On 10/30/26 16:16, Lee Jones wrote:
> > > > [You don't often get email from lee@xxxxxxxxxx. Learn why this is
> > > > important at https://aka.ms/LearnAboutSenderIdentification ]
> > > >
> > > > amd_sdw_manager_probe() publishes amd_manager via dev_set_drvdata()
> > > > before initialising amd_sdw_irq_thread and amd_sdw_work, allowing an
> > > > early interrupt to schedule uninitialised work.  During removal,
> > > > amd_sdw_manager_remove() only cancels amd_sdw_work without first
> > > > cancelling amd_sdw_irq_thread, allowing deferred IRQ work to re-queue
> > > > amd_sdw_work or dereference amd_manager after it is freed.
> > > >
> > > > Initialise both work items before calling dev_set_drvdata() in
> > > > amd_sdw_manager_probe(), and cancel amd_sdw_irq_thread before
> > > > amd_sdw_work in amd_sdw_manager_remove().
> > > >
> > > > Fixes: 65f93e4096a0 ("soundwire: amd: add SoundWire manager
> > > > interrupt handling")
> > > > Closes: https://sashiko.dev/#/bug/linux-9890e8dc-4b7d-44d3-
> > > > bfb0-171dc421e28c
> > > > Cc: stable@xxxxxxxxxxxxxxx
> > > > Signed-off-by: Lee Jones <lee@xxxxxxxxxx>
> > > > ---
> > > >    drivers/soundwire/amd_manager.c | 3 ++-
> > > >    1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/
> > > > amd_manager.c
> > > > index 98b09d81f3b8..2e1038ea0f36 100644
> > > > --- a/drivers/soundwire/amd_manager.c
> > > > +++ b/drivers/soundwire/amd_manager.c
> > > > @@ -1185,9 +1185,9 @@ static int amd_sdw_manager_probe(struct
> > > > platform_device *pdev)
> > > >                   sdw_bus_master_delete(&amd_manager->bus);
> > > >                   return ret;
> > > >           }
> > > > -       dev_set_drvdata(dev, amd_manager);
> > > >           INIT_WORK(&amd_manager->amd_sdw_irq_thread, amd_sdw_irq_thread);
> > > >           INIT_WORK(&amd_manager->amd_sdw_work,
> > > > amd_sdw_update_slave_status_work);
> > > > +       dev_set_drvdata(dev, amd_manager);
> > > The probe-side ordering issue is valid — an interrupt arriving after
> > > dev_set_drvdata() but before INIT_WORK() could schedule work on an
> > > uninitialised work_struct.
> > >
> > > The remove-side fix is already posted as part of our v3 patch series:
> > >
> > > https://lore.kernel.org/alsa-devel/20261013085111.2009061-6-
> > > Vijendar.Mukunda@xxxxxxx/T/#m60177687bb84758efdd6f5f09eba8ad32f0e4631
> > >
> > > The probe-side fix will be included in our upcoming v4 patch series,
> > > which contains major driver restructuring and customer-critical bug
> > > fixes. Merging this patch independently would conflict with v4 and
> > > complicate the review process.
> > >
> > > Could you please drop this patch? Both issues will be covered by our
> > > v3/v4 series.
> > Lee,
> >
> > I think as a general statement since there is still these big pending
> > changes already in flight, could you either wait a few weeks until they
> > merge or rebase your series on the in-flight version to see what still
> > actually makes sense?
> I will leave the AMD driver alone.
>
> Please be kind enough to add my Reported-by for any issued that you fix
> as a result of these reports.
Sure, thanks for reporting these issues.

I'll make sure to include your Reported-by: tag for any fixes resulting
from these reports.
> Thanks.
>
> --
> Lee Jones