[alsa-devel] [PATCH 2/4] ASoC: soc-cache: Add support for standard register caching

Dimitris Papastamos dp at opensource.wolfsonmicro.com
Mon Oct 25 10:09:40 CEST 2010


On Sun, 2010-10-24 at 14:35 -0700, Mark Brown wrote:
> On Sun, Oct 24, 2010 at 08:18:59AM -0500, Timur Tabi wrote:
> > On Sat, Oct 23, 2010 at 1:24 PM, Dimitris Papastamos
> 
> > > The only problem I see with the above code, is when
> > > codec_drv->reg_word_size > sizeof (unsigned int) but that can't really
> > > happen in practice.
> 
> Plus if it did happen the rest of the code would fall over fairly badly
> since we've got that assumption embedded in the register read and write
> APIs.
> 
> > I'm going to have to agree with Mark that this code is suspect.  I
> > understand everything you said, but it makes me nervous.  Unless this
> > code is in some kind of fast-path, I would prefer to see it rewritten
> > to avoid any assumption about the sizes of the types involved.
> 
> I think the important thing here is that the code is clear - from a
> maintainability so long as it's clear how and why the code works things
> should be fine, otherwise we'll have people scratching their heads over
> it every time someone looks at the code which is going to be painful.
> This could be done with documentation as well as with code changes,
> though code changes should definitely be considered.

Yes that makes sense.  The reason why I did this in the first place was
to make it work with 8/16-byte reads/writes without using branching.  I
will change this to explicitly dereference a u8/u16 pointer.

Thanks,
Dimitrios




More information about the Alsa-devel mailing list