Skip to contentSkip to conversation

ASoC: codecs: Add PCM186x binding documentation

PATCH Superseded Andrew F. Davis 2 participants 6 messages Nov 29, 2017 – Dec 6, 2017

Subsystems (4)
Device tree bindings Sound ASoC Texas Instruments audio (ASoC/HDA) drivers

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Andrew F. Davis

author
#0 lore raw

PATCH v2 1/2 ASoC: codecs: Add PCM186x binding documentation

Add the dt-binding documentation for the TI PCM186x 2ch and 4ch Audio ADCs With Universal Front End.

Signed-off-by Andrew F. Davis <[email protected]>
Acked-by Rob Herring <[email protected]>

---

Changes from v1:
- pcm186x@4a -> audio-codec@4a
- Added Acked-by

 .../devicetree/bindings/sound/pcm186x.txt          | 42 ++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/pcm186x.txt
Documentation/devicetree/bindings/sound/pcm186x.txt new file +42 -0
new file mode 100644
index 000000000000..1087f4855980
@@ -0,0 +1,42 @@
+Texas Instruments PCM186x Universal Audio ADC
+
+These devices support both I2C and SPI (configured with pin strapping
+on the board).
+
+Required properties:
+
+ - compatible : "ti,pcm1862",
+                "ti,pcm1863",
+                "ti,pcm1864",
+                "ti,pcm1865"
+
+ - reg : The I2C address of the device for I2C, the chip select
+         number for SPI.
+
+ - avdd-supply: Analog core power supply (3.3v)
+ - dvdd-supply: Digital core power supply
+ - iovdd-supply: Digital IO power supply
+        See regulator/regulator.txt for more information
+
+CODEC input pins:
+ * VINL1
+ * VINR1
+ * VINL2
+ * VINR2
+ * VINL3
+ * VINR3
+ * VINL4
+ * VINR4
+
+The pins can be used in referring sound node's audio-routing property.
+
+Example:
+
+	pcm186x: audio-codec@4a {
+		compatible = "ti,pcm1865";
+		reg = <0x4a>;
+
+		avdd-supply = <&reg_3v3_analog>;
+		dvdd-supply = <&reg_3v3>;
+		iovdd-supply = <&reg_1v8>;
+	};
Signature · 1 line
2.15.0
author
#1 lore raw

PATCH v2 2/2 ASoC: codecs: Add initial PCM1862/63/64/65 universal ADC driver

From: Andreas Dannenberg <[email protected]>

This is an initial version of the PCM186x codec driver supporting both 2-channel and 4-channel device variants. Not all device features are supported yet such as master/slave mode PLL configuration for which the codec driver currently relies on the PCM186x built-in clock auto-detection feature or the connection of digital microphones. However here is what's here and what should work:

- Support for SPI and I2C low-level interfaces
- Regmap support and basic register definitions
- Input Mixer and Mux selection
- I2C, LJ, and TDM DAI format support

Signed-off-by Andreas Dannenberg <[email protected]>
Signed-off-by Michael Stecklein <[email protected]>
Signed-off-by Andrew F. Davis <[email protected]>

---

Changes from v1:
- Remove extra debug statements
- Fix volume control names
- Power up supplies before device reset

 sound/soc/codecs/Kconfig       |  17 +
 sound/soc/codecs/Makefile      |   6 +
 sound/soc/codecs/pcm186x-i2c.c |  69 ++++
 sound/soc/codecs/pcm186x-spi.c |  69 ++++
 sound/soc/codecs/pcm186x.c     | 773 +++++++++++++++++++++++++++++++++++++++++
 sound/soc/codecs/pcm186x.h     | 221 ++++++++++++
 6 files changed, 1155 insertions(+)
 create mode 100644 sound/soc/codecs/pcm186x-i2c.c
 create mode 100644 sound/soc/codecs/pcm186x-spi.c
 create mode 100644 sound/soc/codecs/pcm186x.c
 create mode 100644 sound/soc/codecs/pcm186x.h
sound/soc/codecs/Kconfig +17 -0
index a42ddbc93f3d..dda8c01170b3 100644
@@ -109,6 +109,8 @@ config SND_SOC_ALL_CODECS
 	select SND_SOC_PCM1681 if I2C
 	select SND_SOC_PCM179X_I2C if I2C
 	select SND_SOC_PCM179X_SPI if SPI_MASTER
