Re: [PATCH v3] ASoC: rt5514: add rt5514 SPI driver

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1086 bytes --]

On Tue, May 17, 2016 at 02:33:16AM +0000, Oder Chiou wrote:
> > > +static void rt5514_spi_copy_work(struct work_struct *work)
> > > +{
> > > +	struct rt5514_dsp *rt5514_dsp =
> > > +		container_of(work, struct rt5514_dsp, copy_work.work);
> > > +	struct snd_pcm_runtime *runtime = rt5514_dsp->substream->runtime;
> > > +	size_t period_bytes, truncated_bytes = 0;

> > I can't see anything that cancels this work when the PCM stream to
> > userspace is stopped or if things power off otherwise?

> In the beginning of the function "rt5514_spi_copy_work", there is a if
> condition to check the variable "rt5514_dsp->substream", and the function
> "rt5514_spi_hw_free" will set the variable "rt5514_dsp->substream" to NULL.
> It also will make the delayed work stop and won't schedule again.

No, that won't stop the currently running work which means that on
driver removal we could for example try to free the driver data while
the work is still running.  The driver needs to explicitly ensure that
there is no work running when it shouldn't be, not just stop any new
work being added.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]