[alsa-devel] [PATCH v7 1/7] ASoC: Intel: mrfld: add the gain controls

[Mark Brown]

On Fri, Sep 19, 2014 at 04:46:02PM +0530, Subhransu S. Prusty wrote:

> +/* Gain helper with min/max set */
> +#define SST_GAIN(name, path_id, task_id, instance, gain_var)				\
> +	SST_GAIN_KCONTROLS(name, "Gain", SST_GAIN_MIN_VALUE, SST_GAIN_MAX_VALUE,	\
> +		SST_GAIN_TC_MIN, SST_GAIN_TC_MAX,					\

As far as I can tell from following the macros through this looks like
it will create controls called "name Gain Volume" which doesn't seem
excellent.

> +#define SST_NUM_GAINS 36
> +static struct sst_gain_value sst_gains[SST_NUM_GAINS];

> +static const struct snd_kcontrol_new sst_gain_controls[] = {
> +	SST_GAIN("media0_in", SST_PATH_INDEX_MEDIA0_IN, SST_TASK_MMX, 0, &sst_gains[0]),

This table is full of magic number references into the array...

> +	SST_GAIN("codec_out1", SST_PATH_INDEX_CODEC_OUT1, SST_TASK_SBA, 0, &sst_gains[23]),
> +	SST_GAIN("media_loop1_out", SST_PATH_INDEX_MEDIA_LOOP1_OUT, SST_TASK_SBA, 0, &sst_gains[30]),

...with holes in the array.  This seems really error prone.

>  	char params[0];
>  } __packed;
> +/**** widget defines *****/
> +
> +#include <sound/soc.h>
> +#include <sound/tlv.h>
> +struct sst_ids {

This is adding includes in the middle of the file (which isn't the usual
kernel coding style) and is missing a blank lines.  This sort of stuff
is getting quite repetitive and hence annoying, especially the blank
lines thing which comes up again and again and just makes everything
even harder to read.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20140925/c12d9a30/attachment.sig>