Re: Adjust the the codec and dai_link close sequence

Re: Adjust the the codec and dai_link close sequence

[Mark Brown]

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

On Mon, Jan 19, 2015 at 06:22:00PM -0800, Zhao Ye wrote:

Please fix your mailer to word wrap within paragraphs at something
confortably less than 80 columns, it makes your messages much easier to
read.

> We meet a issue, when we close the audio path, we found the dai link
> will closed after the codec. And the codec dai register will write
> fail.  This is because our audio codec is a I2C device, and the codec
> dai’s dai_link register is in the I2C device. If we close the codec
> chip fisrt, The whole I2C chip’s power will be off, then we write the
> dai_link’s register will be fail.

I'm a little unclear on the exact scenario here but this sounds like the
CODEC driver is probably buggy - it should either be putting the device
regmap into cache only mode when powered off (if there is no need for
the writes to take immediate effect) or not powering off until after
whatever cleanup is being done in the DAI is done.  Without knowing more
about what the CODEC is trying to do at each stage it's hard to say
exactly but it doesn't sound like an ordering issue.

> So I think we should adjust the
> rtd->dai_link->ops->shutdown(substream); before to
> codec_dai->driver->ops->shutdown(substream, codec_dai); in the
> soc_pcm_close() function.

Doing that then means that we run into trouble on systems where the
CODEC somehow depends on clocks from the DAI (which does happen)...
really there's no perfect ordering here and attempting to shuffle this
stuff around is likely to create bugs in other places as their
dependencies on the current ordering get exposed.  If there is some core
work needed it would be better to do something that explicitly shows the
dependencies so we don't get this sort of fragility.

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

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

Re: Adjust the the codec and dai_link close sequence

[Zhao Ye]

Ok, I understood what do you mean,
And I think we can add a cache in the codec driver.
Let the power off operation take effect after the DAI cleanup work done.



Best Regard
Zhao Ye (叶钊）
zhaoy@marvell.com
+61942433
Marvell Tech (SH) Co. Ltd.

-----Original Message-----
From: Mark Brown [mailto:broonie@kernel.org] 
Sent: 2015年2月24日 23:03
To: Zhao Ye
Cc: Liam Girdwood; tiwai@suse.de; perex@perex.cz; alsa-devel@alsa-project.org; Qiao Zhou
Subject: Re: Adjust the the codec and dai_link close sequence

On Mon, Jan 19, 2015 at 06:22:00PM -0800, Zhao Ye wrote:

Please fix your mailer to word wrap within paragraphs at something confortably less than 80 columns, it makes your messages much easier to read.

> We meet a issue, when we close the audio path, we found the dai link 
> will closed after the codec. And the codec dai register will write 
> fail.  This is because our audio codec is a I2C device, and the codec 
> dai’s dai_link register is in the I2C device. If we close the codec 
> chip fisrt, The whole I2C chip’s power will be off, then we write the 
> dai_link’s register will be fail.

I'm a little unclear on the exact scenario here but this sounds like the CODEC driver is probably buggy - it should either be putting the device regmap into cache only mode when powered off (if there is no need for the writes to take immediate effect) or not powering off until after whatever cleanup is being done in the DAI is done.  Without knowing more about what the CODEC is trying to do at each stage it's hard to say exactly but it doesn't sound like an ordering issue.

> So I think we should adjust the
> rtd->dai_link->ops->shutdown(substream); before to
> codec_dai->driver->ops->shutdown(substream, codec_dai); in the
> soc_pcm_close() function.

Doing that then means that we run into trouble on systems where the CODEC somehow depends on clocks from the DAI (which does happen)...
really there's no perfect ordering here and attempting to shuffle this stuff around is likely to create bugs in other places as their dependencies on the current ordering get exposed.  If there is some core work needed it would be better to do something that explicitly shows the dependencies so we don't get this sort of fragility.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Adjust the the codec and dai_link close sequence

[Mark Brown]

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

On Tue, Feb 24, 2015 at 09:03:36PM -0800, Zhao Ye wrote:
> Ok, I understood what do you mean,
> And I think we can add a cache in the codec driver.
> Let the power off operation take effect after the DAI cleanup work done.

If you're using regmap (which you should be) then this should be very
straightforward.

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

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