On Tue, Jul 26, 2022 at 09:17:46PM +0800, Zhu Ning wrote:
+static struct snd_pcm_hw_constraint_list es8326_constraints = {
- .count = ARRAY_SIZE(es8326_rates),
- .list = es8326_rates,
+};
+static int es8326_set_dai_sysclk(struct snd_soc_dai *codec_dai,
int clk_id, unsigned int freq, int dir)+{
- struct snd_soc_component *codec = codec_dai->component;
- struct es8326_priv *es8326 = snd_soc_component_get_drvdata(codec);
- es8326->sysclk = freq;
- if (freq == 0) {
es8326->sysclk_constraints->list = NULL;es8326->sysclk_constraints->count = 0;return 0;- }
- es8326->sysclk_constraints = &es8326_constraints;
Nothing ever restores the constraints if a clock is specified again, and in general it's odd that the enable/disable don't match up - if we're setting variable constraints I'd expect that in the freq == 0 case we should be setting ->sysclk_constraints to NULL rather than the contents. Indeed, we'll segfault here if the frequency is set to 0 without having first been set to some actual value. It's also bad to modify static data potentially shared between multiple instances of the device in a system.
Having said all that though I'm not clear why we're doing this constraint stuff at all, we never reference sysclk_constraints during startup and teardown and you'd usually do this because you want to set constraints that depend on the sysclk but this is just a constant set of constraints that should be set in the DAI desription.
- if (coeff >= 0) {
regmap_write(es8326->regmap, ES8326_CLK_DIV1_04,coeff_div[coeff].reg4);regmap_write(es8326->regmap, ES8326_CLK_DIV2_05,coeff_div[coeff].reg5);regmap_write(es8326->regmap, ES8326_CLK_DLL_06,coeff_div[coeff].reg6);regmap_write(es8326->regmap, ES8326_CLK_MUX_07,coeff_div[coeff].reg7);regmap_write(es8326->regmap, ES8326_CLK_ADC_SEL_08,coeff_div[coeff].reg8);regmap_write(es8326->regmap, ES8326_CLK_DAC_SEL_09,coeff_div[coeff].reg9);regmap_write(es8326->regmap, ES8326_CLK_ADC_OSR_0A,coeff_div[coeff].rega);regmap_write(es8326->regmap, ES8326_CLK_DAC_OSR_0B,coeff_div[coeff].regb);- }
This will just leave the divider setup at whatever they were last set at if we don't get a value which given the names of the registers I suspect won't go too well, it'd be better to print a warning just in case.
- regmap_write(es8326->regmap, ES8326_INT_SOURCE_58, 0x08);
- regmap_write(es8326->regmap, ES8326_INTOUT_IO_59, 0x45);
This really does look like board specific configuration which should come from DT.