Re: [RFC] ASoC: sgtl5000: Remove cache support

Re: [RFC] ASoC: sgtl5000: Remove cache support

[Fabio Estevam]

Hi Mark,

On Fri, May 3, 2013 at 7:36 PM, Mark Brown <broonie@sirena.org.uk> wrote:
> On Fri, May 03, 2013 at 04:49:09PM -0300, Fabio Estevam wrote:
>
>>       .set_bias_level = sgtl5000_set_bias_level,
>> -     .reg_cache_size = ARRAY_SIZE(sgtl5000_regs),
>> -     .reg_word_size = sizeof(u16),
>> -     .reg_cache_step = 2,
>> -     .reg_cache_default = sgtl5000_regs,
>
> So, the most obvious thing here is to convert the driver to regmap.  It
> has a much more baked in idea of sparse register maps which should help
> if nothing else, and will allow the device initialisation to be moved to
> the normal device probe which is better practice.

Thanks for your suggestion.

I have just sent a patch converting sgtl5000 to use regmap.

Unfortunately the result is the same: the codec cannot probe after a reset.

> Otherwise the normal issue with this sort of thing is a failure to power
> down the device properly, the LDO workaround looks suspicous here as
> does the fact that the driver doesn't restore the register cache when
> powering on the device in resume (nor does it makr the device as cache
> only).

I will try to investigate the power down and LDO workaround as suggested.

Thanks,

Fabio Estevam

Re: [RFC] ASoC: sgtl5000: Remove cache support

[Fabio Estevam]

On Sat, May 4, 2013 at 3:47 PM, Fabio Estevam <festevam@gmail.com> wrote:

> I will try to investigate the power down and LDO workaround as suggested.

If I use a the kernel provided by Freescale the codec reset issue does
not happen.

I see this commit from Freescale kernel that workarounds exactly the same issue:

http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/commit/sound/soc/codecs/sgtl5000.c?h=imx_3.0.35_1.1.0&id=2f7d47e760725d33f6b2eb01024b800d64ff8944

It seems that caching is broking with this codec.

If this is true, should we go with the proposal of this RFC patch?

Re: [RFC] ASoC: sgtl5000: Remove cache support

[Mark Brown]

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

On Sat, May 04, 2013 at 05:25:45PM -0300, Fabio Estevam wrote:

> I see this commit from Freescale kernel that workarounds exactly the same issue:

> http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/commit/sound/soc/codecs/sgtl5000.c?h=imx_3.0.35_1.1.0&id=2f7d47e760725d33f6b2eb01024b800d64ff8944

> It seems that caching is broking with this codec.

This is nonsensical.  Clearly there's something about the use of the
cache that is interacting poorly with the device but we've no idea what
it is.  Simply having a copy of the register map in the host memory is
not going to do anything to the device, there must be some other issue
at work.

> If this is true, should we go with the proposal of this RFC patch?

