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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
