On Tue, Mar 12, 2024 at 07:03:25PM +0100, Alexandre Mergnat wrote:
On 26/02/2024 17:09, Mark Brown wrote:
- case MT6357_ZCD_CON2:
regmap_read(priv->regmap, MT6357_ZCD_CON2, ®);priv->ana_gain[ANALOG_VOLUME_HPOUTL] =(reg & AUD_HPL_GAIN_MASK) >> AUD_HPL_GAIN_SFT;priv->ana_gain[ANALOG_VOLUME_HPOUTR] =(reg & AUD_HPR_GAIN_MASK) >> AUD_HPR_GAIN_SFT;break;
It would probably be less code and would definitely be clearer and simpler to just read the values when we need them rather than constatly keeping a cache separate to the register cache.
Actually you must save the values because the gain selected by the user will be override to do a ramp => volume_ramp(.....):
- When you switch on the HP, you start from gain=-40db to final_gain
(selected by user).
- When you switch off the HP, you start from final_gain (selected by user)
to gain=-40db.
You can just read the value back when you need to do a ramp?
Also, the microphone's gain change when it's enabled/disabled.
I don't understand what this means?
- /* ul channel swap */
- SOC_SINGLE("UL LR Swap", MT6357_AFE_UL_DL_CON0, AFE_UL_LR_SWAP_SFT, 1, 0),
On/off controls should end in Switch.
Sorry, I don't understand your comment. Can you reword it please ?
See control-names.rst. Run mixer-test on a card with this driver and fix all the issues it reports.
+static int hslo_mux_map_value[] = {
- 0x0, 0x1, 0x2, 0x3,
+};
Why not just use a normal mux here, there's no missing values or reordering? Similarly for other muxes.
I've dug into some other codecs and it's done like that, but I've probably misunderstood something.
The only bad thing I see is enum is missing currently:
enum { PGA_MUX_OPEN = 0, PGA_MUX_DACR, PGA_MUX_PB, PGA_MUX_TM, PGA_MUX_MASK = 0x3, };
The whole thing with explicitly specfying the mapping is just completely redundant, you may as well remove it.