Re: [PATCH] ASoC: ak4458: Check reset status after deassert

[Shengjiu Wang]

Hi Alvin, Philipp Zabel

On Sun, Oct 11, 2026 at 3:05 AM Alvin Šipraga <alvin.sipraga@xxxxxxxxxx> wrote:
>
> On Sat, Oct 10, 2026 at 04:40:58PM +0800, shengjiu.wang@xxxxxxxxxxx wrote:
> > From: Shengjiu Wang <shengjiu.wang@xxxxxxx>
> >
> > The reset line of the ak4458 can be shared, for example when two ak4458
> > devices use the same reset GPIO. In that case the two codecs may call
> > reset_control_deassert() concurrently from their runtime resume paths.
> >
> > Only the first caller performs the actual hardware deassert, which may
> > sleep (for example a GPIO-expander based reset controller). A second
> > caller merely increments the shared deassert count and returns
> > immediately, so it can continue while the first caller's .deassert() has
> > not finished releasing the line yet, leaving the device still in reset.
>
> This seems like something that should be fixed in the reset controller
> framework. In fact the issue you describe seems to contradict the
> documentation of reset_control_deassert():
>
> /**
>  * reset_control_deassert - deasserts the reset line
>  * @rstc: reset controller
>  *
>  * After calling this function, the reset is guaranteed to be deasserted.
>
> ^ (1) seems not to be the case as you have found

Agreed. Looking at the documentation again, the guarantee

* After calling this function, the reset is guaranteed to be deasserted.

does not hold for a shared reset control today, so the issue is really in
the reset core. For a shared reset only the first caller runs the
hardware operation; later callers just bump the refcount and return
immediately, e.g. in reset_control_deassert():

if (atomic_inc_return(&rstc->deassert_count) != 1)
return 0;

So when two consumers of the same shared reset (here two ak4458 sharing
one reset GPIO) call concurrently, the second caller can return before
the first caller's .deassert() - which may sleep, e.g. a GPIO-expander
based controller - has actually released the line. assert() and reset()
have the same pattern.

Rather than work around it in the codec, I'd like to propose a reset-core
fix for your review. The idea is to take the existing per-control
rstc->lock across the shared refcount update and the hardware
.assert()/.deassert()/.reset(), so a second concurrent caller waits for
the first caller's hardware operation to complete. The lock is taken
before the SRCU section, matching reset_control_acquire(), and these
paths do not take rcdev->lock, so no new lock ordering is introduced:

if (reset_control_is_array(rstc))
return reset_control_array_deassert(rstc_to_array(rstc));

guard(mutex)(&rstc->lock);
guard(srcu)(&rstc->srcu);
...

Does this approach look reasonable to you? If so I will send it as a
proper patch and drop the ASoC: ak4458 change.

>
>  * Consumers must not use reset_control_reset on shared reset lines when
>  * reset_control_(de)assert has been used.
>
> ^ (2) isn't this rule also being broken in the suspend/resume path of
>       this driver?

I don't think so: the ak4458 suspend/resume path only uses
reset_control_assert()/reset_control_deassert(); it never calls
reset_control_reset(), so this rule isn't violated here. Please let me
know if you had a different concern in mind.

Best regards
Shengjiu Wang
>
>  *
>  * If rstc is NULL it is an optional reset and the function will just
>  * return 0.
>  */
> int reset_control_deassert(struct reset_control *rstc)
> ...
>
>
> (1) being fixed in the framework would solve your problem.
>
> But (2) seems a bit more intractable. Shared reset GPIOs have always
> been a pain... I wonder if there is a better approach?
>
> Kind regards,
> Alvin
>