Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1338734 > unrolled thread

[PATCH 1/4] ASoC: wm9713: add binding for WM9713 codec

Started byRobert Jarzmik <robert.jarzmik@free.fr>
First post2016-02-20 15:40 +0100
Last post2016-02-21 03:00 +0100
Articles 10 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 1/4] ASoC: wm9713: add binding for WM9713 codec Robert Jarzmik <robert.jarzmik@free.fr> - 2016-02-20 15:40 +0100
    [PATCH 2/4] ASoC: wm9713: add device tree support Robert Jarzmik <robert.jarzmik@free.fr> - 2016-02-20 15:40 +0100
    [PATCH 3/4] ASoC: pxa: add binding for pxa2xx-ac97 audio complex Robert Jarzmik <robert.jarzmik@free.fr> - 2016-02-20 15:40 +0100
    Re: [PATCH 1/4] ASoC: wm9713: add binding for WM9713 codec Mark Brown <broonie@kernel.org> - 2016-02-20 18:20 +0100
      Re: [PATCH 1/4] ASoC: wm9713: add binding for WM9713 codec Robert Jarzmik <robert.jarzmik@free.fr> - 2016-02-20 19:30 +0100
        Re: [PATCH 1/4] ASoC: wm9713: add binding for WM9713 codec Mark Brown <broonie@kernel.org> - 2016-02-20 21:10 +0100
          Re: [PATCH 1/4] ASoC: wm9713: add binding for WM9713 codec Robert Jarzmik <robert.jarzmik@free.fr> - 2016-02-20 21:40 +0100
            Re: [PATCH 1/4] ASoC: wm9713: add binding for WM9713 codec Mark Brown <broonie@kernel.org> - 2016-02-20 22:20 +0100
              Re: [PATCH 1/4] ASoC: wm9713: add binding for WM9713 codec Robert Jarzmik <robert.jarzmik@free.fr> - 2016-02-20 23:30 +0100
                Re: [PATCH 1/4] ASoC: wm9713: add binding for WM9713 codec Mark Brown <broonie@kernel.org> - 2016-02-21 03:00 +0100

#1338734 — [PATCH 1/4] ASoC: wm9713: add binding for WM9713 codec

FromRobert Jarzmik <robert.jarzmik@free.fr>
Date2016-02-20 15:40 +0100
Subject[PATCH 1/4] ASoC: wm9713: add binding for WM9713 codec
Message-ID<r4gRr-727-11@gated-at.bofh.it>
This adds a binding for the Wolfson WM9713 audio codec.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 Documentation/devicetree/bindings/sound/wm9713.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/wm9713.txt

diff --git a/Documentation/devicetree/bindings/sound/wm9713.txt b/Documentation/devicetree/bindings/sound/wm9713.txt
new file mode 100644
index 000000000000..761001bc3201
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/wm9713.txt
@@ -0,0 +1,14 @@
+WM9713 audio CODEC
+
+This devices supports I2C.
+
+Required properties:
+  - compatible :
+	"wlf,wm9713"
+
+Example:
+	wm9713: wm9713@0 {
+		compatible = "wlf,wm9713";
+		#sound-dai-cells = <1>;
+		status = "okay";
+	};
-- 
2.1.4

[toc] | [next] | [standalone]


#1338735 — [PATCH 2/4] ASoC: wm9713: add device tree support

