> On 10. 5. 2023, at 4:16, Mark Brown <broonie@xxxxxxxxxx> wrote: > > On Tue, May 09, 2023 at 06:38:28PM +0200, Martin Povišer wrote: > >> +static int ssm3515_setup(struct snd_soc_component *component) >> +{ >> + struct ssm3515_data *data = >> + snd_soc_component_get_drvdata(component); >> + int ret; >> + >> + ret = snd_soc_component_update_bits(component, SSM3515_GEC, >> + SSM3515_GEC_ANA_GAIN, >> + FIELD_PREP(SSM3515_GEC_ANA_GAIN, data->ana_gain)); >> + if (ret < 0) >> + return ret; >> + >> + /* Start out muted */ >> + ret = snd_soc_component_update_bits(component, SSM3515_DAC, >> + SSM3515_DAC_MUTE, SSM3515_DAC_MUTE); >> + if (ret < 0) >> + return ret; > > Why are we not using the chip defaults here? We use those for most > things as what's appropraite for one user might not be appropriate for > another and it's easier to agree to follow what the chip does than to > select things. There's some exceptions like for zero cross options but > not typically for gains and mutes. This bit is controlled by the mute DAI op, where it is expected the component starts out muted. The datasheet promises pop-free experience if this bit is sequenced with the disablement of clocks, so it seems like a good fit for said op. >> +static int ssm3515_probe(struct snd_soc_component *component) >> +{ >> + struct ssm3515_data *data = >> + snd_soc_component_get_drvdata(component); >> + int ret; >> + >> + ret = ssm3515_reset(component); >> + if (ret < 0) >> + return ret; >> + regmap_reinit_cache(data->regmap, &ssm3515_i2c_regmap); >> + >> + return ssm3515_setup(component); >> +} > > We don't normally reset things on component probe, only on bus level > probe... I don’t think I have a strong reason to do this.