diff mbox

[v4,2/5] ASoC: Move standard kcontrol helpers to the component level

Message ID 1398165797-22997-3-git-send-email-lars@metafoo.de (mailing list archive)
State New, archived
Headers show

Commit Message

Lars-Peter Clausen April 22, 2014, 11:23 a.m. UTC
After moving the IO layer inside ASoC to the component level we can now easily
move the standard control helpers also to the component level. This allows to
reuse the same standard helper control implementations for other components.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 include/sound/soc.h  |  20 ++++-
 sound/soc/soc-core.c | 221 ++++++++++++++++++++++++++++++++-------------------
 2 files changed, 156 insertions(+), 85 deletions(-)

Comments

Mark Brown April 22, 2014, 12:38 p.m. UTC | #1
On Tue, Apr 22, 2014 at 01:23:14PM +0200, Lars-Peter Clausen wrote:
> After moving the IO layer inside ASoC to the component level we can now easily
> move the standard control helpers also to the component level. This allows to
> reuse the same standard helper control implementations for other components.

Applied, thanks.  This also depended on the multicodec changes.
Shawn Guo May 9, 2014, 3 p.m. UTC | #2
Hi Lars,

On Tue, Apr 22, 2014 at 01:23:14PM +0200, Lars-Peter Clausen wrote:
> After moving the IO layer inside ASoC to the component level we can now easily
> move the standard control helpers also to the component level. This allows to
> reuse the same standard helper control implementations for other components.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

I'm running next-20140508 on imx6q-sabresd board and seeing repeated
'amixer: Mixer hw:0 load error: Device or resource busy' message with
Debian wheezy rootfs.  The git bisect points me to this patch.

== message output before this patch ==

