Register cache different from the real register values

Register cache different from the real register values

[Leon Romanovsky]

Hi, I need your suggestions, I'm trying to add new codec to use the
asoc subsystem, all read/write operations done via standard snd_soc_*
functions with cache enabled, but register cache is different from the
real register values. Can you point me to what can I check?

Our code is based on 2.6.38 release.

Thanks

-- 
Leon Romanovsky | Independent Linux Consultant
        www.leon.nu | leon@leon.nu
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Register cache different from the real register values

[Mark Brown]

On Sun, Oct 23, 2011 at 03:09:18PM +0200, Leon Romanovsky wrote:
> Hi, I need your suggestions, I'm trying to add new codec to use the
> asoc subsystem, all read/write operations done via standard snd_soc_*
> functions with cache enabled, but register cache is different from the
> real register values. Can you point me to what can I check?

Always CC maintainers on mails...  Post your code for review and we
might be able to spot something.  At a guess you're doing cache_only and
something's going wrong there, the register defaults are wrong or the
register word size is wrong.

Re: Register cache different from the real register values

[Leon Romanovsky]

On Sun, Oct 23, 2011 at 15:51, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Sun, Oct 23, 2011 at 03:09:18PM +0200, Leon Romanovsky wrote:
>> Hi, I need your suggestions, I'm trying to add new codec to use the
>> asoc subsystem, all read/write operations done via standard snd_soc_*
>> functions with cache enabled, but register cache is different from the
>> real register values. Can you point me to what can I check?
>
> Always CC maintainers on mails...
Sorry, I didn't know it.

> Post your code for review and we
> might be able to spot something.
Our code is located at
http://gitorious.org/~marvin24/ac100/marvin24s-kernel/blobs/chromeos-ac100-2.6.38/sound/soc/codecs/alc5632.c

>  At a guess you're doing cache_only and
> something's going wrong there
I also tried to add codec->cache_sync = 1 to the _probe function, but
without luck.
http://gitorious.org/~marvin24/ac100/marvin24s-kernel/blobs/chromeos-ac100-2.6.38/sound/soc/codecs/alc5632.c#line998

> the register defaults are wrong or the
Our style is a little different, because it is a port from the
android, we will be change it later, before merging into the mainline
to be more convenient.
http://gitorious.org/~marvin24/ac100/marvin24s-kernel/blobs/chromeos-ac100-2.6.38/sound/soc/codecs/alc5632.c#line1023

> register word size is wrong.
the register word size is 16 bits (ALC5632 specification).

-- 
Leon Romanovsky | Independent Linux Consultant
        www.leon.nu | leon@leon.nu
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Register cache different from the real register values

[Mark Brown]

On Sun, Oct 23, 2011 at 04:14:07PM +0200, Leon Romanovsky wrote:
> On Sun, Oct 23, 2011 at 15:51, Mark Brown

> > Post your code for review and we
> > might be able to spot something.

> Our code is located at
> http://gitorious.org/~marvin24/ac100/marvin24s-kernel/blobs/chromeos-ac100-2.6.38/sound/soc/codecs/alc5632.c

Please post for upstream, it's much easier to review and ideally we
could even get the driver merged.

> >  At a guess you're doing cache_only and
> > something's going wrong there
> I also tried to add codec->cache_sync = 1 to the _probe function, but
> without luck.
> http://gitorious.org/~marvin24/ac100/marvin24s-kernel/blobs/chromeos-ac100-2.6.38/sound/soc/codecs/alc5632.c#line998

You should only set cache_sync if you dirty the cache and then you need
to call cache_sync() at some point to actually sync the cache.

> > the register defaults are wrong or the

> Our style is a little different, because it is a port from the
> android, we will be change it later, before merging into the mainline
> to be more convenient.
> http://gitorious.org/~marvin24/ac100/marvin24s-kernel/blobs/chromeos-ac100-2.6.38/sound/soc/codecs/alc5632.c#line1023

Android is using the same kernel - there's no differences introduced by
Android here.  If you're seeing differences they're driver quality
issues (probably caused by not being mainline).  

Looking briefly at the code that android_init() function is just broken
and should be removed, any missing control should be implemented in the
driver.  The fill_cache() function looks suspect also - in general all
the code peering directly into the register cache data structure smells
bad.  You should have register defaults hard coded into the driver which
reflect the chip defaults so when we reset we know that's where the chip
is at.

Another thing to check is that the registers in the chip aren't
volatile, if the chip can change register values underneath the driver
then obviously things might get confused.  Multiple copies of the same
control are a common culprit.

Re: Register cache different from the real register values

[Leon Romanovsky]

On Sun, Oct 23, 2011 at 16:37, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Sun, Oct 23, 2011 at 04:14:07PM +0200, Leon Romanovsky wrote:
>> On Sun, Oct 23, 2011 at 15:51, Mark Brown
>
>> > Post your code for review and we
>> > might be able to spot something.
>
>> Our code is located at
>> http://gitorious.org/~marvin24/ac100/marvin24s-kernel/blobs/chromeos-ac100-2.6.38/sound/soc/codecs/alc5632.c
>
> Please post for upstream, it's much easier to review and ideally we
> could even get the driver merged.
>
>> >  At a guess you're doing cache_only and
>> > something's going wrong there
>> I also tried to add codec->cache_sync = 1 to the _probe function, but
>> without luck.
>> http://gitorious.org/~marvin24/ac100/marvin24s-kernel/blobs/chromeos-ac100-2.6.38/sound/soc/codecs/alc5632.c#line998
>
> You should only set cache_sync if you dirty the cache and then you need
> to call cache_sync() at some point to actually sync the cache.
>
>> > the register defaults are wrong or the
>
>> Our style is a little different, because it is a port from the
>> android, we will be change it later, before merging into the mainline
>> to be more convenient.
>> http://gitorious.org/~marvin24/ac100/marvin24s-kernel/blobs/chromeos-ac100-2.6.38/sound/soc/codecs/alc5632.c#line1023
>
> Android is using the same kernel - there's no differences introduced by
> Android here.  If you're seeing differences they're driver quality
> issues (probably caused by not being mainline).
>
> Looking briefly at the code that android_init() function is just broken
> and should be removed, any missing control should be implemented in the
> driver.  The fill_cache() function looks suspect also - in general all
> the code peering directly into the register cache data structure smells
> bad.  You should have register defaults hard coded into the driver which
> reflect the chip defaults so when we reset we know that's where the chip
> is at.
>
> Another thing to check is that the registers in the chip aren't
> volatile, if the chip can change register values underneath the driver
> then obviously things might get confused.  Multiple copies of the same
> control are a common culprit.
>

Thanks, I'll post the patches, as soon as possible.


-- 
Leon Romanovsky | Independent Linux Consultant
        www.leon.nu | leon@leon.nu
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel