[PATCH] ASoC: kirkwood: change the compatible string of the kirkwood-i2s driver

[PATCH] ASoC: kirkwood: change the compatible string of the kirkwood-i2s driver

[Thomas Petazzoni]

The compatible string of the kirkwood-i2s driver was chosen as
"marvell,mvebu-audio". Using such a compatible string is not a good
idea, since "mvebu" is the name of a large family of SOCs, in which
new, unknown SOCs will be coming in the future. It is therefore
impossible to know what will be evolutions of this hardware block in
the next generations of the SOCs. For this reason, the recommandation
for compatible strings of on-SOCs devices has always been to use the
name of the oldest SOC that has the hardware block. New SOCs that have
an exactly compatible hardware block can reference it using the same
compatible string. See [1], [2] and [3] for various cases were this
suggestion was made, including from Rob Herring, a Device Tree binding
maintainer.

As an example, there are already small differences between current
generations:

 * On Kirkwood, only one interrupt is used for audio.
 * On Dove, two interrupts are used, one for audio data and one for
   error reporting.

In the near future, I'll be adding audio support to Armada 370, which
allows has the same hardware block (but maybe with minor variants).

Therefore, this patch changes the driver to accept
"marvell,kirkwood-audio" and "marvell,dove-audio" as compatible
strings instead of the too-generic "marvell,mvebu-audio". The reason
for the two different compatible strings is the difference in the
number of interrupts used by the two SOCs for audio.

This Device Tree binding has never been part of a Linux kernel stable
release so far, so it can be changed now without breaking backward
compatibility.

[1] http://lists.infradead.org/pipermail/linux-mtd/2012-March/040417.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-April/161065.html
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-March/087702.html

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 Documentation/devicetree/bindings/sound/mvebu-audio.txt | 12 ++++++++----
 sound/soc/kirkwood/kirkwood-i2s.c                       |  3 ++-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/mvebu-audio.txt b/Documentation/devicetree/bindings/sound/mvebu-audio.txt
index 7e5fd37..f0062c5 100644
--- a/Documentation/devicetree/bindings/sound/mvebu-audio.txt
+++ b/Documentation/devicetree/bindings/sound/mvebu-audio.txt
@@ -2,13 +2,17 @@
 
 Required properties:
 
-- compatible: "marvell,mvebu-audio"
+- compatible:
+  "marvell,kirkwood-audio" for Kirkwood platforms
+  "marvell,dove-audio" for Dove platforms
 
 - reg: physical base address of the controller and length of memory mapped
   region.
 
-- interrupts: list of two irq numbers.
-  The first irq is used for data flow and the second one is used for errors.
+- interrupts:
+  with "marvell,kirkwood-audio", the audio interrupt
+  with "marvell,dove-audio", a list of two interrupts, the first for
+  the data flow, and the second for errors.
 
 - clocks: one or two phandles.
   The first one is mandatory and defines the internal clock.
