[alsa-devel] [PATCH] ASoC: pcm512x: Add PCM512x driver

Mark Brown broonie at kernel.org
Thu Feb 6 14:50:15 CET 2014


On Thu, Feb 06, 2014 at 02:12:48PM +0100, Takashi Iwai wrote:
> Mark Brown wrote:

> > > The numbers of items in pcm512x_dsp_program_texts[] and _values[]
> > > don't match.

> > Yeah, fixed.

> I'm thinking whether we can check this in the macro.
> I thought of using BUILD_BUG_ON(), but it's unsure whether it aligns
> there well.

Yes, or changing the way we pass things in so it's an array of
key/value.  Usability isn't great for enums in general and particularly
poor for sparse ones.

> > Hrm, it is but this points out an error in the control helpers which has
> > been there since forever - they call that parameter max but it's not a
> > maximum, it's the number of elements in the enumeration.  I bet we have
> > a bunch of other enumerations which miss the last element as a result.

> Yeah, the argument name is really confusing.  I had to double-check
> the code when I reviewed your patch, too :)

> Also it'd be better to have a practice to use either ARRAY_SIZE() or a
> constant there, too.

Indeed - even better would be to just remove the parameter entirely and
use ARRAY_SIZE() on the array of strings we get passed in so there's no
way it could be messed up.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20140206/664fc244/attachment.sig>


More information about the Alsa-devel mailing list