We should understand what the issue is.  Looking at the commit above
it's not removing the cache at all, it's both attempting to work around
an issue with the cache not handling the register step size properly
(this looks like it's just a bug in the current driver) and reordering
some of the startup around the regulator (the comment says it's open
coding cache_bypass but it's also moving blocks of code around).

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

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

Re: [RFC] ASoC: sgtl5000: Remove cache support

[Mark Brown]

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

On Mon, May 06, 2013 at 03:19:27PM -0500, Matt Sealey wrote:

> I have asked before and I will ask again; does anyone have any insight

Not on the list you haven't as far as I can tell...  this is the first
I've heard of any concern about this code, I'd not be surprised if
others were similarly unaware of any concerns.

> The patch came from out of nowhere, and the comments seem to say that
> the LDO gets enabled on every chip higher than a specific version.
> Now, this is totally against the design documentation for the SGTL5000
> which specifically states that using the internal LDO is quite
> possibly the *worst* power consuming case and NOT best practise for
> putting the codec on a design, with the sole benefit being reduced
> external component count.

Are you sure it's purely about power consumption?  A common application
for LDOs in this situation is to clean up noise on the incoming power
supply, improving rejection of noise.  This is mostly irrelevant for
digital supplies but can be desirable for analogue ones.  Of course this
looks like it's being used as a digital supply so...

I have to say your explanation is a little curious, though - normally an
LDO would require an external capacitor and so would either increase the
component count (adding the capactior) or have no effect (if one is
already present for decoupling).  What is the supply for this LDO and
where does the output go?

> What happens if we just take out the internal LDO setup conditional on
> version, and only initialize it if VDDD is missing? Do boards that

If the LDO is enabled and connected to a supply which is also driven
externally that's not a clever idea, the two regulators will both be
trying to regulate the same supply.

> I am not entirely sure how well the commit linked by Fabio would work
> on mainline since if it uses hw_read/write it should also be updating
> the cache with those values to be sure they can later be modified...
> the BSP code does that then fills the cache with the existing values
> afterwards.

That commit is just bogus in the way it interacts with the cache, half
of it looks awfully like it's trying to work around some breakage in the
way the cache was being set up.

> So maybe we don't disable the cache, or it gets ported to regmap, but
> the order should be enable regulators in a non-cached way then fill
> the cache AFTERWARDS - the code at ToT mainline doesn't fix the cache

I'm sorry, this makes no sense to me.  If we know the physical defaults
for the device and we can get the device into that state then we should
have no need to read from the cache.  If we don't know what they are
then they shouldn't be listed.

> after it does regulator/power regs setup, I assume because cache
> filling is now somehow automated in ASoC (and if this is the case,

There is no cache filling to automate, if (as is the case here) register
defaults are provided they're used.

> then the cache io setup needs to be done after regulator/power setup,
> the defaults list/cache needs to be updated for those registers before
> any register caching io is done..). And maybe setting up the regulator
> is kind of pointless anyway (meaning that whole caching/not-caching
> thing just falls away?)

Again this makes no sense, it seems like you're very confused here.  The
whole purpose of the register defaults is to reflect the default values
of the registers.  The infrastructure relies on this being the case,
otherwise it will get confused for example during cache sync when it
decides if a write to the device is required.  

I think the point that you lost me was your assumption that the
regulator enable needs to be done without the cache.  What makes you
believe that this is the case?

Note that as I said in another post the driver is currently totally
broken with regard to anything that actually powers off the device,
there's never any attempt made to restore the register cache.

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

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

Re: [RFC] ASoC: sgtl5000: Remove cache support

[Fabio Estevam]

On Tue, May 7, 2013 at 5:06 PM, Matt Sealey <matt@genesi-usa.com> wrote:

> Right, and in theory - I can't find an example of anyone implementing
> VDDD as unconnected - if you float VDDD then the SGTL5000 will start

Actually I can't find see anyone that implemented VDDD connected, except Efika.

All these boards have sgtl5000 VDDD unconnected:
mx51evk, mx28evk, mx53smd, mx35pdk, mx25pdk.

mx6qsabrelite connects VDD to GND.

Re: [RFC] ASoC: sgtl5000: Remove cache support

[Troy Kisky]

On 5/7/2013 4:26 PM, Fabio Estevam wrote:
> On Tue, May 7, 2013 at 5:06 PM, Matt Sealey <matt@genesi-usa.com> wrote:
>
>> Right, and in theory - I can't find an example of anyone implementing
>> VDDD as unconnected - if you float VDDD then the SGTL5000 will start
> Actually I can't find see anyone that implemented VDDD connected, except Efika.
>
> All these boards have sgtl5000 VDDD unconnected:
> mx51evk, mx28evk, mx53smd, mx35pdk, mx25pdk.
>
> mx6qsabrelite connects VDD to GND.
> .
>
Sabrelite connects VDD to a cap only.

Troy

Re: [RFC] ASoC: sgtl5000: Remove cache support

[Mark Brown]

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

On Tue, May 07, 2013 at 03:06:39PM -0500, Matt Sealey wrote:
> On Mon, May 6, 2013 at 5:06 PM, Mark Brown <broonie@kernel.org> wrote:

> Just for reference the supplies on the Efika MX are VDDD 1.2V, VDDDA
> 1.65V and VDDIO 2.775V respectively. For the sake of a couple 0.1uF
> capacitors you're not really saving much money in the lowest cost
> configuration, and you're doubling to tripling power consumption. If
> 15mW makes a difference to you (especially if you're supplying the
> above from rails connected to other devices and you're concerned about
> current limiting etc.) it makes sense to go for the lower power one.

Well. some applications just don't care about power consumption.

> It is hard to imagine an i.MX board without 1.2V, 1.6V and 3.3V from a
> PMIC to supply the codec so letting VDDD float and using the internal
> LDO seems like an exercise in selling the codec to people who aren't
> using other Freescale parts..

Even if there's physically a supply at the appropriate voltage people
can have concerns about running noisy digital stuff near their analogue,
the on-chip regulators aren't purely about component cost.  Not sure
that this is a consideration here but...

> I think most of the weird problems is the LDO configuration itself -
> Linux assumes that it's not set up, when if VDDD isn't there, it is
> already. Weird sequencing of supplies by boards (either not bringing
> any supplies down or not bringing them down correctly in a reasonable
> order) on warm reboot and enabling it *regardless* at revision 0x11 or
> above which is just turning (on our boards) a 1.2V supply into a
> better filtered 1.2V supply and making the chip run a little hotter.

Is it actually having a postive impact on performance?

> I am curious about the version check more than anything. Either the
> internal LDO is required to filter poor supplies on some boards (which
> ones?) or revisions equal or above 0x11 had some DIFFERENT digital
> supply requirement - I seem to remember the reference manual stating
> 1.2V was a suitable input supply, but the minimum is 1.1V in the
> latest RM. Maybe if you run it at 1.2V there's some problem. It's not
> documented why that decision would be made.. no errata..

That sounds like tolerances doesn't it?  Spec of 1.2V.

> >> So maybe we don't disable the cache, or it gets ported to regmap, but
> >> the order should be enable regulators in a non-cached way then fill
> >> the cache AFTERWARDS - the code at ToT mainline doesn't fix the cache

> > I'm sorry, this makes no sense to me.  If we know the physical defaults
> > for the device and we can get the device into that state then we should
> > have no need to read from the cache.  If we don't know what they are
> > then they shouldn't be listed.

> In the case VDDD isn't supplied when the other two supplies are
> brought up, then the LDO is enabled, so if the register controlling
> the LDO configuration is in the cache.. that might explain a lot...

I still don't understand this at all.  Is the power on value for the
register variable?

> > I think the point that you lost me was your assumption that the
> > regulator enable needs to be done without the cache.  What makes you
> > believe that this is the case?

> Because that's what Freescale did in their code as per their comments
> - if only because they needed to do this kind of setup before the
> cache/defaults were populated. I assume they populate the cache from
> the chip at probe because the defaults change per revision and the
> presence of VDDD or not may change some of the registers in the cache.

We just have no idea why they're doing what they're doing; this is all
guesswork.  I've not seen anything that indicates that anyone who's sent
any code has any understanding of what they're changing.

> It may also be on boards like i.MX6 Sabrelite that the "defaults" are
> not the "defaults" over a warm reboot, since the chip never powers off
> (just "optionally" loses clock depending on whether CLKO is
> reconfigured or not at some point) and probably retains everything you
> ever set.

This is why most drivers do a hardware reset of the device on gaining
control of it, in order to ensure that the hardware is in a known state.
I see  that the driver doesn't do that except through the regulators.  I
see this driver doesn't do that which isn't terribly clever, though it's
not clear that the chip has suitable support.

> We want to do two possible things...

> * At probe, dump some known good defaults into the chip and use those as a cache

> * At probe, read out all the registers assuming they're already known
> good defaults into a cache

Clearly the first option is more sane since otherwise you've no idea if
the chip is configured sensibly.

> The intent being in the first case to make for damn sure the codec is
> in a well known state, and in the second case JUST to reduce i2c
> writes further down the line. Freescale seem to like the second one,

This isn't the sole function of the register cache, it's also used to
implement most of suspend and resume and to allow the device to be
powered off when idle but still present controls.

> Mainline is doing the first set, although it's definitely possible
> those defaults are not the true defaults if they were derived from a
> board like the Sabrelite where a warm reboot might retain all the
> state from previous boots.. the precedence of that list of defaults is
> basically unknown.. and I recall Freescale updating them in the BSP a
> lot.

I'd *really* hope that the defaults come from the chip datasheet.  If
they don't that's extremely disappointing.

> So the problem here might be some quirky board designs, a random
> implementation of an erratum workaround with no documentation, a bunch

My impression based on what I'm seeing here, including things like your
comments about lots of register default changes, is that people working
on the driver haven't had much understanding of what they're doing and
have got confused as a result.  Sounds like a good first step is getting
the register defaults sane...

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

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

Re: [RFC] ASoC: sgtl5000: Remove cache support

[Eric Nelson]

Hi Mark,

On 05/08/2013 03:31 AM, Mark Brown wrote:
> On Tue, May 07, 2013 at 03:06:39PM -0500, Matt Sealey wrote:
>> On Mon, May 6, 2013 at 5:06 PM, Mark Brown <broonie@kernel.org> wrote:
 >
 > ...
 >
>> Because that's what Freescale did in their code as per their comments
>> - if only because they needed to do this kind of setup before the
>> cache/defaults were populated. I assume they populate the cache from
>> the chip at probe because the defaults change per revision and the
>> presence of VDDD or not may change some of the registers in the cache.
>
> We just have no idea why they're doing what they're doing; this is all
> guesswork.  I've not seen anything that indicates that anyone who's sent
> any code has any understanding of what they're changing.
>
>> It may also be on boards like i.MX6 Sabrelite that the "defaults" are
>> not the "defaults" over a warm reboot, since the chip never powers off
>> (just "optionally" loses clock depending on whether CLKO is
>> reconfigured or not at some point) and probably retains everything you
>> ever set.

That's exactly right for SABRE Lite. Hitting the reset button or
invoking a processor reset doesn't reset the SGTL5000, so when
Linux boots, the registers are in an unknown state.

>
> This is why most drivers do a hardware reset of the device on gaining
> control of it, in order to ensure that the hardware is in a known state.
> I see  that the driver doesn't do that except through the regulators.  I
> see this driver doesn't do that which isn't terribly clever, though it's
> not clear that the chip has suitable support.
>
>> We want to do two possible things...
>> * At probe, dump some known good defaults into the chip and use those as
 >> a cache
>> * At probe, read out all the registers assuming they're already known
>> good defaults into a cache
>
> Clearly the first option is more sane since otherwise you've no idea if
> the chip is configured sensibly.
>

I agree. It seems that the 'cache' is currently mixing two things:
     1.) **presumed** power-on defaults for the chip, and
     2.) cached values of the registers