FromRobert Jarzmik <robert.jarzmik@free.fr>
Date2016-02-20 15:40 +0100
Subject[PATCH 2/4] ASoC: wm9713: add device tree support
Message-ID<r4gRs-727-21@gated-at.bofh.it>
In reply to#1338734
Add the code to be able to use this codec in a devicetree platform.
This is tested with the zylonite pxa310 board.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 sound/soc/codecs/wm9713.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/sound/soc/codecs/wm9713.c b/sound/soc/codecs/wm9713.c
index 79e143625ac3..4e522302bb8a 100644
--- a/sound/soc/codecs/wm9713.c
+++ b/sound/soc/codecs/wm9713.c
@@ -1272,9 +1272,19 @@ static int wm9713_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id wm9713_dt_ids[] = {
+	{ .compatible = "wlf,wm9713", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, wm9713_dt_ids);
+
+#endif
+
 static struct platform_driver wm9713_codec_driver = {
 	.driver = {
 			.name = "wm9713-codec",
+			.of_match_table = of_match_ptr(wm9713_dt_ids),
 	},
 
 	.probe = wm9713_probe,
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]


#1338736 — [PATCH 3/4] ASoC: pxa: add binding for pxa2xx-ac97 audio complex

FromRobert Jarzmik <robert.jarzmik@free.fr>
Date2016-02-20 15:40 +0100
Subject[PATCH 3/4] ASoC: pxa: add binding for pxa2xx-ac97 audio complex
Message-ID<r4gRs-727-23@gated-at.bofh.it>
In reply to#1338734
This adds a binding for the Marvell PXA audio complex, available in
pxa2xx and pxa3xx variants.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 .../bindings/sound/marvell,pxa2xx-ac97.txt         | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/marvell,pxa2xx-ac97.txt

diff --git a/Documentation/devicetree/bindings/sound/marvell,pxa2xx-ac97.txt b/Documentation/devicetree/bindings/sound/marvell,pxa2xx-ac97.txt
new file mode 100644
index 000000000000..b3f2882d9c7d
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/marvell,pxa2xx-ac97.txt
@@ -0,0 +1,25 @@
+Marvell PXA2xx audio complex
+
+This descriptions matches the AC97 controller found in pxa2xx and pxa3xx series.
+
+Required properties:
+  - compatible: "marvell,pxa2xx-ac97"
+  - reg: device MMIO address space
+  - interrupts: single interrupt generated by AC97 IP
+  - clocks: input clock of the AC97 IP, refer to clock-bindings.txt
+
+Optional properties:
+  - pinctrl-names, pinctrl-0: refer to pinctrl-bindings.txt
+  - reset-gpio: gpio used for AC97 reset, refer to gpio.txt
+
+Example:
+	ac97: sound@40500000 {
+	      	compatible = "marvell,pxa2xx-ac97";
+		reg = < 0x40500000 0x1000 >;
+		interrupts = <14>;
+		reset-gpio = <&gpio 113 GPIO_ACTIVE_HIGH>;
+		#sound-dai-cells = <1>;
+		pinctrl-names = "default";
+		pinctrl-0 = < &pmux_ac97_default >;
+		status = "okay";
+	};
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]


#1338751

FromMark Brown <broonie@kernel.org>
Date2016-02-20 18:20 +0100
Message-ID<r4jmi-xc-21@gated-at.bofh.it>
In reply to#1338734

[Multipart message — attachments visible in raw view] — view raw

On Sat, Feb 20, 2016 at 03:37:56PM +0100, Robert Jarzmik wrote:

> +WM9713 audio CODEC
> +
> +This devices supports I2C.

