Re: [PATCH v3 3/4] ASoC: es9039q2m: add ESS Technology ES9039Q2M codec driver

> > +out:
> > + regmap_update_bits(priv->regmap, ES9039_PROG_RAM_CTRL,
> > +   ES9039_PROG_COEFF_WE, 0);
> > + kfree(buf);
> > + if (ret)
> > + return ret;
>
> How does this work with suspend and resume if the device is powered down
> in suspend?  It looks like this is doing some windowing stuff and needs
> a specific write sequence.

You are right, and it is broken as posted.

The coefficient RAM is write-only: PROG_COEFF_OUT is documented as a
readback register but returns the last coefficient written rather than the
addressed one, as the commit message describes.  So regmap cannot cache it
and nothing else in the system holds a copy.  On a board that keeps the
part powered through suspend nothing is lost, but where the supplies go
away the RAM comes back undefined while regcache_sync() restores the
filter selection from the cache - so selecting the programmable filter
would point the interpolator at whatever the RAM powered up holding.

For v4 the driver keeps a shadow of the last upload in its private struct,
the upload loop is split into a helper, and resume calls that helper again
after regcache_sync() for any stage that was ever written.  Stages never
uploaded keep the part's own defaults.

You are right about the sequence too.  It is address, data, raise
PROG_COEFF_WE, lower it, once per coefficient - a per-coefficient strobe
rather than a gate held open across the upload.  Splitting the helper out
exposed a second problem I had missed: both FIR controls drive the same
address, data and strobe registers and nothing serialised them, so two
concurrent uploads could interleave into each other's RAM.  The mutex that
orders an upload against resume closes that as well.

For completeness: an automated review on this thread raised the same
suspend question a few hours before your mail, so the fix above was
already written and building by the time it arrived.

I will hold v4 until your review is finished rather than resending
piecemeal.

Thanks,
Karl