[RFC 0/4] DT support for rx51-audio

[RFC 0/4] DT support for rx51-audio

[Sebastian Reichel]

Hi,

This patchset adds DT support in rx51-audio. I tested it on the Nokia N900
and was able to play sound with aplay using earphones and earspeaker. The
Loudspeakers did not work. I don't know the reason.

The patchset consists of 4 patches:

1. convert rx51-audio to snd_soc_register_card()
2. init audio in rx51 platform file
3. add some DT helpers to ASoC core
4. add DT support to rx51-audio

Please note, that the patch is RFC level for now. For example non DT boot
does not work, because I did not yet add pdata in boardcode.

Proposed DTS node would look like this:

sound: rx51-audio {
       compatible = "nokia,rx51-audio";

       ti,mcbsp = <&mcbsp2>;
       ti,codec = <&tlv320aic3x>;
       ti,codec_aux = <&tlv320aic3x_aux>;
       ti,headset_amp = <&tpa6130a2>;

       tvout-selection-gpio = <&gpio2 8 GPIO_ACTIVE_HIGH>; /* 40 */
       jack-detection-gpio = <&gpio6 17 GPIO_ACTIVE_HIGH>; /* 177 */
       eci-sw-gpio = <&gpio6 22 GPIO_ACTIVE_HIGH>; /* 182 */
       speaker-amp-gpio = <&twl_gpio 7 GPIO_ACTIVE_HIGH>;
};

-- Sebastian

Pali Roh?r (2):
  ASoC: omap: rx51: Use snd_soc_register_card
  ARM: OMAP: rx51: Register audio device

Sebastian Reichel (2):
  ASoC: Allow Aux Codecs to be specified using DT
  ASoC: RX-51: Add DT support to sound driver

 .../devicetree/bindings/sound/nokia,rx51.txt       |  28 +++
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 arch/arm/mach-omap2/board-rx51-peripherals.c       |  11 ++
 include/sound/rx51.h                               |  18 ++
 include/sound/soc.h                                |  13 +-
 sound/soc/omap/omap-mcbsp.c                        |   5 +-
 sound/soc/omap/omap-mcbsp.h                        |   2 +-
 sound/soc/omap/rx51.c                              | 212 +++++++++++++++------
 sound/soc/soc-core.c                               |  68 +++++--
 9 files changed, 277 insertions(+), 81 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/nokia,rx51.txt
 create mode 100644 include/sound/rx51.h

-- 
1.8.4.rc3

[RFC 4/4] ASoC: RX-51: Add DT support to sound driver

[Kumar Gala]

On Oct 27, 2013, at 4:24 PM, Sebastian Reichel wrote:

> This patch adds device tree support to the Nokia N900 audio driver.
> It also removes GPIO defines and gets them from platform data /
> device tree, since some GPIO numbers may be different with DT boot.
> 
> The binding also changes a helper function in omap-mcbsp, which
> is currently only used by the Nokia N900. This change is needed,
> because DT instanced omap-mcbsp driver has no valid id assigned.
> 
> Last but not least the DT binding is documented.
> 
> Signed-off-by: Sebastian Reichel <sre@debian.org>
> ---
> .../devicetree/bindings/sound/nokia,rx51.txt       |  28 ++++
> .../devicetree/bindings/vendor-prefixes.txt        |   1 +
> include/sound/rx51.h                               |  18 +++
> sound/soc/omap/omap-mcbsp.c                        |   5 +-
> sound/soc/omap/omap-mcbsp.h                        |   2 +-
> sound/soc/omap/rx51.c                              | 175 ++++++++++++++++-----
> 6 files changed, 184 insertions(+), 45 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/sound/nokia,rx51.txt
> create mode 100644 include/sound/rx51.h
> 
> diff --git a/Documentation/devicetree/bindings/sound/nokia,rx51.txt b/Documentation/devicetree/bindings/sound/nokia,rx51.txt
> new file mode 100644
> index 0000000..4f985ca
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/nokia,rx51.txt
> @@ -0,0 +1,28 @@
> +* Nokia N900 audio setup

