[alsa-devel] [PATCH v4] ASoC: ak4613: call dummy write for PW_MGMT1/3 when Playback

[Kuninori Morimoto]

Hi Takashi

Thank you for your review

> > +static void ak4613_dummy_write(struct work_struct *work)
> > +{
> > +	struct ak4613_priv *priv = container_of(work,
> > +						struct ak4613_priv,
> > +						dummy_write_work);
> > +	struct snd_soc_component *component = priv->component;
> > +	unsigned int mgmt1;
> > +	unsigned int mgmt3;
> > +
> > +	/* wait 5 LR clocks */
> > +	udelay(5000000 / priv->rate);
> > +
> > +	snd_soc_component_read(component, PW_MGMT1, &mgmt1);
> > +	snd_soc_component_read(component, PW_MGMT3, &mgmt3);
> > +
> > +	snd_soc_component_write(component, PW_MGMT1, mgmt1);
> > +	snd_soc_component_write(component, PW_MGMT3, mgmt3);
> > +}
> 
> In my understanding, it's better to have care of kernel preemption in
> this case because data transmission is already activated and these
> should be executed as quick as possible to prevent much presentation
> loss.

Hmm.. it was included from v2 patch.
I need to create v5 patch again...

> > +	/*
> > +	 * FIXME
> > +	 *
> > +	 * PW_MGMT1 / PW_MGMT3 needs dummy write at least after 5 LR clocks
> > +	 * from Power Down Release. Otherwise, Playback volume will be 0dB.
> > +	 * To avoid complex multiple delay/dummy_write method from
> > +	 * ak4613_set_bias_level() / SND_SOC_DAPM_DAC_E("DACx", ...),
> > +	 * call it once here.
> > +	 *
> > +	 * But, unfortunately, we can't "write" here because here is atomic
> > +	 * context (It uses I2C access for write).
> > +	 * Thus, use delayed work with 0 delay to switch to normal context
> > +	 * immediately, and wait 5 LR clocks and do dummy_write there.
> 
> You don't use 'struct delayed_work' anymore.

Ahh yes indeed.

> Overall, no information about possibilities of presentation loss in
> the beginning of playback, which I addressed several times.

Its my fault.
I will add it in v5

> Before posting revised patches, please have a time to re-evaluate it
> in your side. It often brings easy mistakes because some points are
> newly added against its original shape. The points can causes
> contradictions and lose consistency.

I don't understand where is contradictions.
Anyway, basically indeed it is my fault, but one reason is that
someone is pointing very randomly, thus the patch because to v4 now.
Now I got new point which exist from v1 patch.
If these random review were included into one, my easy mistakes
can be reduced :P

Best regards
---
Kuninori Morimoto