On Thu, Dec 06, 2018 at 03:04:46PM -0600, James Schulman wrote: > Add driver support for Cirrus Logic CS35L36 boosted > speaker amplifier > > Signed-off-by: James Schulman <james.schulman@xxxxxxxxxx> > --- Again just a couple of very minor nit picks from me. > +static int cs35l36_dai_set_sysclk(struct snd_soc_dai *dai, int clk_id, > + unsigned int freq, int dir) > +{ > + struct snd_soc_component *component = dai->component; > + struct cs35l36_private *cs35l36 = > + snd_soc_component_get_drvdata(component); > + int fs1, fs2, reg; > + > + if (freq > CS35L36_FS_NOM_6MHZ) { > + fs1 = CS35L36_FS1_DEFAULT_VAL; > + fs2 = CS35L36_FS2_DEFAULT_VAL; > + } else { > + fs1 = 3 * ((CS35L36_FS_NOM_6MHZ * 4 + freq - 1) / freq) + 4; > + fs2 = 5 * ((CS35L36_FS_NOM_6MHZ * 4 + freq - 1) / freq) + 4; > + } > + > + regmap_write(cs35l36->regmap, CS35L36_TESTKEY_CTRL, > + CS35L36_TEST_UNLOCK1); > + regmap_write(cs35l36->regmap, CS35L36_TESTKEY_CTRL, > + CS35L36_TEST_UNLOCK2); > + > + regmap_read(cs35l36->regmap, CS35L36_TST_FS_MON0, ®); > + reg &= ~(CS35L36_FS1_WINDOW_MASK | CS35L36_FS2_WINDOW_MASK); > + reg |= ((fs1 & CS35L36_FS1_WINDOW_MASK) | > + (fs2 << CS35L36_FS2_WINDOW_SHIFT & CS35L36_FS2_WINDOW_MASK)); > + regmap_write(cs35l36->regmap, CS35L36_TST_FS_MON0, reg); This is just open coding update_bits probably better to just use update_bits. > + > + regmap_write(cs35l36->regmap, CS35L36_TESTKEY_CTRL, > + CS35L36_TEST_LOCK1); > + regmap_write(cs35l36->regmap, CS35L36_TESTKEY_CTRL, > + CS35L36_TEST_LOCK2); > + return 0; > +} > + > +typedef char CS35L36_MAX_NUM_REGS[__LINE__]; Not sure I am the greatest fan of this, is it perhaps just worth combining the tables file and the regular file? Then you don't need any fancyness. Thanks, Charles _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel