Re: [PATCH 2/2] ASoC: max98396: add amplifier driver

On Fri, Apr 15, 2022 at 05:40:24PM -0700, Ryan Lee wrote:

This looks mostly good - some issues below but nothing structural.

• switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
• case SND_SOC_DAIFMT_NB_NF:
• case SND_SOC_DAIFMT_NB_IF:

• break;
  

One of these must be wrong - the device needs to know if it's handling a normal or inverted frame clock, otherwise the audio will be corrupted.

+static int max98396_mux_put(struct snd_kcontrol *kcontrol,

• 	    struct snd_ctl_elem_value *ucontrol)
  

+{

• struct snd_soc_component *component =

• snd_soc_dapm_kcontrol_component(kcontrol);
  

• struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kcontrol);
• struct max98396_priv *max98396 = snd_soc_component_get_drvdata(component);
• struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
• unsigned int *item = ucontrol->value.enumerated.item;
• int reg, val;
• if (item[0] >= e->items)

• return -EINVAL;
  

• val = snd_soc_enum_item_to_val(e, item[0]) << e->shift_l;
• if (max98396->device_id == CODEC_TYPE_MAX98396)

• reg = MAX98396_R2055_PCM_RX_SRC1;
  

• else

• reg = MAX98397_R2056_PCM_RX_SRC1;
  

• regmap_update_bits(max98396->regmap, reg,

• 	   MAX98396_PCM_RX_MASK, val);
  

• snd_soc_dapm_mux_update_power(dapm, kcontrol, item[0], e, NULL);
• return 0;

+}

This should return 1 if the value changed - you should get an error reported by mixer-test from selftests if you run them on a sound card with the driver.

• /* L/R mix configuration */
• if (max98396->device_id == CODEC_TYPE_MAX98396) {

• regmap_write(max98396->regmap,
  

• 	     MAX98396_R2055_PCM_RX_SRC1, 0x02);
  

• regmap_write(max98396->regmap,
  

• 	     MAX98396_R2056_PCM_RX_SRC2, 0x10);
  

• } else {

• regmap_write(max98396->regmap,
  

• 	     MAX98397_R2056_PCM_RX_SRC1, 0x02);
  

• regmap_write(max98396->regmap,
  

• 	     MAX98397_R2057_PCM_RX_SRC2, 0x10);
  

• }

Shouldn't these be user controllable? Most of the setup being done here looks system specific, especially the routing stuff.