On Tue, Sep 01, 2015 at 01:41:40PM +0800, Songjun Wu wrote:
+static const char * const mono_text[] = {
- "stereo", "mono"
+};
+static SOC_ENUM_SINGLE_DECL(classd_mono_enum,
CLASSD_INTPMR, CLASSD_INTPMR_MONO_SHIFT,mono_text);
This looks like it should be a simple Switch control called something like "Stereo Switch" or "Mono Switch".
+static const char * const deemp_text[] = {
- "disabled", "enabled"
+};
+static SOC_ENUM_SINGLE_DECL(classd_deemp_enum,
CLASSD_INTPMR, CLASSD_INTPMR_DEEMP_SHIFT,deemp_text);
Similarly this looks like it should be "Deemph Switch".
+static const char * const eqcfg_bass_text[] = {
- "-12 dB", "-6 dB", "0 dB", "+6 dB", "+12 dB"
+};
+static const unsigned int eqcfg_bass_value[] = {
- CLASSD_INTPMR_EQCFG_B_CUT_12,
- CLASSD_INTPMR_EQCFG_B_CUT_6, CLASSD_INTPMR_EQCFG_FLAT,
- CLASSD_INTPMR_EQCFG_B_BOOST_6, CLASSD_INTPMR_EQCFG_B_BOOST_12
+};
This should be a Volume control with TLV information, as should the following few controls.
+static const struct snd_kcontrol_new atmel_classd_snd_controls[] = { +SOC_SINGLE_TLV("Left Volume", CLASSD_INTPMR,
CLASSD_INTPMR_ATTL_SHIFT, 78, 1, classd_digital_tlv),+SOC_SINGLE_TLV("Right Volume", CLASSD_INTPMR,
CLASSD_INTPMR_ATTR_SHIFT, 78, 1, classd_digital_tlv),
This should be a single stereo control rather than separate left and right controls.
+static const char * const pwm_type[] = {
- "single-ended", "differential"
+};
The normal style for ALSA controls is to capitalise strings so "Single ended" and "Differential".
- if (pdata->non_overlap_enable) {
val |= (CLASSD_MR_NON_OVERLAP_EN<< CLASSD_MR_NON_OVERLAP_SHIFT);mask |= CLASSD_MR_NOVR_VAL_MASK;switch (pdata->non_overlap_time) {case 5:val |= (CLASSD_MR_NOVR_VAL_5NS<< CLASSD_MR_NOVR_VAL_SHIFT);break;case 10:val |= (CLASSD_MR_NOVR_VAL_10NS<< CLASSD_MR_NOVR_VAL_SHIFT);break;case 15:val |= (CLASSD_MR_NOVR_VAL_15NS<< CLASSD_MR_NOVR_VAL_SHIFT);break;case 20:val |= (CLASSD_MR_NOVR_VAL_20NS<< CLASSD_MR_NOVR_VAL_SHIFT);break;default:val |= (CLASSD_MR_NOVR_VAL_10NS<< CLASSD_MR_NOVR_VAL_SHIFT);break;}
I'd expect at least a warning if the user trys to specify an invalid value (if they didn't specify any value then I'd expect the DT parsing function to assign the default value).
+static struct regmap *atmel_classd_codec_get_remap(struct device *dev) +{
- struct atmel_classd *dd = dev_get_drvdata(dev);
- return dd->regmap;
+}
Can you just use dev_get_regmap()?
+static int atmel_classd_codec_dai_startup(struct snd_pcm_substream *substream,
struct snd_soc_dai *codec_dai)+{
- struct atmel_classd *dd = snd_soc_dai_get_drvdata(codec_dai);
- clk_prepare_enable(dd->aclk);
- clk_prepare_enable(dd->gclk);
Should check for errors here.
- dev_info(dev,
"Atmel Class D Amplifier (CLASSD) device at 0x%p (irq %d)\n",io_base, dd->irq);
This is a bit noisy and not really based on interaction with the hardware... dev_dbg() seems better.