+	select SND_SOC_PCM186X_I2C if I2C
+	select SND_SOC_PCM186X_SPI if SPI_MASTER
 	select SND_SOC_PCM3008
 	select SND_SOC_PCM3168A_I2C if I2C
 	select SND_SOC_PCM3168A_SPI if SPI_MASTER

@@ -661,6 +663,21 @@ config SND_SOC_PCM179X_SPI
 	  Enable support for Texas Instruments PCM179x CODEC.
 	  Select this if your PCM179x is connected via an SPI bus.
 
+config SND_SOC_PCM186X
+	tristate
+
+config SND_SOC_PCM186X_I2C
+	tristate "Texas Instruments PCM186x CODECs - I2C"
+	depends on I2C
+	select SND_SOC_PCM186X
+	select REGMAP_I2C
+
+config SND_SOC_PCM186X_SPI
+	tristate "Texas Instruments PCM186x CODECs - SPI"
+	depends on SPI_MASTER
+	select SND_SOC_PCM186X
+	select REGMAP_SPI
+
 config SND_SOC_PCM3008
        tristate
sound/soc/codecs/Makefile +6 -0
index 0001069ce2a7..146e48a60098 100644
@@ -105,6 +105,9 @@ snd-soc-pcm1681-objs := pcm1681.o
 snd-soc-pcm179x-codec-objs := pcm179x.o
 snd-soc-pcm179x-i2c-objs := pcm179x-i2c.o
 snd-soc-pcm179x-spi-objs := pcm179x-spi.o
+snd-soc-pcm186x-objs := pcm186x.o
+snd-soc-pcm186x-i2c-objs := pcm186x-i2c.o
+snd-soc-pcm186x-spi-objs := pcm186x-spi.o
 snd-soc-pcm3008-objs := pcm3008.o
 snd-soc-pcm3168a-objs := pcm3168a.o
 snd-soc-pcm3168a-i2c-objs := pcm3168a-i2c.o

@@ -345,6 +348,9 @@ obj-$(CONFIG_SND_SOC_PCM1681)	+= snd-soc-pcm1681.o
 obj-$(CONFIG_SND_SOC_PCM179X)	+= snd-soc-pcm179x-codec.o
 obj-$(CONFIG_SND_SOC_PCM179X_I2C)	+= snd-soc-pcm179x-i2c.o
 obj-$(CONFIG_SND_SOC_PCM179X_SPI)	+= snd-soc-pcm179x-spi.o
+obj-$(CONFIG_SND_SOC_PCM186X)	+= snd-soc-pcm186x.o
+obj-$(CONFIG_SND_SOC_PCM186X_I2C)	+= snd-soc-pcm186x-i2c.o
+obj-$(CONFIG_SND_SOC_PCM186X_SPI)	+= snd-soc-pcm186x-spi.o
 obj-$(CONFIG_SND_SOC_PCM3008)	+= snd-soc-pcm3008.o
 obj-$(CONFIG_SND_SOC_PCM3168A)	+= snd-soc-pcm3168a.o
 obj-$(CONFIG_SND_SOC_PCM3168A_I2C)	+= snd-soc-pcm3168a-i2c.o
sound/soc/codecs/pcm186x-i2c.c new file +69 -0

69 lines not on this page · raw message

sound/soc/codecs/pcm186x-spi.c new file +69 -0

69 lines not on this page · raw message

sound/soc/codecs/pcm186x.c new file +773 -0

773 lines not on this page · raw message

sound/soc/codecs/pcm186x.h new file +221 -0

221 lines not on this page · raw message

Signature · 1 line
2.15.0
maintainer
#2 lore raw

Re: [PATCH v2 2/2] ASoC: codecs: Add initial PCM1862/63/64/65 universal ADC driver

sound/soc/codecs/pcm186x.c line 615

Quoted 13 lines · Andrew F. Davis
sound/soc/codecs/pcm186x.c new file +12 -0
+	case SND_SOC_BIAS_STANDBY:
+		pcm186x_power_on(codec);
+		break;
+	case SND_SOC_BIAS_OFF:
+		pcm186x_power_off(codec);
+		break;
+	}
 
+/*
+ * The PCM186x's page register is located on every page, allowing to program it
+ * without having to switch pages. Take advantage of this by defining the range
+ * such to have this register located inside the data window.
+ */

That sounds like a normal page register?

Quoted 17 lines
sound/soc/codecs/pcm186x.c new file +15 -0
+int pcm186x_probe(struct device *dev, enum pcm186x_type type, int irq,
+		  struct regmap *regmap)
+{
 
+	ret = regulator_bulk_disable(ARRAY_SIZE(priv->supplies),
+				     priv->supplies);
+	if (ret) {
+		dev_err(dev, "failed disable supplies: %d\n", ret);
+		return ret;
+	}
 
+static int __maybe_unused pcm186x_resume(struct device *dev)
+{
+	struct pcm186x_priv *priv = dev_get_drvdata(dev);
+	int ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(priv->supplies),

sound/soc/codecs/pcm186x.c line 768

Quoted 10 lines
sound/soc/codecs/pcm186x.c new file +9 -0
+				    priv->supplies);
+	if (ret != 0) {
+		dev_err(dev, "failed to enable supplies: %d\n", ret);
+		return ret;
+	}
 
+const struct dev_pm_ops pcm186x_pm_ops = {
+	SET_RUNTIME_PM_OPS(pcm186x_suspend, pcm186x_resume, NULL)
+};
+EXPORT_SYMBOL_GPL(pcm186x_pm_ops);

There's no code in the driver that enables runtime PM so this isn't going to do anything. I'm also not clear that the power management handling is in general joined up - we leave the regulators disabled at the end of probe, relying on the bias level configuration to reenable them but then the runtime PM configuration also tries to enable and disable them. Based on what I think the intention is I'd suggest removing the bias level handling and then having probe enable runtime PM with the device flagged as active, letting runtime PM do any disabling if the device is idle.

I'd also expect to see some system suspend handling.

author
#3 lore raw

Re: [PATCH v2 2/2] ASoC: codecs: Add initial PCM1862/63/64/65 universal ADC driver

Quoted 10 lines · Mark Brown
Quoted 7 lines · Andrew F. Davis
sound/soc/codecs/pcm186x.c new file +7 -0
+	case SND_SOC_BIAS_STANDBY:
+		pcm186x_power_on(codec);
+		break;
+	case SND_SOC_BIAS_OFF:
+		pcm186x_power_off(codec);
+		break;
+	}
Quoted 8 lines
Quoted 5 lines

+/*
+ * The PCM186x's page register is located on every page, allowing to program it
+ * without having to switch pages. Take advantage of this by defining the range
+ * such to have this register located inside the data window.
+ */

That sounds like a normal page register?

It is, I believe Andreas commented this for his own reference, I'll drop it.

Quoted 23 lines
Quoted 22 lines
sound/soc/codecs/pcm186x.c new file +20 -0
+int pcm186x_probe(struct device *dev, enum pcm186x_type type, int irq,
+		  struct regmap *regmap)
+{
 
+	ret = regulator_bulk_disable(ARRAY_SIZE(priv->supplies),
+				     priv->supplies);
+	if (ret) {
+		dev_err(dev, "failed disable supplies: %d\n", ret);
+		return ret;
+	}
 
+static int __maybe_unused pcm186x_resume(struct device *dev)
+{
+	struct pcm186x_priv *priv = dev_get_drvdata(dev);
+	int ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(priv->supplies),
+				    priv->supplies);
+	if (ret != 0) {
+		dev_err(dev, "failed to enable supplies: %d\n", ret);
+		return ret;
+	}
Quoted 15 lines

sound/soc/codecs/pcm186x.c line 768

Quoted 4 lines
sound/soc/codecs/pcm186x.c new file +4 -0
+const struct dev_pm_ops pcm186x_pm_ops = {
+	SET_RUNTIME_PM_OPS(pcm186x_suspend, pcm186x_resume, NULL)
+};
+EXPORT_SYMBOL_GPL(pcm186x_pm_ops);

There's no code in the driver that enables runtime PM so this isn't going to do anything. I'm also not clear that the power management handling is in general joined up - we leave the regulators disabled at the end of probe, relying on the bias level configuration to reenable them but then the runtime PM configuration also tries to enable and disable them. Based on what I think the intention is I'd suggest removing the bias level handling and then having probe enable runtime PM with the device flagged as active, letting runtime PM do any disabling if the device is idle.

I beleive this was meant to be be SIMPLE_DEV_PM_OPS and not SET_RUNTIME_PM_OPS. I'll fix this all up for v3.

Just thinking, the sound core sets SND_SOC_BIAS_OFF before suspend anyway, right? So the results would be similar just having all the PM stuff in the bias level handling for consistency, but I'm open to whatever is the preferred way.

Quoted 2 lines

I'd also expect to see some system suspend handling.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

maintainer
#4 lore raw

Re: [PATCH v2 2/2] ASoC: codecs: Add initial PCM1862/63/64/65 universal ADC driver

Quoted 9 lines · Andrew F. Davis
Quoted 4 lines · Mark Brown

disable them. Based on what I think the intention is I'd suggest removing the bias level handling and then having probe enable runtime PM with the device flagged as active, letting runtime PM do any disabling if the device is idle.

I beleive this was meant to be be SIMPLE_DEV_PM_OPS and not SET_RUNTIME_PM_OPS. I'll fix this all up for v3.

I was wondering that.

Quoted 4 lines

Just thinking, the sound core sets SND_SOC_BIAS_OFF before suspend anyway, right? So the results would be similar just having all the PM stuff in the bias level handling for consistency, but I'm open to whatever is the preferred way.

It doesn't matter that much, if you do it only in set_bias_level() then unless you set idle_bias_off there will be no runtime PM which may or may not be what you want and you'll also not give the user the ability to control if runtime PM happens via the sysfs files but I'm not convinced that anyone ever actually does that. Either approach is fine really.

maintainer
#5 lore raw

Applied "ASoC: pcm186x: Add PCM186x binding documentation" to the asoc tree

In reply to Andrew F. Davis

The patch

ASoC: pcm186x: Add PCM186x binding documentation

has been applied to the asoc tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

Quoted 1 line

From eb2a8168b9fd69f66199d9d7e86d23fecfab4e33 Mon Sep 17 00:00:00 2001

From: "Andrew F. Davis" <[email protected]>
Date: Tue, 5 Dec 2017 14:52:55 -0600
Subject: [PATCH] ASoC: pcm186x: Add PCM186x binding documentation

Add the dt-binding documentation for the TI PCM186x 2ch and 4ch Audio ADCs With Universal Front End.

Signed-off-by Andrew F. Davis <[email protected]>
Acked-by Rob Herring <[email protected]>
Signed-off-by Mark Brown <[email protected]>
---
 .../devicetree/bindings/sound/pcm186x.txt          | 42 ++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/pcm186x.txt
Documentation/devicetree/bindings/sound/pcm186x.txt new file +42 -0
new file mode 100644
index 000000000000..1087f4855980
@@ -0,0 +1,42 @@
+Texas Instruments PCM186x Universal Audio ADC
+
+These devices support both I2C and SPI (configured with pin strapping
+on the board).
+
+Required properties:
+
+ - compatible : "ti,pcm1862",
+                "ti,pcm1863",
+                "ti,pcm1864",
+                "ti,pcm1865"
+
+ - reg : The I2C address of the device for I2C, the chip select
+         number for SPI.
+
+ - avdd-supply: Analog core power supply (3.3v)
+ - dvdd-supply: Digital core power supply
+ - iovdd-supply: Digital IO power supply
+        See regulator/regulator.txt for more information
+
+CODEC input pins:
+ * VINL1
+ * VINR1
+ * VINL2
+ * VINR2
+ * VINL3
+ * VINR3
+ * VINL4
+ * VINR4
+
+The pins can be used in referring sound node's audio-routing property.
+
+Example:
+
+	pcm186x: audio-codec@4a {
+		compatible = "ti,pcm1865";
+		reg = <0x4a>;
+
+		avdd-supply = <&reg_3v3_analog>;
+		dvdd-supply = <&reg_3v3>;
+		iovdd-supply = <&reg_1v8>;
+	};
Signature · 1 line
2.15.0