[alsa-devel] [PATCH 1/1] ASoC: core: cache index fix

[Takashi Iwai]

At Tue, 2 Aug 2011 10:55:25 +0000,
Dong Aisheng-B29396 wrote:
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai at suse.de]
> > Sent: Tuesday, August 02, 2011 6:34 PM
> > To: Dong Aisheng-B29396
> > Cc: Mark Brown; alsa-devel at alsa-project.org; s.hauer at pengutronix.de;
> > lrg at ti.com; linux-arm-kernel at lists.infradead.org; w.sang at pengutronix.de
> > Subject: Re: [alsa-devel] [PATCH 1/1] ASoC: core: cache index fix
> > 
> > At Tue, 2 Aug 2011 09:41:22 +0000,
> > Dong Aisheng-B29396 wrote:
> > >
> > > > -----Original Message-----
> > > > From: Mark Brown [mailto:broonie at opensource.wolfsonmicro.com]
> > > > Sent: Tuesday, August 02, 2011 4:39 PM
> > > > To: Dong Aisheng-B29396
> > > > Cc: alsa-devel at alsa-project.org;
> > > > linux-arm-kernel at lists.infradead.org;
> > > > lrg at ti.com; s.hauer at pengutronix.de; w.sang at pengutronix.de
> > > > Subject: Re: [PATCH 1/1] ASoC: core: cache index fix
> > > >
> > > > On Tue, Aug 02, 2011 at 08:03:23AM +0000, Dong Aisheng-B29396 wrote:
> > > >
> > > >
> > > > > > ...hw_read() shouldn't need to know about this stuff
> > > > > Here the reg_cache_size is the maximum cache index in the register
> > > > cache array.
> > > > > Therefore, the original code using reg to compare with
> > > > > reg_cache_size is not correct when the reg_cache_step is not 1.
> > > > > e.g. the reg to read is 0x30 and reg_cache_size maybe only 0x16.
> > > > > So we use idx to check if it exceeds the cache size.
> > > >
> > > > I see what you're doing, but like I say this is just legacy code
> > > > that shouldn't be peering at the cache size any more and layering
> > > > additional stuff in is just going to make the situation worse.
> > > >
> > > > > BTW, do you mean the soc_io layer does not need to know the
> > > > > details of idx&reg Conversion?
> > > >
> > > > Yes.
> > > >
> > > > > Do I need to implement a help function like reg_is_cachable(reg)
> > > > > to be
> > > > used here?
> > > >
> > > > No, I think we should hide these decisions completely within the
> > > > cache code.
> > >
> > > Yes, but the issue is that hw_read will check if reg is in cache array
> > > And checking like " if (reg >= codec->driver->reg_cache_size ||" is
> > > wrong when the step is not 1 that will cause registers with their
> > > index are greater than cache index will not be able to fetch data from
> > cache.
> > 
> > reg_cache_size is supposed to be the real size of the cache table.
> > This isn't influenced by reg_cache_step value.  So, the behavior in soc-
> > io.c (and other ASoC core) is correct.
> But the reg is related to step.
> So reg and reg_cache_size are un-match when step > 1, right?

I'm not sure what is referred here as reg, but the argument passed to
snd_soc_{read|write}() is the raw register index.  reg = 2 is 2 no
matter whether reg_cache_step is 1 or 2.  This is passed down to
hw_read(), thus reg and reg_cache_size do match even when step > 1.


Takashi