Since we can't ensure that we're starting with power-on-reset, the
entire initial cache is potentially wrong.

>> The intent being in the first case to make for damn sure the codec is
>> in a well known state, and in the second case JUST to reduce i2c
>> writes further down the line. Freescale seem to like the second one,
>
> This isn't the sole function of the register cache, it's also used to
> implement most of suspend and resume and to allow the device to be
> powered off when idle but still present controls.
>
>> Mainline is doing the first set, although it's definitely possible
>> those defaults are not the true defaults if they were derived from a
>> board like the Sabrelite where a warm reboot might retain all the
>> state from previous boots.. the precedence of that list of defaults is
>> basically unknown.. and I recall Freescale updating them in the BSP a
>> lot.
>
> I'd *really* hope that the defaults come from the chip datasheet.  If
> they don't that's extremely disappointing.
>
>> So the problem here might be some quirky board designs, a random
>> implementation of an erratum workaround with no documentation, a bunch
>
> My impression based on what I'm seeing here, including things like your
> comments about lots of register default changes, is that people working
> on the driver haven't had much understanding of what they're doing and
> have got confused as a result.  Sounds like a good first step is getting
> the register defaults sane...
>

An initial pass of writing all of the default register values with a
sort of 'cache flush' appears to be the right thing.

Then it doesn't even matter if the values match the datasheet, or if
the spec changes over time. If the values are reasonable, the device
will function properly.

