On Wed, Jun 19, 2024 at 04:46:48PM +0200, amergnat@baylibre.com wrote:

• /* gain default values*/
• regmap_update_bits(priv->regmap, MT6357_AUDENC_ANA_CON0, MT6357_AUDPREAMPLGAIN_MASK,

• 	   UL_GAIN_0DB << MT6357_AUDPREAMPLGAIN_SFT);
  

• regmap_update_bits(priv->regmap, MT6357_AUDENC_ANA_CON1, MT6357_AUDPREAMPRGAIN_MASK,

• 	   UL_GAIN_0DB << MT6357_AUDPREAMPRGAIN_SFT);
  

• regmap_update_bits(priv->regmap, MT6357_ZCD_CON1,

• 	   MT6357_AUD_LOL_GAIN_MASK |
  

• 	   MT6357_AUD_LOR_GAIN_MASK,
  

• 	   DL_GAIN_0DB << MT6357_AUD_LOL_GAIN_SFT |
  

• 	   DL_GAIN_0DB << MT6357_AUD_LOR_GAIN_SFT);
  

• regmap_update_bits(priv->regmap, MT6357_ZCD_CON2,

• 	   MT6357_AUD_HPL_GAIN_MASK |
  

• 	   MT6357_AUD_HPR_GAIN_MASK,
  

• 	   DL_GAIN_0DB << MT6357_AUD_HPL_GAIN_SFT |
  

• 	   DL_GAIN_0DB << MT6357_AUD_HPR_GAIN_SFT);
  

• regmap_update_bits(priv->regmap, MT6357_ZCD_CON3,

• 	   MT6357_AUD_HS_GAIN_MASK, DL_GAIN_0DB);
  

We generally leave everything at chip defaults, why is this different?

+static int mt6357_codec_probe(struct snd_soc_component *codec) +{

• struct mt6357_priv *priv = snd_soc_component_get_drvdata(codec);
• mt6357_codec_init(priv);
• return 0;

+}

Why not just inline mt6357_codec_init() into the one user?

+static unsigned int mt6357_read(struct snd_soc_component *codec, unsigned int reg) +{

• struct mt6357_priv *priv = snd_soc_component_get_drvdata(codec);
• unsigned int val;
• regmap_read(priv->regmap, reg, &val);
• return val;

+}

+static int mt6357_write(struct snd_soc_component *codec, unsigned int reg, unsigned int value) +{

• struct mt6357_priv *priv = snd_soc_component_get_drvdata(codec);
• return regmap_update_bits(priv->regmap, reg, 0xffff, value);

+}

Why open code these, the core has standard adaptors for regmap?

+static const u32 micbias_values[MT6357_MICBIAS_ARRAY_SIZE] = {

• 1700000, 1800000, 1900000, 2000000,
• 2100000, 2500000, 2600000, 2700000

+};

Just use ARRAY_SIZE() for the size then the number can't be out of sync.