This needs more description 

> +
> +Required properties:
> +- compatible: "nokia,rx51-audio"
> +- ti,mcbsp: phandle for the McBSP node
> +- ti,codec: phandle for the main TLV320AIC3X node
> +- ti,codec_aux: phandle for the main TLV320AIC3X auxiliary node
> +- ti,headset_amp: phandle for the TPA6130A2 node

Hmm, I don't think these should be ti prefixed but nokia.

> +- tvout-selection-gpio: GPIO for tvout selection
> +- jack-detection-gpio: GPIO for jack detection
> +- eci-sw-gpio: GPIO for ECI SW
> +- speaker-amp-gpio: GPIO for Speaker Amp

why aren't the gpio properties vendor prefixed?

> +
> +Example:
> +
> +sound {
> +	compatible = "nokia,rx51-audio";
> +
> +	ti,mcbsp = <&mcbsp2>;
> +	ti,codec = <&tlv320aic3x>;
> +	ti,codec_aux = <&tlv320aic3x_aux>;
> +	ti,headset_amp = <&tpa6130a2>;
> +
> +	tvout-selection-gpio = <&gpio2 8 GPIO_ACTIVE_HIGH>; /* 40 */
> +	jack-detection-gpio = <&gpio6 17 GPIO_ACTIVE_HIGH>; /* 177 */
> +	eci-sw-gpio = <&gpio6 22 GPIO_ACTIVE_HIGH>; /* 182 */
> +	speaker-amp-gpio = <&twl_gpio 7 GPIO_ACTIVE_HIGH>;
> +};

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[RFC 1/4] ASoC: omap: rx51: Use snd_soc_register_card

[Mark Brown]

On Sun, Oct 27, 2013 at 10:24:43PM +0100, Sebastian Reichel wrote:
> From: Pali Roh?r <pali.rohar@gmail.com>
> 
> This patch converts the rx51 ASoC module to use
> snd_soc_register_card. It also adds module alias
> to support driver autoloading.

You should really use devm_snd_soc_register_card() here.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131028/9750074b/attachment.sig>

[RFC 3/4] ASoC: Allow Aux Codecs to be specified using DT

[Mark Brown]

On Sun, Oct 27, 2013 at 10:24:45PM +0100, Sebastian Reichel wrote:
> This patch adds support for specifying auxiliary codecs and
> codec configuration via device tree phandles.

This doesn't apply against current git, please check and resend.
Please also consider your CC list more carefully, there's an awful lot
of people on this...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131028/ed30203b/attachment.sig>

[RFC 3/4] ASoC: Allow Aux Codecs to be specified using DT

[Sebastian Reichel]

On Mon, Oct 28, 2013 at 09:37:21AM -0700, Mark Brown wrote:
> On Sun, Oct 27, 2013 at 10:24:45PM +0100, Sebastian Reichel wrote:
> > This patch adds support for specifying auxiliary codecs and
> > codec configuration via device tree phandles.
> 
> This doesn't apply against current git, please check and resend.

Can you give me a git url to rebase against? I used current git
master from torvalds.

> Please also consider your CC list more carefully, there's an awful lot
> of people on this...

I added exactly those recipients, that were returned by
./scripts/get_maintainer.pl for the patchset.

The list of involved people is quite long, because the patchset
touches devicetree, documentation, boardcode, asoc and the specific
driver.

-- Sebastian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131028/a0e96a24/attachment.sig>

[RFC 3/4] ASoC: Allow Aux Codecs to be specified using DT

[Mark Brown]

On Mon, Oct 28, 2013 at 06:53:42PM +0100, Sebastian Reichel wrote:
> On Mon, Oct 28, 2013 at 09:37:21AM -0700, Mark Brown wrote:

> > This doesn't apply against current git, please check and resend.

> Can you give me a git url to rebase against? I used current git
> master from torvalds.

>From MAINTAINERS:

SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEMENT (ASoC)
M:      Liam Girdwood <lgirdwood@gmail.com>
M:      Mark Brown <broonie@kernel.org>
T:      git git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
L:      alsa-devel at alsa-project.org (moderated for non-subscribers)
W:      http://alsa-project.org/main/index.php/ASoC
S:      Supported
F:      Documentation/sound/alsa/soc/
F:      sound/soc/
F:      include/sound/soc*

> > Please also consider your CC list more carefully, there's an awful lot
> > of people on this...

> I added exactly those recipients, that were returned by
> ./scripts/get_maintainer.pl for the patchset.

> The list of involved people is quite long, because the patchset
> touches devicetree, documentation, boardcode, asoc and the specific
> driver.

You shouldn't be using the output of get_maintainers directly, you
should understand why everyone it's including is in the list and if they
really should be.  This is especially important if you are sending a
series that covers many subsystems, or nested subsystems.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131028/78280c9a/attachment.sig>

[RFC 4/4] ASoC: RX-51: Add DT support to sound driver

[Linus Walleij]

On Sun, Oct 27, 2013 at 2:24 PM, Sebastian Reichel <sre@debian.org> wrote:

> +- eci-sw-gpio: GPIO for ECI SW
> +- speaker-amp-gpio: GPIO for Speaker Amp

Can you please spell out the acronyms? ECI? SW? Amp?

I guess SW=switch, Amp=amplifier, but ECI beats me.

Yours,
Linus Walleij

[RFC 4/4] ASoC: RX-51: Add DT support to sound driver

[Mark Brown]

On Thu, Oct 31, 2013 at 07:44:22AM -0700, Linus Walleij wrote:
> On Sun, Oct 27, 2013 at 2:24 PM, Sebastian Reichel <sre@debian.org> wrote:

> > +- eci-sw-gpio: GPIO for ECI SW
> > +- speaker-amp-gpio: GPIO for Speaker Amp

> Can you please spell out the acronyms? ECI? SW? Amp?

> I guess SW=switch, Amp=amplifier, but ECI beats me.

ECI is the name of Nokia's accesory interface spec, it should be as
clear as GPIO for anyone working on Nokia hardware (which this is).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131031/4c452316/attachment-0001.sig>

[RFC 1/4] ASoC: omap: rx51: Use snd_soc_register_card

[Pavel Machek]

On Sun 2013-10-27 22:24:43, Sebastian Reichel wrote:
> From: Pali Roh?r <pali.rohar@gmail.com>
> 
> This patch converts the rx51 ASoC module to use
> snd_soc_register_card. It also adds module alias
> to support driver autoloading.
> 
> Signed-off-by: Pali Roh?r <pali.rohar@gmail.com>
> Signed-off-by: Sebastian Reichel <sre@debian.org>

Reviewed-by: Pavel Machek <pavel@ucw.cz>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[RFC 2/4] ARM: OMAP: rx51: Register audio device

[Pavel Machek]

Hi!

> From: Pali Roh?r <pali.rohar@gmail.com>
> 
> This patch adds support for the audio chip to the legacy
> boardcode of the Nokia N900.

In 0/4, you said:

> For example non DT boot
> does not work, because I did not yet add pdata in boardcode.

So, perhaps we don't need this patch for now and can rely only do
audio on device-tree-enabled configurations?

Thanks,
							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[RFC 3/4] ASoC: Allow Aux Codecs to be specified using DT

[Pavel Machek]

Hi!

> This patch adds support for specifying auxiliary codecs and
> codec configuration via device tree phandles.
> 
> This change adds new fields to snd_soc_aux_dev and snd_soc_codec_conf
> and adds support for the changes to SoC core methods.
> 
> Signed-off-by: Sebastian Reichel <sre@debian.org>
> ---
>  include/sound/soc.h  | 13 +++++++++-
>  sound/soc/soc-core.c | 68 ++++++++++++++++++++++++++++++++++++----------------
>  2 files changed, 59 insertions(+), 22 deletions(-)

> @@ -1527,15 +1529,23 @@ static void soc_unregister_ac97_dai_link(struct snd_soc_codec *codec)
>  static int soc_check_aux_dev(struct snd_soc_card *card, int num)
>  {
>  	struct snd_soc_aux_dev *aux_dev = &card->aux_dev[num];
> +	const char *codecname = aux_dev->codec_name;
>  	struct snd_soc_codec *codec;
>  
>  	/* find CODEC from registered CODECs*/
>  	list_for_each_entry(codec, &codec_list, list) {
> -		if (!strcmp(codec->name, aux_dev->codec_name))
> +		if (aux_dev->codec_of_node &&
> +				codec->dev->of_node == aux_dev->codec_of_node)
> +			return 0;
> +		if (aux_dev->codec_name &&
> +				!strcmp(codec->name, aux_dev->codec_name))
>  			return 0;

You can use codecname here.

> -		if (!strcmp(codec->name, aux_dev->codec_name)) {
> -			if (codec->probed) {
> -				dev_err(codec->dev,
> -					"ASoC: codec already probed");
> -				ret = -EBUSY;
> -				goto out;
> -			}
> -			goto found;
> +		if (aux_dev->codec_of_node &&
> +				codec->dev->of_node != aux_dev->codec_of_node)
> +			continue;
> +		if (aux_dev->codec_name &&
> +				strcmp(codec->name, aux_dev->codec_name))
> +			continue;

So in (error) case of ! aux_dev->codec_of_node && ! aux_dev->codec_name 
we match first possible codec?

Given code similarity between this and the one above, should there be
helper function that does the comparison (or even walks the list)?

> @@ -1644,12 +1663,19 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card)
>  		/* check to see if we need to override the compress_type */
>  		for (i = 0; i < card->num_configs; ++i) {
>  			codec_conf = &card->codec_conf[i];
> -			if (!strcmp(codec->name, codec_conf->dev_name)) {
> -				compress_type = codec_conf->compress_type;
> -				if (compress_type && compress_type
> -				    != codec->compress_type)
> -					break;
> -			}
> +
> +			if (codec_conf->of_node &&
> +				codec->dev->of_node != codec_conf->of_node)
> +				continue;
> +
> +			if (codec_conf->dev_name &&
> +				strcmp(codec->name, codec_conf->dev_name))
> +				continue;
> +

..third copy of codec matching.

Thanks for doing all the good work,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[RFC 4/4] ASoC: RX-51: Add DT support to sound driver

[Pavel Machek]

Hi!

> This patch adds device tree support to the Nokia N900 audio driver.

Note, N900 audio driver.

> +- compatible: "nokia,rx51-audio"

Still, it is rx51-audio. If it is feasible, could we move away from
rx51 naming and into n900 naming that is familiar to mere humans?

We can rename the sources later, but I guess we should get dts binding
right first time...
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[RFC 3/4] ASoC: Allow Aux Codecs to be specified using DT

[Pavel Machek]

Hi!


> So in (error) case of ! aux_dev->codec_of_node && ! aux_dev->codec_name 
> we match first possible codec?
> 
> Given code similarity between this and the one above, should there be
> helper function that does the comparison (or even walks the list)?

Something like this? soc_probe_aux_dev now looks better... (Only
compile tested).

Signed-off-by: Pavel Machek <pavel@ucw.cz>

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 392f479..3097e17 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1526,69 +1526,65 @@ static void soc_unregister_ac97_dai_link(struct snd_soc_codec *codec)
 }
 #endif
 
-static int soc_check_aux_dev(struct snd_soc_card *card, int num)
+struct snd_soc_codec *soc_find_matching_codec(struct snd_soc_card *card, int num)
 {
 	struct snd_soc_aux_dev *aux_dev = &card->aux_dev[num];
-	const char *codecname = aux_dev->codec_name;
 	struct snd_soc_codec *codec;
 
-	/* find CODEC from registered CODECs*/
+	/* find CODEC from registered CODECs */
 	list_for_each_entry(codec, &codec_list, list) {
-		if (aux_dev->codec_of_node &&
-				codec->dev->of_node == aux_dev->codec_of_node)
-			return 0;
-		if (aux_dev->codec_name &&
-				!strcmp(codec->name, aux_dev->codec_name))
-			return 0;
+		if (aux_dev->codec_of_node && 
+		    (codec->dev->of_node != aux_dev->codec_of_node))
+			continue;
+		if (aux_dev->codec_name && strcmp(codec->name, aux_dev->codec_name))
+			continue;
+		return codec;
 	}
+	return NULL;
+}
 
+static int soc_check_aux_dev(struct snd_soc_card *card, int num)
+{
+	struct snd_soc_aux_dev *aux_dev = &card->aux_dev[num];
+	const char *codecname = aux_dev->codec_name;
+	struct snd_soc_codec *codec = soc_find_matching_codec(card, num);
+
+	if (codec)
+		return 0;
 	if (aux_dev->codec_of_node)
 		codecname = of_node_full_name(aux_dev->codec_of_node);
 
 	dev_err(card->dev, "ASoC: %s not registered\n", codecname);
-
 	return -EPROBE_DEFER;
 }
 
+
 static int soc_probe_aux_dev(struct snd_soc_card *card, int num)
 {
 	struct snd_soc_aux_dev *aux_dev = &card->aux_dev[num];
-	struct snd_soc_codec *codec;
 	const char *codecname = aux_dev->codec_name;
 	int ret = -ENODEV;
+	struct snd_soc_codec *codec = soc_find_matching_codec(card, num);
 
-	/* find CODEC from registered CODECs*/
-	list_for_each_entry(codec, &codec_list, list) {
-		if (aux_dev->codec_of_node &&
-				codec->dev->of_node != aux_dev->codec_of_node)
-			continue;
-		if (aux_dev->codec_name &&
-				strcmp(codec->name, aux_dev->codec_name))
-			continue;
+	if (!codec) {
+		if (aux_dev->codec_of_node)
+			codecname = of_node_full_name(aux_dev->codec_of_node);
 
-		if (codec->probed) {
-			dev_err(codec->dev, "ASoC: codec already probed");
-			ret = -EBUSY;
-			goto out;
-		}
-		goto found;
+		/* codec not found */
+		dev_err(card->dev, "ASoC: codec %s not found", codecname);
+		return -EPROBE_DEFER;
 	}
 
-	if (aux_dev->codec_of_node)
-		codecname = of_node_full_name(aux_dev->codec_of_node);
-
-	/* codec not found */
-	dev_err(card->dev, "ASoC: codec %s not found", codecname);
-	return -EPROBE_DEFER;
+	if (codec->probed) {
+		dev_err(codec->dev, "ASoC: codec already probed");
+		return -EBUSY;
+	}
 
-found:
 	ret = soc_probe_codec(card, codec);
 	if (ret < 0)
 		return ret;
 
 	ret = soc_post_component_init(card, codec, num, 1);
-
-out:
 	return ret;
 }
 


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[RFC 4/4] ASoC: RX-51: Add DT support to sound driver

[Jarkko Nikula]

Hi

On 11/06/2013 04:32 PM, Pavel Machek wrote:
> Hi!
> 
>> This patch adds device tree support to the Nokia N900 audio driver.
> 
> Note, N900 audio driver.
> 
>> +- compatible: "nokia,rx51-audio"
> 
> Still, it is rx51-audio. If it is feasible, could we move away from
> rx51 naming and into n900 naming that is familiar to mere humans?
> 
> We can rename the sources later, but I guess we should get dts binding
> right first time...

Sorry the long response. I definitely would like n900 naming more in dts.

History is that the basic board file for N900 went to mainline before
sales started and used the hw name in sources and Kconfig. Audio machine
driver then followed the same rx51 hw naming scheme with rest of sources.

-- 
Jarkko