On Thu, Sep 25, 2014 at 03:20:26PM +0100, Mark Brown wrote:
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.
The convenstion we used was <pipeline identifer> and then module. So if gain block is in media 0 input we refer to it as "media 0 in Gain Volume" This helps to identify the module location and help in understanding control by looking at the name
+#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.
I will fix the holes here..
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.
Sorry about this one, i will make sure this is fixed properly and not repeated