Re: [PATCH 2/2] ASoC: ssm3515: Add new amp driver

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1369 bytes --]

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.

> +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...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]