[PATCH v2 0/2] ASoC: Add support for DAC PCM1789

[PATCH v2 0/2] ASoC: Add support for DAC PCM1789

[Mylène Josserand]

Hello everyone,

The current series is the second version to add the support of Texas
Instrument's DAC PCM1789. This DAC is very minimalist and does
not have many registers.

It is important to notice that this DAC needs to always have clocks
enabled (even without any data) otherwise it will be in a "desynchronized"
state and can not send data correctly.
This issue has been solved by performing a reset each time a sound
is played. This reset can produce a "pop" noise.

Depending on your DAI, you will need to provide and enable the MCLK
to be able to communicate with this codec throught i2c.

Changes since v1:
	- Create a new file to support pcm1789 instead of converting the
	existing pcm179x driver. All the patches are merged into one patch.
	- Update the code to use gpiod for the reset.
	- Add some fixes according to Thomas Petazzoni's reviews
	- Create a new patch to add device-tree bindings for this new DAC.

Thank you in advance for any review.

Best regards,
Mylène

Mylène Josserand (2):
  ASoC: codecs: Add support for PCM1789
  ASoC: Add bindings for PCM1789

 .../devicetree/bindings/sound/pcm1789.txt          |  21 ++
 sound/soc/codecs/Kconfig                           |  12 +
 sound/soc/codecs/Makefile                          |   4 +
 sound/soc/codecs/pcm1789-i2c.c                     |  76 ++++++
 sound/soc/codecs/pcm1789.c                         | 288 +++++++++++++++++++++
 sound/soc/codecs/pcm1789.h                         |  28 ++
 6 files changed, 429 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/pcm1789.txt
 create mode 100644 sound/soc/codecs/pcm1789-i2c.c
 create mode 100644 sound/soc/codecs/pcm1789.c
 create mode 100644 sound/soc/codecs/pcm1789.h

-- 
2.11.0

[PATCH v2 0/2] ASoC: Add support for DAC PCM1789

[Mylène Josserand]

Hello everyone,

The current series is the second version to add the support of Texas
Instrument's DAC PCM1789. This DAC is very minimalist and does
not have many registers.

It is important to notice that this DAC needs to always have clocks
enabled (even without any data) otherwise it will be in a "desynchronized"
state and can not send data correctly.
This issue has been solved by performing a reset each time a sound
is played. This reset can produce a "pop" noise.

Depending on your DAI, you will need to provide and enable the MCLK
to be able to communicate with this codec throught i2c.

Changes since v1:
	- Create a new file to support pcm1789 instead of converting the
	existing pcm179x driver. All the patches are merged into one patch.
	- Update the code to use gpiod for the reset.
	- Add some fixes according to Thomas Petazzoni's reviews
	- Create a new patch to add device-tree bindings for this new DAC.

Thank you in advance for any review.

Best regards,
Mylène

Mylène Josserand (2):
  ASoC: codecs: Add support for PCM1789
  ASoC: Add bindings for PCM1789

 .../devicetree/bindings/sound/pcm1789.txt          |  21 ++
 sound/soc/codecs/Kconfig                           |  12 +
 sound/soc/codecs/Makefile                          |   4 +
 sound/soc/codecs/pcm1789-i2c.c                     |  76 ++++++
 sound/soc/codecs/pcm1789.c                         | 288 +++++++++++++++++++++
 sound/soc/codecs/pcm1789.h                         |  28 ++
 6 files changed, 429 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/pcm1789.txt
 create mode 100644 sound/soc/codecs/pcm1789-i2c.c
 create mode 100644 sound/soc/codecs/pcm1789.c
 create mode 100644 sound/soc/codecs/pcm1789.h

-- 
2.11.0

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH v2 1/2] ASoC: codecs: Add support for PCM1789

[Mylène Josserand]

Add Texas Instruments's PCM1789 DAC support.
It is a simple DAC and does not have many registers.

One particularity about this DAC is that the clocks must be
always enabled. Also, an entire software reset is necessary
while starting to play a sound otherwise, the clocks are not
synchronized (so the DAC is not able to send data).

Signed-off-by: Mylène Josserand <mylene.josserand@bootlin.com>
---
 sound/soc/codecs/Kconfig       |  12 ++
 sound/soc/codecs/Makefile      |   4 +
 sound/soc/codecs/pcm1789-i2c.c |  76 +++++++++++
 sound/soc/codecs/pcm1789.c     | 288 +++++++++++++++++++++++++++++++++++++++++
 sound/soc/codecs/pcm1789.h     |  28 ++++
 5 files changed, 408 insertions(+)
 create mode 100644 sound/soc/codecs/pcm1789-i2c.c
 create mode 100644 sound/soc/codecs/pcm1789.c
 create mode 100644 sound/soc/codecs/pcm1789.h

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 822df8d3d4f9..0314a9faae36 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -112,6 +112,7 @@ config SND_SOC_ALL_CODECS
 	select SND_SOC_NAU8825 if I2C
 	select SND_SOC_HDMI_CODEC
 	select SND_SOC_PCM1681 if I2C
+	select SND_SOC_PCM1789_I2C if I2C
 	select SND_SOC_PCM179X_I2C if I2C
 	select SND_SOC_PCM179X_SPI if SPI_MASTER
 	select SND_SOC_PCM186X_I2C if I2C
@@ -676,6 +677,17 @@ config SND_SOC_PCM1681
 	tristate "Texas Instruments PCM1681 CODEC"
 	depends on I2C
 
+config SND_SOC_PCM1789
+	tristate
+
+config SND_SOC_PCM1789_I2C
+	tristate "Texas Instruments PCM1789 CODEC (I2C)"
+	depends on I2C
+	select SND_SOC_PCM1789
+	help
+	  Enable support for Texas Instruments PCM1789 CODEC.
+	  Select this if your PCM1789 is connected via an I2C bus.
+
 config SND_SOC_PCM179X
 	tristate
 
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index bc9425b4ba0c..b1654909582f 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -108,6 +108,8 @@ snd-soc-nau8824-objs := nau8824.o
 snd-soc-nau8825-objs := nau8825.o
 snd-soc-hdmi-codec-objs := hdmi-codec.o
 snd-soc-pcm1681-objs := pcm1681.o
