Thanks for the review Mark,
+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.
Interesting. The properties are the same in I2C and SoundWire mode, so would we need a completely different file that just specifies the SoundWire DeviceID, e.g.
properties: compatible: const: sdw10217201000
What's the process for such dual-mode devices?
- /* 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.
The same sequence is already used in the I2C probe. if this needs to change, it's got to be applied for both cases.
- /* 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.
It's also copied as is from the existing I2C parts.
We should probably cut the common parts out, as done for rt5682. Ryan, can you look into this.
+/* SPDX-License-Identifier: GPL-2.0-only
AIUI this needs the trailing */ on the same line for the license compliance people.
Indeed, that's a miss.