Re: [RFC] ASoC: sgtl5000: Remove cache support

[Mark Brown]

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

On Wed, May 08, 2013 at 07:26:22AM -0700, Eric Nelson wrote:

> An initial pass of writing all of the default register values with a
> sort of 'cache flush' appears to be the right thing.

> Then it doesn't even matter if the values match the datasheet, or if
> the spec changes over time. If the values are reasonable, the device
> will function properly.

No, it does matter - when we do things like cache syncs on resume the
core will suppress writes of registers which have their power on value
since either the register will have been reset to that value by power
loss or retained the value due to power being maintained.  This is a
useful win when resuming to audio activity, I2C is pretty slow.

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

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

Re: [RFC] ASoC: sgtl5000: Remove cache support

[Mark Brown]

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

On Wed, May 08, 2013 at 03:59:19PM +0100, Mark Brown wrote:
> On Wed, May 08, 2013 at 07:26:22AM -0700, Eric Nelson wrote:

> > An initial pass of writing all of the default register values with a
> > sort of 'cache flush' appears to be the right thing.

> > Then it doesn't even matter if the values match the datasheet, or if
> > the spec changes over time. If the values are reasonable, the device
> > will function properly.

> No, it does matter - when we do things like cache syncs on resume the
> core will suppress writes of registers which have their power on value
> since either the register will have been reset to that value by power
> loss or retained the value due to power being maintained.  This is a
> useful win when resuming to audio activity, I2C is pretty slow.

Actually the simplest way to implement this solution (at least with
regmap, IIRC there might've been some issues with the ASoC cache code)
is just to discard the register defaults.  The core will fall back to
reading the hardware for any register it doesn't have cached but will
continue to cache values that are written.  

Something that iterates all the known registers and reads the default
values in will be required in order to support control access with the
device powered down.  I'd therefore suggest just not telling regmap
about the current defaults and instead changing the code to loop over
them and write them out during startup.  Probably with a big fat comment
about why we're doing this.

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

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

Re: [RFC] ASoC: sgtl5000: Remove cache support

[Fabio Estevam]

Hi Mark,

On Wed, May 8, 2013 at 12:11 PM, Mark Brown <broonie@kernel.org> wrote:

> Actually the simplest way to implement this solution (at least with
> regmap, IIRC there might've been some issues with the ASoC cache code)
> is just to discard the register defaults.  The core will fall back to
> reading the hardware for any register it doesn't have cached but will
> continue to cache values that are written.

Shouldn't we use defaults_raw with regmap?

The patch below fixes the reset issue for me:

>From d9018614781f9a4626d75dabbb495b8b44c74d09 Mon Sep 17 00:00:00 2001
From: Fabio Estevam <fabio.estevam@freescale.com>
Date: Wed, 8 May 2013 12:56:44 -0300
Subject: [PATCH] ASoC: sgtl5000: Use 'defaults_raw'

When using regmap, the 'defaults_raw' variants must be used, as these are the
ones checked inside drivers/base/regmap/regcache.c

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 sound/soc/codecs/sgtl5000.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c
index 327b443..c59fae8 100644
--- a/sound/soc/codecs/sgtl5000.c
+++ b/sound/soc/codecs/sgtl5000.c
@@ -1470,8 +1470,8 @@ static const struct regmap_config sgtl5000_regmap = {
 	.readable_reg = sgtl5000_readable,

 	.cache_type = REGCACHE_RBTREE,
-	.reg_defaults = sgtl5000_reg_defaults,
-	.num_reg_defaults = ARRAY_SIZE(sgtl5000_reg_defaults),
+	.reg_defaults_raw = sgtl5000_reg_defaults,
+	.num_reg_defaults_raw = ARRAY_SIZE(sgtl5000_reg_defaults),
 };

 static int sgtl5000_i2c_probe(struct i2c_client *client,
-- 
1.7.9.5

If you think this is the right approach I can submit this change and
fix the other codecs that use regmap as well.

Regards,

Fabio Estevam

Re: [RFC] ASoC: sgtl5000: Remove cache support

[Fabio Estevam]

On Wed, May 8, 2013 at 1:02 PM, Fabio Estevam <festevam@gmail.com> wrote:

> When using regmap, the 'defaults_raw' variants must be used, as these are the
> ones checked inside drivers/base/regmap/regcache.c

Please ignore this comment. I fixed it in the patch I have just submitted.

[RFC] ASoC: sgtl5000: Remove cache support

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@freescale.com>

Currently sgtl5000 driver is probed correctly after a power-on reset, but after
audio is played and the board is reset, we are no longer able to probe again:

sgtl5000 0-000a: Device with ID register ffff is not a sgtl5000
sgtl5000 0-000a: ASoC: failed to probe CODEC -19
imx-sgtl5000 sound.12: ASoC: failed to instantiate card -19
imx-sgtl5000 sound.12: snd_soc_register_card failed (-19)

Removing the cache support mechanism allows us to probe sgtl5000 driver 
successfully in subsequent board resets.

Tested on a mx6qsabrelite board.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
I would like to fix it properly and looking for some ideas/feedbacks as to what
a proper fix would be.

Should I complete the sgtl5000_regs array with all the sgtl5000 registers?

Thanks!

 sound/soc/codecs/sgtl5000.c |   31 -------------------------------
 1 file changed, 31 deletions(-)

diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c
index 92bbfec..3884162 100644
--- a/sound/soc/codecs/sgtl5000.c
+++ b/sound/soc/codecs/sgtl5000.c
@@ -33,33 +33,6 @@
 #define SGTL5000_DAP_REG_OFFSET	0x0100
 #define SGTL5000_MAX_REG_OFFSET	0x013A
 
-/* default value of sgtl5000 registers */
-static const u16 sgtl5000_regs[SGTL5000_MAX_REG_OFFSET] =  {
-	[SGTL5000_CHIP_CLK_CTRL] = 0x0008,
-	[SGTL5000_CHIP_I2S_CTRL] = 0x0010,
-	[SGTL5000_CHIP_SSS_CTRL] = 0x0008,
-	[SGTL5000_CHIP_DAC_VOL] = 0x3c3c,
-	[SGTL5000_CHIP_PAD_STRENGTH] = 0x015f,
-	[SGTL5000_CHIP_ANA_HP_CTRL] = 0x1818,
-	[SGTL5000_CHIP_ANA_CTRL] = 0x0111,
-	[SGTL5000_CHIP_LINE_OUT_VOL] = 0x0404,
-	[SGTL5000_CHIP_ANA_POWER] = 0x7060,
-	[SGTL5000_CHIP_PLL_CTRL] = 0x5000,
-	[SGTL5000_DAP_BASS_ENHANCE] = 0x0040,
-	[SGTL5000_DAP_BASS_ENHANCE_CTRL] = 0x051f,
-	[SGTL5000_DAP_SURROUND] = 0x0040,
-	[SGTL5000_DAP_EQ_BASS_BAND0] = 0x002f,
-	[SGTL5000_DAP_EQ_BASS_BAND1] = 0x002f,
-	[SGTL5000_DAP_EQ_BASS_BAND2] = 0x002f,
-	[SGTL5000_DAP_EQ_BASS_BAND3] = 0x002f,
-	[SGTL5000_DAP_EQ_BASS_BAND4] = 0x002f,
-	[SGTL5000_DAP_MAIN_CHAN] = 0x8000,
-	[SGTL5000_DAP_AVC_CTRL] = 0x0510,
-	[SGTL5000_DAP_AVC_THRESHOLD] = 0x1473,
-	[SGTL5000_DAP_AVC_ATTACK] = 0x0028,
-	[SGTL5000_DAP_AVC_DECAY] = 0x0050,
-};
-
 /* regulator supplies for sgtl5000, VDDD is an optional external supply */
 enum sgtl5000_regulator_supplies {
 	VDDA,
@@ -1391,10 +1364,6 @@ static struct snd_soc_codec_driver sgtl5000_driver = {
 	.suspend = sgtl5000_suspend,
 	.resume = sgtl5000_resume,
 	.set_bias_level = sgtl5000_set_bias_level,
-	.reg_cache_size = ARRAY_SIZE(sgtl5000_regs),
-	.reg_word_size = sizeof(u16),
-	.reg_cache_step = 2,
-	.reg_cache_default = sgtl5000_regs,
 	.volatile_register = sgtl5000_volatile_register,
 	.controls = sgtl5000_snd_controls,
 	.num_controls = ARRAY_SIZE(sgtl5000_snd_controls),
-- 
1.7.9.5