No, it clearly doesn't...  The problem with doing this is that since
AC'97 is an enumerable bus we really shouldn't need to list AC'97 CODECs
in the device tree.  Instead we should be probing at runtime (as the
non-ASoC AC'97 code does) or something similar.

[toc] | [prev] | [next] | [standalone]


#1338770

FromRobert Jarzmik <robert.jarzmik@free.fr>
Date2016-02-20 19:30 +0100
Message-ID<r4ks2-1qv-13@gated-at.bofh.it>
In reply to#1338751
Mark Brown <broonie@kernel.org> writes:

> On Sat, Feb 20, 2016 at 03:37:56PM +0100, Robert Jarzmik wrote:
>
>> +WM9713 audio CODEC
>> +
>> +This devices supports I2C.
>
> No, it clearly doesn't...
Right, it supports AC97.

> The problem with doing this is that since AC'97 is an enumerable bus we really
> shouldn't need to list AC'97 CODECs in the device tree.
Ok, I understand that.

> Instead we should be probing at runtime (as the non-ASoC AC'97 code does) or
> something similar.
When you say "non-ASoC AC'97 code", which file are you referring to ? Is it
sound/pci/ac97/ac97_codec.c ?

If so is there already a table of tuples (AC97_VENDOR_ID1, AC97_VENDOR_ID2) ->
(platform device, platform device data) and a matching mechanism already
available to the ASoC drivers ?

Cheers.

-- 
Robert

[toc] | [prev] | [next] | [standalone]


#1338782

FromMark Brown <broonie@kernel.org>
Date2016-02-20 21:10 +0100
Message-ID<r4m0N-2IT-1@gated-at.bofh.it>
In reply to#1338770

[Multipart message — attachments visible in raw view] — view raw

On Sat, Feb 20, 2016 at 07:22:04PM +0100, Robert Jarzmik wrote:
> Mark Brown <broonie@kernel.org> writes:

> > Instead we should be probing at runtime (as the non-ASoC AC'97 code does) or
> > something similar.

> When you say "non-ASoC AC'97 code", which file are you referring to ? Is it
> sound/pci/ac97/ac97_codec.c ?

Yes.

> If so is there already a table of tuples (AC97_VENDOR_ID1, AC97_VENDOR_ID2) ->
> (platform device, platform device data) and a matching mechanism already
> available to the ASoC drivers ?

ASoC doesn't really support the enumeration very well, you can use
ac97.c as the CODEC but that's about it.  There is a generic AC'97 PXA
driver in sound/arm, if your system can use that that'd be a better
route to DT integration for it I think.  Did you try that, if there are
problems with that perhaps we can improve that driver, it should  be
simpler.

[toc] | [prev] | [next] | [standalone]


#1338784

FromRobert Jarzmik <robert.jarzmik@free.fr>
Date2016-02-20 21:40 +0100
Message-ID<r4mtP-2SH-1@gated-at.bofh.it>
In reply to#1338782
Mark Brown <broonie@kernel.org> writes:

> On Sat, Feb 20, 2016 at 07:22:04PM +0100, Robert Jarzmik wrote:
>> Mark Brown <broonie@kernel.org> writes:
Removed DT people from this conversation.

>> > Instead we should be probing at runtime (as the non-ASoC AC'97 code does) or
>> > something similar.
>
>> When you say "non-ASoC AC'97 code", which file are you referring to ? Is it
>> sound/pci/ac97/ac97_codec.c ?
>
> Yes.
>
>> If so is there already a table of tuples (AC97_VENDOR_ID1, AC97_VENDOR_ID2) ->
>> (platform device, platform device data) and a matching mechanism already
>> available to the ASoC drivers ?
>
> ASoC doesn't really support the enumeration very well, you can use
> ac97.c as the CODEC but that's about it.

> There is a generic AC'97 PXA driver in sound/arm, if your system can use that
> that'd be a better route to DT integration for it I think.
I'm open on the topic.

Historically, I use sound/soc/pxa/pxa2xx-ac97.c since 2008. I know it works, but
if you think I should examine sound/arm/pxa2xx-ac97.c, let's do that.

> Did you try that, if there are problems with that perhaps we can improve that
> driver, it should be simpler.
I will. By now I fail to see how this will help in the wm9713 probing and
detection ...

Until I make the try, here is what I have as a device-tree extract in [1], which
is my candidate for sound/soc/pxa/zylonite.c replacement.. If we conclude that
wm9713 shouldn't be in device-tree, then I'm curious how the DAI bindings
(simple-audio-card,dai-link*) should be handled.

Cheers.

-- 
Robert

[1] Zylonite DT extract
	ssp3: ssp@41900000 {
		compatible = "mrvl,pxa3xx-ssp";
		reg = <0x41900000 0x40>;
		interrupts = <0>;
		clocks = < &clks CLK_SSP3 >;
		dmas = <&pdma 66 3
			&pdma 67 3>;
		dma-names = "rx", "tx";
		pinctrl-names = "default";
		pinctrl-0 = < &pmux_ssp3_low_default
			      &pmux_ssp3_float_default >;
		status = "okay";
	};

	ssp_dai0: ssp_dai@0 {
		compatible = "mrvl,pxa-ssp-dai";
		port = <&ssp3>;
		#sound-dai-cells = <0>;
	};
	ac97: sound@40500000 {
	      	compatible = "marvell,pxa2xx-ac97";
		reg = < 0x40500000 0x1000 >;
		interrupts = <14>;
		reset-gpio = <&gpio 113 GPIO_ACTIVE_HIGH>;
		#sound-dai-cells = <1>;
		pinctrl-names = "default";
		pinctrl-0 = < &pmux_ac97_default >;
		status = "okay";
	};

	pxa_pcm_audio: snd_soc_pxa_audio {
		compatible = "mrvl,pxa-pcm-audio";
		#sound-dai-cells = <1>;
	};
	pxa_ssp_dai: snd_soc_pxa_audio {
		compatible = "mrvl,pxa-pcm-audio";
		#sound-dai-cells = <1>;
	};

	wm9713: wm9713@0 {
		compatible = "wlf,wm9713";
		#sound-dai-cells = <1>;
		pinctrl-names = "default";
		pinctrl-0 = < &pmux_wm9713_default >;
		status = "okay";
	};

	sound {
		compatible = "simple-audio-card";
		simple-audio-card,name = "Zylonite-Sound-Card";
		simple-audio-card,format = "ac97";
		simple-audio-card,widgets =
			"Headphone", "Headphone",
			"Microphone", "Headset Microphone",
			"Microphone", "Handset Microphone",
			"Speaker", "Multiactor",
			"Speaker", "Headset Earpiece";
		simple-audio-card,routing =
			"Headphone", "HPL",		/* Headphone output */
			"Headphone", "HPR",		/* connected to HPL/HPR */
			"Headset Earpiece", "OUT3",	/* On-board earpiece */
			"MIC2A", "Mic Bias",			/* Headphone mic */
			"Mic Bias", "Headset Microphone",	/* Headphone mic */
			"MIC1", "Mic Bias",			/* On-board mic */
			"Mic Bias", "Handset Microphone",	/* On-board mic */
			"Multiactor", "SPKL",		/* Multiactor differentially */
			"Multiactor", "SPKR";		/* connected over SPKL/SPKR */
	
		simple-audio-card,dai-link@0 {		/* AC97 */
			format = "ac97";
			cpu {
				sound-dai = <&ac97 0>;
			};
			codec {
				sound-dai = <&wm9713 0>;
			};
			plat {
				sound-dai = <&pxa_pcm_audio 0>;
			};
		};
		simple-audio-card,dai-link@1 {		/* AC97 Aux */
			format = "ac97";
			cpu {
				sound-dai = <&ac97 1>;
			};
			codec {
				sound-dai = <&wm9713 1>;
			};
			plat {
				sound-dai = <&pxa_pcm_audio 0>;
			};
		};
		simple-audio-card,dai-link@2 {		/* AC97 Voice */
			format = "i2s";
			cpu {
				sound-dai = <&ssp_dai0>;
			};
			codec {
				sound-dai = <&wm9713 2>;
			};
			plat {
				sound-dai = <&pxa_pcm_audio 0>;
			};
		};
	};

[toc] | [prev] | [next] | [standalone]


#1338794

FromMark Brown <broonie@kernel.org>
Date2016-02-20 22:20 +0100
Message-ID<r4n6x-3nw-13@gated-at.bofh.it>
In reply to#1338784

[Multipart message — attachments visible in raw view] — view raw

On Sat, Feb 20, 2016 at 09:32:58PM +0100, Robert Jarzmik wrote:
> Mark Brown <broonie@kernel.org> writes:
> > On Sat, Feb 20, 2016 at 07:22:04PM +0100, Robert Jarzmik wrote:

Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns.  Doing this makes your messages much
easier to read and reply to.

> > There is a generic AC'97 PXA driver in sound/arm, if your system can use that
> > that'd be a better route to DT integration for it I think.
> I'm open on the topic.

> Historically, I use sound/soc/pxa/pxa2xx-ac97.c since 2008. I know it works, but
> if you think I should examine sound/arm/pxa2xx-ac97.c, let's do that.
> 
> > Did you try that, if there are problems with that perhaps we can improve that
> > driver, it should be simpler.

> I will. By now I fail to see how this will help in the wm9713 probing and
> detection ...

It will eumerate the AC'97 bus by itself and does not need the CODEC to
be described.

> Until I make the try, here is what I have as a device-tree extract in [1], which
> is my candidate for sound/soc/pxa/zylonite.c replacement.. If we conclude that
> wm9713 shouldn't be in device-tree, then I'm curious how the DAI bindings
> (simple-audio-card,dai-link*) should be handled.

They should be created as a function of enumerating the CODEC.  If you
use the genric AC'97 stuff it doesn't use ASoC at all and this happens
as a side effect.

[toc] | [prev] | [next] | [standalone]


#1338806

FromRobert Jarzmik <robert.jarzmik@free.fr>
Date2016-02-20 23:30 +0100
Message-ID<r4oci-4j5-25@gated-at.bofh.it>
In reply to#1338794
Mark Brown <broonie@kernel.org> writes:

> On Sat, Feb 20, 2016 at 09:32:58PM +0100, Robert Jarzmik wrote:
>> Mark Brown <broonie@kernel.org> writes:
>> > On Sat, Feb 20, 2016 at 07:22:04PM +0100, Robert Jarzmik wrote:
>> I will. By now I fail to see how this will help in the wm9713 probing and
>> detection ...
>
> It will eumerate the AC'97 bus by itself and does not need the CODEC to
> be described.
I think I still don't get it.

So let's rephrase it another way : how will the function wm9713_probe() be
called, ie. what is the possible function backtrace leading to that call ?

>> Until I make the try, here is what I have as a device-tree extract in [1],
>> which is my candidate for sound/soc/pxa/zylonite.c replacement.. If we
>> conclude that wm9713 shouldn't be in device-tree, then I'm curious how the
>> DAI bindings (simple-audio-card,dai-link*) should be handled.
>
> They should be created as a function of enumerating the CODEC.  If you
> use the genric AC'97 stuff it doesn't use ASoC at all and this happens
> as a side effect.
I don't get that either. For me sound/soc/pci/ac97/ac97_codec.c is PCI specific,
not generic, so what is "generic AC'97 stuff" ? I will never be able to use it
as on my platforms CONFIG_PCI=n.

Do you have a devicetree example somewhere, with (ac97 host, audio codec) pair I
can have a look at to understand ?

Cheers.

-- 
Robert

[toc] | [prev] | [next] | [standalone]


#1338838

FromMark Brown <broonie@kernel.org>
Date2016-02-21 03:00 +0100
Message-ID<r4rtw-6KY-23@gated-at.bofh.it>
In reply to#1338806

[Multipart message — attachments visible in raw view] — view raw

On Sat, Feb 20, 2016 at 11:24:02PM +0100, Robert Jarzmik wrote:

Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns.  Doing this makes your messages much
easier to read and reply to.

> Mark Brown <broonie@kernel.org> writes:
> > On Sat, Feb 20, 2016 at 09:32:58PM +0100, Robert Jarzmik wrote:

> > It will eumerate the AC'97 bus by itself and does not need the CODEC to
> > be described.

> I think I still don't get it.

> So let's rephrase it another way : how will the function wm9713_probe() be
> called, ie. what is the possible function backtrace leading to that call ?

It will not be called, the generic AC'97 code will be used.

> > They should be created as a function of enumerating the CODEC.  If you
> > use the genric AC'97 stuff it doesn't use ASoC at all and this happens
> > as a side effect.

> I don't get that either. For me sound/soc/pci/ac97/ac97_codec.c is PCI specific,
> not generic, so what is "generic AC'97 stuff" ? I will never be able to use it
> as on my platforms CONFIG_PCI=n.

That is the generic code, there is no PCI dependency.

> Do you have a devicetree example somewhere, with (ac97 host, audio codec) pair I
> can have a look at to understand ?

Some Atmel boards do this IIRC, as does the AACI driver (via AMBA but
same effect).

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web