* [PATCH 1/3] ASoC: fsl_ssi: mark SACNT register volatile
@ 2015-12-20 20:30 Maciej S. Szmigiero
2015-12-23 13:12 ` Fabio Estevam
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Maciej S. Szmigiero @ 2015-12-20 20:30 UTC (permalink / raw)
To: alsa-devel@alsa-project•org
Cc: Timur Tabi, Nicolin Chen, Xiubo Li, Liam Girdwood, Mark Brown,
linuxppc-dev@lists•ozlabs.org, linux-kernel, Fabio Estevam
SACNT register should be marked volatile since
its WR and RD bits are cleared by SSI after
completing the relevant operation.
This unbreaks AC'97 register access.
Fixes: 05cf237972fe ("ASoC: fsl_ssi: Add driver suspend and resume to support MEGA Fast")
Signed-off-by: Maciej S. Szmigiero <mail@maciej•szmigiero.name>
---
sound/soc/fsl/fsl_ssi.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index e3abad5f980a..cc22354d7758 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -146,6 +146,7 @@ static bool fsl_ssi_volatile_reg(struct device *dev, unsigned int reg)
case CCSR_SSI_SRX1:
case CCSR_SSI_SISR:
case CCSR_SSI_SFCSR:
+ case CCSR_SSI_SACNT:
case CCSR_SSI_SACADD:
case CCSR_SSI_SACDAT:
case CCSR_SSI_SATAG:
@@ -239,8 +240,9 @@ struct fsl_ssi_private {
unsigned int baudclk_streams;
unsigned int bitclk_freq;
- /*regcache for SFCSR*/
+ /* regcache for volatile regs */
u32 regcache_sfcsr;
+ u32 regcache_sacnt;
/* DMA params */
struct snd_dmaengine_dai_dma_data dma_params_tx;
@@ -1587,6 +1589,8 @@ static int fsl_ssi_suspend(struct device *dev)
regmap_read(regs, CCSR_SSI_SFCSR,
&ssi_private->regcache_sfcsr);
+ regmap_read(regs, CCSR_SSI_SACNT,
+ &ssi_private->regcache_sacnt);
regcache_cache_only(regs, true);
regcache_mark_dirty(regs);
@@ -1605,6 +1609,8 @@ static int fsl_ssi_resume(struct device *dev)
CCSR_SSI_SFCSR_RFWM1_MASK | CCSR_SSI_SFCSR_TFWM1_MASK |
CCSR_SSI_SFCSR_RFWM0_MASK | CCSR_SSI_SFCSR_TFWM0_MASK,
ssi_private->regcache_sfcsr);
+ regmap_write(regs, CCSR_SSI_SACNT,
+ ssi_private->regcache_sacnt);
return regcache_sync(regs);
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] ASoC: fsl_ssi: mark SACNT register volatile
2015-12-20 20:30 [PATCH 1/3] ASoC: fsl_ssi: mark SACNT register volatile Maciej S. Szmigiero
@ 2015-12-23 13:12 ` Fabio Estevam
2015-12-24 16:12 ` [alsa-devel] " Timur Tabi
2016-01-10 21:33 ` Timur Tabi
2 siblings, 0 replies; 6+ messages in thread
From: Fabio Estevam @ 2015-12-23 13:12 UTC (permalink / raw)
To: Maciej S. Szmigiero
Cc: alsa-devel@alsa-project•org, Timur Tabi, Nicolin Chen, Xiubo Li,
Liam Girdwood, Mark Brown, linuxppc-dev@lists•ozlabs.org,
linux-kernel
On Sun, Dec 20, 2015 at 6:30 PM, Maciej S. Szmigiero
<mail@maciej•szmigiero.name> wrote:
> SACNT register should be marked volatile since
> its WR and RD bits are cleared by SSI after
> completing the relevant operation.
> This unbreaks AC'97 register access.
>
> Fixes: 05cf237972fe ("ASoC: fsl_ssi: Add driver suspend and resume to support MEGA Fast")
>
> Signed-off-by: Maciej S. Szmigiero <mail@maciej•szmigiero.name>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp•com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [alsa-devel] [PATCH 1/3] ASoC: fsl_ssi: mark SACNT register volatile
2015-12-20 20:30 [PATCH 1/3] ASoC: fsl_ssi: mark SACNT register volatile Maciej S. Szmigiero
2015-12-23 13:12 ` Fabio Estevam
@ 2015-12-24 16:12 ` Timur Tabi
2016-01-05 14:57 ` Fabio Estevam
2016-01-10 21:33 ` Timur Tabi
2 siblings, 1 reply; 6+ messages in thread
From: Timur Tabi @ 2015-12-24 16:12 UTC (permalink / raw)
To: Maciej S. Szmigiero
Cc: alsa-devel@alsa-project•org, Xiubo Li, linux-kernel,
Liam Girdwood, Nicolin Chen, Mark Brown, Fabio Estevam,
linuxppc-dev@lists•ozlabs.org
On Sun, Dec 20, 2015 at 2:30 PM, Maciej S. Szmigiero
<mail@maciej•szmigiero.name> wrote:
> SACNT register should be marked volatile since
> its WR and RD bits are cleared by SSI after
> completing the relevant operation.
> This unbreaks AC'97 register access.
>
> Fixes: 05cf237972fe ("ASoC: fsl_ssi: Add driver suspend and resume to support MEGA Fast")
>
> Signed-off-by: Maciej S. Szmigiero <mail@maciej•szmigiero.name>
These patches seem okay, but can we hold off merging them until I get
back from vacation and have a chance to review them?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [alsa-devel] [PATCH 1/3] ASoC: fsl_ssi: mark SACNT register volatile
2015-12-24 16:12 ` [alsa-devel] " Timur Tabi
@ 2016-01-05 14:57 ` Fabio Estevam
0 siblings, 0 replies; 6+ messages in thread
From: Fabio Estevam @ 2016-01-05 14:57 UTC (permalink / raw)
To: Timur Tabi
Cc: Maciej S. Szmigiero, alsa-devel@alsa-project•org, Xiubo Li,
linux-kernel, Liam Girdwood, Nicolin Chen, Mark Brown,
linuxppc-dev@lists•ozlabs.org
Timur,
On Thu, Dec 24, 2015 at 2:12 PM, Timur Tabi <timur@tabi•org> wrote:
> On Sun, Dec 20, 2015 at 2:30 PM, Maciej S. Szmigiero
> <mail@maciej•szmigiero.name> wrote:
>> SACNT register should be marked volatile since
>> its WR and RD bits are cleared by SSI after
>> completing the relevant operation.
>> This unbreaks AC'97 register access.
>>
>> Fixes: 05cf237972fe ("ASoC: fsl_ssi: Add driver suspend and resume to support MEGA Fast")
>>
>> Signed-off-by: Maciej S. Szmigiero <mail@maciej•szmigiero.name>
>
> These patches seem okay, but can we hold off merging them until I get
> back from vacation and have a chance to review them?
Have you had a chance to review these patches?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] ASoC: fsl_ssi: mark SACNT register volatile
2015-12-20 20:30 [PATCH 1/3] ASoC: fsl_ssi: mark SACNT register volatile Maciej S. Szmigiero
2015-12-23 13:12 ` Fabio Estevam
2015-12-24 16:12 ` [alsa-devel] " Timur Tabi
@ 2016-01-10 21:33 ` Timur Tabi
2016-01-11 15:52 ` Maciej S. Szmigiero
2 siblings, 1 reply; 6+ messages in thread
From: Timur Tabi @ 2016-01-10 21:33 UTC (permalink / raw)
To: Maciej S. Szmigiero, alsa-devel@alsa-project•org
Cc: Nicolin Chen, Xiubo Li, Liam Girdwood, Mark Brown,
linuxppc-dev@lists•ozlabs.org, linux-kernel, Fabio Estevam
Maciej S. Szmigiero wrote:
> + regmap_write(regs, CCSR_SSI_SACNT,
> + ssi_private->regcache_sacnt);
So I'm not familiar with all of the regcache features, but I understand
this patch. I was wondering if it makes sense to write the same exact
value that was read previously. Isn't it possible for the WR or RD bits
to change between fsl_ssi_suspend() and fsl_ssi_resume()? That is,
should we be doing this instead?
u32 temp;
regmap_read(regs, CCSR_SSI_SACNT, &temp);
temp &= 0x18; // preserve WR and RD
regmap_write(regs, CCSR_SSI_SACNT, (ssi_private->regcache_sacnt & ~0x18)
| temp);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] ASoC: fsl_ssi: mark SACNT register volatile
2016-01-10 21:33 ` Timur Tabi
@ 2016-01-11 15:52 ` Maciej S. Szmigiero
0 siblings, 0 replies; 6+ messages in thread
From: Maciej S. Szmigiero @ 2016-01-11 15:52 UTC (permalink / raw)
To: Timur Tabi, alsa-devel@alsa-project•org
Cc: Nicolin Chen, Xiubo Li, Liam Girdwood, Mark Brown,
linuxppc-dev@lists•ozlabs.org, linux-kernel, Fabio Estevam,
Zidan Wang
Hi Timur,
Thanks for review.
On 10.01.2016 22:33, Timur Tabi wrote:
> Maciej S. Szmigiero wrote:
>> + regmap_write(regs, CCSR_SSI_SACNT,
>> + ssi_private->regcache_sacnt);
>
> So I'm not familiar with all of the regcache features, but I understand this patch.
> I was wondering if it makes sense to write the same exact value that was read previously.
> Isn't it possible for the WR or RD bits to change between fsl_ssi_suspend() and fsl_ssi_resume()?
These bits are only set in fsl_ssi_ac97_{read,write} which then wait 100usecs
before returning. This should be enough for SSI core to finish the relevant
operation and clear the bits again, so theoretically they shouldn't be set
outside these functions.
However, if AC'97 register access is done concurrently with suspend or resume
the read / written reg data might be corrupted.
It looks to me this is indeed possible since SSI PM callbacks are set in its
platform driver struct but ASoC core only calls PM callbacks in snd_soc_dai_driver
(which SSI driver don't set).
If I am correct with this reasoning then these callbacks need to be added to
snd_soc_dai_driver but platform driver ones should still be provided in case
the driver is loaded but the sound card is not yet registered.
I've CCed Zidan since he originally added PM support to this driver.
> That is, should we be doing this instead?
>
> u32 temp;
> regmap_read(regs, CCSR_SSI_SACNT, &temp);
> temp &= 0x18; // preserve WR and RD
> regmap_write(regs, CCSR_SSI_SACNT, (ssi_private->regcache_sacnt & ~0x18) | temp);
>
Maciej
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-01-11 15:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-20 20:30 [PATCH 1/3] ASoC: fsl_ssi: mark SACNT register volatile Maciej S. Szmigiero
2015-12-23 13:12 ` Fabio Estevam
2015-12-24 16:12 ` [alsa-devel] " Timur Tabi
2016-01-05 14:57 ` Fabio Estevam
2016-01-10 21:33 ` Timur Tabi
2016-01-11 15:52 ` Maciej S. Szmigiero
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox