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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux