Re: [PATCH 15/24] ASoC: codecs: cs42l43: Use guard() and PM runtime scope helpers

[Bui Duc Phuc]

On Fri, Jun 26, 2026 at 7:57 PM Bui Duc Phuc <phucduc.bui@xxxxxxxxx> wrote:
>
> > > ---
> > > -     if (!jack)
> > > -             goto done;
> > > +     if (!jack) {
> > > +             regmap_update_bits(cs42l43->regmap, CS42L43_HS_BIAS_SENSE_AND_CLAMP_AUTOCONTROL,
> > > +                                CS42L43_JACKDET_MODE_MASK |
> > > +                                CS42L43_S0_AUTO_ADCMUTE_DISABLE_MASK |
> > > +                                CS42L43_HSBIAS_SENSE_TRIP_MASK, autocontrol);
> > > +             regmap_update_bits(cs42l43->regmap, CS42L43_PDNCNTL,
> > > +                                CS42L43_RING_SENSE_EN_MASK, pdncntl);
> > > +             dev_dbg(priv->dev, "Successfully configured accessory detect\n");
> > > +
> > > +             return 0;
> >
> > I really don't like this duplicating of code blocks, it takes us
> > to a situation we always need to remember to update both blocks
> > in sync which feels like it makes reviewing future patches very
> > error prone.
> >
>
> Hi Charles,
>
> I understand your concern about duplicating the code.
>
> My reasoning is that duplicating this block once lets us eliminate 12
> `goto` labels,
> which I think makes the overall flow easier to follow. To me, that's a
> reasonable trade-off.
>
> If you have a cleaner approach that avoids both the duplicated block
> and the 12 `goto` labels,
> I'd be happy to adopt it.
>
> Best regards,
> Phuc

Hi Charles,

I'll factor the duplicated code into a helper and use it from both
paths in v2, for example:

static void cs42l43_apply_accdet_config(struct cs42l43_codec *priv,
                    unsigned int autocontrol, unsigned int pdncntl)
{
    struct cs42l43 *cs42l43 = priv->core;

    regmap_update_bits(cs42l43->regmap,
CS42L43_HS_BIAS_SENSE_AND_CLAMP_AUTOCONTROL,
               CS42L43_JACKDET_MODE_MASK |
CS42L43_S0_AUTO_ADCMUTE_DISABLE_MASK |
               CS42L43_HSBIAS_SENSE_TRIP_MASK, autocontrol);
    regmap_update_bits(cs42l43->regmap, CS42L43_PDNCNTL,
               CS42L43_RING_SENSE_EN_MASK, pdncntl);

    dev_dbg(priv->dev, "Successfully configured accessory detect\n");
}

Then both paths can simply call cs42l43_apply_accdet_config(),
avoiding the duplicated code.
Does that address your concern?

Best regards,
Phuc