@@ -21,7 +25,7 @@ Required properties:
 Example:
 
 i2s1: audio-controller@b4000 {
-	compatible = "marvell,mvebu-audio";
+	compatible = "marvell,dove-audio";
 	reg = <0xb4000 0x2210>;
 	interrupts = <21>, <22>;
 	clocks = <&gate_clk 13>;
diff --git a/sound/soc/kirkwood/kirkwood-i2s.c b/sound/soc/kirkwood/kirkwood-i2s.c
index 7fce340..0f3d73d 100644
--- a/sound/soc/kirkwood/kirkwood-i2s.c
+++ b/sound/soc/kirkwood/kirkwood-i2s.c
@@ -559,7 +559,8 @@ static int kirkwood_i2s_dev_remove(struct platform_device *pdev)
 
 #ifdef CONFIG_OF
 static struct of_device_id mvebu_audio_of_match[] = {
-	{ .compatible = "marvell,mvebu-audio" },
+	{ .compatible = "marvell,kirkwood-audio" },
+	{ .compatible = "marvell,dove-audio" },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, mvebu_audio_of_match);
-- 
1.8.1.2

Re: [PATCH] ASoC: kirkwood: change the compatible string of the kirkwood-i2s driver

[Jean-Francois Moine]

On Thu,  5 Sep 2013 14:56:31 +0200
Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:

> The compatible string of the kirkwood-i2s driver was chosen as
> "marvell,mvebu-audio". Using such a compatible string is not a good
> idea, since "mvebu" is the name of a large family of SOCs, in which
> new, unknown SOCs will be coming in the future. It is therefore
> impossible to know what will be evolutions of this hardware block in
> the next generations of the SOCs. For this reason, the recommandation
> for compatible strings of on-SOCs devices has always been to use the
> name of the oldest SOC that has the hardware block. New SOCs that have
> an exactly compatible hardware block can reference it using the same
> compatible string. See [1], [2] and [3] for various cases were this
> suggestion was made, including from Rob Herring, a Device Tree binding
> maintainer.
> 
> As an example, there are already small differences between current
> generations:
> 
>  * On Kirkwood, only one interrupt is used for audio.
>  * On Dove, two interrupts are used, one for audio data and one for
>    error reporting.
> 
> In the near future, I'll be adding audio support to Armada 370, which
> allows has the same hardware block (but maybe with minor variants).
> 
> Therefore, this patch changes the driver to accept
> "marvell,kirkwood-audio" and "marvell,dove-audio" as compatible
> strings instead of the too-generic "marvell,mvebu-audio". The reason
> for the two different compatible strings is the difference in the
> number of interrupts used by the two SOCs for audio.
> 
> This Device Tree binding has never been part of a Linux kernel stable
> release so far, so it can be changed now without breaking backward
> compatibility.
> 
> [1] http://lists.infradead.org/pipermail/linux-mtd/2012-March/040417.html
> [2] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-April/161065.html
> [3] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-March/087702.html
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---

The name of the kirwood audio driver has been changed from
"kirkwood-i2s" to "mvebu-audio" by Russell King
(http://www.spinics.net/lists/arm-kernel/msg264185.html).
I have just used this name in the DT.

Then, the second interrupt in Dove is not used in the driver. If it
should, it would be present in the DTs of the dove boards only, so,
actually, there is not need to have different compatibles.

Eventually, you are talking about some other mvebu SoCs. Before adding
any new compatible, it would be nice to know if, for example, some
changes are needed in the kirkwood/dove audio driver for the Armada 370...

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] ASoC: kirkwood: change the compatible string of the kirkwood-i2s driver

[Thomas Petazzoni]

Dear Jean-Francois Moine,

On Thu, 5 Sep 2013 18:54:56 +0200, Jean-Francois Moine wrote:

> The name of the kirwood audio driver has been changed from
> "kirkwood-i2s" to "mvebu-audio" by Russell King
> (http://www.spinics.net/lists/arm-kernel/msg264185.html).
> I have just used this name in the DT.

I don't care about the driver name: it can be changed whenever you want.

On the opposite, the DT compatible string becomes part of the kernel
ABI, and therefore cannot be changed once it has been defined. This is
why I worry about this one.

And the compatible string doesn't necessarily need to match the driver
name.

> Then, the second interrupt in Dove is not used in the driver. If it
> should, it would be present in the DTs of the dove boards only, so,
> actually, there is not need to have different compatibles.

Indeed the second interrupt is not used in the driver, but the binding
still specifies that two interrupts have to be mentioned, which is
incorrect for Kirkwood. The fact that on Kirkwood there is one
interrupt, while Dove has two, is an indication that the IP block is
not exactly the same. Should this error interrupt become useful in the
driver, it should be done only for Dove, so it makes sense to have two
different compatible strings.

> Eventually, you are talking about some other mvebu SoCs. Before adding
> any new compatible, it would be nice to know if, for example, some
> changes are needed in the kirkwood/dove audio driver for the Armada 370...

So far, it looks like no changes should be needed. I'm sure this will
bring you to the conclusion that 'mvebu-audio' is a good compatible
string: it is not because while we know what Armada 370 looks today, we
have absolutely no idea what the next Armada <something> will look
like. Maybe it will use the same audio IP block, with just a few little
variations.

Again, please read the mails from Rob Herring and Arnd Bergmann. It has
been said numerous times that using the name of the oldest SoC that has
the compatible IP block was the right choice for the compatible string.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH] ASoC: kirkwood: change the compatible string of the kirkwood-i2s driver

[Mark Brown]

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

On Thu, Sep 05, 2013 at 07:03:41PM +0200, Thomas Petazzoni wrote:

> not exactly the same. Should this error interrupt become useful in the
> driver, it should be done only for Dove, so it makes sense to have two
> different compatible strings.

In general it's reasonable to add compatible strings for the SoCs anyway
since even if we don't know of any reason why we'd want to distingush
between them right now it's possible that in the future some reason
might be discovered (such as an erratum being found).  Worst case the
kernel just treats both compatible strings identically which isn't a
great loss.  It's not essential though.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

Re: [PATCH] ASoC: kirkwood: change the compatible string of the kirkwood-i2s driver

[Jean-Francois Moine]

On Thu, 5 Sep 2013 19:03:41 +0200
Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:

> Again, please read the mails from Rob Herring and Arnd Bergmann. It has
> been said numerous times that using the name of the oldest SoC that has
> the compatible IP block was the right choice for the compatible string.

OK. Thanks.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] ASoC: kirkwood: change the compatible string of the kirkwood-i2s driver

[Mark Brown]

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

On Thu, Sep 05, 2013 at 02:56:31PM +0200, Thomas Petazzoni wrote:
> The compatible string of the kirkwood-i2s driver was chosen as
> "marvell,mvebu-audio". Using such a compatible string is not a good
> idea, since "mvebu" is the name of a large family of SOCs, in which
> new, unknown SOCs will be coming in the future. It is therefore

Applied, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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