Re: [PATCH] ASoC: codecs: Added MAX98373 Soundwire Driver

[Mark Brown <broonie@kernel.org>]

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

On Wed, Jun 17, 2020 at 11:30:15AM -0500, Pierre-Louis Bossart wrote:

> +static void max98373_read_config(struct sdw_slave *slave)
> +{
> +	int value;
> +	struct device *dev = &slave->dev;
> +	struct max98373_priv *max98373 = dev_get_drvdata(dev);
> +
> +	if (!device_property_read_u32(dev, "maxim,vmon-slot-no", &value))
> +		max98373->v_slot = value & 0xF;
> +	else
> +		max98373->v_slot = 0;

The DT bindings need updating to add SoundWire support.

> +	/* L/R mix configuration */
> +	regmap_write(max98373->regmap,
> +		     MAX98373_R2029_PCM_TO_SPK_MONO_MIX_1,
> +		     0x80);
> +	regmap_write(max98373->regmap,
> +		     MAX98373_R202A_PCM_TO_SPK_MONO_MIX_2,
> +		     0x1);
> +	/* Set initial volume (0dB) */
> +	regmap_write(max98373->regmap,
> +		     MAX98373_R203D_AMP_DIG_VOL_CTRL,
> +		     0x00);
> +	regmap_write(max98373->regmap,
> +		     MAX98373_R203E_AMP_PATH_GAIN,
> +		     0x00);

I'd expect these to be chip defaults, especially the volumes.

> +	/* Speaker enable */
> +	regmap_update_bits(max98373->regmap,
> +			   MAX98373_R2043_AMP_EN,
> +			   MAX98373_SPK_EN_MASK, 1);

I'd expect this to be managed via DAPM.

> +/* SPDX-License-Identifier: GPL-2.0-only

AIUI this needs the trailing */ on the same line for the license
compliance people.

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