All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] New feature and clean up for pcm1792a driver
@ 2015-11-20 10:09 Raphael Poggi
  2015-11-20 10:09   ` Raphael Poggi
                   ` (3 more replies)
  0 siblings, 4 replies; 23+ messages in thread
From: Raphael Poggi @ 2015-11-20 10:09 UTC (permalink / raw)
  To: perex, tiwai; +Cc: devicetree, linux-kernel, alsa-devel, Raphael Poggi

From: Raphael Poggi <poggi.raph@gmail.com>

Hi,

This patch series support for the channel selection (stereo/left/right) based on device tree bindings.
It also adds some registers defines and cleans up weird test on function exit.

Raphael Poggi (4):
  sound: codecs: pcm1792a: clean function exit
  sound: codecs: pcm1792a: add define for register
  dt-bindings: sound: add binding for pcm1792a
  sound: codecs: pcm1792a: introduce channel side

 .../devicetree/bindings/sound/pcm1792a.txt         |   5 +
 sound/soc/codecs/pcm1792a.c                        | 107 +++++++++++++++------
 2 files changed, 83 insertions(+), 29 deletions(-)

-- 
2.1.0


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 1/4] sound: codecs: pcm1792a: clean function exit
@ 2015-11-20 10:09   ` Raphael Poggi
  0 siblings, 0 replies; 23+ messages in thread
From: Raphael Poggi @ 2015-11-20 10:09 UTC (permalink / raw)
  To: perex, tiwai; +Cc: devicetree, linux-kernel, alsa-devel, Raphael Poggi

From: Raphael Poggi <poggi.raph@gmail.com>

Signed-off-by: Raphael Poggi <poggi.raph@gmail.com>
---
 sound/soc/codecs/pcm1792a.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/sound/soc/codecs/pcm1792a.c b/sound/soc/codecs/pcm1792a.c
index 08bb486..cf42b24 100644
--- a/sound/soc/codecs/pcm1792a.c
+++ b/sound/soc/codecs/pcm1792a.c
@@ -91,14 +91,9 @@ static int pcm1792a_digital_mute(struct snd_soc_dai *dai, int mute)
 {
 	struct snd_soc_codec *codec = dai->codec;
 	struct pcm1792a_private *priv = snd_soc_codec_get_drvdata(codec);
-	int ret;
 
-	ret = regmap_update_bits(priv->regmap, PCM1792A_SOFT_MUTE,
+	return regmap_update_bits(priv->regmap, PCM1792A_SOFT_MUTE,
 				 PCM1792A_MUTE_MASK, !!mute);
-	if (ret < 0)
-		return ret;
-
-	return 0;
 }
 
 static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
@@ -107,7 +102,8 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
 {
 	struct snd_soc_codec *codec = dai->codec;
 	struct pcm1792a_private *priv = snd_soc_codec_get_drvdata(codec);
-	int val = 0, ret;
+	int val = 0;
+	int ret = 0;
 
 	priv->rate = params_rate(params);
 
@@ -147,10 +143,8 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
 
 	ret = regmap_update_bits(priv->regmap, PCM1792A_FMT_CONTROL,
 				 PCM1792A_FMT_MASK | PCM1792A_ATLD_ENABLE, val);
-	if (ret < 0)
-		return ret;
 
-	return 0;
+	return ret;
 }
 
 static const struct snd_soc_dai_ops pcm1792a_dai_ops = {
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 1/4] sound: codecs: pcm1792a: clean function exit
@ 2015-11-20 10:09   ` Raphael Poggi
  0 siblings, 0 replies; 23+ messages in thread
From: Raphael Poggi @ 2015-11-20 10:09 UTC (permalink / raw)
  To: perex-/Fr2/VpizcU, tiwai-IBi9RG/b67k
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Raphael Poggi

From: Raphael Poggi <poggi.raph-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Signed-off-by: Raphael Poggi <poggi.raph-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 sound/soc/codecs/pcm1792a.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/sound/soc/codecs/pcm1792a.c b/sound/soc/codecs/pcm1792a.c
index 08bb486..cf42b24 100644
--- a/sound/soc/codecs/pcm1792a.c
+++ b/sound/soc/codecs/pcm1792a.c
@@ -91,14 +91,9 @@ static int pcm1792a_digital_mute(struct snd_soc_dai *dai, int mute)
 {
 	struct snd_soc_codec *codec = dai->codec;
 	struct pcm1792a_private *priv = snd_soc_codec_get_drvdata(codec);
-	int ret;
 
-	ret = regmap_update_bits(priv->regmap, PCM1792A_SOFT_MUTE,
+	return regmap_update_bits(priv->regmap, PCM1792A_SOFT_MUTE,
 				 PCM1792A_MUTE_MASK, !!mute);
-	if (ret < 0)
-		return ret;
-
-	return 0;
 }
 
 static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
@@ -107,7 +102,8 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
 {
 	struct snd_soc_codec *codec = dai->codec;
 	struct pcm1792a_private *priv = snd_soc_codec_get_drvdata(codec);
-	int val = 0, ret;
+	int val = 0;
+	int ret = 0;
 
 	priv->rate = params_rate(params);
 
@@ -147,10 +143,8 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
 
 	ret = regmap_update_bits(priv->regmap, PCM1792A_FMT_CONTROL,
 				 PCM1792A_FMT_MASK | PCM1792A_ATLD_ENABLE, val);
-	if (ret < 0)
-		return ret;
 
-	return 0;
+	return ret;
 }
 
 static const struct snd_soc_dai_ops pcm1792a_dai_ops = {
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 2/4] sound: codecs: pcm1792a: add define for register
  2015-11-20 10:09 [PATCH 0/4] New feature and clean up for pcm1792a driver Raphael Poggi
  2015-11-20 10:09   ` Raphael Poggi
@ 2015-11-20 10:09 ` Raphael Poggi
  2015-11-21  9:36   ` [alsa-devel] " Michael Trimarchi
  2015-11-20 10:09   ` Raphael Poggi
  2015-11-20 10:09 ` [PATCH 4/4] sound: codecs: pcm1792a: introduce channel side Raphael Poggi
  3 siblings, 1 reply; 23+ messages in thread
From: Raphael Poggi @ 2015-11-20 10:09 UTC (permalink / raw)
  To: perex, tiwai; +Cc: devicetree, linux-kernel, alsa-devel, Raphael Poggi

From: Raphael Poggi <poggi.raph@gmail.com>

Use define for register instead of number, when it is possible.
Doing this, make easier to read and use in the code.

Signed-off-by: Raphael Poggi <poggi.raph@gmail.com>
---
 sound/soc/codecs/pcm1792a.c | 47 +++++++++++++++++++++++++++------------------
 1 file changed, 28 insertions(+), 19 deletions(-)

diff --git a/sound/soc/codecs/pcm1792a.c b/sound/soc/codecs/pcm1792a.c
index cf42b24..febaa48 100644
--- a/sound/soc/codecs/pcm1792a.c
+++ b/sound/soc/codecs/pcm1792a.c
@@ -33,27 +33,36 @@
 
 #include "pcm1792a.h"
 
-#define PCM1792A_DAC_VOL_LEFT	0x10
-#define PCM1792A_DAC_VOL_RIGHT	0x11
-#define PCM1792A_FMT_CONTROL	0x12
-#define PCM1792A_MODE_CONTROL	0x13
-#define PCM1792A_SOFT_MUTE	PCM1792A_FMT_CONTROL
-
-#define PCM1792A_FMT_MASK	0x70
-#define PCM1792A_FMT_SHIFT	4
-#define PCM1792A_MUTE_MASK	0x01
-#define PCM1792A_MUTE_SHIFT	0
-#define PCM1792A_ATLD_ENABLE	(1 << 7)
+#define PCM1792A_DAC_VOL_LEFT		0x10
+#define PCM1792A_DAC_VOL_RIGHT		0x11
+#define PCM1792A_FMT_CONTROL		0x12
+#define PCM1792A_MODE_CONTROL		0x13
+#define PCM1792A_ZERO_OUTPUT		0x15
+#define PCM1792A_SOFT_MUTE		PCM1792A_FMT_CONTROL
+#define PCM1792A_ZERO_FLAG		0x16
+#define PCM1792A_DEVICE_ID		0x17
+
+#define PCM1792A_CHANNEL		0x14
+#define PCM1792A_CHANNEL_MONO_MASK	0x8
+#define PCM1792A_CHANNEL_MONO_SHIFT	0x8
+#define PCM1792A_CHANNEL_MONO_LEFT	(0x0 << PCM1792A_CHANNEL_MONO_SHIFT)
+#define PCM1792A_CHANNEL_MONO_RIGHT	(0x1 << PCM1792A_CHANNEL_MONO_SHIFT)
+
+#define PCM1792A_FMT_MASK		0x70
+#define PCM1792A_FMT_SHIFT		4
+#define PCM1792A_MUTE_MASK		0x01
+#define PCM1792A_MUTE_SHIFT		0
+#define PCM1792A_ATLD_ENABLE		(1 << 7)
 
 static const struct reg_default pcm1792a_reg_defaults[] = {
-	{ 0x10, 0xff },
-	{ 0x11, 0xff },
-	{ 0x12, 0x50 },
-	{ 0x13, 0x00 },
-	{ 0x14, 0x00 },
-	{ 0x15, 0x01 },
-	{ 0x16, 0x00 },
-	{ 0x17, 0x00 },
+	{ PCM1792A_DAC_VOL_LEFT, 0xff },
+	{ PCM1792A_DAC_VOL_RIGHT, 0xff },
+	{ PCM1792A_FMT_CONTROL, 0x50 },
+	{ PCM1792A_MODE_CONTROL, 0x00 },
+	{ PCM1792A_CHANNEL, 0x00 },
+	{ PCM1792A_ZERO_OUTPUT, 0x01 },
+	{ PCM1792A_ZERO_FLAG, 0x00 },
+	{ PCM1792A_DEVICE_ID, 0x00 },
 };
 
 static bool pcm1792a_accessible_reg(struct device *dev, unsigned int reg)
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 3/4] dt-bindings: sound: add binding for pcm1792a
@ 2015-11-20 10:09   ` Raphael Poggi
  0 siblings, 0 replies; 23+ messages in thread
From: Raphael Poggi @ 2015-11-20 10:09 UTC (permalink / raw)
  To: perex, tiwai; +Cc: devicetree, linux-kernel, alsa-devel, Raphael Poggi

From: Raphael Poggi <poggi.raph@gmail.com>

Add a binding for specify channel selection.

Signed-off-by: Raphael Poggi <poggi.raph@gmail.com>
---
 Documentation/devicetree/bindings/sound/pcm1792a.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/pcm1792a.txt b/Documentation/devicetree/bindings/sound/pcm1792a.txt
index 970ba1e..6e87a3a 100644
--- a/Documentation/devicetree/bindings/sound/pcm1792a.txt
+++ b/Documentation/devicetree/bindings/sound/pcm1792a.txt
@@ -9,10 +9,15 @@ Required properties:
 For required properties on SPI, please consult
 Documentation/devicetree/bindings/spi/spi-bus.txt
 
+Optional properties:
+- ti,side: specifie the channel side
+  insertion, removal.
+
 Examples:
 
 	codec_spi: 1792a@0 {
 		compatible = "ti,pcm1792a";
 		spi-max-frequency = <600000>;
+		ti,side = <left>
 	};
 
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 3/4] dt-bindings: sound: add binding for pcm1792a
@ 2015-11-20 10:09   ` Raphael Poggi
  0 siblings, 0 replies; 23+ messages in thread
From: Raphael Poggi @ 2015-11-20 10:09 UTC (permalink / raw)
  To: perex-/Fr2/VpizcU, tiwai-IBi9RG/b67k
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Raphael Poggi

From: Raphael Poggi <poggi.raph-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Add a binding for specify channel selection.

Signed-off-by: Raphael Poggi <poggi.raph-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 Documentation/devicetree/bindings/sound/pcm1792a.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/pcm1792a.txt b/Documentation/devicetree/bindings/sound/pcm1792a.txt
index 970ba1e..6e87a3a 100644
--- a/Documentation/devicetree/bindings/sound/pcm1792a.txt
+++ b/Documentation/devicetree/bindings/sound/pcm1792a.txt
@@ -9,10 +9,15 @@ Required properties:
 For required properties on SPI, please consult
 Documentation/devicetree/bindings/spi/spi-bus.txt
 
+Optional properties:
+- ti,side: specifie the channel side
+  insertion, removal.
+
 Examples:
 
 	codec_spi: 1792a@0 {
 		compatible = "ti,pcm1792a";
 		spi-max-frequency = <600000>;
+		ti,side = <left>
 	};
 
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 4/4] sound: codecs: pcm1792a: introduce channel side
  2015-11-20 10:09 [PATCH 0/4] New feature and clean up for pcm1792a driver Raphael Poggi
                   ` (2 preceding siblings ...)
  2015-11-20 10:09   ` Raphael Poggi
@ 2015-11-20 10:09 ` Raphael Poggi
  2015-11-21  9:37     ` Michael Trimarchi
  3 siblings, 1 reply; 23+ messages in thread
From: Raphael Poggi @ 2015-11-20 10:09 UTC (permalink / raw)
  To: perex, tiwai; +Cc: devicetree, linux-kernel, alsa-devel, Raphael Poggi

From: Raphael Poggi <poggi.raph@gmail.com>

Add possibility to choose the channel side using the device tree,
and also modify it using alsa ctrl.

Signed-off-by: Raphael Poggi <poggi.raph@gmail.com>
---
 sound/soc/codecs/pcm1792a.c | 46 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/sound/soc/codecs/pcm1792a.c b/sound/soc/codecs/pcm1792a.c
index febaa48..005b679 100644
--- a/sound/soc/codecs/pcm1792a.c
+++ b/sound/soc/codecs/pcm1792a.c
@@ -65,6 +65,12 @@ static const struct reg_default pcm1792a_reg_defaults[] = {
 	{ PCM1792A_DEVICE_ID, 0x00 },
 };
 
+enum __pcm1792_side {
+	STEREO,
+	MONORAL_LEFT,
+	MONORAL_RIGHT
+};
+
 static bool pcm1792a_accessible_reg(struct device *dev, unsigned int reg)
 {
 	return reg >= 0x10 && reg <= 0x17;
@@ -83,6 +89,7 @@ struct pcm1792a_private {
 	struct regmap *regmap;
 	unsigned int format;
 	unsigned int rate;
+	unsigned int side;
 };
 
 static int pcm1792a_set_dai_fmt(struct snd_soc_dai *codec_dai,
@@ -153,6 +160,13 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
 	ret = regmap_update_bits(priv->regmap, PCM1792A_FMT_CONTROL,
 				 PCM1792A_FMT_MASK | PCM1792A_ATLD_ENABLE, val);
 
+	if (priv->side == MONORAL_LEFT)
+		val = PCM1792A_CHANNEL_MONO_LEFT;
+	else if (priv->side == MONORAL_RIGHT)
+		val = PCM1792A_CHANNEL_MONO_RIGHT;
+
+	regmap_update_bits(priv->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, val);
+
 	return ret;
 }
 
@@ -222,10 +236,31 @@ static struct snd_soc_codec_driver soc_codec_dev_pcm1792a = {
 	.num_dapm_routes	= ARRAY_SIZE(pcm1792a_dapm_routes),
 };
 
+static int pcm1792a_of_init(struct spi_device *spi)
+{
+	int ret = 0;
+	struct pcm1792a_private *pcm1792a = dev_get_drvdata(&spi->dev);
+	const char *name;
+	struct device_node *np = spi->dev.of_node;
+
+	name = of_get_property(np, "ti,side", NULL);
+	if (name) {
+		if (!strcmp(name, "left"))
+			pcm1792a->side = MONORAL_LEFT;
+		else
+			pcm1792a->side = MONORAL_RIGHT;
+	}
+	else
+		pcm1792a->side = STEREO;
+
+	return ret;
+}
+
 static int pcm1792a_spi_probe(struct spi_device *spi)
 {
 	struct pcm1792a_private *pcm1792a;
 	int ret;
+	struct device_node *np = spi->dev.of_node;
 
 	pcm1792a = devm_kzalloc(&spi->dev, sizeof(struct pcm1792a_private),
 				GFP_KERNEL);
@@ -241,6 +276,17 @@ static int pcm1792a_spi_probe(struct spi_device *spi)
 		return ret;
 	}
 
+	if (np) {
+		ret = pcm1792a_of_init(spi);
+		if (ret)
+			return ret;
+	}
+
+	if (pcm1792a->side == MONORAL_LEFT)
+		regmap_update_bits(pcm1792a->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, PCM1792A_CHANNEL_MONO_LEFT);
+	else if (pcm1792a->side == MONORAL_RIGHT)
+		regmap_update_bits(pcm1792a->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, PCM1792A_CHANNEL_MONO_RIGHT);
+
 	return snd_soc_register_codec(&spi->dev,
 			&soc_codec_dev_pcm1792a, &pcm1792a_dai, 1);
 }
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/4] dt-bindings: sound: add binding for pcm1792a
  2015-11-20 10:09   ` Raphael Poggi
  (?)
