[PATCH] ASoC: sun4i-spdif: Use guard() for spin locks

[Bui Duc Phuc]

Hi,

I am not entirely sure whether the issue is exactly as
described in the Sashiko review.

However, I also looked at a similar case in the
micfil_rate_set() function from the fsl_micfil driver:
https://elixir.bootlin.com/linux/v7.1-rc3/source/sound/soc/fsl/fsl_micfil.c

There, pm_runtime_resume_and_get() is also used to ensure
that the device is runtime-resumed before accessing the
registers.

To address the review comments and avoid mixing functional
changes with the cleanup/refactoring using guard/scoped_guard,
I am planning to split this into two separate patches:

Patch 1: replace spin_lock_irqsave() /
spin_unlock_irqrestore() with scoped_guard() / guard()

+ scoped_guard(spinlock_irqsave, &host->lock) {

    regmap_read(...);

+ }

Patch 2: add runtime PM handling around register accesses

+ ret = pm_runtime_resume_and_get(dev);
+ if (ret < 0)
+     return ret;

  scoped_guard(spinlock_irqsave, &host->lock) {

      regmap_read(...);

  }

+ pm_runtime_put(dev);

This should make the review easier and clearly separate the
functional change from the refactoring.
Does this approach look reasonable to you?
If so, please let me know and I will prepare the
corresponding patches.

Best Regard,
Phuc




On Thu, May 14, 2026 at 6:22 PM Bui Duc Phuc <phucduc.bui at gmail.com> wrote:
>
> Thanks for the review.
>
> This seems to be pre-existing behavior rather than something
> introduced by this patch, since the current change only converts
> the locking to use guard() helpers and does not modify the
> runtime PM handling.
>
> If this turns out to be a real issue, it would probably be better
> addressed separately from this cleanup patch.
>
>
> On Thu, May 14, 2026 at 11:33 AM <sashiko-bot at kernel.org> wrote:
> >
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [High] Hardware State / PM Violation leading to bus hang
> > --
> >
> > commit 0686001584788d984a0f16403183bcad78fe4176
> > Author: bui duc phuc <phucduc.bui at gmail.com>
> >
> > ASoC: sun4i-spdif: Use guard() for spin locks
> >
> > This commit refactors the locking in the sun4i-spdif driver to use the
> > guard() macro for spinlocks. It replaces manual spin_lock_irqsave() and
> > spin_unlock_irqrestore() calls with scope-based cleanup.
> >
> > > diff --git a/sound/soc/sunxi/sun4i-spdif.c b/sound/soc/sunxi/sun4i-spdif.c
> > > --- a/sound/soc/sunxi/sun4i-spdif.c
> > > +++ b/sound/soc/sunxi/sun4i-spdif.c
> > > @@ -427,10 +427,9 @@ static int sun4i_spdif_get_status(struct snd_kcontrol *kcontrol,
> > >       struct snd_soc_dai *cpu_dai = snd_kcontrol_chip(kcontrol);
> > >       struct sun4i_spdif_dev *host = snd_soc_dai_get_drvdata(cpu_dai);
> > >       u8 *status = ucontrol->value.iec958.status;
> > > -     unsigned long flags;
> > >       unsigned int reg;
> > >
> > > -     spin_lock_irqsave(&host->lock, flags);
> > > +     guard(spinlock_irqsave)(&host->lock);
> > >
> > >       regmap_read(host->regmap, SUN4I_SPDIF_TXCHSTA0, &reg);
> >
> > While this issue isn't new to this patch, does this code risk a bus hang if
> > the device is runtime-suspended?
> >
> > The regmap configuration lacks a cache, and these ALSA kcontrols can be
> > accessed by userspace at any time. If the audio stream is idle, the device
> > will be runtime-suspended and its APB bus clock (apb_clk) will be disabled.
> >
> > Accessing the physical registers with the clock disabled will trigger an
> > asynchronous external abort.
> >
> > Would it make sense to ensure the device is runtime-resumed (perhaps via
> > pm_runtime_get_sync() outside the spinlock guard) before touching the
> > hardware?
> >
> > [ ... ]
> >
> > --
> > Sashiko AI review · https://sashiko.dev/#/patchset/20260513105003.81880-1-phucduc.bui@gmail.com?part=1