On Tue, Apr 05, 2022 at 09:12:20PM +0800, Zhu Ning wrote:
The ES8326 codec is not compatible with ES8316 and requires a dedicated driver.
This looks mostly good - a few things below but nothing huge that should require big restructurings or anything.
+++ b/sound/soc/codecs/es8326.c @@ -0,0 +1,753 @@ +// SPDX-License-Identifier: GPL-2.0-only +/*
- es8326.c -- es8326 ALSA SoC audio driver
- Copyright Everest Semiconductor Co., Ltd
Please make the entire comment a C++ one so things look more intentional.
+#include <linux/clk.h> +#include <linux/gpio.h>
The driver doesn't actually seem to use the GPIO APIs?
- /* The lock protects the situation that an irq is generated
* while the previous irq is still being processed.*/- struct mutex lock;
It doesn't seem to, it seems to protect registrations and deregistrations from racing against each other? The actual interrupt handling doesn't use it.
- SOC_SINGLE_TLV("ADC Analog PGA Gain", ES8326_PAGGAIN_23, 0, 10, 0, adc_analog_pga_tlv),
Control name should end in Volume - see control-names.rst.
- regmap_write(es8326->regmap, ES8326_CLK_CTL_01, ES8326_CLK_ON);
- /* Two channel ADC */
- regmap_write(es8326->regmap, ES8326_PULLUP_CTL_F9, 0x02);
- regmap_write(es8326->regmap, ES8326_CLK_INV_02, 0x00);
- regmap_write(es8326->regmap, ES8326_CLK_DIV_CPC_0C, 0x1F);
- regmap_write(es8326->regmap, ES8326_CLK_TRI_0E, 0x00);
- regmap_write(es8326->regmap, ES8326_CLK_VMIDS1_10, 0xC8);
- regmap_write(es8326->regmap, ES8326_CLK_VMIDS2_11, 0x88);
- regmap_write(es8326->regmap, ES8326_CLK_CAL_TIME_12, 0x20);
- regmap_write(es8326->regmap, ES8326_DAC_MUTE_14, 0x00);
- regmap_write(es8326->regmap, ES8326_ANA_LOWPOWER_19, 0xF0);
- regmap_write(es8326->regmap, ES8326_SYS_BIAS_1D, 0x08);
- regmap_write(es8326->regmap, ES8326_DAC2HPMIX_25, 0x22);
- regmap_write(es8326->regmap, ES8326_ADC_SCALE_29, 0x00);
- regmap_write(es8326->regmap, ES8326_ADC1_SRC_2A, es8326->mic1_src);
- regmap_write(es8326->regmap, ES8326_ADC2_SRC_2B, es8326->mic2_src);
- regmap_write(es8326->regmap, ES8326_HP_CAL_4A, 0x00);
- regmap_write(es8326->regmap, ES8326_DAC_DSM_4D, 0x08);
- regmap_write(es8326->regmap, ES8326_DAC_RAMPRATE_4E, 0x20);
- regmap_write(es8326->regmap, ES8326_DAC_VPPSCALE_4F, 0x15);
- regmap_write(es8326->regmap, ES8326_HPJACK_TIMER_56, 0x88);
- regmap_write(es8326->regmap, ES8326_HP_DET_57,
ES8326_HP_DET_SRC_PIN9 | es8326->jack_pol);- regmap_write(es8326->regmap, ES8326_INT_SOURCE_58, 0x08);
- regmap_write(es8326->regmap, ES8326_INTOUT_IO_59, 0x45);
- regmap_write(es8326->regmap, ES8326_RESET_00, ES8326_CSM_ON);
- snd_soc_component_update_bits(component, ES8326_PAGGAIN_23,
ES8326_MIC_SEL_MASK, ES8326_MIC1_SEL);
This looks like some or all of it is doing routing which would normally be configured from userspace using controls - the ADC1/2 sources and DAC controls jump out for example. Some of it like the pullups might make sense fixed, or as DT properties like the jack detect polarity is here, but not all of it.
+#ifdef CONFIG_OF +static const struct i2c_device_id es8326_i2c_id[] = {
- {"es8326", 0 },
- {}
+}; +MODULE_DEVICE_TABLE(i2c, es8326_i2c_id); +#endif
This should be unconditional, the ifdefs should be around es8326_of_match instead.