Re: [PATCH 2/2] ASoC: rt9120: Add rt9210 audio amplifier support

ChiYuan Huang [off-list ref] 於 2021年10月5日 週二 下午8:36寫道：

Mark Brown [off-list ref] 於 2021年10月5日 週二 下午7:54寫道：

quoted

On Tue, Oct 05, 2021 at 09:16:55AM +0800, cy_huang wrote:

quoted

+static const char * const sdo_select_text[] = {
+     "NONE", "INTF", "FINAL", "RMS Detect"
+};

Why not None and Final?

Just follow the datasheet to write the text.
Whatever, Ack in next.

quoted

quoted

+     if (event == SND_SOC_DAPM_PRE_PMU)
+             snd_soc_component_write(comp, RT9120_REG_ERRRPT, 0);
+     else if (event == SND_SOC_DAPM_POST_PMU)
+             msleep(RT9120_AMPON_WAITMS);
+     else if (event == SND_SOC_DAPM_POST_PMD)
+             msleep(RT9120_AMPOFF_WAITMS);

This should be a switch statement, it'd be clearer.

Ack in next.

quoted

quoted

+     /* Default config volume to 0dB */
+     snd_soc_component_write(comp, RT9120_REG_MSVOL, 0x180);
+     /* Mute by default */
+     snd_soc_component_update_bits(comp, RT9120_REG_VOLRAMP,
+                                   RT9120_MUTE_MASK, RT9120_MUTE_MASK);

As ever you should leave the defaults at whatever the hardware defaults
to, the defaults for one machine may not be suitable for another so we
shouldn't be trying to do that in software.

The default volume will be kept in value 0x7ff (mute).
I just want to follow the ASoC flow to control mute/unmute mask by AMP on/off.
If to default set volume to 0dB and mute is improper, user have to use
mixer control to configure the volume.
Does mute function also need to be removed also?

Sorry, loop all again.