On Sat, Apr 18, 2015 at 4:36 AM, Mark Brown broonie@kernel.org wrote:
On Wed, Apr 15, 2015 at 02:42:20PM -0700, Kevin Cernekee wrote:
+static int tas571x_set_sysclk(struct snd_soc_dai *dai,
int clk_id, unsigned int freq, int dir)+{
/** TAS5717 datasheet pg 21: "The DAP can autodetect and set the* internal clock-control logic to the appropriate settings for all* supported clock rates as defined in the clock control register."*/return 0;+}
Remove empty functions, at best they waste space at worst they break things.
Without the empty function, we run into problems with drivers that abort when they get -ENOTSUPP here:
sound/soc/atmel/atmel_wm8904.c: ret = snd_soc_dai_set_sysclk(codec_dai, WM8904_CLK_FLL, sound/soc/atmel/atmel_wm8904.c- 0, SND_SOC_CLOCK_IN); sound/soc/atmel/atmel_wm8904.c- if (ret < 0) { sound/soc/atmel/atmel_wm8904.c- pr_err("%s -failed to set wm8904 SYSCLK\n", __func__); sound/soc/atmel/atmel_wm8904.c- return ret; sound/soc/atmel/atmel_wm8904.c- }
Is there a stub version that I can use instead? Nothing jumped out at me when looking at the other codec drivers.
+static int tas571x_set_shutdown(struct tas571x_private *priv, bool is_shutdown) +{
return regmap_update_bits(priv->regmap, TAS571X_SYS_CTRL_2_REG,TAS571X_SYS_CTRL_2_SDN_MASK,is_shutdown ? TAS571X_SYS_CTRL_2_SDN_MASK : 0);+}
ret = tas571x_set_shutdown(priv, false);if (ret)return ret;break;case SND_SOC_BIAS_STANDBY:ret = tas571x_set_shutdown(priv, true);if (ret)return ret;This looks like it'd be clearer just as direct register updates, I'm not sure a function to set a single bit is addinng much.
It might be useful if another tas571x variant put the bit somewhere else, but that hasn't happened yet so I can nuke the helper function for now.
case SND_SOC_BIAS_OFF:/* Note that this kills I2C accesses. */assert_pdn = 1;No, the GPIO set associated with it kills I2C access. I'd also expect to see the regmap being marked cache only before we do this and a resync of the register map when we power back up (assuming that is actually a power down).
Hmm, not sure if this actually resets the registers back to power-on defaults, but I'll check.
/** The master volume defaults to 0x3ff (mute), but we ignore* (zero) the LSB because the hardware step size is 0.125 dB* and TLV_DB_SCALE_ITEM has a resolution of 0.01 dB.*/if (regmap_write(priv->regmap, TAS571X_MVOL_REG, 0x3fe))return -EIO;I don't understand this - is the LSB a mute bit or sommething?
The 10-bit master volume field on 5717/5719 works like:
0x3ff: MUTE (power-on default) 0x3fe: -103.750 dB 0x3fd: -103.625 dB [lots more options, in 0.125 dB increments] 0x001: 23.875 dB 0x000: 24.000 dB
Since we only have a resolution of 0.01 dB, the driver forces the LSB to 0 and uses 0.25 dB increments instead of 0.125 dB. Mute is handled through the dedicated per-channel soft mute register bits instead of the 0x3ff volume setting.