[alsa-devel] [PATCH 5/7] ASoC: Intel: sst: add power management handling

[Mark Brown]

On Fri, Jul 18, 2014 at 07:53:09PM +0530, Vinod Koul wrote:
> On Fri, Jul 18, 2014 at 12:46:11PM +0100, Mark Brown wrote:

> > > +	/* Move the SST state to Reset */
> > > +	sst_set_fw_state_locked(ctx, SST_RESET);

> > > +	flush_workqueue(ctx->post_msg_wq);
> > > +	synchronize_irq(ctx->irq_num);

> > This is very strange - we're flushing the work *after* resetting the DSP
> > which suggests there might be something else trying to access the DSP
> > while it is being put into reset.  Presumably that'd be bad.

> the macro sst_set_fw_state_locked is used here :)

> And state is marked reset, we dont do HW reset, so this is okay. After we return the
> suspend handler, PCI will put the device to D3.

It's still setting off alarm bells from a code review point of view -
having the DSP marked as in reset may well break something that some of
the other activity that's going on is doing.

> > > +static int intel_sst_suspend(struct device *dev)
> > > +{
> > > +
> > > +	return intel_sst_runtime_suspend(dev);
> > > +}

> > You currently need to check that the device isn't runtime suspended
> > before you try to reuse runtime PM ops in suspend.

> Sorry didnt quite get why?

The driver should be restoring the device to the state it was in prior
to suspend, and not repeating work that's already been done.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20140718/ea4b66aa/attachment-0001.sig>