@ 2015-11-20 14:42   ` Rob Herring
  2015-11-20 15:04       ` Michael Trimarchi
  -1 siblings, 1 reply; 23+ messages in thread
From: Rob Herring @ 2015-11-20 14:42 UTC (permalink / raw)
  To: Raphael Poggi; +Cc: perex, tiwai, devicetree, linux-kernel, alsa-devel

On Fri, Nov 20, 2015 at 11:09:16AM +0100, Raphael Poggi wrote:
> From: Raphael Poggi <poggi.raph@gmail.com>
> 
> Add a binding for specify channel selection.
> 
> Signed-off-by: Raphael Poggi <poggi.raph@gmail.com>
> ---
>  Documentation/devicetree/bindings/sound/pcm1792a.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/pcm1792a.txt b/Documentation/devicetree/bindings/sound/pcm1792a.txt
> index 970ba1e..6e87a3a 100644
> --- a/Documentation/devicetree/bindings/sound/pcm1792a.txt
> +++ b/Documentation/devicetree/bindings/sound/pcm1792a.txt
> @@ -9,10 +9,15 @@ Required properties:
>  For required properties on SPI, please consult
>  Documentation/devicetree/bindings/spi/spi-bus.txt
>  
> +Optional properties:
> +- ti,side: specifie the channel side

typo

> +  insertion, removal.

why the line wrap?

I don't understand what this property is/does. Both the name and 
description are a bit terse.

> +
>  Examples:
>  
>  	codec_spi: 1792a@0 {
>  		compatible = "ti,pcm1792a";
>  		spi-max-frequency = <600000>;
> +		ti,side = <left>
                                ^
missing ;

>  	};
>  
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [alsa-devel] [PATCH 3/4] dt-bindings: sound: add binding for pcm1792a
  2015-11-20 14:42   ` Rob Herring
@ 2015-11-20 15:04       ` Michael Trimarchi
  0 siblings, 0 replies; 23+ messages in thread
From: Michael Trimarchi @ 2015-11-20 15:04 UTC (permalink / raw)
  To: Rob Herring; +Cc: Raphael Poggi, devicetree, alsa-devel, tiwai, LKML

Hi Raphael


On Fri, Nov 20, 2015 at 3:42 PM, Rob Herring <robh@kernel.org> wrote:
> On Fri, Nov 20, 2015 at 11:09:16AM +0100, Raphael Poggi wrote:
>> From: Raphael Poggi <poggi.raph@gmail.com>
>>
>> Add a binding for specify channel selection.
>>
>> Signed-off-by: Raphael Poggi <poggi.raph@gmail.com>
>> ---
>>  Documentation/devicetree/bindings/sound/pcm1792a.txt | 5 +++++
>>  1 file changed, 5 insertions(+)
>>

can you add me in the patch-set?

Michael

>> diff --git a/Documentation/devicetree/bindings/sound/pcm1792a.txt b/Documentation/devicetree/bindings/sound/pcm1792a.txt
>> index 970ba1e..6e87a3a 100644
>> --- a/Documentation/devicetree/bindings/sound/pcm1792a.txt
>> +++ b/Documentation/devicetree/bindings/sound/pcm1792a.txt
>> @@ -9,10 +9,15 @@ Required properties:
>>  For required properties on SPI, please consult
>>  Documentation/devicetree/bindings/spi/spi-bus.txt
>>
>> +Optional properties:
>> +- ti,side: specifie the channel side
>
> typo
>
>> +  insertion, removal.
>
> why the line wrap?
>
> I don't understand what this property is/does. Both the name and
> description are a bit terse.
>
>> +
>>  Examples:
>>
>>       codec_spi: 1792a@0 {
>>               compatible = "ti,pcm1792a";
>>               spi-max-frequency = <600000>;
>> +             ti,side = <left>
>                                 ^
> missing ;
>
>>       };
>>
>> --
>> 2.1.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel



-- 
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/4] dt-bindings: sound: add binding for pcm1792a
@ 2015-11-20 15:04       ` Michael Trimarchi
  0 siblings, 0 replies; 23+ messages in thread
From: Michael Trimarchi @ 2015-11-20 15:04 UTC (permalink / raw)
  To: Rob Herring; +Cc: devicetree, alsa-devel, Raphael Poggi, tiwai, LKML

Hi Raphael


On Fri, Nov 20, 2015 at 3:42 PM, Rob Herring <robh@kernel.org> wrote:
> On Fri, Nov 20, 2015 at 11:09:16AM +0100, Raphael Poggi wrote:
>> From: Raphael Poggi <poggi.raph@gmail.com>
>>
>> Add a binding for specify channel selection.
>>
>> Signed-off-by: Raphael Poggi <poggi.raph@gmail.com>
>> ---
>>  Documentation/devicetree/bindings/sound/pcm1792a.txt | 5 +++++
>>  1 file changed, 5 insertions(+)
>>

can you add me in the patch-set?

Michael

>> diff --git a/Documentation/devicetree/bindings/sound/pcm1792a.txt b/Documentation/devicetree/bindings/sound/pcm1792a.txt
>> index 970ba1e..6e87a3a 100644
>> --- a/Documentation/devicetree/bindings/sound/pcm1792a.txt
>> +++ b/Documentation/devicetree/bindings/sound/pcm1792a.txt
>> @@ -9,10 +9,15 @@ Required properties:
>>  For required properties on SPI, please consult
>>  Documentation/devicetree/bindings/spi/spi-bus.txt
>>
>> +Optional properties:
>> +- ti,side: specifie the channel side
>
> typo
>
>> +  insertion, removal.
>
> why the line wrap?
>
> I don't understand what this property is/does. Both the name and
> description are a bit terse.
>
>> +
>>  Examples:
>>
>>       codec_spi: 1792a@0 {
>>               compatible = "ti,pcm1792a";
>>               spi-max-frequency = <600000>;
>> +             ti,side = <left>
>                                 ^
> missing ;
>
>>       };
>>
>> --
>> 2.1.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel



-- 
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [alsa-devel] [PATCH 1/4] sound: codecs: pcm1792a: clean function exit
@ 2015-11-21  9:31     ` Michael Trimarchi
  0 siblings, 0 replies; 23+ messages in thread