+snd-soc-pcm1789-codec-objs := pcm1789.o
+snd-soc-pcm1789-i2c-objs := pcm1789-i2c.o
 snd-soc-pcm179x-codec-objs := pcm179x.o
 snd-soc-pcm179x-i2c-objs := pcm179x-i2c.o
 snd-soc-pcm179x-spi-objs := pcm179x-spi.o
@@ -359,6 +361,8 @@ obj-$(CONFIG_SND_SOC_NAU8825)   += snd-soc-nau8825.o
 obj-$(CONFIG_SND_SOC_HDMI_CODEC)	+= snd-soc-hdmi-codec.o
 obj-$(CONFIG_SND_SOC_PCM1681)	+= snd-soc-pcm1681.o
 obj-$(CONFIG_SND_SOC_PCM179X)	+= snd-soc-pcm179x-codec.o
+obj-$(CONFIG_SND_SOC_PCM1789_I2C)	+= snd-soc-pcm1789-i2c.o
+obj-$(CONFIG_SND_SOC_PCM1789)	+= snd-soc-pcm1789-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
diff --git a/sound/soc/codecs/pcm1789-i2c.c b/sound/soc/codecs/pcm1789-i2c.c
new file mode 100644
index 000000000000..9a34790cd0de
--- /dev/null
+++ b/sound/soc/codecs/pcm1789-i2c.c
@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PCM1789 ASoC I2C driver
+ *
+ * Copyright (c) Bootlin 2018
+ *
+ * Mylène Josserand <mylene.josserand@bootlin.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+
+#include "pcm1789.h"
+
+static int pcm1789_i2c_probe(struct i2c_client *client,
+			     const struct i2c_device_id *id)
+{
+	struct regmap *regmap;
+	int ret;
+
+	regmap = devm_regmap_init_i2c(client, &pcm1789_regmap_config);
+	if (IS_ERR(regmap)) {
+		ret = PTR_ERR(regmap);
+		dev_err(&client->dev, "Failed to allocate regmap: %d\n", ret);
+		return ret;
+	}
+
+	return pcm1789_common_init(&client->dev, regmap);
+}
+
+static int pcm1789_i2c_remove(struct i2c_client *client)
+{
+	return pcm1789_common_exit(&client->dev);
+}
+
+static const struct of_device_id pcm1789_of_match[] = {
+	{ .compatible = "ti,pcm1789", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pcm1789_of_match);
+
+static const struct i2c_device_id pcm1789_i2c_ids[] = {
+	{ "pcm1789", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, pcm1789_i2c_ids);
+
+static struct i2c_driver pcm1789_i2c_driver = {
+	.driver = {
+		.name	= "pcm1789",
+		.of_match_table = of_match_ptr(pcm1789_of_match),
+	},
+	.id_table	= pcm1789_i2c_ids,
+	.probe		= pcm1789_i2c_probe,
+	.remove	= pcm1789_i2c_remove,
+};
+
+module_i2c_driver(pcm1789_i2c_driver);
+
+MODULE_DESCRIPTION("ASoC PCM1789 I2C driver");
+MODULE_AUTHOR("Mylène Josserand <mylene.josserand@bootlin.com>");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/codecs/pcm1789.c b/sound/soc/codecs/pcm1789.c
new file mode 100644
index 000000000000..93fad175c4ef
--- /dev/null
+++ b/sound/soc/codecs/pcm1789.c
@@ -0,0 +1,288 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PCM1789 ASoC codec driver
+ *
+ * Copyright (c) Bootlin 2018
+ *
+ * Mylène Josserand <mylene.josserand@bootlin.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/gpio.h>
+#include <linux/module.h>
+#include <linux/workqueue.h>
+
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/tlv.h>
+
+#include "pcm1789.h"
+
+#define PCM1789_MUTE_CONTROL	0x10
+#define PCM1789_FMT_CONTROL	0x11
+#define PCM1789_SOFT_MUTE	0x14
+#define PCM1789_DAC_VOL_LEFT	0x18
+#define PCM1789_DAC_VOL_RIGHT	0x19
+
+#define PCM1789_FMT_MASK	0x07
+#define PCM1789_MUTE_MASK	0x03
+#define PCM1789_MUTE_SRET	0x06
+
+struct pcm1789_private {
+	struct regmap *regmap;
+	unsigned int format;
+	unsigned int rate;
+	struct gpio_desc *reset;
+	struct work_struct work;
+	struct device *dev;
+};
+
+static const struct reg_default pcm1789_reg_defaults[] = {
+	{ PCM1789_FMT_CONTROL, 0x00 },
+	{ PCM1789_SOFT_MUTE, 0x00 },
+	{ PCM1789_DAC_VOL_LEFT, 0xff },
+	{ PCM1789_DAC_VOL_RIGHT, 0xff },
+};
+
+static bool pcm1789_accessible_reg(struct device *dev, unsigned int reg)
+{
+	return reg >= PCM1789_MUTE_CONTROL && reg <= PCM1789_DAC_VOL_RIGHT;
+}
+
+static bool pcm1789_writeable_reg(struct device *dev, unsigned int reg)
+{
+	return pcm1789_accessible_reg(dev, reg);
+}
+
+static int pcm1789_set_dai_fmt(struct snd_soc_dai *codec_dai,
+			       unsigned int format)
+{
+	struct snd_soc_component *component = codec_dai->component;
+	struct pcm1789_private *priv = snd_soc_component_get_drvdata(component);
+
+	priv->format = format;
+
+	return 0;
+}
+
+static int pcm1789_digital_mute(struct snd_soc_dai *codec_dai, int mute)
+{
+	struct snd_soc_component *component = codec_dai->component;
+	struct pcm1789_private *priv = snd_soc_component_get_drvdata(component);
+
+	return regmap_update_bits(priv->regmap, PCM1789_SOFT_MUTE,
+				  PCM1789_MUTE_MASK,
+				  mute ? 0 : PCM1789_MUTE_MASK);
+}
+
+static int pcm1789_hw_params(struct snd_pcm_substream *substream,
+			     struct snd_pcm_hw_params *params,
+			     struct snd_soc_dai *codec_dai)
+{
+	struct snd_soc_component *component = codec_dai->component;
+	struct pcm1789_private *priv = snd_soc_component_get_drvdata(component);
+	int val = 0, ret;
+
+	priv->rate = params_rate(params);
+
+	switch (priv->format & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_RIGHT_J:
+		switch (params_width(params)) {
+		case 24:
+			val = 2;
+			break;
+		case 16:
+			val = 3;
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	case SND_SOC_DAIFMT_I2S:
+		switch (params_width(params)) {
+		case 16:
+		case 24:
+		case 32:
+			val = 0;
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	case SND_SOC_DAIFMT_LEFT_J:
+		switch (params_width(params)) {
+		case 16:
+		case 24:
+		case 32:
+			val = 1;
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	default:
+		dev_err(component->dev, "Invalid DAI format\n");
+		return -EINVAL;
+	}
+
+	ret = regmap_update_bits(priv->regmap, PCM1789_FMT_CONTROL,
+				 PCM1789_FMT_MASK, val);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static void pcm1789_work_queue(struct work_struct *work)
+{
+	struct pcm1789_private *priv = container_of(work,
+						    struct pcm1789_private,
+						    work);
+
+	/* Perform a software reset to remove codec from desynchronized state */
+	if (regmap_update_bits(priv->regmap, PCM1789_MUTE_CONTROL,
+			       0x3 << PCM1789_MUTE_SRET, 0) < 0)
+		dev_err(priv->dev, "Error while setting SRET");
+}
+
+static int pcm1789_trigger(struct snd_pcm_substream *substream, int cmd,
+			   struct snd_soc_dai *dai)
+{
+	struct snd_soc_component *component = dai->component;
+	struct pcm1789_private *priv = snd_soc_component_get_drvdata(component);
+	int ret = 0;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		schedule_work(&priv->work);
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+static const struct snd_soc_dai_ops pcm1789_dai_ops = {
+	.set_fmt	= pcm1789_set_dai_fmt,
+	.hw_params	= pcm1789_hw_params,
+	.digital_mute	= pcm1789_digital_mute,
+	.trigger	= pcm1789_trigger,
+};
+
+static const DECLARE_TLV_DB_SCALE(pcm1789_dac_tlv, -12000, 50, 1);
+
+static const struct snd_kcontrol_new pcm1789_controls[] = {
+	SOC_DOUBLE_R_RANGE_TLV("DAC Playback Volume", PCM1789_DAC_VOL_LEFT,
+			       PCM1789_DAC_VOL_RIGHT, 0, 0xf, 0xff, 0,
+			       pcm1789_dac_tlv),
+};
+
+static const struct snd_soc_dapm_widget pcm1789_dapm_widgets[] = {
+	SND_SOC_DAPM_OUTPUT("IOUTL+"),
+	SND_SOC_DAPM_OUTPUT("IOUTL-"),
+	SND_SOC_DAPM_OUTPUT("IOUTR+"),
+	SND_SOC_DAPM_OUTPUT("IOUTR-"),
+};
+
+static const struct snd_soc_dapm_route pcm1789_dapm_routes[] = {
+	{ "IOUTL+", NULL, "Playback" },
+	{ "IOUTL-", NULL, "Playback" },
+	{ "IOUTR+", NULL, "Playback" },
+	{ "IOUTR-", NULL, "Playback" },
+};
+
+static struct snd_soc_dai_driver pcm1789_dai = {
+	.name = "pcm1789-hifi",
+	.playback = {
+		.stream_name = "Playback",
+		.channels_min = 2,
+		.channels_max = 2,
+		.rates = SNDRV_PCM_RATE_CONTINUOUS,
+		.rate_min = 10000,
+		.rate_max = 200000,
+		.formats = PCM1789_FORMATS,
+	},
+	.ops = &pcm1789_dai_ops,
+};
+
+const struct regmap_config pcm1789_regmap_config = {
+	.reg_bits		= 8,
+	.val_bits		= 8,
+	.max_register		= PCM1789_DAC_VOL_RIGHT,
+	.reg_defaults		= pcm1789_reg_defaults,
+	.num_reg_defaults	= ARRAY_SIZE(pcm1789_reg_defaults),
+	.writeable_reg		= pcm1789_writeable_reg,
+	.readable_reg		= pcm1789_accessible_reg,
+};
+EXPORT_SYMBOL_GPL(pcm1789_regmap_config);
+
+static const struct snd_soc_component_driver soc_component_dev_pcm1789 = {
+	.controls		= pcm1789_controls,
+	.num_controls		= ARRAY_SIZE(pcm1789_controls),
+	.dapm_widgets		= pcm1789_dapm_widgets,
+	.num_dapm_widgets	= ARRAY_SIZE(pcm1789_dapm_widgets),
+	.dapm_routes		= pcm1789_dapm_routes,
+	.num_dapm_routes	= ARRAY_SIZE(pcm1789_dapm_routes),
+	.idle_bias_on		= 1,
+	.use_pmdown_time	= 1,
+	.endianness		= 1,
+	.non_legacy_dai_naming	= 1,
+};
+
+int pcm1789_common_init(struct device *dev, struct regmap *regmap)
+{
+	struct pcm1789_private *pcm1789;
+
+	pcm1789 = devm_kzalloc(dev, sizeof(struct pcm1789_private),
+			       GFP_KERNEL);
+	if (!pcm1789)
+		return -ENOMEM;
+
+	pcm1789->regmap = regmap;
+	pcm1789->dev = dev;
+	dev_set_drvdata(dev, pcm1789);
+
+	pcm1789->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(pcm1789->reset))
+		return PTR_ERR(pcm1789->reset);
+
+	gpiod_set_value_cansleep(pcm1789->reset, 0);
+	msleep(300);
+
+	INIT_WORK(&pcm1789->work, pcm1789_work_queue);
+
+	return devm_snd_soc_register_component(dev, &soc_component_dev_pcm1789,
+					       &pcm1789_dai, 1);
+}
+EXPORT_SYMBOL_GPL(pcm1789_common_init);
+
+int pcm1789_common_exit(struct device *dev)
+{
+	struct pcm1789_private *priv = dev_get_drvdata(dev);
+
+	if (&priv->work)
+		flush_work(&priv->work);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pcm1789_common_exit);
+
+MODULE_DESCRIPTION("ASoC PCM1789 driver");
+MODULE_AUTHOR("Mylène Josserand <mylene.josserand@free-electrons.com>");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/codecs/pcm1789.h b/sound/soc/codecs/pcm1789.h
new file mode 100644
index 000000000000..05c2c8ad2bad
--- /dev/null
+++ b/sound/soc/codecs/pcm1789.h
@@ -0,0 +1,28 @@
+/*
+ * definitions for PCM1789
+ *
+ * Copyright (c) Bootlin 2018
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __PCM1789_H__
+#define __PCM1789_H__
+
+#define PCM1789_FORMATS (SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S24_LE | \
+			 SNDRV_PCM_FMTBIT_S16_LE)
+
+extern const struct regmap_config pcm1789_regmap_config;
+
+int pcm1789_common_init(struct device *dev, struct regmap *regmap);
+int pcm1789_common_exit(struct device *dev);
+
+#endif
-- 
2.11.0

[PATCH v2 1/2] ASoC: codecs: Add support for PCM1789

[Mylène Josserand]

Add Texas Instruments's PCM1789 DAC support.
It is a simple DAC and does not have many registers.

One particularity about this DAC is that the clocks must be
always enabled. Also, an entire software reset is necessary
while starting to play a sound otherwise, the clocks are not
synchronized (so the DAC is not able to send data).

Signed-off-by: Mylène Josserand <mylene.josserand@bootlin.com>
---
 sound/soc/codecs/Kconfig       |  12 ++
 sound/soc/codecs/Makefile      |   4 +
 sound/soc/codecs/pcm1789-i2c.c |  76 +++++++++++
 sound/soc/codecs/pcm1789.c     | 288 +++++++++++++++++++++++++++++++++++++++++
 sound/soc/codecs/pcm1789.h     |  28 ++++
 5 files changed, 408 insertions(+)
 create mode 100644 sound/soc/codecs/pcm1789-i2c.c
 create mode 100644 sound/soc/codecs/pcm1789.c
 create mode 100644 sound/soc/codecs/pcm1789.h

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 822df8d3d4f9..0314a9faae36 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -112,6 +112,7 @@ config SND_SOC_ALL_CODECS
 	select SND_SOC_NAU8825 if I2C
 	select SND_SOC_HDMI_CODEC
 	select SND_SOC_PCM1681 if I2C
+	select SND_SOC_PCM1789_I2C if I2C
 	select SND_SOC_PCM179X_I2C if I2C
 	select SND_SOC_PCM179X_SPI if SPI_MASTER
 	select SND_SOC_PCM186X_I2C if I2C
@@ -676,6 +677,17 @@ config SND_SOC_PCM1681
 	tristate "Texas Instruments PCM1681 CODEC"
 	depends on I2C
 
+config SND_SOC_PCM1789
+	tristate
+
+config SND_SOC_PCM1789_I2C
+	tristate "Texas Instruments PCM1789 CODEC (I2C)"
+	depends on I2C
+	select SND_SOC_PCM1789
+	help
+	  Enable support for Texas Instruments PCM1789 CODEC.
+	  Select this if your PCM1789 is connected via an I2C bus.
+
 config SND_SOC_PCM179X
 	tristate
 
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index bc9425b4ba0c..b1654909582f 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -108,6 +108,8 @@ snd-soc-nau8824-objs := nau8824.o
 snd-soc-nau8825-objs := nau8825.o
 snd-soc-hdmi-codec-objs := hdmi-codec.o
 snd-soc-pcm1681-objs := pcm1681.o
+snd-soc-pcm1789-codec-objs := pcm1789.o
+snd-soc-pcm1789-i2c-objs := pcm1789-i2c.o
 snd-soc-pcm179x-codec-objs := pcm179x.o
 snd-soc-pcm179x-i2c-objs := pcm179x-i2c.o
 snd-soc-pcm179x-spi-objs := pcm179x-spi.o
@@ -359,6 +361,8 @@ obj-$(CONFIG_SND_SOC_NAU8825)   += snd-soc-nau8825.o
 obj-$(CONFIG_SND_SOC_HDMI_CODEC)	+= snd-soc-hdmi-codec.o
 obj-$(CONFIG_SND_SOC_PCM1681)	+= snd-soc-pcm1681.o
 obj-$(CONFIG_SND_SOC_PCM179X)	+= snd-soc-pcm179x-codec.o
+obj-$(CONFIG_SND_SOC_PCM1789_I2C)	+= snd-soc-pcm1789-i2c.o
+obj-$(CONFIG_SND_SOC_PCM1789)	+= snd-soc-pcm1789-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
diff --git a/sound/soc/codecs/pcm1789-i2c.c b/sound/soc/codecs/pcm1789-i2c.c
new file mode 100644
index 000000000000..9a34790cd0de
--- /dev/null
+++ b/sound/soc/codecs/pcm1789-i2c.c
@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PCM1789 ASoC I2C driver
+ *
+ * Copyright (c) Bootlin 2018
+ *
+ * Mylène Josserand <mylene.josserand@bootlin.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+
+#include "pcm1789.h"
+
+static int pcm1789_i2c_probe(struct i2c_client *client,
+			     const struct i2c_device_id *id)
+{
+	struct regmap *regmap;
+	int ret;
+
+	regmap = devm_regmap_init_i2c(client, &pcm1789_regmap_config);
+	if (IS_ERR(regmap)) {
+		ret = PTR_ERR(regmap);
+		dev_err(&client->dev, "Failed to allocate regmap: %d\n", ret);
+		return ret;
+	}
+
+	return pcm1789_common_init(&client->dev, regmap);
+}
+
+static int pcm1789_i2c_remove(struct i2c_client *client)
+{
+	return pcm1789_common_exit(&client->dev);
+}
+
+static const struct of_device_id pcm1789_of_match[] = {
+	{ .compatible = "ti,pcm1789", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pcm1789_of_match);
+
+static const struct i2c_device_id pcm1789_i2c_ids[] = {
+	{ "pcm1789", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, pcm1789_i2c_ids);
+
+static struct i2c_driver pcm1789_i2c_driver = {
+	.driver = {
+		.name	= "pcm1789",
+		.of_match_table = of_match_ptr(pcm1789_of_match),
+	},
+	.id_table	= pcm1789_i2c_ids,
+	.probe		= pcm1789_i2c_probe,
+	.remove	= pcm1789_i2c_remove,
+};
+
+module_i2c_driver(pcm1789_i2c_driver);
+
+MODULE_DESCRIPTION("ASoC PCM1789 I2C driver");
+MODULE_AUTHOR("Mylène Josserand <mylene.josserand@bootlin.com>");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/codecs/pcm1789.c b/sound/soc/codecs/pcm1789.c
new file mode 100644
index 000000000000..93fad175c4ef
--- /dev/null
+++ b/sound/soc/codecs/pcm1789.c
@@ -0,0 +1,288 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PCM1789 ASoC codec driver
+ *
+ * Copyright (c) Bootlin 2018
+ *
+ * Mylène Josserand <mylene.josserand@bootlin.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/gpio.h>
+#include <linux/module.h>
+#include <linux/workqueue.h>
+
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/tlv.h>
+
+#include "pcm1789.h"
+
+#define PCM1789_MUTE_CONTROL	0x10
+#define PCM1789_FMT_CONTROL	0x11
+#define PCM1789_SOFT_MUTE	0x14
+#define PCM1789_DAC_VOL_LEFT	0x18
+#define PCM1789_DAC_VOL_RIGHT	0x19
+
+#define PCM1789_FMT_MASK	0x07
+#define PCM1789_MUTE_MASK	0x03
+#define PCM1789_MUTE_SRET	0x06
+
+struct pcm1789_private {
+	struct regmap *regmap;
+	unsigned int format;
+	unsigned int rate;
+	struct gpio_desc *reset;
+	struct work_struct work;
+	struct device *dev;
+};
+
+static const struct reg_default pcm1789_reg_defaults[] = {
+	{ PCM1789_FMT_CONTROL, 0x00 },
+	{ PCM1789_SOFT_MUTE, 0x00 },
+	{ PCM1789_DAC_VOL_LEFT, 0xff },
+	{ PCM1789_DAC_VOL_RIGHT, 0xff },
+};
+
+static bool pcm1789_accessible_reg(struct device *dev, unsigned int reg)
+{
+	return reg >= PCM1789_MUTE_CONTROL && reg <= PCM1789_DAC_VOL_RIGHT;
+}
+
+static bool pcm1789_writeable_reg(struct device *dev, unsigned int reg)
+{
+	return pcm1789_accessible_reg(dev, reg);
+}
+
+static int pcm1789_set_dai_fmt(struct snd_soc_dai *codec_dai,
+			       unsigned int format)
+{
+	struct snd_soc_component *component = codec_dai->component;
+	struct pcm1789_private *priv = snd_soc_component_get_drvdata(component);
+
+	priv->format = format;
+
+	return 0;
+}
+
+static int pcm1789_digital_mute(struct snd_soc_dai *codec_dai, int mute)
+{
+	struct snd_soc_component *component = codec_dai->component;
+	struct pcm1789_private *priv = snd_soc_component_get_drvdata(component);
+
+	return regmap_update_bits(priv->regmap, PCM1789_SOFT_MUTE,
+				  PCM1789_MUTE_MASK,
+				  mute ? 0 : PCM1789_MUTE_MASK);
+}
+
+static int pcm1789_hw_params(struct snd_pcm_substream *substream,
+			     struct snd_pcm_hw_params *params,
+			     struct snd_soc_dai *codec_dai)
+{
+	struct snd_soc_component *component = codec_dai->component;
+	struct pcm1789_private *priv = snd_soc_component_get_drvdata(component);
+	int val = 0, ret;
+
+	priv->rate = params_rate(params);
+
+	switch (priv->format & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_RIGHT_J:
+		switch (params_width(params)) {
+		case 24:
+			val = 2;
+			break;
+		case 16:
+			val = 3;
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	case SND_SOC_DAIFMT_I2S:
+		switch (params_width(params)) {
+		case 16:
+		case 24:
+		case 32:
+			val = 0;
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	case SND_SOC_DAIFMT_LEFT_J:
+		switch (params_width(params)) {
+		case 16:
+		case 24:
+		case 32:
+			val = 1;
+			break;
+		default:
+			return -EINVAL;
+		}
+		break;
+	default:
+		dev_err(component->dev, "Invalid DAI format\n");
+		return -EINVAL;
+	}
+
+	ret = regmap_update_bits(priv->regmap, PCM1789_FMT_CONTROL,
+				 PCM1789_FMT_MASK, val);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static void pcm1789_work_queue(struct work_struct *work)
+{
+	struct pcm1789_private *priv = container_of(work,
+						    struct pcm1789_private,
+						    work);
+
+	/* Perform a software reset to remove codec from desynchronized state */
+	if (regmap_update_bits(priv->regmap, PCM1789_MUTE_CONTROL,
+			       0x3 << PCM1789_MUTE_SRET, 0) < 0)
+		dev_err(priv->dev, "Error while setting SRET");
+}
+
+static int pcm1789_trigger(struct snd_pcm_substream *substream, int cmd,
+			   struct snd_soc_dai *dai)
+{
+	struct snd_soc_component *component = dai->component;
+	struct pcm1789_private *priv = snd_soc_component_get_drvdata(component);
+	int ret = 0;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		schedule_work(&priv->work);
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+static const struct snd_soc_dai_ops pcm1789_dai_ops = {
+	.set_fmt	= pcm1789_set_dai_fmt,
+	.hw_params	= pcm1789_hw_params,
+	.digital_mute	= pcm1789_digital_mute,
+	.trigger	= pcm1789_trigger,
+};
+
+static const DECLARE_TLV_DB_SCALE(pcm1789_dac_tlv, -12000, 50, 1);
+
+static const struct snd_kcontrol_new pcm1789_controls[] = {
+	SOC_DOUBLE_R_RANGE_TLV("DAC Playback Volume", PCM1789_DAC_VOL_LEFT,
+			       PCM1789_DAC_VOL_RIGHT, 0, 0xf, 0xff, 0,
+			       pcm1789_dac_tlv),
+};
+
+static const struct snd_soc_dapm_widget pcm1789_dapm_widgets[] = {
+	SND_SOC_DAPM_OUTPUT("IOUTL+"),
+	SND_SOC_DAPM_OUTPUT("IOUTL-"),
+	SND_SOC_DAPM_OUTPUT("IOUTR+"),
+	SND_SOC_DAPM_OUTPUT("IOUTR-"),
+};
+
+static const struct snd_soc_dapm_route pcm1789_dapm_routes[] = {
+	{ "IOUTL+", NULL, "Playback" },
+	{ "IOUTL-", NULL, "Playback" },
+	{ "IOUTR+", NULL, "Playback" },
+	{ "IOUTR-", NULL, "Playback" },
+};
+
+static struct snd_soc_dai_driver pcm1789_dai = {
+	.name = "pcm1789-hifi",
+	.playback = {
+		.stream_name = "Playback",
+		.channels_min = 2,
+		.channels_max = 2,
+		.rates = SNDRV_PCM_RATE_CONTINUOUS,
+		.rate_min = 10000,
+		.rate_max = 200000,
+		.formats = PCM1789_FORMATS,
+	},
+	.ops = &pcm1789_dai_ops,
+};
+
+const struct regmap_config pcm1789_regmap_config = {
+	.reg_bits		= 8,
+	.val_bits		= 8,
+	.max_register		= PCM1789_DAC_VOL_RIGHT,
+	.reg_defaults		= pcm1789_reg_defaults,
+	.num_reg_defaults	= ARRAY_SIZE(pcm1789_reg_defaults),
+	.writeable_reg		= pcm1789_writeable_reg,
+	.readable_reg		= pcm1789_accessible_reg,
+};
+EXPORT_SYMBOL_GPL(pcm1789_regmap_config);
+
+static const struct snd_soc_component_driver soc_component_dev_pcm1789 = {
+	.controls		= pcm1789_controls,
+	.num_controls		= ARRAY_SIZE(pcm1789_controls),
+	.dapm_widgets		= pcm1789_dapm_widgets,
+	.num_dapm_widgets	= ARRAY_SIZE(pcm1789_dapm_widgets),
+	.dapm_routes		= pcm1789_dapm_routes,
+	.num_dapm_routes	= ARRAY_SIZE(pcm1789_dapm_routes),
+	.idle_bias_on		= 1,
+	.use_pmdown_time	= 1,
+	.endianness		= 1,
+	.non_legacy_dai_naming	= 1,
+};
+
+int pcm1789_common_init(struct device *dev, struct regmap *regmap)
+{
+	struct pcm1789_private *pcm1789;
+
+	pcm1789 = devm_kzalloc(dev, sizeof(struct pcm1789_private),
+			       GFP_KERNEL);
+	if (!pcm1789)
+		return -ENOMEM;
+
+	pcm1789->regmap = regmap;
+	pcm1789->dev = dev;
+	dev_set_drvdata(dev, pcm1789);
+
+	pcm1789->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(pcm1789->reset))
+		return PTR_ERR(pcm1789->reset);
+
+	gpiod_set_value_cansleep(pcm1789->reset, 0);
+	msleep(300);
+
+	INIT_WORK(&pcm1789->work, pcm1789_work_queue);
+
+	return devm_snd_soc_register_component(dev, &soc_component_dev_pcm1789,
+					       &pcm1789_dai, 1);
+}
+EXPORT_SYMBOL_GPL(pcm1789_common_init);
+
+int pcm1789_common_exit(struct device *dev)
+{
+	struct pcm1789_private *priv = dev_get_drvdata(dev);
+
+	if (&priv->work)
+		flush_work(&priv->work);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pcm1789_common_exit);
+
+MODULE_DESCRIPTION("ASoC PCM1789 driver");
+MODULE_AUTHOR("Mylène Josserand <mylene.josserand@free-electrons.com>");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/codecs/pcm1789.h b/sound/soc/codecs/pcm1789.h
new file mode 100644
index 000000000000..05c2c8ad2bad
--- /dev/null
+++ b/sound/soc/codecs/pcm1789.h
@@ -0,0 +1,28 @@
+/*
+ * definitions for PCM1789
+ *
+ * Copyright (c) Bootlin 2018
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __PCM1789_H__
+#define __PCM1789_H__
+
+#define PCM1789_FORMATS (SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S24_LE | \
+			 SNDRV_PCM_FMTBIT_S16_LE)
+
+extern const struct regmap_config pcm1789_regmap_config;
+
+int pcm1789_common_init(struct device *dev, struct regmap *regmap);
+int pcm1789_common_exit(struct device *dev);
+
+#endif
-- 
2.11.0

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH v2 2/2] ASoC: Add bindings for PCM1789

[Mylène Josserand]

Add a device-tree binding for Texas Instrument's PCM1789 codec.
For the moment, only I2C bus is supported but SPI could be added
in future.

Signed-off-by: Mylène Josserand <mylene.josserand@bootlin.com>
---
 Documentation/devicetree/bindings/sound/pcm1789.txt | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/pcm1789.txt

diff --git a/Documentation/devicetree/bindings/sound/pcm1789.txt b/Documentation/devicetree/bindings/sound/pcm1789.txt
new file mode 100644
index 000000000000..a6cdc2c83c98
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/pcm1789.txt
@@ -0,0 +1,21 @@
+Texas Instruments pcm1789 DT bindings
+
+This driver supports the I2C bus.
+
+Required properties:
+
+ - compatible: "ti,pcm1789"
+
+Required properties on I2C:
+
+ - reg: the I2C address
+ - reset-gpios: GPIO to control the RESET pin
+
+Examples:
+
+	codec: pcm1789@4c {
+		compatible = "ti,pcm1789";
+		reg = <0x4c>;
+		reset-gpios = <&gpio2 14 GPIO_ACTIVE_LOW>;
+		#sound-dai-cells = <0>;
+	};
-- 
2.11.0

Re: [PATCH v2 1/2] ASoC: codecs: Add support for PCM1789

[Mark Brown]

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

On Mon, Mar 05, 2018 at 01:48:16PM +0100, Mylène Josserand wrote:

> --- /dev/null
> +++ b/sound/soc/codecs/pcm1789-i2c.c
> @@ -0,0 +1,76 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PCM1789 ASoC I2C driver
> + *
> + * Copyright (c) Bootlin 2018
> + *
> + * Mylène Josserand <mylene.josserand@bootlin.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License

Please don't mix C and C++ style comments in a single comment like this
- it looks unintentional.  Just use the same style for the whole thing.
You also don't need to include all the boilerplate with the SPDX header.

> +static bool pcm1789_accessible_reg(struct device *dev, unsigned int reg)
> +{
> +	return reg >= PCM1789_MUTE_CONTROL && reg <= PCM1789_DAC_VOL_RIGHT;
> +}
> +
> +static bool pcm1789_writeable_reg(struct device *dev, unsigned int reg)
> +{
> +	return pcm1789_accessible_reg(dev, reg);
> +}

> +static void pcm1789_work_queue(struct work_struct *work)
> +{
> +	struct pcm1789_private *priv = container_of(work,
> +						    struct pcm1789_private,
> +						    work);
> +
> +	/* Perform a software reset to remove codec from desynchronized state */
> +	if (regmap_update_bits(priv->regmap, PCM1789_MUTE_CONTROL,
> +			       0x3 << PCM1789_MUTE_SRET, 0) < 0)
> +		dev_err(priv->dev, "Error while setting SRET");
> +}

A reset sounds like we'd also need to resync the register map,
presumably this is something else?  The comment could be clearer.

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

Re: [PATCH v2 1/2] ASoC: codecs: Add support for PCM1789

[Alexandre Belloni]

On 05/03/2018 at 15:49:10 +0000, Mark Brown wrote:
> On Mon, Mar 05, 2018 at 01:48:16PM +0100, Mylène Josserand wrote:
> 
> > --- /dev/null
> > +++ b/sound/soc/codecs/pcm1789-i2c.c
> > @@ -0,0 +1,76 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * PCM1789 ASoC I2C driver
> > + *
> > + * Copyright (c) Bootlin 2018
> > + *
> > + * Mylène Josserand <mylene.josserand@bootlin.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> 
> Please don't mix C and C++ style comments in a single comment like this
> - it looks unintentional.  Just use the same style for the whole thing.
> You also don't need to include all the boilerplate with the SPDX header.
> 

I think Linus made it clear that he wants the SPDX header to be a
C++ comment, the documentation has:

"
2. Style:

   The SPDX license identifier is added in form of a comment.  The comment
   style depends on the file type::

      C source: // SPDX-License-Identifier: <SPDX License Expression>
"

I agree the boilerplate should be removed.


-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 1/2] ASoC: codecs: Add support for PCM1789

[Mark Brown]

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

On Mon, Mar 05, 2018 at 04:57:15PM +0100, Alexandre Belloni wrote:
> On 05/03/2018 at 15:49:10 +0000, Mark Brown wrote:

> > Please don't mix C and C++ style comments in a single comment like this
> > - it looks unintentional.  Just use the same style for the whole thing.
> > You also don't need to include all the boilerplate with the SPDX header.

> I think Linus made it clear that he wants the SPDX header to be a
> C++ comment, the documentation has:

Sure, so make the entire thing a C++ comment then.

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

Re: [PATCH v2 1/2] ASoC: codecs: Add support for PCM1789

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 509 bytes --]

On Mon, Mar 05, 2018 at 04:57:15PM +0100, Alexandre Belloni wrote:
> On 05/03/2018 at 15:49:10 +0000, Mark Brown wrote:

> > Please don't mix C and C++ style comments in a single comment like this
> > - it looks unintentional.  Just use the same style for the whole thing.
> > You also don't need to include all the boilerplate with the SPDX header.

> I think Linus made it clear that he wants the SPDX header to be a
> C++ comment, the documentation has:

Sure, so make the entire thing a C++ comment then.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v2 1/2] ASoC: codecs: Add support for PCM1789

[Mylène Josserand]

Hello Mark,

Thanks for the review.

On Mon, 5 Mar 2018 15:59:38 +0000
Mark Brown <broonie@kernel.org> wrote:

> On Mon, Mar 05, 2018 at 04:57:15PM +0100, Alexandre Belloni wrote:
> > On 05/03/2018 at 15:49:10 +0000, Mark Brown wrote:  
> 
> > > Please don't mix C and C++ style comments in a single comment like this
> > > - it looks unintentional.  Just use the same style for the whole thing.
> > > You also don't need to include all the boilerplate with the SPDX header.  
> 
> > I think Linus made it clear that he wants the SPDX header to be a
> > C++ comment, the documentation has:  
> 
> Sure, so make the entire thing a C++ comment then.

Ok, I saw other drivers with this mix C++/C comment code so I wrote it
like this but no problem using only C++ comment.

I will send a v3 with this modification only (except if I have other
reviews).

Best regards,

-- 
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

Re: [PATCH v2 1/2] ASoC: codecs: Add support for PCM1789

[Mylène Josserand]

Hello Mark,

Thanks for the review.

On Mon, 5 Mar 2018 15:59:38 +0000
Mark Brown <broonie@kernel.org> wrote:

> On Mon, Mar 05, 2018 at 04:57:15PM +0100, Alexandre Belloni wrote:
> > On 05/03/2018 at 15:49:10 +0000, Mark Brown wrote:  
> 
> > > Please don't mix C and C++ style comments in a single comment like this
> > > - it looks unintentional.  Just use the same style for the whole thing.
> > > You also don't need to include all the boilerplate with the SPDX header.  
> 
> > I think Linus made it clear that he wants the SPDX header to be a
> > C++ comment, the documentation has:  
> 
> Sure, so make the entire thing a C++ comment then.

Ok, I saw other drivers with this mix C++/C comment code so I wrote it
like this but no problem using only C++ comment.

I will send a v3 with this modification only (except if I have other
reviews).

Best regards,

-- 
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH v2 2/2] ASoC: Add bindings for PCM1789

[Rob Herring]

On Mon, Mar 05, 2018 at 01:48:17PM +0100, Mylène Josserand wrote:
> Add a device-tree binding for Texas Instrument's PCM1789 codec.
> For the moment, only I2C bus is supported but SPI could be added
> in future.
> 
> Signed-off-by: Mylène Josserand <mylene.josserand@bootlin.com>
> ---
>  Documentation/devicetree/bindings/sound/pcm1789.txt | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/pcm1789.txt
> 
> diff --git a/Documentation/devicetree/bindings/sound/pcm1789.txt b/Documentation/devicetree/bindings/sound/pcm1789.txt
> new file mode 100644
> index 000000000000..a6cdc2c83c98
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/pcm1789.txt
> @@ -0,0 +1,21 @@
> +Texas Instruments pcm1789 DT bindings

What function does this device provide?

> +
> +This driver supports the I2C bus.

Bindings describe h/w, not drivers.

> +
> +Required properties:
> +
> + - compatible: "ti,pcm1789"
> +
> +Required properties on I2C:
> +
> + - reg: the I2C address
> + - reset-gpios: GPIO to control the RESET pin
> +
> +Examples:
> +
> +	codec: pcm1789@4c {

audio-codec@4c

If that's the function...

> +		compatible = "ti,pcm1789";
> +		reg = <0x4c>;
> +		reset-gpios = <&gpio2 14 GPIO_ACTIVE_LOW>;
> +		#sound-dai-cells = <0>;
> +	};
> -- 
> 2.11.0
> 

Re: [PATCH v2 2/2] ASoC: Add bindings for PCM1789

[Mylène Josserand]

Hello Rob,

Thank you for the review.

On Wed, 7 Mar 2018 16:30:48 -0600
Rob Herring <robh@kernel.org> wrote:

> On Mon, Mar 05, 2018 at 01:48:17PM +0100, Mylène Josserand wrote:
> > Add a device-tree binding for Texas Instrument's PCM1789 codec.
> > For the moment, only I2C bus is supported but SPI could be added
> > in future.
> > 
> > Signed-off-by: Mylène Josserand <mylene.josserand@bootlin.com>
> > ---
> >  Documentation/devicetree/bindings/sound/pcm1789.txt | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/sound/pcm1789.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/sound/pcm1789.txt b/Documentation/devicetree/bindings/sound/pcm1789.txt
> > new file mode 100644
> > index 000000000000..a6cdc2c83c98
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/pcm1789.txt
> > @@ -0,0 +1,21 @@
> > +Texas Instruments pcm1789 DT bindings  
> 
> What function does this device provide?

Sorry, I forgot to mention this information.

> 
> > +
> > +This driver supports the I2C bus.  
> 
> Bindings describe h/w, not drivers.

True, I will reformulate it.

> 
> > +
> > +Required properties:
> > +
> > + - compatible: "ti,pcm1789"
> > +
> > +Required properties on I2C:
> > +
> > + - reg: the I2C address
> > + - reset-gpios: GPIO to control the RESET pin
> > +
> > +Examples:
> > +
> > +	codec: pcm1789@4c {  
> 
> audio-codec@4c
> 
> If that's the function...

Yep, I will change that.

> 
> > +		compatible = "ti,pcm1789";
> > +		reg = <0x4c>;
> > +		reset-gpios = <&gpio2 14 GPIO_ACTIVE_LOW>;
> > +		#sound-dai-cells = <0>;
> > +	};
> > -- 
> > 2.11.0
> >   

Thanks,

Best regards,

-- 
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

Re: [PATCH v2 2/2] ASoC: Add bindings for PCM1789

[Mylène Josserand]

Hello Rob,

Thank you for the review.

On Wed, 7 Mar 2018 16:30:48 -0600
Rob Herring <robh@kernel.org> wrote:

> On Mon, Mar 05, 2018 at 01:48:17PM +0100, Mylène Josserand wrote:
> > Add a device-tree binding for Texas Instrument's PCM1789 codec.
> > For the moment, only I2C bus is supported but SPI could be added
> > in future.
> > 
> > Signed-off-by: Mylène Josserand <mylene.josserand@bootlin.com>
> > ---
> >  Documentation/devicetree/bindings/sound/pcm1789.txt | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/sound/pcm1789.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/sound/pcm1789.txt b/Documentation/devicetree/bindings/sound/pcm1789.txt
> > new file mode 100644
> > index 000000000000..a6cdc2c83c98
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/pcm1789.txt
> > @@ -0,0 +1,21 @@
> > +Texas Instruments pcm1789 DT bindings  
> 
> What function does this device provide?

Sorry, I forgot to mention this information.

> 
> > +
> > +This driver supports the I2C bus.  
> 
> Bindings describe h/w, not drivers.

True, I will reformulate it.

> 
> > +
> > +Required properties:
> > +
> > + - compatible: "ti,pcm1789"
> > +
> > +Required properties on I2C:
> > +
> > + - reg: the I2C address
> > + - reset-gpios: GPIO to control the RESET pin
> > +
> > +Examples:
> > +
> > +	codec: pcm1789@4c {  
> 
> audio-codec@4c
> 
> If that's the function...

Yep, I will change that.

> 
> > +		compatible = "ti,pcm1789";
> > +		reg = <0x4c>;
> > +		reset-gpios = <&gpio2 14 GPIO_ACTIVE_LOW>;
> > +		#sound-dai-cells = <0>;
> > +	};
> > -- 
> > 2.11.0
> >   

Thanks,

Best regards,

-- 
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel