Re: [PATCH v2] ASoC: wm9713: convert to regmap
On Mon, Oct 26, 2015 at 06:53:27PM +0100, Robert Jarzmik wrote: > Charles Keepax writes: > > > On Sat, Oct 24, 2015 at 11:37:07PM +0200, Robert Jarzmik wrote: > >> Convert the Wolfson WM9713 to regmap API. This will leverage all the > >> regmap functions (debug, registers update, etc ...). > >> > >> As a bonus, this will pave the path to gpio chip introduction, and > >> devicetree support. > >> > >> Signed-off-by: Robert Jarzmik > >> --- > > I am assuming we don't want to be doing a cache_sync just after we > > have marked the regmap as cache only. > Ah, this denotes my lack of understanding of regmap cache behavior. > Basically what I want at this point is that : > 1) All previous writes should hit the cache (cache coherency barrier) > 2) All subsequent writes should not go into the cache (uncached writes) > => especially the next writes in wm9713_soc_suspend() Cache only means that writes will only go into the cache. Cache sync writes the current state of the cache out to the hardware. Normally you would put the cache into cache only during suspend this allows controls etc. to still function but the writes go to the cache and thus the user doesn't need to worry if the codec is active or not. Then on resume you would turn cache only off such that writes will go to the hardware again and do a cache sync (you will need a call to mark the cache as dirty at some point as well), this will bring the hardware up to date with the writes that happened whilst it was suspended. Doing an update_bits that bypasses the cache can be difficult. You can use regcache_cache_bypass to effectively turn the cache off, but you need to synchronise it with everything else to ensure that no writes you want to hit the cache happen whilst it is disabled. Also you can use multi_write_bypassed to do a bunch of writes atomically that don't hit the cache, but those are whole writes rather than update_bits. > > Also as you have already marked the regmap as cache only these > > writes won't go to the hardware. > Ah, exactly the opposite of what I wanted :( > > Would you have an example driver I can inspire myself from. If not, I'll dig > in > more in the regmap cache API. wm8962 for example shows fairly typical use of these APIs. Thanks, Charles -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ASoC: wm9713: convert to regmap
Charles Keepax writes:
> On Sat, Oct 24, 2015 at 11:37:07PM +0200, Robert Jarzmik wrote:
>> Convert the Wolfson WM9713 to regmap API. This will leverage all the
>> regmap functions (debug, registers update, etc ...).
>>
>> As a bonus, this will pave the path to gpio chip introduction, and
>> devicetree support.
>>
>> Signed-off-by: Robert Jarzmik
>> ---
>> Since v1: fix suspend/resume (that specific part is not tested yet)
>> ---
>> sound/soc/codecs/Kconfig | 1 +
>> sound/soc/codecs/wm9713.c | 296
>> +++---
>> 2 files changed, 152 insertions(+), 145 deletions(-)
>
>> @@ -1156,16 +1165,17 @@ static int wm9713_set_bias_level(struct
>> snd_soc_codec *codec,
>>
>> static int wm9713_soc_suspend(struct snd_soc_codec *codec)
>> {
>> -u16 reg;
>> +regcache_cache_only(codec->component.regmap, true);
>> +snd_soc_cache_sync(codec);
>
> I am assuming we don't want to be doing a cache_sync just after we
> have marked the regmap as cache only.
Ah, this denotes my lack of understanding of regmap cache behavior.
Basically what I want at this point is that :
1) All previous writes should hit the cache (cache coherency barrier)
2) All subsequent writes should not go into the cache (uncached writes)
=> especially the next writes in wm9713_soc_suspend()
>> +snd_soc_update_bits(codec, AC97_EXTENDED_MID, 0x7fff,
>> + 0x7fff);
>> +snd_soc_write(codec, AC97_EXTENDED_MSTATUS, 0x);
>> +snd_soc_write(codec, AC97_POWERDOWN, 0x6f00);
>> +snd_soc_write(codec, AC97_POWERDOWN, 0x);
>
> Also as you have already marked the regmap as cache only these
> writes won't go to the hardware.
Ah, exactly the opposite of what I wanted :(
Would you have an example driver I can inspire myself from. If not, I'll dig in
more in the regmap cache API.
Cheers.
--
Robert
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ASoC: wm9713: convert to regmap
On Sat, Oct 24, 2015 at 11:37:07PM +0200, Robert Jarzmik wrote:
> Convert the Wolfson WM9713 to regmap API. This will leverage all the
> regmap functions (debug, registers update, etc ...).
>
> As a bonus, this will pave the path to gpio chip introduction, and
> devicetree support.
>
> Signed-off-by: Robert Jarzmik
> ---
> Since v1: fix suspend/resume (that specific part is not tested yet)
> ---
> sound/soc/codecs/Kconfig | 1 +
> sound/soc/codecs/wm9713.c | 296
> +++---
> 2 files changed, 152 insertions(+), 145 deletions(-)
> @@ -1156,16 +1165,17 @@ static int wm9713_set_bias_level(struct snd_soc_codec
> *codec,
>
> static int wm9713_soc_suspend(struct snd_soc_codec *codec)
> {
> - u16 reg;
> + regcache_cache_only(codec->component.regmap, true);
> + snd_soc_cache_sync(codec);
I am assuming we don't want to be doing a cache_sync just after we
have marked the regmap as cache only.
>
> /* Disable everything except touchpanel - that will be handled
>* by the touch driver and left disabled if touch is not in
>* use. */
> - reg = ac97_read(codec, AC97_EXTENDED_MID);
> - ac97_write(codec, AC97_EXTENDED_MID, reg | 0x7fff);
> - ac97_write(codec, AC97_EXTENDED_MSTATUS, 0x);
> - ac97_write(codec, AC97_POWERDOWN, 0x6f00);
> - ac97_write(codec, AC97_POWERDOWN, 0x);
> + snd_soc_update_bits(codec, AC97_EXTENDED_MID, 0x7fff,
> + 0x7fff);
> + snd_soc_write(codec, AC97_EXTENDED_MSTATUS, 0x);
> + snd_soc_write(codec, AC97_POWERDOWN, 0x6f00);
> + snd_soc_write(codec, AC97_POWERDOWN, 0x);
Also as you have already marked the regmap as cache only these
writes won't go to the hardware.
Thanks,
Charles
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