[....] Setting up ALSA...warning: 'alsactl restore' failed with error message 'Found hardware: "wm8962-audio" "" "" "" ""
Hardware is initialized using a generic method
[ ok tl: set_control:1328: failed to obtain info for control #117 (No such file or directory)'...done.

== message output after this patch ==

[....] Setting up ALSA...warning: 'alsactl restore' failed with error message 'Found hardware: "wm8962-audio" "" "" "" ""
Hardware is initialized using a generic method
alsactl: set_control:1328: failed to obtain info for control #117 (No such file or directory)'...amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
amixer: Mixer hw:0 load error: Device or resource busy
done.

Why do we start seeing such error message with your patch?  Is this
a problem of rootfs or kernel?

Shawn
Lars-Peter Clausen May 9, 2014, 3:17 p.m. UTC | #3
On 05/09/2014 05:00 PM, Shawn Guo wrote:
> Hi Lars,
>
> On Tue, Apr 22, 2014 at 01:23:14PM +0200, Lars-Peter Clausen wrote:
>> After moving the IO layer inside ASoC to the component level we can now easily
>> move the standard control helpers also to the component level. This allows to
>> reuse the same standard helper control implementations for other components.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>
> I'm running next-20140508 on imx6q-sabresd board and seeing repeated
> 'amixer: Mixer hw:0 load error: Device or resource busy' message with
> Debian wheezy rootfs.  The git bisect points me to this patch.
>
> == message output before this patch ==
>
> [....] Setting up ALSA...warning: 'alsactl restore' failed with error message 'Found hardware: "wm8962-audio" "" "" "" ""
> Hardware is initialized using a generic method
> [ ok tl: set_control:1328: failed to obtain info for control #117 (No such file or directory)'...done.
>
> == message output after this patch ==
>
> [....] Setting up ALSA...warning: 'alsactl restore' failed with error message 'Found hardware: "wm8962-audio" "" "" "" ""
> Hardware is initialized using a generic method
> alsactl: set_control:1328: failed to obtain info for control #117 (No such file or directory)'...amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> amixer: Mixer hw:0 load error: Device or resource busy
> done.
>
> Why do we start seeing such error message with your patch?  Is this
> a problem of rootfs or kernel?

The changes in the patch should mostly be transparent. But what changed is 
how error reporting is handled. If there is a error reading/writing a 
register in the kcontrol callbacks that error is passed on to userspace 
whereas previously it was silently ignored.

There is also the possibility that there is a bug somewhere in the patch 
causing this.

Does the board otherwise work fine?

I actually have the board here, if you tell me which devictree/defconfig I 
can use with an upstream kernel I can give things a try.

- Lars
Fabio Estevam May 9, 2014, 3:34 p.m. UTC | #4
On Fri, May 9, 2014 at 12:17 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:

> The changes in the patch should mostly be transparent. But what changed is
> how error reporting is handled. If there is a error reading/writing a
> register in the kcontrol callbacks that error is passed on to userspace
> whereas previously it was silently ignored.
>
> There is also the possibility that there is a bug somewhere in the patch
> causing this.
>
> Does the board otherwise work fine?
>
> I actually have the board here, if you tell me which devictree/defconfig I
> can use with an upstream kernel I can give things a try.

make imx_v6_v7_defconfig
make imx6q-sabresd.dtb

Thanks
Lars-Peter Clausen May 9, 2014, 4:01 p.m. UTC | #5
On 05/09/2014 05:34 PM, Fabio Estevam wrote:
> On Fri, May 9, 2014 at 12:17 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>
>> The changes in the patch should mostly be transparent. But what changed is
>> how error reporting is handled. If there is a error reading/writing a
>> register in the kcontrol callbacks that error is passed on to userspace
>> whereas previously it was silently ignored.
>>
>> There is also the possibility that there is a bug somewhere in the patch
>> causing this.
>>
>> Does the board otherwise work fine?
>>
>> I actually have the board here, if you tell me which devictree/defconfig I
>> can use with an upstream kernel I can give things a try.
>
> make imx_v6_v7_defconfig
> make imx6q-sabresd.dtb

I just tried 907fe36a2c, e2c330b9b5 and next/master. I get the same behavior 
with all 3, no errors when loading a state file and audio out on the 
headphones works. I disabled DRM though since it deadlocked the system, 
maybe that makes a difference.

Is it possible that the error is coming from the I2C driver? Can you build 
the I2C driver with #define DEBUG and take a look at the output?

- Lars
Fabio Estevam May 9, 2014, 4:11 p.m. UTC | #6
On Fri, May 9, 2014 at 1:01 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:

> I just tried 907fe36a2c, e2c330b9b5 and next/master. I get the same behavior
> with all 3, no errors when loading a state file and audio out on the
> headphones works. I disabled DRM though since it deadlocked the system,
> maybe that makes a difference.

Here is the fix for this hang  (which hasn't reached linux-next yet):
http://www.spinics.net/lists/arm-kernel/msg329063.html

>
> Is it possible that the error is coming from the I2C driver? Can you build
> the I2C driver with #define DEBUG and take a look at the output?

I will let Shawn to try it. With my minimal rootfs I do not see the
error messages when playing audio.
Shawn Guo May 10, 2014, 5:07 a.m. UTC | #7
On Fri, May 09, 2014 at 06:01:43PM +0200, Lars-Peter Clausen wrote:
> On 05/09/2014 05:34 PM, Fabio Estevam wrote:
> >On Fri, May 9, 2014 at 12:17 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> >
> >>The changes in the patch should mostly be transparent. But what changed is
> >>how error reporting is handled. If there is a error reading/writing a
> >>register in the kcontrol callbacks that error is passed on to userspace
> >>whereas previously it was silently ignored.
> >>
> >>There is also the possibility that there is a bug somewhere in the patch
> >>causing this.
> >>
> >>Does the board otherwise work fine?
> >>
> >>I actually have the board here, if you tell me which devictree/defconfig I
> >>can use with an upstream kernel I can give things a try.
> >
> >make imx_v6_v7_defconfig
> >make imx6q-sabresd.dtb
> 
> I just tried 907fe36a2c, e2c330b9b5 and next/master. I get the same
> behavior with all 3, no errors when loading a state file and audio
> out on the headphones works.

For me, e2c330b9b5 is good while 907fe36a2c and next/master expose this
error message.

> I disabled DRM though since it
> deadlocked the system, maybe that makes a difference.

It does not make a difference.

> 
> Is it possible that the error is coming from the I2C driver? Can you
> build the I2C driver with #define DEBUG and take a look at the
> output?

I tried to turn on DEBUG in I2C driver, and did not notice any I2C
message when above error appears.

The error message only shows up with Debian wheezy and does not with
yocto rootfs.  And even when the error message shows, the audio still
functions well on Debian wheezy.  So it's just a noisy error message
for me which is only seen after your kernel patch.

Shawn
Lars-Peter Clausen May 10, 2014, 7:04 a.m. UTC | #8
On 05/10/2014 07:07 AM, Shawn Guo wrote:
> On Fri, May 09, 2014 at 06:01:43PM +0200, Lars-Peter Clausen wrote:
>> On 05/09/2014 05:34 PM, Fabio Estevam wrote:
>>> On Fri, May 9, 2014 at 12:17 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>>>
>>>> The changes in the patch should mostly be transparent. But what changed is
>>>> how error reporting is handled. If there is a error reading/writing a
>>>> register in the kcontrol callbacks that error is passed on to userspace
>>>> whereas previously it was silently ignored.
>>>>
>>>> There is also the possibility that there is a bug somewhere in the patch
>>>> causing this.
>>>>
>>>> Does the board otherwise work fine?
>>>>
>>>> I actually have the board here, if you tell me which devictree/defconfig I
>>>> can use with an upstream kernel I can give things a try.
>>>
>>> make imx_v6_v7_defconfig
>>> make imx6q-sabresd.dtb
>>
>> I just tried 907fe36a2c, e2c330b9b5 and next/master. I get the same
>> behavior with all 3, no errors when loading a state file and audio
>> out on the headphones works.
>
> For me, e2c330b9b5 is good while 907fe36a2c and next/master expose this
> error message.
>
>> I disabled DRM though since it
>> deadlocked the system, maybe that makes a difference.
>
> It does not make a difference.
>
>>
>> Is it possible that the error is coming from the I2C driver? Can you
>> build the I2C driver with #define DEBUG and take a look at the
>> output?
>
> I tried to turn on DEBUG in I2C driver, and did not notice any I2C
> message when above error appears.
>
> The error message only shows up with Debian wheezy and does not with
> yocto rootfs.  And even when the error message shows, the audio still
> functions well on Debian wheezy.  So it's just a noisy error message
> for me which is only seen after your kernel patch.

The keyword here is "seen". The error quite likely predated the commit, but 
it was silently discarded.

regmap_read() returns -EBUSY when there is no cached register value and 
cache_only is set to true. But I'm not sure why that would happen, try to 
add some printks to _regmap_read() to see if this is the source and if it is 
why it is.

- Lars
Lars-Peter Clausen May 10, 2014, 7:11 a.m. UTC | #9
On 05/10/2014 09:04 AM, Lars-Peter Clausen wrote:
[..]
>> The error message only shows up with Debian wheezy and does not with
>> yocto rootfs.  And even when the error message shows, the audio still
>> functions well on Debian wheezy.  So it's just a noisy error message
>> for me which is only seen after your kernel patch.
>
> The keyword here is "seen". The error quite likely predated the commit, but
> it was silently discarded.
>
> regmap_read() returns -EBUSY when there is no cached register value and
> cache_only is set to true. But I'm not sure why that would happen, try to
> add some printks to _regmap_read() to see if this is the source and if it is
> why it is.

Both SGTL5000_CHIP_ANA_ADC_CTRL and SGTL5000_CHIP_MIC_CTRL don't have an 
entry in sgtl5000_reg_defaults. So if cache_only is true, controls which use 
these registers will return -EBUSY when you try to read or write them.
Shawn Guo May 10, 2014, 8:31 a.m. UTC | #10
On Sat, May 10, 2014 at 09:11:04AM +0200, Lars-Peter Clausen wrote:
> On 05/10/2014 09:04 AM, Lars-Peter Clausen wrote:
> [..]
> >>The error message only shows up with Debian wheezy and does not with
> >>yocto rootfs.  And even when the error message shows, the audio still
> >>functions well on Debian wheezy.  So it's just a noisy error message
> >>for me which is only seen after your kernel patch.
> >
> >The keyword here is "seen". The error quite likely predated the commit, but
> >it was silently discarded.
> >
> >regmap_read() returns -EBUSY when there is no cached register value and
> >cache_only is set to true. But I'm not sure why that would happen, try to
> >add some printks to _regmap_read() to see if this is the source and if it is
> >why it is.
> 
> Both SGTL5000_CHIP_ANA_ADC_CTRL and SGTL5000_CHIP_MIC_CTRL don't
> have an entry in sgtl5000_reg_defaults. So if cache_only is true,
> controls which use these registers will return -EBUSY when you try
> to read or write them.

Hmm, it's a wm8962 than sgtl5000 on board imx6q-sabresd.

Shawn
Lars-Peter Clausen May 10, 2014, 8:37 a.m. UTC | #11
On 05/10/2014 10:31 AM, Shawn Guo wrote:
> On Sat, May 10, 2014 at 09:11:04AM +0200, Lars-Peter Clausen wrote:
>> On 05/10/2014 09:04 AM, Lars-Peter Clausen wrote:
>> [..]
>>>> The error message only shows up with Debian wheezy and does not with
>>>> yocto rootfs.  And even when the error message shows, the audio still
>>>> functions well on Debian wheezy.  So it's just a noisy error message
>>>> for me which is only seen after your kernel patch.
>>>
>>> The keyword here is "seen". The error quite likely predated the commit, but
>>> it was silently discarded.
>>>
>>> regmap_read() returns -EBUSY when there is no cached register value and
>>> cache_only is set to true. But I'm not sure why that would happen, try to
>>> add some printks to _regmap_read() to see if this is the source and if it is
>>> why it is.
>>
>> Both SGTL5000_CHIP_ANA_ADC_CTRL and SGTL5000_CHIP_MIC_CTRL don't
>> have an entry in sgtl5000_reg_defaults. So if cache_only is true,
>> controls which use these registers will return -EBUSY when you try
>> to read or write them.
>
> Hmm, it's a wm8962 than sgtl5000 on board imx6q-sabresd.

Ok, misread the boardname. But the issue will be the same. cache_only = true 
and trying to read/update a register that is not in the cache.
Shawn Guo May 10, 2014, 9:12 a.m. UTC | #12
On Sat, May 10, 2014 at 10:37:28AM +0200, Lars-Peter Clausen wrote:
> On 05/10/2014 10:31 AM, Shawn Guo wrote:
> >On Sat, May 10, 2014 at 09:11:04AM +0200, Lars-Peter Clausen wrote:
> >>On 05/10/2014 09:04 AM, Lars-Peter Clausen wrote:
> >>[..]
> >>>>The error message only shows up with Debian wheezy and does not with
> >>>>yocto rootfs.  And even when the error message shows, the audio still
> >>>>functions well on Debian wheezy.  So it's just a noisy error message
> >>>>for me which is only seen after your kernel patch.
> >>>
> >>>The keyword here is "seen". The error quite likely predated the commit, but
> >>>it was silently discarded.
> >>>
> >>>regmap_read() returns -EBUSY when there is no cached register value and
> >>>cache_only is set to true. But I'm not sure why that would happen, try to
> >>>add some printks to _regmap_read() to see if this is the source and if it is
> >>>why it is.
> >>
> >>Both SGTL5000_CHIP_ANA_ADC_CTRL and SGTL5000_CHIP_MIC_CTRL don't
> >>have an entry in sgtl5000_reg_defaults. So if cache_only is true,
> >>controls which use these registers will return -EBUSY when you try
> >>to read or write them.
> >
> >Hmm, it's a wm8962 than sgtl5000 on board imx6q-sabresd.
> 
> Ok, misread the boardname. But the issue will be the same.
> cache_only = true and trying to read/update a register that is not
> in the cache.

Yea, you're right.  When the error message shows up, _regmap_read()
returns -EBUSY for reg WM8962_CLASS_D_CONTROL_1.  But how do I find out
why the reg is not in the cache?

Shawn
Lars-Peter Clausen May 10, 2014, 9:28 a.m. UTC | #13
On 05/10/2014 11:12 AM, Shawn Guo wrote:
> On Sat, May 10, 2014 at 10:37:28AM +0200, Lars-Peter Clausen wrote:
>> On 05/10/2014 10:31 AM, Shawn Guo wrote:
>>> On Sat, May 10, 2014 at 09:11:04AM +0200, Lars-Peter Clausen wrote:
>>>> On 05/10/2014 09:04 AM, Lars-Peter Clausen wrote:
>>>> [..]
>>>>>> The error message only shows up with Debian wheezy and does not with
>>>>>> yocto rootfs.  And even when the error message shows, the audio still
>>>>>> functions well on Debian wheezy.  So it's just a noisy error message
>>>>>> for me which is only seen after your kernel patch.
>>>>>
>>>>> The keyword here is "seen". The error quite likely predated the commit, but
>>>>> it was silently discarded.
>>>>>
>>>>> regmap_read() returns -EBUSY when there is no cached register value and
>>>>> cache_only is set to true. But I'm not sure why that would happen, try to
>>>>> add some printks to _regmap_read() to see if this is the source and if it is
>>>>> why it is.
>>>>
>>>> Both SGTL5000_CHIP_ANA_ADC_CTRL and SGTL5000_CHIP_MIC_CTRL don't
>>>> have an entry in sgtl5000_reg_defaults. So if cache_only is true,
>>>> controls which use these registers will return -EBUSY when you try
>>>> to read or write them.
>>>
>>> Hmm, it's a wm8962 than sgtl5000 on board imx6q-sabresd.
>>
>> Ok, misread the boardname. But the issue will be the same.
>> cache_only = true and trying to read/update a register that is not
>> in the cache.
>
> Yea, you're right.  When the error message shows up, _regmap_read()
> returns -EBUSY for reg WM8962_CLASS_D_CONTROL_1.  But how do I find out
> why the reg is not in the cache?

The register is marked as volatile in the driver and volatile registers are 
never cached. I don't know what the right fix is in this case though.

- Lars
Charles Keepax May 12, 2014, 10:42 a.m. UTC | #14
On Sat, May 10, 2014 at 05:12:11PM +0800, Shawn Guo wrote:
> On Sat, May 10, 2014 at 10:37:28AM +0200, Lars-Peter Clausen wrote:
> > On 05/10/2014 10:31 AM, Shawn Guo wrote:
> > >On Sat, May 10, 2014 at 09:11:04AM +0200, Lars-Peter Clausen wrote:
> > >>On 05/10/2014 09:04 AM, Lars-Peter Clausen wrote:
> > >>[..]
> > >>>>The error message only shows up with Debian wheezy and does not with
> > >>>>yocto rootfs.  And even when the error message shows, the audio still
> > >>>>functions well on Debian wheezy.  So it's just a noisy error message
> > >>>>for me which is only seen after your kernel patch.
> > >>>
> > >>>The keyword here is "seen". The error quite likely predated the commit, but
> > >>>it was silently discarded.
> > >>>
> > >>>regmap_read() returns -EBUSY when there is no cached register value and
> > >>>cache_only is set to true. But I'm not sure why that would happen, try to
> > >>>add some printks to _regmap_read() to see if this is the source and if it is
> > >>>why it is.
> > >>
> > >>Both SGTL5000_CHIP_ANA_ADC_CTRL and SGTL5000_CHIP_MIC_CTRL don't
> > >>have an entry in sgtl5000_reg_defaults. So if cache_only is true,
> > >>controls which use these registers will return -EBUSY when you try
> > >>to read or write them.
> > >
> > >Hmm, it's a wm8962 than sgtl5000 on board imx6q-sabresd.
> > 
> > Ok, misread the boardname. But the issue will be the same.
> > cache_only = true and trying to read/update a register that is not
> > in the cache.
> 
> Yea, you're right.  When the error message shows up, _regmap_read()
> returns -EBUSY for reg WM8962_CLASS_D_CONTROL_1.  But how do I find out
> why the reg is not in the cache?

From what I can see it doesn't look like this register should be
volatile. I am checking with the hardware guys here, incase I am
missing something, but otherwise I will wing out a patch to make
it non-volatile.

Thanks,
Charles
Charles Keepax May 12, 2014, 10:52 a.m. UTC | #15
On Sat, May 10, 2014 at 05:12:11PM +0800, Shawn Guo wrote:
> On Sat, May 10, 2014 at 10:37:28AM +0200, Lars-Peter Clausen wrote:
> > On 05/10/2014 10:31 AM, Shawn Guo wrote:
> > >On Sat, May 10, 2014 at 09:11:04AM +0200, Lars-Peter Clausen wrote:
> > >>On 05/10/2014 09:04 AM, Lars-Peter Clausen wrote:
> > >>[..]
> > >>>>The error message only shows up with Debian wheezy and does not with
> > >>>>yocto rootfs.  And even when the error message shows, the audio still
> > >>>>functions well on Debian wheezy.  So it's just a noisy error message
> > >>>>for me which is only seen after your kernel patch.
> > >>>
> > >>>The keyword here is "seen". The error quite likely predated the commit, but
> > >>>it was silently discarded.
> > >>>
> > >>>regmap_read() returns -EBUSY when there is no cached register value and
> > >>>cache_only is set to true. But I'm not sure why that would happen, try to
> > >>>add some printks to _regmap_read() to see if this is the source and if it is
> > >>>why it is.
> > >>
> > >>Both SGTL5000_CHIP_ANA_ADC_CTRL and SGTL5000_CHIP_MIC_CTRL don't
> > >>have an entry in sgtl5000_reg_defaults. So if cache_only is true,
> > >>controls which use these registers will return -EBUSY when you try
> > >>to read or write them.
> > >
> > >Hmm, it's a wm8962 than sgtl5000 on board imx6q-sabresd.
> > 
> > Ok, misread the boardname. But the issue will be the same.
> > cache_only = true and trying to read/update a register that is not
> > in the cache.
> 
> Yea, you're right.  When the error message shows up, _regmap_read()
> returns -EBUSY for reg WM8962_CLASS_D_CONTROL_1.  But how do I find out
> why the reg is not in the cache?

Looking at this a bit more can you let me know when it is
failing, I assume it is in relation to setting the "Speaker
Switch" control?

Thanks,
Charles
Mark Brown May 12, 2014, 11:03 a.m. UTC | #16
On Mon, May 12, 2014 at 11:42:04AM +0100, Charles Keepax wrote:

> > Yea, you're right.  When the error message shows up, _regmap_read()
> > returns -EBUSY for reg WM8962_CLASS_D_CONTROL_1.  But how do I find out
> > why the reg is not in the cache?

> From what I can see it doesn't look like this register should be
> volatile. I am checking with the hardware guys here, incase I am
> missing something, but otherwise I will wing out a patch to make
> it non-volatile.

IIRC at least one of the bits in the register also appears in another
register.  They aren't volatile but the cache could get confused unless
something is open coded to keep the two cached registers in sync.
Charles Keepax May 12, 2014, 2:09 p.m. UTC | #17
On Mon, May 12, 2014 at 12:03:04PM +0100, Mark Brown wrote:
> On Mon, May 12, 2014 at 11:42:04AM +0100, Charles Keepax wrote:
> 
> > > Yea, you're right.  When the error message shows up, _regmap_read()
> > > returns -EBUSY for reg WM8962_CLASS_D_CONTROL_1.  But how do I find out
> > > why the reg is not in the cache?
> 
> > From what I can see it doesn't look like this register should be
> > volatile. I am checking with the hardware guys here, incase I am
> > missing something, but otherwise I will wing out a patch to make
> > it non-volatile.
> 
> IIRC at least one of the bits in the register also appears in another
> register.  They aren't volatile but the cache could get confused unless
> something is open coded to keep the two cached registers in sync.

Yup, just heard back from the hardware guys, this is exactly it.
The DAC_MUTE bit in registers R5 and R49 both control the same
thing. So if you change one it changes the other.

Looks like either something will need to be added to keep the two
registers in sync, or some manual caching of the speaker switch
until the device is enabled.

Thanks,
Charles
Shawn Guo May 13, 2014, 3:19 a.m. UTC | #18
On Mon, May 12, 2014 at 11:52:57AM +0100, Charles Keepax wrote:
> Looking at this a bit more can you let me know when it is
> failing, I assume it is in relation to setting the "Speaker
> Switch" control?

Yes, it is.

Shawn
diff mbox

Patch

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 554c650..19a82ce 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1250,6 +1250,22 @@  static inline bool snd_soc_codec_is_active(struct snd_soc_codec *codec)
 }
 
 /**
+ * snd_soc_kcontrol_component() - Returns the component that registered the
+ *  control
+ * @kcontrol: The control for which to get the component
+ *
+ * Note: This function will work correctly if the control has been registered
+ * for a component. Either with snd_soc_add_codec_controls() or
+ * snd_soc_add_platform_controls() or via  table based setup for either a
+ * CODEC, a platform or component driver. Otherwise the behavior is undefined.
+ */
+static inline struct snd_soc_component *snd_soc_kcontrol_component(
+	struct snd_kcontrol *kcontrol)
+{
+	return snd_kcontrol_chip(kcontrol);
+}
+
+/**
  * snd_soc_kcontrol_codec() - Returns the CODEC that registered the control
  * @kcontrol: The control for which to get the CODEC
  *
@@ -1260,7 +1276,7 @@  static inline bool snd_soc_codec_is_active(struct snd_soc_codec *codec)
 static inline struct snd_soc_codec *snd_soc_kcontrol_codec(
 	struct snd_kcontrol *kcontrol)
 {
-	return snd_kcontrol_chip(kcontrol);
+	return snd_soc_component_to_codec(snd_soc_kcontrol_component(kcontrol));
 }
 
 /**
@@ -1274,7 +1290,7 @@  static inline struct snd_soc_codec *snd_soc_kcontrol_codec(
 static inline struct snd_soc_platform *snd_soc_kcontrol_platform(
 	struct snd_kcontrol *kcontrol)
 {
-	return snd_kcontrol_chip(kcontrol);
+	return snd_soc_component_to_platform(snd_soc_kcontrol_component(kcontrol));
 }
 
 int snd_soc_util_init(void);
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index df3e293..d5cd80b 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2353,7 +2353,7 @@  int snd_soc_add_codec_controls(struct snd_soc_codec *codec,
 	struct snd_card *card = codec->card->snd_card;
 
 	return snd_soc_add_controls(card, codec->dev, controls, num_controls,
-			codec->name_prefix, codec);
+			codec->name_prefix, &codec->component);
 }
 EXPORT_SYMBOL_GPL(snd_soc_add_codec_controls);
 
@@ -2373,7 +2373,7 @@  int snd_soc_add_platform_controls(struct snd_soc_platform *platform,
 	struct snd_card *card = platform->card->snd_card;
 
 	return snd_soc_add_controls(card, platform->dev, controls, num_controls,
-			NULL, platform);
+			NULL, &platform->component);
 }
 EXPORT_SYMBOL_GPL(snd_soc_add_platform_controls);
 
@@ -2457,12 +2457,15 @@  EXPORT_SYMBOL_GPL(snd_soc_info_enum_double);
 int snd_soc_get_enum_double(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *ucontrol)
 {
-	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
 	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
 	unsigned int val, item;
 	unsigned int reg_val;
+	int ret;
 
-	reg_val = snd_soc_read(codec, e->reg);
+	ret = snd_soc_component_read(component, e->reg, &reg_val);
+	if (ret)
+		return ret;
 	val = (reg_val >> e->shift_l) & e->mask;
 	item = snd_soc_enum_val_to_item(e, val);
 	ucontrol->value.enumerated.item[0] = item;
@@ -2488,7 +2491,7 @@  EXPORT_SYMBOL_GPL(snd_soc_get_enum_double);
 int snd_soc_put_enum_double(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *ucontrol)
 {
-	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
 	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
 	unsigned int *item = ucontrol->value.enumerated.item;
 	unsigned int val;
@@ -2505,38 +2508,48 @@  int snd_soc_put_enum_double(struct snd_kcontrol *kcontrol,
 		mask |= e->mask << e->shift_r;
 	}
 
-	return snd_soc_update_bits_locked(codec, e->reg, mask, val);
+	return snd_soc_component_update_bits(component, e->reg, mask, val);
 }
 EXPORT_SYMBOL_GPL(snd_soc_put_enum_double);
 
 /**
  * snd_soc_read_signed - Read a codec register and interprete as signed value
- * @codec: codec
+ * @component: component
  * @reg: Register to read
  * @mask: Mask to use after shifting the register value
  * @shift: Right shift of register value
  * @sign_bit: Bit that describes if a number is negative or not.
+ * @signed_val: Pointer to where the read value should be stored
  *
  * This functions reads a codec register. The register value is shifted right
  * by 'shift' bits and masked with the given 'mask'. Afterwards it translates
  * the given registervalue into a signed integer if sign_bit is non-zero.
  *
- * Returns the register value as signed int.
+ * Returns 0 on sucess, otherwise an error value
  */
-static int snd_soc_read_signed(struct snd_soc_codec *codec, unsigned int reg,
-		unsigned int mask, unsigned int shift, unsigned int sign_bit)
+static int snd_soc_read_signed(struct snd_soc_component *component,
+	unsigned int reg, unsigned int mask, unsigned int shift,
+	unsigned int sign_bit, int *signed_val)
 {
 	int ret;
 	unsigned int val;
 
-	val = (snd_soc_read(codec, reg) >> shift) & mask;
+	ret = snd_soc_component_read(component, reg, &val);
+	if (ret < 0)
+		return ret;
+
+	val = (val >> shift) & mask;
 
-	if (!sign_bit)
-		return val;
+	if (!sign_bit) {
+		*signed_val = val;
+		return 0;
+	}
 
 	/* non-negative number */
-	if (!(val & BIT(sign_bit)))
-		return val;
+	if (!(val & BIT(sign_bit))) {
+		*signed_val = val;
+		return 0;
+	}
 
 	ret = val;
 
@@ -2548,7 +2561,9 @@  static int snd_soc_read_signed(struct snd_soc_codec *codec, unsigned int reg,
 	 */
 	ret |= ~((int)(BIT(sign_bit) - 1));
 
-	return ret;
+	*signed_val = ret;
+
+	return 0;
 }
 
 /**
@@ -2597,9 +2612,9 @@  EXPORT_SYMBOL_GPL(snd_soc_info_volsw);
 int snd_soc_get_volsw(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *ucontrol)
 {
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
 	struct soc_mixer_control *mc =
 		(struct soc_mixer_control *)kcontrol->private_value;
-	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
 	unsigned int reg = mc->reg;
 	unsigned int reg2 = mc->rreg;
 	unsigned int shift = mc->shift;
@@ -2609,25 +2624,32 @@  int snd_soc_get_volsw(struct snd_kcontrol *kcontrol,
 	int sign_bit = mc->sign_bit;
 	unsigned int mask = (1 << fls(max)) - 1;
 	unsigned int invert = mc->invert;
+	int val;
+	int ret;
 
 	if (sign_bit)
 		mask = BIT(sign_bit + 1) - 1;
 
-	ucontrol->value.integer.value[0] = snd_soc_read_signed(codec, reg, mask,
-			shift, sign_bit) - min;
+	ret = snd_soc_read_signed(component, reg, mask, shift, sign_bit, &val);
+	if (ret)
+		return ret;
+
+	ucontrol->value.integer.value[0] = val - min;
 	if (invert)
 		ucontrol->value.integer.value[0] =
 			max - ucontrol->value.integer.value[0];
 
 	if (snd_soc_volsw_is_stereo(mc)) {
 		if (reg == reg2)
-			ucontrol->value.integer.value[1] =
-				snd_soc_read_signed(codec, reg, mask, rshift,
-						sign_bit) - min;
+			ret = snd_soc_read_signed(component, reg, mask, rshift,
+				sign_bit, &val);
 		else
-			ucontrol->value.integer.value[1] =
-				snd_soc_read_signed(codec, reg2, mask, shift,
-						sign_bit) - min;
+			ret = snd_soc_read_signed(component, reg2, mask, shift,
+				sign_bit, &val);
+		if (ret)
+			return ret;
+
+		ucontrol->value.integer.value[1] = val - min;
 		if (invert)
 			ucontrol->value.integer.value[1] =
 				max - ucontrol->value.integer.value[1];
@@ -2650,9 +2672,9 @@  EXPORT_SYMBOL_GPL(snd_soc_get_volsw);
 int snd_soc_put_volsw(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *ucontrol)
 {
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
 	struct soc_mixer_control *mc =
 		(struct soc_mixer_control *)kcontrol->private_value;
-	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
 	unsigned int reg = mc->reg;
 	unsigned int reg2 = mc->rreg;
 	unsigned int shift = mc->shift;
@@ -2687,12 +2709,13 @@  int snd_soc_put_volsw(struct snd_kcontrol *kcontrol,
 			type_2r = true;
 		}
 	}
-	err = snd_soc_update_bits_locked(codec, reg, val_mask, val);
+	err = snd_soc_component_update_bits(component, reg, val_mask, val);
 	if (err < 0)
 		return err;
 
 	if (type_2r)
-		err = snd_soc_update_bits_locked(codec, reg2, val_mask, val2);
+		err = snd_soc_component_update_bits(component, reg2, val_mask,
+			val2);
 
 	return err;
 }
@@ -2711,10 +2734,9 @@  EXPORT_SYMBOL_GPL(snd_soc_put_volsw);
 int snd_soc_get_volsw_sx(struct snd_kcontrol *kcontrol,
 		      struct snd_ctl_elem_value *ucontrol)
 {
-	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
 	struct soc_mixer_control *mc =
 	    (struct soc_mixer_control *)kcontrol->private_value;
-
 	unsigned int reg = mc->reg;
 	unsigned int reg2 = mc->rreg;
 	unsigned int shift = mc->shift;
@@ -2722,13 +2744,23 @@  int snd_soc_get_volsw_sx(struct snd_kcontrol *kcontrol,
 	int max = mc->max;
 	int min = mc->min;
 	int mask = (1 << (fls(min + max) - 1)) - 1;
+	unsigned int val;
+	int ret;
 
-	ucontrol->value.integer.value[0] =
-	    ((snd_soc_read(codec, reg) >> shift) - min) & mask;
+	ret = snd_soc_component_read(component, reg, &val);
+	if (ret < 0)
+		return ret;
 
-	if (snd_soc_volsw_is_stereo(mc))
-		ucontrol->value.integer.value[1] =
-			((snd_soc_read(codec, reg2) >> rshift) - min) & mask;
+	ucontrol->value.integer.value[0] = ((val >> shift) - min) & mask;
+
+	if (snd_soc_volsw_is_stereo(mc)) {
+		ret = snd_soc_component_read(component, reg2, &val);
+		if (ret < 0)
+			return ret;
+
+		val = ((val >> rshift) - min) & mask;
+		ucontrol->value.integer.value[1] = val;
+	}
 
 	return 0;
 }
@@ -2746,7 +2778,7 @@  EXPORT_SYMBOL_GPL(snd_soc_get_volsw_sx);
 int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol,
 			 struct snd_ctl_elem_value *ucontrol)
 {
-	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
 	struct soc_mixer_control *mc =
 	    (struct soc_mixer_control *)kcontrol->private_value;
 
@@ -2764,7 +2796,7 @@  int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol,
 	val = (ucontrol->value.integer.value[0] + min) & mask;
 	val = val << shift;
 
-	err = snd_soc_update_bits_locked(codec, reg, val_mask, val);
+	err = snd_soc_component_update_bits(component, reg, val_mask, val);
 	if (err < 0)
 		return err;
 
@@ -2773,10 +2805,10 @@  int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol,
 		val2 = (ucontrol->value.integer.value[1] + min) & mask;
 		val2 = val2 << rshift;
 
-		if (snd_soc_update_bits_locked(codec, reg2, val_mask, val2))
-			return err;
+		err = snd_soc_component_update_bits(component, reg2, val_mask,
+			val2);
 	}
-	return 0;
+	return err;
 }
 EXPORT_SYMBOL_GPL(snd_soc_put_volsw_sx);
 
@@ -2823,10 +2855,15 @@  int snd_soc_get_volsw_s8(struct snd_kcontrol *kcontrol,
 {
 	struct soc_mixer_control *mc =
 		(struct soc_mixer_control *)kcontrol->private_value;
-	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
 	unsigned int reg = mc->reg;
+	unsigned int val;
 	int min = mc->min;
-	int val = snd_soc_read(codec, reg);
+	int ret;
+
+	ret = snd_soc_component_read(component, reg, &val);
+	if (ret)
+		return ret;
 
 	ucontrol->value.integer.value[0] =
 		((signed char)(val & 0xff))-min;
@@ -2850,7 +2887,7 @@  int snd_soc_put_volsw_s8(struct snd_kcontrol *kcontrol,
 {
 	struct soc_mixer_control *mc =
 		(struct soc_mixer_control *)kcontrol->private_value;
-	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
 	unsigned int reg = mc->reg;
 	int min = mc->min;
 	unsigned int val;
@@ -2858,7 +2895,7 @@  int snd_soc_put_volsw_s8(struct snd_kcontrol *kcontrol,
 	val = (ucontrol->value.integer.value[0]+min) & 0xff;
 	val |= ((ucontrol->value.integer.value[1]+min) & 0xff) << 8;
 
-	return snd_soc_update_bits_locked(codec, reg, 0xffff, val);
+	return snd_soc_component_update_bits(component, reg, 0xffff, val);
 }
 EXPORT_SYMBOL_GPL(snd_soc_put_volsw_s8);
 
@@ -2907,7 +2944,7 @@  int snd_soc_put_volsw_range(struct snd_kcontrol *kcontrol,
 {
 	struct soc_mixer_control *mc =
 		(struct soc_mixer_control *)kcontrol->private_value;
-	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
 	unsigned int reg = mc->reg;
 	unsigned int rreg = mc->rreg;
 	unsigned int shift = mc->shift;
@@ -2924,7 +2961,7 @@  int snd_soc_put_volsw_range(struct snd_kcontrol *kcontrol,
 	val_mask = mask << shift;
 	val = val << shift;
 
-	ret = snd_soc_update_bits_locked(codec, reg, val_mask, val);
+	ret = snd_soc_component_update_bits(component, reg, val_mask, val);
 	if (ret < 0)
 		return ret;
 
@@ -2935,7 +2972,8 @@  int snd_soc_put_volsw_range(struct snd_kcontrol *kcontrol,
 		val_mask = mask << shift;
 		val = val << shift;
 
-		ret = snd_soc_update_bits_locked(codec, rreg, val_mask, val);
+		ret = snd_soc_component_update_bits(component, rreg, val_mask,
+			val);
 	}
 
 	return ret;
@@ -2954,9 +2992,9 @@  EXPORT_SYMBOL_GPL(snd_soc_put_volsw_range);
 int snd_soc_get_volsw_range(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *ucontrol)
 {
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
 	struct soc_mixer_control *mc =
 		(struct soc_mixer_control *)kcontrol->private_value;
-	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
 	unsigned int reg = mc->reg;
 	unsigned int rreg = mc->rreg;
 	unsigned int shift = mc->shift;
@@ -2964,9 +3002,14 @@  int snd_soc_get_volsw_range(struct snd_kcontrol *kcontrol,
 	int max = mc->max;
 	unsigned int mask = (1 << fls(max)) - 1;
 	unsigned int invert = mc->invert;
+	unsigned int val;
+	int ret;
 
-	ucontrol->value.integer.value[0] =
-		(snd_soc_read(codec, reg) >> shift) & mask;
+	ret = snd_soc_component_read(component, reg, &val);
+	if (ret)
+		return ret;
+
+	ucontrol->value.integer.value[0] = (val >> shift) & mask;
 	if (invert)
 		ucontrol->value.integer.value[0] =
 			max - ucontrol->value.integer.value[0];
@@ -2974,8 +3017,11 @@  int snd_soc_get_volsw_range(struct snd_kcontrol *kcontrol,
 		ucontrol->value.integer.value[0] - min;
 
 	if (snd_soc_volsw_is_stereo(mc)) {
-		ucontrol->value.integer.value[1] =
-			(snd_soc_read(codec, rreg) >> shift) & mask;
+		ret = snd_soc_component_read(component, rreg, &val);
+		if (ret)
+			return ret;
+
+		ucontrol->value.integer.value[1] = (val >> shift) & mask;
 		if (invert)
 			ucontrol->value.integer.value[1] =
 				max - ucontrol->value.integer.value[1];
@@ -3029,11 +3075,11 @@  EXPORT_SYMBOL_GPL(snd_soc_limit_volume);
 int snd_soc_bytes_info(struct snd_kcontrol *kcontrol,
 		       struct snd_ctl_elem_info *uinfo)
 {
-	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
 	struct soc_bytes *params = (void *)kcontrol->private_value;
 
 	uinfo->type = SNDRV_CTL_ELEM_TYPE_BYTES;
-	uinfo->count = params->num_regs * codec->component.val_bytes;
+	uinfo->count = params->num_regs * component->val_bytes;
 
 	return 0;
 }
@@ -3042,20 +3088,20 @@  EXPORT_SYMBOL_GPL(snd_soc_bytes_info);
 int snd_soc_bytes_get(struct snd_kcontrol *kcontrol,
 		      struct snd_ctl_elem_value *ucontrol)
 {
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
 	struct soc_bytes *params = (void *)kcontrol->private_value;
-	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
 	int ret;
 
-	if (codec->component.regmap)
-		ret = regmap_raw_read(codec->component.regmap, params->base,
+	if (component->regmap)
+		ret = regmap_raw_read(component->regmap, params->base,
 				      ucontrol->value.bytes.data,
-				      params->num_regs * codec->component.val_bytes);
+				      params->num_regs * component->val_bytes);
 	else
 		ret = -EINVAL;
 
 	/* Hide any masked bytes to ensure consistent data reporting */
 	if (ret == 0 && params->mask) {
-		switch (codec->component.val_bytes) {
+		switch (component->val_bytes) {
 		case 1:
 			ucontrol->value.bytes.data[0] &= ~params->mask;
 			break;
@@ -3079,16 +3125,16 @@  EXPORT_SYMBOL_GPL(snd_soc_bytes_get);
 int snd_soc_bytes_put(struct snd_kcontrol *kcontrol,
 		      struct snd_ctl_elem_value *ucontrol)
 {
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
 	struct soc_bytes *params = (void *)kcontrol->private_value;
-	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
 	int ret, len;
 	unsigned int val, mask;
 	void *data;
 
-	if (!codec->component.regmap)
+	if (!component->regmap)
 		return -EINVAL;
 
-	len = params->num_regs * codec->component.val_bytes;
+	len = params->num_regs * component->val_bytes;
 
 	data = kmemdup(ucontrol->value.bytes.data, len, GFP_KERNEL | GFP_DMA);
 	if (!data)
@@ -3100,27 +3146,27 @@  int snd_soc_bytes_put(struct snd_kcontrol *kcontrol,
 	 * copy.
 	 */
 	if (params->mask) {
-		ret = regmap_read(codec->component.regmap, params->base, &val);
+		ret = regmap_read(component->regmap, params->base, &val);
 		if (ret != 0)
 			goto out;
 
 		val &= params->mask;
 
-		switch (codec->component.val_bytes) {
+		switch (component->val_bytes) {
 		case 1:
 			((u8 *)data)[0] &= ~params->mask;
 			((u8 *)data)[0] |= val;
 			break;
 		case 2:
 			mask = ~params->mask;
-			ret = regmap_parse_val(codec->component.regmap,
+			ret = regmap_parse_val(component->regmap,
 							&mask, &mask);
 			if (ret != 0)
 				goto out;
 
 			((u16 *)data)[0] &= mask;
 
-			ret = regmap_parse_val(codec->component.regmap,
+			ret = regmap_parse_val(component->regmap,
 							&val, &val);
 			if (ret != 0)
 				goto out;
@@ -3129,14 +3175,14 @@  int snd_soc_bytes_put(struct snd_kcontrol *kcontrol,
 			break;
 		case 4:
 			mask = ~params->mask;
-			ret = regmap_parse_val(codec->component.regmap,
+			ret = regmap_parse_val(component->regmap,
 							&mask, &mask);
 			if (ret != 0)
 				goto out;
 
 			((u32 *)data)[0] &= mask;
 
-			ret = regmap_parse_val(codec->component.regmap,
+			ret = regmap_parse_val(component->regmap,
 							&val, &val);
 			if (ret != 0)
 				goto out;
@@ -3149,7 +3195,7 @@  int snd_soc_bytes_put(struct snd_kcontrol *kcontrol,
 		}
 	}
 
-	ret = regmap_raw_write(codec->component.regmap, params->base,
+	ret = regmap_raw_write(component->regmap, params->base,
 			       data, len);
 
 out:
@@ -3200,24 +3246,27 @@  EXPORT_SYMBOL_GPL(snd_soc_info_xr_sx);
 int snd_soc_get_xr_sx(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *ucontrol)
 {
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
 	struct soc_mreg_control *mc =
 		(struct soc_mreg_control *)kcontrol->private_value;
-	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
 	unsigned int regbase = mc->regbase;
 	unsigned int regcount = mc->regcount;
-	unsigned int regwshift = codec->component.val_bytes * BITS_PER_BYTE;
+	unsigned int regwshift = component->val_bytes * BITS_PER_BYTE;
 	unsigned int regwmask = (1<<regwshift)-1;
 	unsigned int invert = mc->invert;
 	unsigned long mask = (1UL<<mc->nbits)-1;
 	long min = mc->min;
 	long max = mc->max;
 	long val = 0;
-	unsigned long regval;
+	unsigned int regval;
 	unsigned int i;
+	int ret;
 
 	for (i = 0; i < regcount; i++) {
-		regval = snd_soc_read(codec, regbase+i) & regwmask;
-		val |= regval << (regwshift*(regcount-i-1));
+		ret = snd_soc_component_read(component, regbase+i, &regval);
+		if (ret)
+			return ret;
+		val |= (regval & regwmask) << (regwshift*(regcount-i-1));
 	}
 	val &= mask;
 	if (min < 0 && val > max)
@@ -3246,12 +3295,12 @@  EXPORT_SYMBOL_GPL(snd_soc_get_xr_sx);
 int snd_soc_put_xr_sx(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *ucontrol)
 {
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
 	struct soc_mreg_control *mc =
 		(struct soc_mreg_control *)kcontrol->private_value;
-	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
 	unsigned int regbase = mc->regbase;
 	unsigned int regcount = mc->regcount;
-	unsigned int regwshift = codec->component.val_bytes * BITS_PER_BYTE;
+	unsigned int regwshift = component->val_bytes * BITS_PER_BYTE;
 	unsigned int regwmask = (1<<regwshift)-1;
 	unsigned int invert = mc->invert;
 	unsigned long mask = (1UL<<mc->nbits)-1;
@@ -3266,7 +3315,7 @@  int snd_soc_put_xr_sx(struct snd_kcontrol *kcontrol,
 	for (i = 0; i < regcount; i++) {
 		regval = (val >> (regwshift*(regcount-i-1))) & regwmask;
 		regmask = (mask >> (regwshift*(regcount-i-1))) & regwmask;
-		err = snd_soc_update_bits_locked(codec, regbase+i,
+		err = snd_soc_component_update_bits(component, regbase+i,
 				regmask, regval);
 		if (err < 0)
 			return err;
@@ -3288,14 +3337,21 @@  EXPORT_SYMBOL_GPL(snd_soc_put_xr_sx);
 int snd_soc_get_strobe(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *ucontrol)
 {
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
 	struct soc_mixer_control *mc =
 		(struct soc_mixer_control *)kcontrol->private_value;
-	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
 	unsigned int reg = mc->reg;
 	unsigned int shift = mc->shift;
 	unsigned int mask = 1 << shift;
 	unsigned int invert = mc->invert != 0;
-	unsigned int val = snd_soc_read(codec, reg) & mask;
+	unsigned int val;
+	int ret;
+
+	ret = snd_soc_component_read(component, reg, &val);
+	if (ret)
+		return ret;
+
+	val &= mask;
 
 	if (shift != 0 && val != 0)
 		val = val >> shift;
@@ -3318,9 +3374,9 @@  EXPORT_SYMBOL_GPL(snd_soc_get_strobe);
 int snd_soc_put_strobe(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *ucontrol)
 {
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
 	struct soc_mixer_control *mc =
 		(struct soc_mixer_control *)kcontrol->private_value;
-	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
 	unsigned int reg = mc->reg;
 	unsigned int shift = mc->shift;
 	unsigned int mask = 1 << shift;
@@ -3330,12 +3386,11 @@  int snd_soc_put_strobe(struct snd_kcontrol *kcontrol,
 	unsigned int val2 = (strobe ^ invert) ? 0 : mask;
 	int err;
 
-	err = snd_soc_update_bits_locked(codec, reg, mask, val1);
+	err = snd_soc_component_update_bits(component, reg, mask, val1);
 	if (err < 0)
 		return err;
 
-	err = snd_soc_update_bits_locked(codec, reg, mask, val2);
-	return err;
+	return snd_soc_component_update_bits(component, reg, mask, val2);
 }
 EXPORT_SYMBOL_GPL(snd_soc_put_strobe);