On Wed, 12 Dec 2018, Charles Keepax wrote:
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@cirrus.com
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.
I did this because it's a register that we didn't want to be visible in userspace, but now I realize that just means it's precious. Will add it to the precious register list and resubmit.
- 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.
Ya it's probably not worth trying to get fancy... I'll just nuke the tables file.
Thanks, Charles