From: Michael Trimarchi @ 2015-11-21  9:31 UTC (permalink / raw)
  To: Raphael Poggi; +Cc: perex, tiwai, devicetree, alsa-devel, LKML

Hi

On Fri, Nov 20, 2015 at 11:09 AM, Raphael Poggi <poggi.raph@gmail.com> wrote:
> From: Raphael Poggi <poggi.raph@gmail.com>
>
> Signed-off-by: Raphael Poggi <poggi.raph@gmail.com>
> ---
>  sound/soc/codecs/pcm1792a.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/sound/soc/codecs/pcm1792a.c b/sound/soc/codecs/pcm1792a.c
> index 08bb486..cf42b24 100644
> --- a/sound/soc/codecs/pcm1792a.c
> +++ b/sound/soc/codecs/pcm1792a.c
> @@ -91,14 +91,9 @@ static int pcm1792a_digital_mute(struct snd_soc_dai *dai, int mute)
>  {
>         struct snd_soc_codec *codec = dai->codec;
>         struct pcm1792a_private *priv = snd_soc_codec_get_drvdata(codec);
> -       int ret;
>
> -       ret = regmap_update_bits(priv->regmap, PCM1792A_SOFT_MUTE,
> +       return regmap_update_bits(priv->regmap, PCM1792A_SOFT_MUTE,
>                                  PCM1792A_MUTE_MASK, !!mute);
> -       if (ret < 0)
> -               return ret;
> -

Ok, I have seen that almost all the codecs use return regmap_upda...

> -       return 0;
>  }
>
>  static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
> @@ -107,7 +102,8 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
>  {
>         struct snd_soc_codec *codec = dai->codec;
>         struct pcm1792a_private *priv = snd_soc_codec_get_drvdata(codec);
> -       int val = 0, ret;
> +       int val = 0;
> +       int ret = 0;
>
>         priv->rate = params_rate(params);
>
> @@ -147,10 +143,8 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
>
>         ret = regmap_update_bits(priv->regmap, PCM1792A_FMT_CONTROL,
>                                  PCM1792A_FMT_MASK | PCM1792A_ATLD_ENABLE, val);
> -       if (ret < 0)
> -               return ret;
>

Please do the same as before

Michael


> -       return 0;
> +       return ret;
>  }
>
>  static const struct snd_soc_dai_ops pcm1792a_dai_ops = {
> --
> 2.1.0
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel



-- 
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [alsa-devel] [PATCH 1/4] sound: codecs: pcm1792a: clean function exit
@ 2015-11-21  9:31     ` Michael Trimarchi
  0 siblings, 0 replies; 23+ messages in thread
From: Michael Trimarchi @ 2015-11-21  9:31 UTC (permalink / raw)
  To: Raphael Poggi
  Cc: perex-/Fr2/VpizcU, tiwai-IBi9RG/b67k,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, LKML

Hi

On Fri, Nov 20, 2015 at 11:09 AM, Raphael Poggi <poggi.raph-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Raphael Poggi <poggi.raph-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Signed-off-by: Raphael Poggi <poggi.raph-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  sound/soc/codecs/pcm1792a.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/sound/soc/codecs/pcm1792a.c b/sound/soc/codecs/pcm1792a.c
> index 08bb486..cf42b24 100644
> --- a/sound/soc/codecs/pcm1792a.c
> +++ b/sound/soc/codecs/pcm1792a.c
> @@ -91,14 +91,9 @@ static int pcm1792a_digital_mute(struct snd_soc_dai *dai, int mute)
>  {
>         struct snd_soc_codec *codec = dai->codec;
>         struct pcm1792a_private *priv = snd_soc_codec_get_drvdata(codec);
> -       int ret;
>
> -       ret = regmap_update_bits(priv->regmap, PCM1792A_SOFT_MUTE,
> +       return regmap_update_bits(priv->regmap, PCM1792A_SOFT_MUTE,
>                                  PCM1792A_MUTE_MASK, !!mute);
> -       if (ret < 0)
> -               return ret;
> -

Ok, I have seen that almost all the codecs use return regmap_upda...

> -       return 0;
>  }
>
>  static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
> @@ -107,7 +102,8 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
>  {
>         struct snd_soc_codec *codec = dai->codec;
>         struct pcm1792a_private *priv = snd_soc_codec_get_drvdata(codec);
> -       int val = 0, ret;
> +       int val = 0;
> +       int ret = 0;
>
>         priv->rate = params_rate(params);
>
> @@ -147,10 +143,8 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
>
>         ret = regmap_update_bits(priv->regmap, PCM1792A_FMT_CONTROL,
>                                  PCM1792A_FMT_MASK | PCM1792A_ATLD_ENABLE, val);
> -       if (ret < 0)
> -               return ret;
>

Please do the same as before

Michael


> -       return 0;
> +       return ret;
>  }
>
>  static const struct snd_soc_dai_ops pcm1792a_dai_ops = {
> --
> 2.1.0
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel



-- 
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [alsa-devel] [PATCH 2/4] sound: codecs: pcm1792a: add define for register
  2015-11-20 10:09 ` [PATCH 2/4] sound: codecs: pcm1792a: add define for register Raphael Poggi
@ 2015-11-21  9:36   ` Michael Trimarchi
  0 siblings, 0 replies; 23+ messages in thread
From: Michael Trimarchi @ 2015-11-21  9:36 UTC (permalink / raw)
  To: Raphael Poggi; +Cc: perex, tiwai, devicetree, alsa-devel, LKML

Hi

On Fri, Nov 20, 2015 at 11:09 AM, Raphael Poggi <poggi.raph@gmail.com> wrote:
> From: Raphael Poggi <poggi.raph@gmail.com>
>
> Use define for register instead of number, when it is possible.
> Doing this, make easier to read and use in the code.
>
> Signed-off-by: Raphael Poggi <poggi.raph@gmail.com>
> ---
>  sound/soc/codecs/pcm1792a.c | 47 +++++++++++++++++++++++++++------------------
>  1 file changed, 28 insertions(+), 19 deletions(-)
>
> diff --git a/sound/soc/codecs/pcm1792a.c b/sound/soc/codecs/pcm1792a.c
> index cf42b24..febaa48 100644
> --- a/sound/soc/codecs/pcm1792a.c
> +++ b/sound/soc/codecs/pcm1792a.c
> @@ -33,27 +33,36 @@
>
>  #include "pcm1792a.h"
>
> -#define PCM1792A_DAC_VOL_LEFT  0x10
> -#define PCM1792A_DAC_VOL_RIGHT 0x11
> -#define PCM1792A_FMT_CONTROL   0x12
> -#define PCM1792A_MODE_CONTROL  0x13
> -#define PCM1792A_SOFT_MUTE     PCM1792A_FMT_CONTROL
> -
> -#define PCM1792A_FMT_MASK      0x70
> -#define PCM1792A_FMT_SHIFT     4
> -#define PCM1792A_MUTE_MASK     0x01
> -#define PCM1792A_MUTE_SHIFT    0
> -#define PCM1792A_ATLD_ENABLE   (1 << 7)
> +#define PCM1792A_DAC_VOL_LEFT          0x10
> +#define PCM1792A_DAC_VOL_RIGHT         0x11
> +#define PCM1792A_FMT_CONTROL           0x12
> +#define PCM1792A_MODE_CONTROL          0x13
> +#define PCM1792A_ZERO_OUTPUT           0x15
> +#define PCM1792A_SOFT_MUTE             PCM1792A_FMT_CONTROL
> +#define PCM1792A_ZERO_FLAG             0x16
> +#define PCM1792A_DEVICE_ID             0x17
> +
> +#define PCM1792A_CHANNEL               0x14
> +#define PCM1792A_CHANNEL_MONO_MASK     0x8
> +#define PCM1792A_CHANNEL_MONO_SHIFT    0x8
> +#define PCM1792A_CHANNEL_MONO_LEFT     (0x0 << PCM1792A_CHANNEL_MONO_SHIFT)
> +#define PCM1792A_CHANNEL_MONO_RIGHT    (0x1 << PCM1792A_CHANNEL_MONO_SHIFT)
> +

Can you just add what you use? and give an order on those defines?

What is register, what is mask, what is shift?

> +#define PCM1792A_FMT_MASK              0x70
> +#define PCM1792A_FMT_SHIFT             4
> +#define PCM1792A_MUTE_MASK             0x01
> +#define PCM1792A_MUTE_SHIFT            0
> +#define PCM1792A_ATLD_ENABLE           (1 << 7)
>
>  static const struct reg_default pcm1792a_reg_defaults[] = {
> -       { 0x10, 0xff },
> -       { 0x11, 0xff },
> -       { 0x12, 0x50 },
> -       { 0x13, 0x00 },
> -       { 0x14, 0x00 },
> -       { 0x15, 0x01 },
> -       { 0x16, 0x00 },
> -       { 0x17, 0x00 },
> +       { PCM1792A_DAC_VOL_LEFT, 0xff },
> +       { PCM1792A_DAC_VOL_RIGHT, 0xff },
> +       { PCM1792A_FMT_CONTROL, 0x50 },
> +       { PCM1792A_MODE_CONTROL, 0x00 },
> +       { PCM1792A_CHANNEL, 0x00 },
> +       { PCM1792A_ZERO_OUTPUT, 0x01 },
> +       { PCM1792A_ZERO_FLAG, 0x00 },
> +       { PCM1792A_DEVICE_ID, 0x00 },
>  };
>
>  static bool pcm1792a_accessible_reg(struct device *dev, unsigned int reg)

This function should be update too

Michael

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



-- 
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [alsa-devel] [PATCH 4/4] sound: codecs: pcm1792a: introduce channel side
@ 2015-11-21  9:37     ` Michael Trimarchi
  0 siblings, 0 replies; 23+ messages in thread
From: Michael Trimarchi @ 2015-11-21  9:37 UTC (permalink / raw)
  To: Raphael Poggi; +Cc: perex, tiwai, devicetree, alsa-devel, LKML

Hi

On Fri, Nov 20, 2015 at 11:09 AM, Raphael Poggi <poggi.raph@gmail.com> wrote:
> From: Raphael Poggi <poggi.raph@gmail.com>
>
> Add possibility to choose the channel side using the device tree,
> and also modify it using alsa ctrl.
>
> Signed-off-by: Raphael Poggi <poggi.raph@gmail.com>
> ---
>  sound/soc/codecs/pcm1792a.c | 46 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>
> diff --git a/sound/soc/codecs/pcm1792a.c b/sound/soc/codecs/pcm1792a.c
> index febaa48..005b679 100644
> --- a/sound/soc/codecs/pcm1792a.c
> +++ b/sound/soc/codecs/pcm1792a.c
> @@ -65,6 +65,12 @@ static const struct reg_default pcm1792a_reg_defaults[] = {
>         { PCM1792A_DEVICE_ID, 0x00 },
>  };
>
> +enum __pcm1792_side {
> +       STEREO,
> +       MONORAL_LEFT,
> +       MONORAL_RIGHT
> +};
> +
>  static bool pcm1792a_accessible_reg(struct device *dev, unsigned int reg)
>  {
>         return reg >= 0x10 && reg <= 0x17;
> @@ -83,6 +89,7 @@ struct pcm1792a_private {
>         struct regmap *regmap;
>         unsigned int format;
>         unsigned int rate;
> +       unsigned int side;
>  };
>
>  static int pcm1792a_set_dai_fmt(struct snd_soc_dai *codec_dai,
> @@ -153,6 +160,13 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
>         ret = regmap_update_bits(priv->regmap, PCM1792A_FMT_CONTROL,
>                                  PCM1792A_FMT_MASK | PCM1792A_ATLD_ENABLE, val);
>
> +       if (priv->side == MONORAL_LEFT)
> +               val = PCM1792A_CHANNEL_MONO_LEFT;
> +       else if (priv->side == MONORAL_RIGHT)
> +               val = PCM1792A_CHANNEL_MONO_RIGHT;
> +
> +       regmap_update_bits(priv->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, val);
> +
>         return ret;
>  }
>
> @@ -222,10 +236,31 @@ static struct snd_soc_codec_driver soc_codec_dev_pcm1792a = {
>         .num_dapm_routes        = ARRAY_SIZE(pcm1792a_dapm_routes),
>  };
>
> +static int pcm1792a_of_init(struct spi_device *spi)
> +{
> +       int ret = 0;
> +       struct pcm1792a_private *pcm1792a = dev_get_drvdata(&spi->dev);
> +       const char *name;
> +       struct device_node *np = spi->dev.of_node;
> +
> +       name = of_get_property(np, "ti,side", NULL);
> +       if (name) {
> +               if (!strcmp(name, "left"))
> +                       pcm1792a->side = MONORAL_LEFT;
> +               else
> +                       pcm1792a->side = MONORAL_RIGHT;
> +       }
> +       else
> +               pcm1792a->side = STEREO;
> +
> +       return ret;
> +}
> +
>  static int pcm1792a_spi_probe(struct spi_device *spi)
>  {
>         struct pcm1792a_private *pcm1792a;
>         int ret;
> +       struct device_node *np = spi->dev.of_node;
>
>         pcm1792a = devm_kzalloc(&spi->dev, sizeof(struct pcm1792a_private),
>                                 GFP_KERNEL);
> @@ -241,6 +276,17 @@ static int pcm1792a_spi_probe(struct spi_device *spi)
>                 return ret;
>         }
>
> +       if (np) {
> +               ret = pcm1792a_of_init(spi);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       if (pcm1792a->side == MONORAL_LEFT)
> +               regmap_update_bits(pcm1792a->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, PCM1792A_CHANNEL_MONO_LEFT);
> +       else if (pcm1792a->side == MONORAL_RIGHT)
> +               regmap_update_bits(pcm1792a->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, PCM1792A_CHANNEL_MONO_RIGHT);
> +

Why this is a part of dts and not a part of mixer control?

Michael

>         return snd_soc_register_codec(&spi->dev,
>                         &soc_codec_dev_pcm1792a, &pcm1792a_dai, 1);
>  }
> --
> 2.1.0
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel



-- 
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [alsa-devel] [PATCH 4/4] sound: codecs: pcm1792a: introduce channel side
@ 2015-11-21  9:37     ` Michael Trimarchi
  0 siblings, 0 replies; 23+ messages in thread
From: Michael Trimarchi @ 2015-11-21  9:37 UTC (permalink / raw)
  To: Raphael Poggi
  Cc: perex-/Fr2/VpizcU, tiwai-IBi9RG/b67k,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, LKML

Hi

On Fri, Nov 20, 2015 at 11:09 AM, Raphael Poggi <poggi.raph-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Raphael Poggi <poggi.raph-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Add possibility to choose the channel side using the device tree,
> and also modify it using alsa ctrl.
>
> Signed-off-by: Raphael Poggi <poggi.raph-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  sound/soc/codecs/pcm1792a.c | 46 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>
> diff --git a/sound/soc/codecs/pcm1792a.c b/sound/soc/codecs/pcm1792a.c
> index febaa48..005b679 100644
> --- a/sound/soc/codecs/pcm1792a.c
> +++ b/sound/soc/codecs/pcm1792a.c
> @@ -65,6 +65,12 @@ static const struct reg_default pcm1792a_reg_defaults[] = {
>         { PCM1792A_DEVICE_ID, 0x00 },
>  };
>
> +enum __pcm1792_side {
> +       STEREO,
> +       MONORAL_LEFT,
> +       MONORAL_RIGHT
> +};
> +
>  static bool pcm1792a_accessible_reg(struct device *dev, unsigned int reg)
>  {
>         return reg >= 0x10 && reg <= 0x17;
> @@ -83,6 +89,7 @@ struct pcm1792a_private {
>         struct regmap *regmap;
>         unsigned int format;
>         unsigned int rate;
> +       unsigned int side;
>  };
>
>  static int pcm1792a_set_dai_fmt(struct snd_soc_dai *codec_dai,
> @@ -153,6 +160,13 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
>         ret = regmap_update_bits(priv->regmap, PCM1792A_FMT_CONTROL,
>                                  PCM1792A_FMT_MASK | PCM1792A_ATLD_ENABLE, val);
>
> +       if (priv->side == MONORAL_LEFT)
> +               val = PCM1792A_CHANNEL_MONO_LEFT;
> +       else if (priv->side == MONORAL_RIGHT)
> +               val = PCM1792A_CHANNEL_MONO_RIGHT;
> +
> +       regmap_update_bits(priv->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, val);
> +
>         return ret;
>  }
>
> @@ -222,10 +236,31 @@ static struct snd_soc_codec_driver soc_codec_dev_pcm1792a = {
>         .num_dapm_routes        = ARRAY_SIZE(pcm1792a_dapm_routes),
>  };
>
> +static int pcm1792a_of_init(struct spi_device *spi)
> +{
> +       int ret = 0;
> +       struct pcm1792a_private *pcm1792a = dev_get_drvdata(&spi->dev);
> +       const char *name;
> +       struct device_node *np = spi->dev.of_node;
> +
> +       name = of_get_property(np, "ti,side", NULL);
> +       if (name) {
> +               if (!strcmp(name, "left"))
> +                       pcm1792a->side = MONORAL_LEFT;
> +               else
> +                       pcm1792a->side = MONORAL_RIGHT;
> +       }
> +       else
> +               pcm1792a->side = STEREO;
> +
> +       return ret;
> +}
> +
>  static int pcm1792a_spi_probe(struct spi_device *spi)
>  {
>         struct pcm1792a_private *pcm1792a;
>         int ret;
> +       struct device_node *np = spi->dev.of_node;
>
>         pcm1792a = devm_kzalloc(&spi->dev, sizeof(struct pcm1792a_private),
>                                 GFP_KERNEL);
> @@ -241,6 +276,17 @@ static int pcm1792a_spi_probe(struct spi_device *spi)
>                 return ret;
>         }
>
> +       if (np) {
> +               ret = pcm1792a_of_init(spi);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       if (pcm1792a->side == MONORAL_LEFT)
> +               regmap_update_bits(pcm1792a->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, PCM1792A_CHANNEL_MONO_LEFT);
> +       else if (pcm1792a->side == MONORAL_RIGHT)
> +               regmap_update_bits(pcm1792a->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, PCM1792A_CHANNEL_MONO_RIGHT);
> +

Why this is a part of dts and not a part of mixer control?

Michael

>         return snd_soc_register_codec(&spi->dev,
>                         &soc_codec_dev_pcm1792a, &pcm1792a_dai, 1);
>  }
> --
> 2.1.0
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel



-- 
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [alsa-devel] [PATCH 1/4] sound: codecs: pcm1792a: clean function exit
  2015-11-21  9:31     ` Michael Trimarchi
  (?)
@ 2015-11-21 16:16     ` Raphaël Poggi
  2015-11-21 16:33         ` Michael Trimarchi
  -1 siblings, 1 reply; 23+ messages in thread
From: Raphaël Poggi @ 2015-11-21 16:16 UTC (permalink / raw)
  To: Michael Trimarchi; +Cc: perex, tiwai, devicetree, alsa-devel, LKML

Hi,

I know that almost all the codec do as before, but what is the goal of
using an extra condition when we can avoid it ?

Raphael

2015-11-21 10:31 GMT+01:00 Michael Trimarchi <michael@amarulasolutions.com>:
> Hi
>
> On Fri, Nov 20, 2015 at 11:09 AM, Raphael Poggi <poggi.raph@gmail.com> wrote:
>> From: Raphael Poggi <poggi.raph@gmail.com>
>>
>> Signed-off-by: Raphael Poggi <poggi.raph@gmail.com>
>> ---
>>  sound/soc/codecs/pcm1792a.c | 14 ++++----------
>>  1 file changed, 4 insertions(+), 10 deletions(-)
>>
>> diff --git a/sound/soc/codecs/pcm1792a.c b/sound/soc/codecs/pcm1792a.c
>> index 08bb486..cf42b24 100644
>> --- a/sound/soc/codecs/pcm1792a.c
>> +++ b/sound/soc/codecs/pcm1792a.c
>> @@ -91,14 +91,9 @@ static int pcm1792a_digital_mute(struct snd_soc_dai *dai, int mute)
>>  {
>>         struct snd_soc_codec *codec = dai->codec;
>>         struct pcm1792a_private *priv = snd_soc_codec_get_drvdata(codec);
>> -       int ret;
>>
>> -       ret = regmap_update_bits(priv->regmap, PCM1792A_SOFT_MUTE,
>> +       return regmap_update_bits(priv->regmap, PCM1792A_SOFT_MUTE,
>>                                  PCM1792A_MUTE_MASK, !!mute);
>> -       if (ret < 0)
>> -               return ret;
>> -
>
> Ok, I have seen that almost all the codecs use return regmap_upda...
>
>> -       return 0;
>>  }
>>
>>  static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
>> @@ -107,7 +102,8 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
>>  {
>>         struct snd_soc_codec *codec = dai->codec;
>>         struct pcm1792a_private *priv = snd_soc_codec_get_drvdata(codec);
>> -       int val = 0, ret;
>> +       int val = 0;
>> +       int ret = 0;
>>
>>         priv->rate = params_rate(params);
>>
>> @@ -147,10 +143,8 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
>>
>>         ret = regmap_update_bits(priv->regmap, PCM1792A_FMT_CONTROL,
>>                                  PCM1792A_FMT_MASK | PCM1792A_ATLD_ENABLE, val);
>> -       if (ret < 0)
>> -               return ret;
>>
>
> Please do the same as before
>
> Michael
>
>
>> -       return 0;
>> +       return ret;
>>  }
>>
>>  static const struct snd_soc_dai_ops pcm1792a_dai_ops = {
>> --
>> 2.1.0
>>
>> _______________________________________________
>> Alsa-devel mailing list
>> Alsa-devel@alsa-project.org
>> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
>
>
> --
> | Michael Nazzareno Trimarchi                     Amarula Solutions BV |
> | COO  -  Founder                                      Cruquiuskade 47 |
> | +31(0)851119172                                 Amsterdam 1018 AM NL |
> |                  [`as] http://www.amarulasolutions.com               |
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [alsa-devel] [PATCH 1/4] sound: codecs: pcm1792a: clean function exit
  2015-11-21 16:16     ` Raphaël Poggi
@ 2015-11-21 16:33         ` Michael Trimarchi
  0 siblings, 0 replies; 23+ messages in thread
From: Michael Trimarchi @ 2015-11-21 16:33 UTC (permalink / raw)
  To: Raphaël Poggi; +Cc: perex, tiwai, devicetree, alsa-devel, LKML

Hi

On Sat, Nov 21, 2015 at 05:16:53PM +0100, Raphaël Poggi wrote:
> Hi,
> 
> I know that almost all the codec do as before, but what is the goal of
> using an extra condition when we can avoid it ?
> 

If you use top posting I can not follow. I said that I'm agree on the
first one chage and I suggest to do the same for the next one.

Michael

> Raphael
> 
> 2015-11-21 10:31 GMT+01:00 Michael Trimarchi <michael@amarulasolutions.com>:
> > Hi
> >
> > On Fri, Nov 20, 2015 at 11:09 AM, Raphael Poggi <poggi.raph@gmail.com> wrote:
> >> From: Raphael Poggi <poggi.raph@gmail.com>
> >>
> >> Signed-off-by: Raphael Poggi <poggi.raph@gmail.com>
> >> ---
> >>  sound/soc/codecs/pcm1792a.c | 14 ++++----------
> >>  1 file changed, 4 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/sound/soc/codecs/pcm1792a.c b/sound/soc/codecs/pcm1792a.c
> >> index 08bb486..cf42b24 100644
> >> --- a/sound/soc/codecs/pcm1792a.c
> >> +++ b/sound/soc/codecs/pcm1792a.c
> >> @@ -91,14 +91,9 @@ static int pcm1792a_digital_mute(struct snd_soc_dai *dai, int mute)
> >>  {
> >>         struct snd_soc_codec *codec = dai->codec;
> >>         struct pcm1792a_private *priv = snd_soc_codec_get_drvdata(codec);
> >> -       int ret;
> >>
> >> -       ret = regmap_update_bits(priv->regmap, PCM1792A_SOFT_MUTE,
> >> +       return regmap_update_bits(priv->regmap, PCM1792A_SOFT_MUTE,
> >>                                  PCM1792A_MUTE_MASK, !!mute);
> >> -       if (ret < 0)
> >> -               return ret;
> >> -
> >
> > Ok, I have seen that almost all the codecs use return regmap_upda...
> >
> >> -       return 0;
> >>  }
> >>
> >>  static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
> >> @@ -107,7 +102,8 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
> >>  {
> >>         struct snd_soc_codec *codec = dai->codec;
> >>         struct pcm1792a_private *priv = snd_soc_codec_get_drvdata(codec);
> >> -       int val = 0, ret;
> >> +       int val = 0;
> >> +       int ret = 0;
> >>
> >>         priv->rate = params_rate(params);
> >>
> >> @@ -147,10 +143,8 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
> >>
> >>         ret = regmap_update_bits(priv->regmap, PCM1792A_FMT_CONTROL,
> >>                                  PCM1792A_FMT_MASK | PCM1792A_ATLD_ENABLE, val);
> >> -       if (ret < 0)
> >> -               return ret;
> >>
> >
> > Please do the same as before
> >
> > Michael
> >
> >
> >> -       return 0;
> >> +       return ret;
> >>  }
> >>
> >>  static const struct snd_soc_dai_ops pcm1792a_dai_ops = {
> >> --
> >> 2.1.0
> >>
> >> _______________________________________________
> >> Alsa-devel mailing list
> >> Alsa-devel@alsa-project.org
> >> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> >
> >
> >
> > --
> > | Michael Nazzareno Trimarchi                     Amarula Solutions BV |
> > | COO  -  Founder                                      Cruquiuskade 47 |
> > | +31(0)851119172                                 Amsterdam 1018 AM NL |
> > |                  [`as] http://www.amarulasolutions.com               |
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/

-- 
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/4] sound: codecs: pcm1792a: clean function exit
@ 2015-11-21 16:33         ` Michael Trimarchi
  0 siblings, 0 replies; 23+ messages in thread
From: Michael Trimarchi @ 2015-11-21 16:33 UTC (permalink / raw)
  To: Raphaël Poggi; +Cc: devicetree, alsa-devel, tiwai, LKML

Hi

On Sat, Nov 21, 2015 at 05:16:53PM +0100, Raphaël Poggi wrote:
> Hi,
> 
> I know that almost all the codec do as before, but what is the goal of
> using an extra condition when we can avoid it ?
> 

If you use top posting I can not follow. I said that I'm agree on the
first one chage and I suggest to do the same for the next one.

Michael

> Raphael
> 
> 2015-11-21 10:31 GMT+01:00 Michael Trimarchi <michael@amarulasolutions.com>:
> > Hi
> >
> > On Fri, Nov 20, 2015 at 11:09 AM, Raphael Poggi <poggi.raph@gmail.com> wrote:
> >> From: Raphael Poggi <poggi.raph@gmail.com>
> >>
> >> Signed-off-by: Raphael Poggi <poggi.raph@gmail.com>
> >> ---
> >>  sound/soc/codecs/pcm1792a.c | 14 ++++----------
> >>  1 file changed, 4 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/sound/soc/codecs/pcm1792a.c b/sound/soc/codecs/pcm1792a.c
> >> index 08bb486..cf42b24 100644
> >> --- a/sound/soc/codecs/pcm1792a.c
> >> +++ b/sound/soc/codecs/pcm1792a.c
> >> @@ -91,14 +91,9 @@ static int pcm1792a_digital_mute(struct snd_soc_dai *dai, int mute)
> >>  {
> >>         struct snd_soc_codec *codec = dai->codec;
> >>         struct pcm1792a_private *priv = snd_soc_codec_get_drvdata(codec);
> >> -       int ret;
> >>
> >> -       ret = regmap_update_bits(priv->regmap, PCM1792A_SOFT_MUTE,
> >> +       return regmap_update_bits(priv->regmap, PCM1792A_SOFT_MUTE,
> >>                                  PCM1792A_MUTE_MASK, !!mute);
> >> -       if (ret < 0)
> >> -               return ret;
> >> -
> >
> > Ok, I have seen that almost all the codecs use return regmap_upda...
> >
> >> -       return 0;
> >>  }
> >>
> >>  static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
> >> @@ -107,7 +102,8 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
> >>  {
> >>         struct snd_soc_codec *codec = dai->codec;
> >>         struct pcm1792a_private *priv = snd_soc_codec_get_drvdata(codec);
> >> -       int val = 0, ret;
> >> +       int val = 0;
> >> +       int ret = 0;
> >>
> >>         priv->rate = params_rate(params);
> >>
> >> @@ -147,10 +143,8 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
> >>
> >>         ret = regmap_update_bits(priv->regmap, PCM1792A_FMT_CONTROL,
> >>                                  PCM1792A_FMT_MASK | PCM1792A_ATLD_ENABLE, val);
> >> -       if (ret < 0)
> >> -               return ret;
> >>
> >
> > Please do the same as before
> >
> > Michael
> >
> >
> >> -       return 0;
> >> +       return ret;
> >>  }
> >>
> >>  static const struct snd_soc_dai_ops pcm1792a_dai_ops = {
> >> --
> >> 2.1.0
> >>
> >> _______________________________________________
> >> Alsa-devel mailing list
> >> Alsa-devel@alsa-project.org
> >> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> >
> >
> >
> > --
> > | Michael Nazzareno Trimarchi                     Amarula Solutions BV |
> > | COO  -  Founder                                      Cruquiuskade 47 |
> > | +31(0)851119172                                 Amsterdam 1018 AM NL |
> > |                  [`as] http://www.amarulasolutions.com               |
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/

-- 
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [alsa-devel] [PATCH 4/4] sound: codecs: pcm1792a: introduce channel side
  2015-11-21  9:37     ` Michael Trimarchi
  (?)
@ 2015-11-24  8:21     ` Raphaël Poggi
  2015-11-24  8:27       ` Michael Trimarchi
  -1 siblings, 1 reply; 23+ messages in thread
From: Raphaël Poggi @ 2015-11-24  8:21 UTC (permalink / raw)
  To: Michael Trimarchi; +Cc: perex, tiwai, devicetree, alsa-devel, LKML

Hi,

2015-11-21 10:37 GMT+01:00 Michael Trimarchi <michael@amarulasolutions.com>:
> Hi
>
> On Fri, Nov 20, 2015 at 11:09 AM, Raphael Poggi <poggi.raph@gmail.com> wrote:
>> From: Raphael Poggi <poggi.raph@gmail.com>
>>
>> Add possibility to choose the channel side using the device tree,
>> and also modify it using alsa ctrl.
>>
>> Signed-off-by: Raphael Poggi <poggi.raph@gmail.com>
>> ---
>>  sound/soc/codecs/pcm1792a.c | 46 +++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 46 insertions(+)
>>
>> diff --git a/sound/soc/codecs/pcm1792a.c b/sound/soc/codecs/pcm1792a.c
>> index febaa48..005b679 100644
>> --- a/sound/soc/codecs/pcm1792a.c
>> +++ b/sound/soc/codecs/pcm1792a.c
>> @@ -65,6 +65,12 @@ static const struct reg_default pcm1792a_reg_defaults[] = {
>>         { PCM1792A_DEVICE_ID, 0x00 },
>>  };
>>
>> +enum __pcm1792_side {
>> +       STEREO,
>> +       MONORAL_LEFT,
>> +       MONORAL_RIGHT
>> +};
>> +
>>  static bool pcm1792a_accessible_reg(struct device *dev, unsigned int reg)
>>  {
>>         return reg >= 0x10 && reg <= 0x17;
>> @@ -83,6 +89,7 @@ struct pcm1792a_private {
>>         struct regmap *regmap;
>>         unsigned int format;
>>         unsigned int rate;
>> +       unsigned int side;
>>  };
>>
>>  static int pcm1792a_set_dai_fmt(struct snd_soc_dai *codec_dai,
>> @@ -153,6 +160,13 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
>>         ret = regmap_update_bits(priv->regmap, PCM1792A_FMT_CONTROL,
>>                                  PCM1792A_FMT_MASK | PCM1792A_ATLD_ENABLE, val);
>>
>> +       if (priv->side == MONORAL_LEFT)
>> +               val = PCM1792A_CHANNEL_MONO_LEFT;
>> +       else if (priv->side == MONORAL_RIGHT)
>> +               val = PCM1792A_CHANNEL_MONO_RIGHT;
>> +
>> +       regmap_update_bits(priv->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, val);
>> +
>>         return ret;
>>  }
>>
>> @@ -222,10 +236,31 @@ static struct snd_soc_codec_driver soc_codec_dev_pcm1792a = {
>>         .num_dapm_routes        = ARRAY_SIZE(pcm1792a_dapm_routes),
>>  };
>>
>> +static int pcm1792a_of_init(struct spi_device *spi)
>> +{
>> +       int ret = 0;
>> +       struct pcm1792a_private *pcm1792a = dev_get_drvdata(&spi->dev);
>> +       const char *name;
>> +       struct device_node *np = spi->dev.of_node;
>> +
>> +       name = of_get_property(np, "ti,side", NULL);
>> +       if (name) {
>> +               if (!strcmp(name, "left"))
>> +                       pcm1792a->side = MONORAL_LEFT;
>> +               else
>> +                       pcm1792a->side = MONORAL_RIGHT;
>> +       }
>> +       else
>> +               pcm1792a->side = STEREO;
>> +
>> +       return ret;
>> +}
>> +
>>  static int pcm1792a_spi_probe(struct spi_device *spi)
>>  {
>>         struct pcm1792a_private *pcm1792a;
>>         int ret;
>> +       struct device_node *np = spi->dev.of_node;
>>
>>         pcm1792a = devm_kzalloc(&spi->dev, sizeof(struct pcm1792a_private),
>>                                 GFP_KERNEL);
>> @@ -241,6 +276,17 @@ static int pcm1792a_spi_probe(struct spi_device *spi)
>>                 return ret;
>>         }
>>
>> +       if (np) {
>> +               ret = pcm1792a_of_init(spi);
>> +               if (ret)
>> +                       return ret;
>> +       }
>> +
>> +       if (pcm1792a->side == MONORAL_LEFT)
>> +               regmap_update_bits(pcm1792a->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, PCM1792A_CHANNEL_MONO_LEFT);
>> +       else if (pcm1792a->side == MONORAL_RIGHT)
>> +               regmap_update_bits(pcm1792a->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, PCM1792A_CHANNEL_MONO_RIGHT);
>> +
>
> Why this is a part of dts and not a part of mixer control?
>
> Michael
>

The "channel side" bindings should be used when the pcm7192a is used
in monaural mode (which requires two DACs, both DACs operate in a
balanced mode for one channel of audio input data).
This is a hardware level design, it will never change at runtime.
I feel that a device tree binding is more appropriate than a control.

>>         return snd_soc_register_codec(&spi->dev,
>>                         &soc_codec_dev_pcm1792a, &pcm1792a_dai, 1);
>>  }
>> --
>> 2.1.0
>>
>> _______________________________________________
>> Alsa-devel mailing list
>> Alsa-devel@alsa-project.org
>> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
>
>
> --
> | Michael Nazzareno Trimarchi                     Amarula Solutions BV |
> | COO  -  Founder                                      Cruquiuskade 47 |
> | +31(0)851119172                                 Amsterdam 1018 AM NL |
> |                  [`as] http://www.amarulasolutions.com               |
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [alsa-devel] [PATCH 4/4] sound: codecs: pcm1792a: introduce channel side
  2015-11-24  8:21     ` Raphaël Poggi
@ 2015-11-24  8:27       ` Michael Trimarchi
  2015-11-24 16:04           ` Raphaël Poggi
  0 siblings, 1 reply; 23+ messages in thread
From: Michael Trimarchi @ 2015-11-24  8:27 UTC (permalink / raw)
  To: Raphaël Poggi; +Cc: perex, tiwai, devicetree, alsa-devel, LKML

Hi

On Tue, Nov 24, 2015 at 9:21 AM, Raphaël Poggi <poggi.raph@gmail.com> wrote:
> Hi,
>
> 2015-11-21 10:37 GMT+01:00 Michael Trimarchi <michael@amarulasolutions.com>:
>> Hi
>>
>> On Fri, Nov 20, 2015 at 11:09 AM, Raphael Poggi <poggi.raph@gmail.com> wrote:
>>> From: Raphael Poggi <poggi.raph@gmail.com>
>>>
>>> Add possibility to choose the channel side using the device tree,
>>> and also modify it using alsa ctrl.
>>>
>>> Signed-off-by: Raphael Poggi <poggi.raph@gmail.com>
>>> ---
>>>  sound/soc/codecs/pcm1792a.c | 46 +++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 46 insertions(+)
>>>
>>> diff --git a/sound/soc/codecs/pcm1792a.c b/sound/soc/codecs/pcm1792a.c
>>> index febaa48..005b679 100644
>>> --- a/sound/soc/codecs/pcm1792a.c
>>> +++ b/sound/soc/codecs/pcm1792a.c
>>> @@ -65,6 +65,12 @@ static const struct reg_default pcm1792a_reg_defaults[] = {
>>>         { PCM1792A_DEVICE_ID, 0x00 },
>>>  };
>>>
>>> +enum __pcm1792_side {
>>> +       STEREO,
>>> +       MONORAL_LEFT,
>>> +       MONORAL_RIGHT
>>> +};
>>> +
>>>  static bool pcm1792a_accessible_reg(struct device *dev, unsigned int reg)
>>>  {
>>>         return reg >= 0x10 && reg <= 0x17;
>>> @@ -83,6 +89,7 @@ struct pcm1792a_private {
>>>         struct regmap *regmap;
>>>         unsigned int format;
>>>         unsigned int rate;
>>> +       unsigned int side;
>>>  };
>>>
>>>  static int pcm1792a_set_dai_fmt(struct snd_soc_dai *codec_dai,
>>> @@ -153,6 +160,13 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
>>>         ret = regmap_update_bits(priv->regmap, PCM1792A_FMT_CONTROL,
>>>                                  PCM1792A_FMT_MASK | PCM1792A_ATLD_ENABLE, val);
>>>
>>> +       if (priv->side == MONORAL_LEFT)
>>> +               val = PCM1792A_CHANNEL_MONO_LEFT;
>>> +       else if (priv->side == MONORAL_RIGHT)
>>> +               val = PCM1792A_CHANNEL_MONO_RIGHT;
>>> +
>>> +       regmap_update_bits(priv->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, val);
>>> +
>>>         return ret;
>>>  }
>>>
>>> @@ -222,10 +236,31 @@ static struct snd_soc_codec_driver soc_codec_dev_pcm1792a = {
>>>         .num_dapm_routes        = ARRAY_SIZE(pcm1792a_dapm_routes),
>>>  };
>>>
>>> +static int pcm1792a_of_init(struct spi_device *spi)
>>> +{
>>> +       int ret = 0;
>>> +       struct pcm1792a_private *pcm1792a = dev_get_drvdata(&spi->dev);
>>> +       const char *name;
>>> +       struct device_node *np = spi->dev.of_node;
>>> +
>>> +       name = of_get_property(np, "ti,side", NULL);
>>> +       if (name) {
>>> +               if (!strcmp(name, "left"))
>>> +                       pcm1792a->side = MONORAL_LEFT;
>>> +               else
>>> +                       pcm1792a->side = MONORAL_RIGHT;
>>> +       }
>>> +       else
>>> +               pcm1792a->side = STEREO;
>>> +
>>> +       return ret;
>>> +}
>>> +
>>>  static int pcm1792a_spi_probe(struct spi_device *spi)
>>>  {
>>>         struct pcm1792a_private *pcm1792a;
>>>         int ret;
>>> +       struct device_node *np = spi->dev.of_node;
>>>
>>>         pcm1792a = devm_kzalloc(&spi->dev, sizeof(struct pcm1792a_private),
>>>                                 GFP_KERNEL);
>>> @@ -241,6 +276,17 @@ static int pcm1792a_spi_probe(struct spi_device *spi)
>>>                 return ret;
>>>         }
>>>
>>> +       if (np) {
>>> +               ret = pcm1792a_of_init(spi);
>>> +               if (ret)
>>> +                       return ret;
>>> +       }
>>> +
>>> +       if (pcm1792a->side == MONORAL_LEFT)
>>> +               regmap_update_bits(pcm1792a->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, PCM1792A_CHANNEL_MONO_LEFT);
>>> +       else if (pcm1792a->side == MONORAL_RIGHT)
>>> +               regmap_update_bits(pcm1792a->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, PCM1792A_CHANNEL_MONO_RIGHT);
>>> +
>>
>> Why this is a part of dts and not a part of mixer control?
>>
>> Michael
>>
>
> The "channel side" bindings should be used when the pcm7192a is used
> in monaural mode (which requires two DACs, both DACs operate in a
> balanced mode for one channel of audio input data).
> This is a hardware level design, it will never change at runtime.
> I feel that a device tree binding is more appropriate than a control.
>

Well I know exactly why is used for, but I don't really know if it's
somenthing that should be
in dts. Do I want to do channel inversion?

Michael

>>>         return snd_soc_register_codec(&spi->dev,
>>>                         &soc_codec_dev_pcm1792a, &pcm1792a_dai, 1);
>>>  }
>>> --
>>> 2.1.0
>>>
>>> _______________________________________________
>>> Alsa-devel mailing list
>>> Alsa-devel@alsa-project.org
>>> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>>
>>
>>
>> --
>> | Michael Nazzareno Trimarchi                     Amarula Solutions BV |
>> | COO  -  Founder                                      Cruquiuskade 47 |
>> | +31(0)851119172                                 Amsterdam 1018 AM NL |
>> |                  [`as] http://www.amarulasolutions.com               |
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/



-- 
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [alsa-devel] [PATCH 4/4] sound: codecs: pcm1792a: introduce channel side
@ 2015-11-24 16:04           ` Raphaël Poggi
  0 siblings, 0 replies; 23+ messages in thread
From: Raphaël Poggi @ 2015-11-24 16:04 UTC (permalink / raw)
  To: Michael Trimarchi; +Cc: perex, tiwai, devicetree, alsa-devel, LKML

Hi,

2015-11-24 9:27 GMT+01:00 Michael Trimarchi <michael@amarulasolutions.com>:
> Hi
>
> On Tue, Nov 24, 2015 at 9:21 AM, Raphaël Poggi <poggi.raph@gmail.com> wrote:
>> Hi,
>>
>> 2015-11-21 10:37 GMT+01:00 Michael Trimarchi <michael@amarulasolutions.com>:
>>> Hi
>>>
>>> On Fri, Nov 20, 2015 at 11:09 AM, Raphael Poggi <poggi.raph@gmail.com> wrote:
>>>> From: Raphael Poggi <poggi.raph@gmail.com>
>>>>
>>>> Add possibility to choose the channel side using the device tree,
>>>> and also modify it using alsa ctrl.
>>>>
>>>> Signed-off-by: Raphael Poggi <poggi.raph@gmail.com>
>>>> ---
>>>>  sound/soc/codecs/pcm1792a.c | 46 +++++++++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 46 insertions(+)
>>>>
>>>> diff --git a/sound/soc/codecs/pcm1792a.c b/sound/soc/codecs/pcm1792a.c
>>>> index febaa48..005b679 100644
>>>> --- a/sound/soc/codecs/pcm1792a.c
>>>> +++ b/sound/soc/codecs/pcm1792a.c
>>>> @@ -65,6 +65,12 @@ static const struct reg_default pcm1792a_reg_defaults[] = {
>>>>         { PCM1792A_DEVICE_ID, 0x00 },
>>>>  };
>>>>
>>>> +enum __pcm1792_side {
>>>> +       STEREO,
>>>> +       MONORAL_LEFT,
>>>> +       MONORAL_RIGHT
>>>> +};
>>>> +
>>>>  static bool pcm1792a_accessible_reg(struct device *dev, unsigned int reg)
>>>>  {
>>>>         return reg >= 0x10 && reg <= 0x17;
>>>> @@ -83,6 +89,7 @@ struct pcm1792a_private {
>>>>         struct regmap *regmap;
>>>>         unsigned int format;
>>>>         unsigned int rate;
>>>> +       unsigned int side;
>>>>  };
>>>>
>>>>  static int pcm1792a_set_dai_fmt(struct snd_soc_dai *codec_dai,
>>>> @@ -153,6 +160,13 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
>>>>         ret = regmap_update_bits(priv->regmap, PCM1792A_FMT_CONTROL,
>>>>                                  PCM1792A_FMT_MASK | PCM1792A_ATLD_ENABLE, val);
>>>>
>>>> +       if (priv->side == MONORAL_LEFT)
>>>> +               val = PCM1792A_CHANNEL_MONO_LEFT;
>>>> +       else if (priv->side == MONORAL_RIGHT)
>>>> +               val = PCM1792A_CHANNEL_MONO_RIGHT;
>>>> +
>>>> +       regmap_update_bits(priv->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, val);
>>>> +
>>>>         return ret;
>>>>  }
>>>>
>>>> @@ -222,10 +236,31 @@ static struct snd_soc_codec_driver soc_codec_dev_pcm1792a = {
>>>>         .num_dapm_routes        = ARRAY_SIZE(pcm1792a_dapm_routes),
>>>>  };
>>>>
>>>> +static int pcm1792a_of_init(struct spi_device *spi)
>>>> +{
>>>> +       int ret = 0;
>>>> +       struct pcm1792a_private *pcm1792a = dev_get_drvdata(&spi->dev);
>>>> +       const char *name;
>>>> +       struct device_node *np = spi->dev.of_node;
>>>> +
>>>> +       name = of_get_property(np, "ti,side", NULL);
>>>> +       if (name) {
>>>> +               if (!strcmp(name, "left"))
>>>> +                       pcm1792a->side = MONORAL_LEFT;
>>>> +               else
>>>> +                       pcm1792a->side = MONORAL_RIGHT;
>>>> +       }
>>>> +       else
>>>> +               pcm1792a->side = STEREO;
>>>> +
>>>> +       return ret;
>>>> +}
>>>> +
>>>>  static int pcm1792a_spi_probe(struct spi_device *spi)
>>>>  {
>>>>         struct pcm1792a_private *pcm1792a;
>>>>         int ret;
>>>> +       struct device_node *np = spi->dev.of_node;
>>>>
>>>>         pcm1792a = devm_kzalloc(&spi->dev, sizeof(struct pcm1792a_private),
>>>>                                 GFP_KERNEL);
>>>> @@ -241,6 +276,17 @@ static int pcm1792a_spi_probe(struct spi_device *spi)
>>>>                 return ret;
>>>>         }
>>>>
>>>> +       if (np) {
>>>> +               ret = pcm1792a_of_init(spi);
>>>> +               if (ret)
>>>> +                       return ret;
>>>> +       }
>>>> +
>>>> +       if (pcm1792a->side == MONORAL_LEFT)
>>>> +               regmap_update_bits(pcm1792a->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, PCM1792A_CHANNEL_MONO_LEFT);
>>>> +       else if (pcm1792a->side == MONORAL_RIGHT)
>>>> +               regmap_update_bits(pcm1792a->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, PCM1792A_CHANNEL_MONO_RIGHT);
>>>> +
>>>
>>> Why this is a part of dts and not a part of mixer control?
>>>
>>> Michael
>>>
>>
>> The "channel side" bindings should be used when the pcm7192a is used
>> in monaural mode (which requires two DACs, both DACs operate in a
>> balanced mode for one channel of audio input data).
>> This is a hardware level design, it will never change at runtime.
>> I feel that a device tree binding is more appropriate than a control.
>>
>
> Well I know exactly why is used for, but I don't really know if it's
> somenthing that should be
> in dts. Do I want to do channel inversion?
>
> Michael
>

I am not sure to understand. You mean "What if I want to do channel
inversion", right ?
If so, we have two differents "problems", one at hardware level design
because monaural mode requires two DACs, and an another one at
software level
"channel inversion".
I have always thought that hardware description should be in dts,
that's why I implement monaural mode using dts.

Honestly, I don't know what is the best solution between "this is a
hardware design, it should be in dts and shouldn't change" vs "this
should be an alsa controler because we want extra features independent
from hardware design"

Raphaël

>>>>         return snd_soc_register_codec(&spi->dev,
>>>>                         &soc_codec_dev_pcm1792a, &pcm1792a_dai, 1);
>>>>  }
>>>> --
>>>> 2.1.0
>>>>
>>>> _______________________________________________
>>>> Alsa-devel mailing list
>>>> Alsa-devel@alsa-project.org
>>>> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>>>
>>>
>>>
>>> --
>>> | Michael Nazzareno Trimarchi                     Amarula Solutions BV |
>>> | COO  -  Founder                                      Cruquiuskade 47 |
>>> | +31(0)851119172                                 Amsterdam 1018 AM NL |
>>> |                  [`as] http://www.amarulasolutions.com               |
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at  http://www.tux.org/lkml/
>
>
>
> --
> | Michael Nazzareno Trimarchi                     Amarula Solutions BV |
> | COO  -  Founder                                      Cruquiuskade 47 |
> | +31(0)851119172                                 Amsterdam 1018 AM NL |
> |                  [`as] http://www.amarulasolutions.com               |
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [alsa-devel] [PATCH 4/4] sound: codecs: pcm1792a: introduce channel side
@ 2015-11-24 16:04           ` Raphaël Poggi
  0 siblings, 0 replies; 23+ messages in thread
From: Raphaël Poggi @ 2015-11-24 16:04 UTC (permalink / raw)
  To: Michael Trimarchi
  Cc: perex-/Fr2/VpizcU, tiwai-IBi9RG/b67k,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, LKML

Hi,

2015-11-24 9:27 GMT+01:00 Michael Trimarchi <michael@amarulasolutions.com>:
> Hi
>
> On Tue, Nov 24, 2015 at 9:21 AM, Raphaël Poggi <poggi.raph@gmail.com> wrote:
>> Hi,
>>
>> 2015-11-21 10:37 GMT+01:00 Michael Trimarchi <michael@amarulasolutions.com>:
>>> Hi
>>>
>>> On Fri, Nov 20, 2015 at 11:09 AM, Raphael Poggi <poggi.raph@gmail.com> wrote:
>>>> From: Raphael Poggi <poggi.raph-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>
>>>> Add possibility to choose the channel side using the device tree,
>>>> and also modify it using alsa ctrl.
>>>>
>>>> Signed-off-by: Raphael Poggi <poggi.raph-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> ---
>>>>  sound/soc/codecs/pcm1792a.c | 46 +++++++++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 46 insertions(+)
>>>>
>>>> diff --git a/sound/soc/codecs/pcm1792a.c b/sound/soc/codecs/pcm1792a.c
>>>> index febaa48..005b679 100644
>>>> --- a/sound/soc/codecs/pcm1792a.c
>>>> +++ b/sound/soc/codecs/pcm1792a.c
>>>> @@ -65,6 +65,12 @@ static const struct reg_default pcm1792a_reg_defaults[] = {
>>>>         { PCM1792A_DEVICE_ID, 0x00 },
>>>>  };
>>>>
>>>> +enum __pcm1792_side {
>>>> +       STEREO,
>>>> +       MONORAL_LEFT,
>>>> +       MONORAL_RIGHT
>>>> +};
>>>> +
>>>>  static bool pcm1792a_accessible_reg(struct device *dev, unsigned int reg)
>>>>  {
>>>>         return reg >= 0x10 && reg <= 0x17;
>>>> @@ -83,6 +89,7 @@ struct pcm1792a_private {
>>>>         struct regmap *regmap;
>>>>         unsigned int format;
>>>>         unsigned int rate;
>>>> +       unsigned int side;
>>>>  };
>>>>
>>>>  static int pcm1792a_set_dai_fmt(struct snd_soc_dai *codec_dai,
>>>> @@ -153,6 +160,13 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
>>>>         ret = regmap_update_bits(priv->regmap, PCM1792A_FMT_CONTROL,
>>>>                                  PCM1792A_FMT_MASK | PCM1792A_ATLD_ENABLE, val);
>>>>
>>>> +       if (priv->side == MONORAL_LEFT)
>>>> +               val = PCM1792A_CHANNEL_MONO_LEFT;
>>>> +       else if (priv->side == MONORAL_RIGHT)
>>>> +               val = PCM1792A_CHANNEL_MONO_RIGHT;
>>>> +
>>>> +       regmap_update_bits(priv->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, val);
>>>> +
>>>>         return ret;
>>>>  }
>>>>
>>>> @@ -222,10 +236,31 @@ static struct snd_soc_codec_driver soc_codec_dev_pcm1792a = {
>>>>         .num_dapm_routes        = ARRAY_SIZE(pcm1792a_dapm_routes),
>>>>  };
>>>>
>>>> +static int pcm1792a_of_init(struct spi_device *spi)
>>>> +{
>>>> +       int ret = 0;
>>>> +       struct pcm1792a_private *pcm1792a = dev_get_drvdata(&spi->dev);
>>>> +       const char *name;
>>>> +       struct device_node *np = spi->dev.of_node;
>>>> +
>>>> +       name = of_get_property(np, "ti,side", NULL);
>>>> +       if (name) {
>>>> +               if (!strcmp(name, "left"))
>>>> +                       pcm1792a->side = MONORAL_LEFT;
>>>> +               else
>>>> +                       pcm1792a->side = MONORAL_RIGHT;
>>>> +       }
>>>> +       else
>>>> +               pcm1792a->side = STEREO;
>>>> +
>>>> +       return ret;
>>>> +}
>>>> +
>>>>  static int pcm1792a_spi_probe(struct spi_device *spi)
>>>>  {
>>>>         struct pcm1792a_private *pcm1792a;
>>>>         int ret;
>>>> +       struct device_node *np = spi->dev.of_node;
>>>>
>>>>         pcm1792a = devm_kzalloc(&spi->dev, sizeof(struct pcm1792a_private),
>>>>                                 GFP_KERNEL);
>>>> @@ -241,6 +276,17 @@ static int pcm1792a_spi_probe(struct spi_device *spi)
>>>>                 return ret;
>>>>         }
>>>>
>>>> +       if (np) {
>>>> +               ret = pcm1792a_of_init(spi);
>>>> +               if (ret)
>>>> +                       return ret;
>>>> +       }
>>>> +
>>>> +       if (pcm1792a->side == MONORAL_LEFT)
>>>> +               regmap_update_bits(pcm1792a->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, PCM1792A_CHANNEL_MONO_LEFT);
>>>> +       else if (pcm1792a->side == MONORAL_RIGHT)
>>>> +               regmap_update_bits(pcm1792a->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, PCM1792A_CHANNEL_MONO_RIGHT);
>>>> +
>>>
>>> Why this is a part of dts and not a part of mixer control?
>>>
>>> Michael
>>>
>>
>> The "channel side" bindings should be used when the pcm7192a is used
>> in monaural mode (which requires two DACs, both DACs operate in a
>> balanced mode for one channel of audio input data).
>> This is a hardware level design, it will never change at runtime.
>> I feel that a device tree binding is more appropriate than a control.
>>
>
> Well I know exactly why is used for, but I don't really know if it's
> somenthing that should be
> in dts. Do I want to do channel inversion?
>
> Michael
>

I am not sure to understand. You mean "What if I want to do channel
inversion", right ?
If so, we have two differents "problems", one at hardware level design
because monaural mode requires two DACs, and an another one at
software level
"channel inversion".
I have always thought that hardware description should be in dts,
that's why I implement monaural mode using dts.

Honestly, I don't know what is the best solution between "this is a
hardware design, it should be in dts and shouldn't change" vs "this
should be an alsa controler because we want extra features independent
from hardware design"

Raphaël

>>>>         return snd_soc_register_codec(&spi->dev,
>>>>                         &soc_codec_dev_pcm1792a, &pcm1792a_dai, 1);
>>>>  }
>>>> --
>>>> 2.1.0
>>>>
>>>> _______________________________________________
>>>> Alsa-devel mailing list
>>>> Alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org
>>>> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>>>
>>>
>>>
>>> --
>>> | Michael Nazzareno Trimarchi                     Amarula Solutions BV |
>>> | COO  -  Founder                                      Cruquiuskade 47 |
>>> | +31(0)851119172                                 Amsterdam 1018 AM NL |
>>> |                  [`as] http://www.amarulasolutions.com               |
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at  http://www.tux.org/lkml/
>
>
>
> --
> | Michael Nazzareno Trimarchi                     Amarula Solutions BV |
> | COO  -  Founder                                      Cruquiuskade 47 |
> | +31(0)851119172                                 Amsterdam 1018 AM NL |
> |                  [`as] http://www.amarulasolutions.com               |
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [alsa-devel] [PATCH 4/4] sound: codecs: pcm1792a: introduce channel side
  2015-11-24 16:04           ` Raphaël Poggi
  (?)
@ 2015-11-24 20:10           ` Michael Trimarchi
  -1 siblings, 0 replies; 23+ messages in thread
From: Michael Trimarchi @ 2015-11-24 20:10 UTC (permalink / raw)
  To: Raphaël Poggi; +Cc: perex, tiwai, devicetree, alsa-devel, LKML

Hi

On Tue, Nov 24, 2015 at 5:04 PM, Raphaël Poggi <poggi.raph@gmail.com> wrote:
> Hi,
>
> 2015-11-24 9:27 GMT+01:00 Michael Trimarchi <michael@amarulasolutions.com>:
>> Hi
>>
>> On Tue, Nov 24, 2015 at 9:21 AM, Raphaël Poggi <poggi.raph@gmail.com> wrote:
>>> Hi,
>>>
>>> 2015-11-21 10:37 GMT+01:00 Michael Trimarchi <michael@amarulasolutions.com>:
>>>> Hi
>>>>
>>>> On Fri, Nov 20, 2015 at 11:09 AM, Raphael Poggi <poggi.raph@gmail.com> wrote:
>>>>> From: Raphael Poggi <poggi.raph@gmail.com>
>>>>>
>>>>> Add possibility to choose the channel side using the device tree,
>>>>> and also modify it using alsa ctrl.
>>>>>
>>>>> Signed-off-by: Raphael Poggi <poggi.raph@gmail.com>
>>>>> ---
>>>>>  sound/soc/codecs/pcm1792a.c | 46 +++++++++++++++++++++++++++++++++++++++++++++
>>>>>  1 file changed, 46 insertions(+)
>>>>>
>>>>> diff --git a/sound/soc/codecs/pcm1792a.c b/sound/soc/codecs/pcm1792a.c
>>>>> index febaa48..005b679 100644
>>>>> --- a/sound/soc/codecs/pcm1792a.c
>>>>> +++ b/sound/soc/codecs/pcm1792a.c
>>>>> @@ -65,6 +65,12 @@ static const struct reg_default pcm1792a_reg_defaults[] = {
>>>>>         { PCM1792A_DEVICE_ID, 0x00 },
>>>>>  };
>>>>>
>>>>> +enum __pcm1792_side {
>>>>> +       STEREO,
>>>>> +       MONORAL_LEFT,
>>>>> +       MONORAL_RIGHT
>>>>> +};
>>>>> +
>>>>>  static bool pcm1792a_accessible_reg(struct device *dev, unsigned int reg)
>>>>>  {
>>>>>         return reg >= 0x10 && reg <= 0x17;
>>>>> @@ -83,6 +89,7 @@ struct pcm1792a_private {
>>>>>         struct regmap *regmap;
>>>>>         unsigned int format;
>>>>>         unsigned int rate;
>>>>> +       unsigned int side;
>>>>>  };
>>>>>
>>>>>  static int pcm1792a_set_dai_fmt(struct snd_soc_dai *codec_dai,
>>>>> @@ -153,6 +160,13 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream,
>>>>>         ret = regmap_update_bits(priv->regmap, PCM1792A_FMT_CONTROL,
>>>>>                                  PCM1792A_FMT_MASK | PCM1792A_ATLD_ENABLE, val);
>>>>>
>>>>> +       if (priv->side == MONORAL_LEFT)
>>>>> +               val = PCM1792A_CHANNEL_MONO_LEFT;
>>>>> +       else if (priv->side == MONORAL_RIGHT)
>>>>> +               val = PCM1792A_CHANNEL_MONO_RIGHT;
>>>>> +
>>>>> +       regmap_update_bits(priv->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, val);
>>>>> +
>>>>>         return ret;
>>>>>  }
>>>>>
>>>>> @@ -222,10 +236,31 @@ static struct snd_soc_codec_driver soc_codec_dev_pcm1792a = {
>>>>>         .num_dapm_routes        = ARRAY_SIZE(pcm1792a_dapm_routes),
>>>>>  };
>>>>>
>>>>> +static int pcm1792a_of_init(struct spi_device *spi)
>>>>> +{
>>>>> +       int ret = 0;
>>>>> +       struct pcm1792a_private *pcm1792a = dev_get_drvdata(&spi->dev);
>>>>> +       const char *name;
>>>>> +       struct device_node *np = spi->dev.of_node;
>>>>> +
>>>>> +       name = of_get_property(np, "ti,side", NULL);
>>>>> +       if (name) {
>>>>> +               if (!strcmp(name, "left"))
>>>>> +                       pcm1792a->side = MONORAL_LEFT;
>>>>> +               else
>>>>> +                       pcm1792a->side = MONORAL_RIGHT;
>>>>> +       }
>>>>> +       else
>>>>> +               pcm1792a->side = STEREO;
>>>>> +
>>>>> +       return ret;
>>>>> +}
>>>>> +
>>>>>  static int pcm1792a_spi_probe(struct spi_device *spi)
>>>>>  {
>>>>>         struct pcm1792a_private *pcm1792a;
>>>>>         int ret;
>>>>> +       struct device_node *np = spi->dev.of_node;
>>>>>
>>>>>         pcm1792a = devm_kzalloc(&spi->dev, sizeof(struct pcm1792a_private),
>>>>>                                 GFP_KERNEL);
>>>>> @@ -241,6 +276,17 @@ static int pcm1792a_spi_probe(struct spi_device *spi)
>>>>>                 return ret;
>>>>>         }
>>>>>
>>>>> +       if (np) {
>>>>> +               ret = pcm1792a_of_init(spi);
>>>>> +               if (ret)
>>>>> +                       return ret;
>>>>> +       }
>>>>> +
>>>>> +       if (pcm1792a->side == MONORAL_LEFT)
>>>>> +               regmap_update_bits(pcm1792a->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, PCM1792A_CHANNEL_MONO_LEFT);
>>>>> +       else if (pcm1792a->side == MONORAL_RIGHT)
>>>>> +               regmap_update_bits(pcm1792a->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, PCM1792A_CHANNEL_MONO_RIGHT);
>>>>> +
>>>>
>>>> Why this is a part of dts and not a part of mixer control?
>>>>
>>>> Michael
>>>>
>>>
>>> The "channel side" bindings should be used when the pcm7192a is used
>>> in monaural mode (which requires two DACs, both DACs operate in a
>>> balanced mode for one channel of audio input data).
>>> This is a hardware level design, it will never change at runtime.
>>> I feel that a device tree binding is more appropriate than a control.
>>>

If you put this in mixer control then you can avoid to have it in dts
because you not suppose to
boot playing music. Basically you can get the same result restore some
alsa config that match
your topology. Even if you have two mono amplifier you can have the
situation that you want to
invert channel left and right and this can be done easily just
reconfigure one as mono left and the other
as mono right or write your own sound card that do in one control,
change both together. I suggest
to wait some other opinions.

Michael

>>
>> Well I know exactly why is used for, but I don't really know if it's
>> somenthing that should be
>> in dts. Do I want to do channel inversion?
>>
>> Michael
>>
>
> I am not sure to understand. You mean "What if I want to do channel
> inversion", right ?
> If so, we have two differents "problems", one at hardware level design
> because monaural mode requires two DACs, and an another one at
> software level
> "channel inversion".
> I have always thought that hardware description should be in dts,
> that's why I implement monaural mode using dts.
>
> Honestly, I don't know what is the best solution between "this is a
> hardware design, it should be in dts and shouldn't change" vs "this
> should be an alsa controler because we want extra features independent
> from hardware design"
>
> Raphaël
>
>>>>>         return snd_soc_register_codec(&spi->dev,
>>>>>                         &soc_codec_dev_pcm1792a, &pcm1792a_dai, 1);
>>>>>  }
>>>>> --
>>>>> 2.1.0
>>>>>
>>>>> _______________________________________________
>>>>> Alsa-devel mailing list
>>>>> Alsa-devel@alsa-project.org
>>>>> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>>>>
>>>>
>>>>
>>>> --
>>>> | Michael Nazzareno Trimarchi                     Amarula Solutions BV |
>>>> | COO  -  Founder                                      Cruquiuskade 47 |
>>>> | +31(0)851119172                                 Amsterdam 1018 AM NL |
>>>> |                  [`as] http://www.amarulasolutions.com               |
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>> Please read the FAQ at  http://www.tux.org/lkml/
>>
>>
>>
>> --
>> | Michael Nazzareno Trimarchi                     Amarula Solutions BV |
>> | COO  -  Founder                                      Cruquiuskade 47 |
>> | +31(0)851119172                                 Amsterdam 1018 AM NL |
>> |                  [`as] http://www.amarulasolutions.com               |
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/



-- 
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2015-11-24 20:11 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-20 10:09 [PATCH 0/4] New feature and clean up for pcm1792a driver Raphael Poggi
2015-11-20 10:09 ` [PATCH 1/4] sound: codecs: pcm1792a: clean function exit Raphael Poggi
2015-11-20 10:09   ` Raphael Poggi
2015-11-21  9:31   ` [alsa-devel] " Michael Trimarchi
2015-11-21  9:31     ` Michael Trimarchi
2015-11-21 16:16     ` Raphaël Poggi
2015-11-21 16:33       ` Michael Trimarchi
2015-11-21 16:33         ` Michael Trimarchi
2015-11-20 10:09 ` [PATCH 2/4] sound: codecs: pcm1792a: add define for register Raphael Poggi
2015-11-21  9:36   ` [alsa-devel] " Michael Trimarchi
2015-11-20 10:09 ` [PATCH 3/4] dt-bindings: sound: add binding for pcm1792a Raphael Poggi
2015-11-20 10:09   ` Raphael Poggi
2015-11-20 14:42   ` Rob Herring
2015-11-20 15:04     ` [alsa-devel] " Michael Trimarchi
2015-11-20 15:04       ` Michael Trimarchi
2015-11-20 10:09 ` [PATCH 4/4] sound: codecs: pcm1792a: introduce channel side Raphael Poggi
2015-11-21  9:37   ` [alsa-devel] " Michael Trimarchi
2015-11-21  9:37     ` Michael Trimarchi
2015-11-24  8:21     ` Raphaël Poggi
2015-11-24  8:27       ` Michael Trimarchi
2015-11-24 16:04         ` Raphaël Poggi
2015-11-24 16:04           ` Raphaël Poggi
2015-11-24 20:10           ` Michael Trimarchi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.