[PATCH 0/5] Simple-audio-card support for HP t5325

[PATCH 0/5] Simple-audio-card support for HP t5325

[Andrew Lunn]

This patchset allows the Kirkwood based HP t5325 device make use of
simple-card to instantiate its audio hardware.

Patch 1 Adds a DT binding to the ALC5623 codec
Patch 2 Makes use of this new binding
Patch 3 Add fixes up the kirkwood sound node in DT
Patch 4 Extends simple-card to support the hw_params() needed
        when using MVEBU sound hardware.
Patch 5 Swaps t5323 to using DT, removing all need for C code.

Andrew Lunn (5):
  ASoC: alc5623: Add device tree binding
  ARM: Kirkwood: Use DT to instansiate codec
  ARM: Kirkwood: DT: Add missing #sound-dai-cells property
  ASoC: simple-card: Support setting mclk via a fixed factor
  ARM: Kirkwood: t5325: Use simple-card to instantiate audio

 .../devicetree/bindings/sound/alc5623.txt          | 23 ++++++++++++
 .../devicetree/bindings/sound/simple-card.txt      |  2 ++
 arch/arm/boot/dts/kirkwood-t5325.dts               | 33 +++++++++++++++++
 arch/arm/boot/dts/kirkwood.dtsi                    |  1 +
 arch/arm/mach-mvebu/board-t5325.c                  | 41 ----------------------
 arch/arm/mach-mvebu/board.h                        |  6 ----
 arch/arm/mach-mvebu/kirkwood.c                     |  3 --
 sound/soc/codecs/alc5623.c                         | 13 +++++++
 sound/soc/generic/simple-card.c                    | 28 +++++++++++++++
 9 files changed, 100 insertions(+), 50 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/alc5623.txt
 delete mode 100644 arch/arm/mach-mvebu/board-t5325.c

-- 
1.9.2

[PATCH 1/5] ASoC: alc5623: Add device tree binding

[Andrew Lunn]

Let the ALC5623 codec be instantiated from DT. Add a simple binding
for the additional control register and the jack detect register.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
I followed the example of the WM8903 binding which allows register
values to be placed into DT.
---
 .../devicetree/bindings/sound/alc5623.txt          | 23 ++++++++++++++++++++++
 sound/soc/codecs/alc5623.c                         | 13 ++++++++++++
 2 files changed, 36 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/alc5623.txt

diff --git a/Documentation/devicetree/bindings/sound/alc5623.txt b/Documentation/devicetree/bindings/sound/alc5623.txt
new file mode 100644
index 000000000000..f7f71346e338
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/alc5623.txt
@@ -0,0 +1,23 @@
+ALC5621/ALC5622/ALC5623 audio Codec
+
+Required properties:
+
+ - compatible:	"realtek,alc5623"
+ - reg:		the I2C address of the device.
+
+Optional properties:
+
+  - add-ctrl:	Default register value for Reg-40h, Additional Control Register.
+		If absent, the default is 0.
+
+  - jack-det-ctrl:	Default register value for Reg-5Ah, Jack Detect
+			Control Register. If absent, the default is 0.
+
+Example:
+
+			alc5621: alc5621 at 1a {
+				compatible = "alc5621";
+				reg = <0x1a>;
+				add-ctrl = <0x3700>;
+				jack-det-ctrl = <0x4810>;
+			};
diff --git a/sound/soc/codecs/alc5623.c b/sound/soc/codecs/alc5623.c
index 2acf82f4a08a..1f9273d20b39 100644
--- a/sound/soc/codecs/alc5623.c
+++ b/sound/soc/codecs/alc5623.c
@@ -23,6 +23,7 @@
 #include <linux/i2c.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
+#include <linux/of.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
@@ -998,8 +999,10 @@ static int alc5623_i2c_probe(struct i2c_client *client,
 {
 	struct alc5623_platform_data *pdata;
 	struct alc5623_priv *alc5623;
+	struct device_node *np;
 	unsigned int vid1, vid2;
 	int ret;
+	u32 val32;
 
 	alc5623 = devm_kzalloc(&client->dev, sizeof(struct alc5623_priv),
 			       GFP_KERNEL);
@@ -1040,6 +1043,16 @@ static int alc5623_i2c_probe(struct i2c_client *client,
 	if (pdata) {
 		alc5623->add_ctrl = pdata->add_ctrl;
 		alc5623->jack_det_ctrl = pdata->jack_det_ctrl;
+	} else {
+		if (client->dev.of_node) {
+			np = client->dev.of_node;
+			ret = of_property_read_u32(np, "add-ctrl", &val32);
+			if (ret >= 0)
+				alc5623->add_ctrl = val32;
+			ret = of_property_read_u32(np, "jack-det-ctrl", &val32);
+			if (ret >= 0)
+				alc5623->jack_det_ctrl = val32;
+		}
 	}
 
 	alc5623->id = vid2;
-- 
1.9.2

[PATCH 1/5] ASoC: alc5623: Add device tree binding

[Mark Brown]

On Thu, Apr 17, 2014 at 05:53:10PM +0200, Andrew Lunn wrote:

> + - compatible:	"realtek,alc5623"

You've not added an ID table to the driver for this.

> +  - add-ctrl:	Default register value for Reg-40h, Additional Control Register.
> +		If absent, the default is 0.

> +  - jack-det-ctrl:	Default register value for Reg-5Ah, Jack Detect
> +			Control Register. If absent, the default is 0.

I would expect the default for these to be to leave the hardware
defaults untouched - why is it different, what does setting to zero mean?
-------------- 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/20140418/055b6a53/attachment.sig>

[PATCH 1/5] ASoC: alc5623: Add device tree binding

[Andrew Lunn]

On Fri, Apr 18, 2014 at 05:33:51PM +0100, Mark Brown wrote:
> On Thu, Apr 17, 2014 at 05:53:10PM +0200, Andrew Lunn wrote:
> 
> > + - compatible:	"realtek,alc5623"
> 
> You've not added an ID table to the driver for this.

The driver already has:

static const struct i2c_device_id alc5623_i2c_table[] = {
        {"alc5621", 0x21},
        {"alc5622", 0x22},
        {"alc5623", 0x23},
        {}
};
MODULE_DEVICE_TABLE(i2c, alc5623_i2c_table);

which is enough for the i2c layer to load the driver when it walks the
nodes under the i2c bus driver in the DT.

> > +  - add-ctrl:	Default register value for Reg-40h, Additional Control Register.
> > +		If absent, the default is 0.
> 
> > +  - jack-det-ctrl:	Default register value for Reg-5Ah, Jack Detect
> > +			Control Register. If absent, the default is 0.
> 
> I would expect the default for these to be to leave the hardware
> defaults untouched - why is it different, what does setting to zero mean?

The description is wrong. I will fix it. If the property is absent, or
the value is zero, the register is left alone.

    Andrew

[PATCH 1/5] ASoC: alc5623: Add device tree binding

[Mark Brown]

On Fri, Apr 18, 2014 at 08:17:35PM +0200, Andrew Lunn wrote:

> The driver already has:

> static const struct i2c_device_id alc5623_i2c_table[] = {
>         {"alc5621", 0x21},
>         {"alc5622", 0x22},
>         {"alc5623", 0x23},
>         {}
> };
> MODULE_DEVICE_TABLE(i2c, alc5623_i2c_table);

> which is enough for the i2c layer to load the driver when it walks the
> nodes under the i2c bus driver in the DT.

You still need to add the compatible strings to the driver explicitly,
device names can be and are duplicated between manufacturers sometimes
and it makes things directly greppable.
-------------- 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/20140418/1d18de28/attachment.sig>

[PATCH 2/5] ARM: Kirkwood: Use DT to instansiate codec

[Andrew Lunn]

Now that the ALC5623 codec has a DT binding, instansiate it using DT
rather than a platform device.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 arch/arm/boot/dts/kirkwood-t5325.dts |  8 ++++++++
 arch/arm/mach-mvebu/board-t5325.c    | 15 ---------------
 2 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/arch/arm/boot/dts/kirkwood-t5325.dts b/arch/arm/boot/dts/kirkwood-t5325.dts
index 0bd70d928c69..dbb730642543 100644
--- a/arch/arm/boot/dts/kirkwood-t5325.dts
+++ b/arch/arm/boot/dts/kirkwood-t5325.dts
@@ -127,6 +127,14 @@
 
 		i2c at 11000 {
 			status = "okay";
+
+			alc5621: alc5621 at 1a {
+				compatible = "realtek,alc5621";
+				reg = <0x1a>;
+				#sound-dai-cells = <0>;
+				add-ctrl = <0x3700>;
+				jack-det-ctrl = <0x4810>;
+			};
 		};
 
 		serial at 12000 {
diff --git a/arch/arm/mach-mvebu/board-t5325.c b/arch/arm/mach-mvebu/board-t5325.c
index 65ace6db9f28..f2401b298218 100644
--- a/arch/arm/mach-mvebu/board-t5325.c
+++ b/arch/arm/mach-mvebu/board-t5325.c
@@ -11,10 +11,8 @@
  */
 
 #include <linux/kernel.h>
-#include <linux/i2c.h>
 #include <linux/init.h>
 #include <linux/platform_device.h>
-#include <sound/alc5623.h>
 #include "board.h"
 
 static struct platform_device hp_t5325_audio_device = {
@@ -22,20 +20,7 @@ static struct platform_device hp_t5325_audio_device = {
 	.id		= -1,
 };
 
-static struct alc5623_platform_data alc5621_data = {
-	.add_ctrl = 0x3700,
-	.jack_det_ctrl = 0x4810,
-};
-
-static struct i2c_board_info i2c_board_info[] __initdata = {
-	{
-		I2C_BOARD_INFO("alc5621", 0x1a),
-		.platform_data = &alc5621_data,
-	},
-};
-
 void __init t5325_init(void)
 {
-	i2c_register_board_info(0, i2c_board_info, ARRAY_SIZE(i2c_board_info));
 	platform_device_register(&hp_t5325_audio_device);
 }
-- 
1.9.2

[PATCH 2/5] ARM: Kirkwood: Use DT to instansiate codec

[Jason Cooper]

Andrew,

First, thanks for following through with the kirkwood DT conversion.

I have one small request below:

On Thu, Apr 17, 2014 at 05:53:11PM +0200, Andrew Lunn wrote:
> Now that the ALC5623 codec has a DT binding, instansiate it using DT
> rather than a platform device.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  arch/arm/boot/dts/kirkwood-t5325.dts |  8 ++++++++
>  arch/arm/mach-mvebu/board-t5325.c    | 15 ---------------
>  2 files changed, 8 insertions(+), 15 deletions(-)

Since the audio device isn't critical to boot, can we split this patch
and the other DT patch in two?  That'll prevent branch
dependency/merge conflicts between mvebu/dt and mvebu/soc.

thx,

Jason.

[PATCH 3/5] ARM: Kirkwood: DT: Add missing #sound-dai-cells property

[Andrew Lunn]

The sound node is missing a #sound-dai-cells property. Add it, so that
the sounds node can be used in combination with the simple-audio-card
binding.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 arch/arm/boot/dts/kirkwood.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi
index 90384587c278..919d1f7f2eef 100644
--- a/arch/arm/boot/dts/kirkwood.dtsi
+++ b/arch/arm/boot/dts/kirkwood.dtsi
@@ -314,6 +314,7 @@
 
 		audio0: audio-controller at a0000 {
 			compatible = "marvell,kirkwood-audio";
+			#sound-dai-cells = <0>;
 			reg = <0xa0000 0x2210>;
 			interrupts = <24>;
 			clocks = <&gate_clk 9>;
-- 
1.9.2

[PATCH 4/5] ASoC: simple-card: Support setting mclk via a fixed factor

[Andrew Lunn]

Some platforms require that the codecs mclk is a fixed multiplication
factor of the audio stream rate. Add a optional property to the
binding to hold this factor and implement a hw_params() function to
make use of it.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 .../devicetree/bindings/sound/simple-card.txt      |  2 ++
 sound/soc/generic/simple-card.c                    | 28 ++++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
index 131aa2ad7f1a..c4379c9f9f03 100644
--- a/Documentation/devicetree/bindings/sound/simple-card.txt
+++ b/Documentation/devicetree/bindings/sound/simple-card.txt
@@ -18,6 +18,8 @@ Optional properties:
 					  Each entry is a pair of strings, the first being the
 					  connection's sink, the second being the connection's
 					  source.
+- simple-audio-card,mclk-factor		: Multiplication factor between stream rate and codec
+					  mclk.
 - dai-tdm-slot-num			: Please refer to tdm-slot.txt.
 - dai-tdm-slot-width			: Please refer to tdm-slot.txt.
 
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 21f1ccbdf582..5ecaf57ab712 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -24,6 +24,7 @@ struct simple_card_data {
 		struct asoc_simple_dai cpu_dai;
 		struct asoc_simple_dai codec_dai;
 	} *dai_props;
+	unsigned int mclk_factor;
 	struct snd_soc_dai_link dai_link[];	/* dynamically allocated */
 };
 
@@ -151,6 +152,28 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
 	return 0;
 }
 
+static int simple_card_hw_params(struct snd_pcm_substream *substream,
+				 struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+	struct simple_card_data *priv = snd_soc_card_get_drvdata(rtd->card);
+	unsigned int mclk;
+	int ret = 0;
+
+	if (priv->mclk_factor) {
+		mclk = params_rate(params) * priv->mclk_factor;
+		ret = snd_soc_dai_set_sysclk(codec_dai, 0, mclk,
+					     SND_SOC_CLOCK_IN);
+	}
+
+	return ret;
+}
+
+static struct snd_soc_ops simple_card_ops = {
+	.hw_params = simple_card_hw_params,
+};
+
 static int simple_card_cpu_codec_of(struct device_node *node,
 				int daifmt,
 				struct snd_soc_dai_link *dai_link,
@@ -255,6 +278,7 @@ static int asoc_simple_card_parse_of(struct device_node *node,
 		sprintf(name, "%s-%s", dai_link->cpu_dai_name,
 					dai_link->codec_dai_name);
 		dai_link->name = dai_link->stream_name = name;
+		dai_link->ops = &simple_card_ops;
 
 		if (!multi)
 			break;
@@ -263,6 +287,10 @@ static int asoc_simple_card_parse_of(struct device_node *node,
 		dai_props++;
 	}
 
+	/* Factor to mclk, used in hw_params() */
+	of_property_read_u32(node, "simple-audio-card,mclk-factor",
+			     &priv->mclk_factor);
+
 	/* card name is created from CPU/CODEC dai name */
 	dai_link = priv->snd_card.dai_link;
 	if (!priv->snd_card.name)
-- 
1.9.2

[PATCH 4/5] ASoC: simple-card: Support setting mclk via a fixed factor

[Mark Brown]

On Thu, Apr 17, 2014 at 05:53:13PM +0200, Andrew Lunn wrote:

> +- simple-audio-card,mclk-factor		: Multiplication factor between stream rate and codec
> +					  mclk.

The normal name for this is fs rather than factor.

> +	if (priv->mclk_factor) {
> +		mclk = params_rate(params) * priv->mclk_factor;
> +		ret = snd_soc_dai_set_sysclk(codec_dai, 0, mclk,
> +					     SND_SOC_CLOCK_IN);
> +	}

We also need to consider what happens in cases where a clock is
specified via the clocks property.  I would expect this to be used to
set the rate for that clock as well.
-------------- 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/20140418/968e5651/attachment.sig>

[PATCH 5/5] ARM: Kirkwood: t5325: Use simple-card to instantiate audio

[Andrew Lunn]

Add device tree nodes to instantiate the audio drivers on the HP T5325
device. Remove platform driver instantiating of the audio, which
results in board-t5325.c being removed.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 arch/arm/boot/dts/kirkwood-t5325.dts | 25 +++++++++++++++++++++++++
 arch/arm/mach-mvebu/board-t5325.c    | 26 --------------------------
 arch/arm/mach-mvebu/board.h          |  6 ------
 arch/arm/mach-mvebu/kirkwood.c       |  3 ---
 4 files changed, 25 insertions(+), 35 deletions(-)
 delete mode 100644 arch/arm/mach-mvebu/board-t5325.c

diff --git a/arch/arm/boot/dts/kirkwood-t5325.dts b/arch/arm/boot/dts/kirkwood-t5325.dts
index dbb730642543..2cf3fe38404d 100644
--- a/arch/arm/boot/dts/kirkwood-t5325.dts
+++ b/arch/arm/boot/dts/kirkwood-t5325.dts
@@ -192,6 +192,31 @@
 		gpios = <&gpio1 17 GPIO_ACTIVE_HIGH>;
 	};
 
+	sound {
+		compatible = "simple-audio-card";
+		simple-audio-card,format = "i2s";
+		simple-audio-card,routing =
+			"Headphone Jack", "HPL",
+			"Headphone Jack", "HPR",
+			"Speaker", "SPKOUT",
+			"Speaker", "SPKOUTN",
+			"MIC1", "Mic Jack",
+			"MIC2", "Mic Jack";
+		simple-audio-card,widgets =
+			"Headphone", "Headphone Jack",
+			"Speaker", "Speaker",
+			"Microphone", "Mic Jack";
+
+		simple-audio-card,mclk-factor = <256>;
+
+		simple-audio-card,cpu {
+			sound-dai = <&audio>;
+		};
+
+		simple-audio-card,codec {
+			sound-dai = <&alc5621>;
+		};
+	};
 };
 
 &mdio {
diff --git a/arch/arm/mach-mvebu/board-t5325.c b/arch/arm/mach-mvebu/board-t5325.c
deleted file mode 100644
index f2401b298218..000000000000
--- a/arch/arm/mach-mvebu/board-t5325.c
+++ /dev/null
@@ -1,26 +0,0 @@
-/*
- * HP T5325 Board Setup
- *
- * Copyright (C) 2014
- *
- * Andrew Lunn <andrew@lunn.ch>
- *
- * This file is licensed under the terms of the GNU General Public
- * License version 2.  This program is licensed "as is" without any
- * warranty of any kind, whether express or implied.
- */
-
-#include <linux/kernel.h>
-#include <linux/init.h>
-#include <linux/platform_device.h>
-#include "board.h"
-
-static struct platform_device hp_t5325_audio_device = {
-	.name		= "t5325-audio",
-	.id		= -1,
-};
-
-void __init t5325_init(void)
-{
-	platform_device_register(&hp_t5325_audio_device);
-}
diff --git a/arch/arm/mach-mvebu/board.h b/arch/arm/mach-mvebu/board.h
index de7f0a191394..9c7bb4386f8b 100644
--- a/arch/arm/mach-mvebu/board.h
+++ b/arch/arm/mach-mvebu/board.h
@@ -13,10 +13,4 @@
 #ifndef __ARCH_MVEBU_BOARD_H
 #define __ARCH_MVEBU_BOARD_H
 
-#ifdef CONFIG_MACH_T5325
-void t5325_init(void);
-#else
-static inline void t5325_init(void) {};
-#endif
-
 #endif
diff --git a/arch/arm/mach-mvebu/kirkwood.c b/arch/arm/mach-mvebu/kirkwood.c
index 120207fc36f1..c30bc316f40d 100644
--- a/arch/arm/mach-mvebu/kirkwood.c
+++ b/arch/arm/mach-mvebu/kirkwood.c
@@ -180,9 +180,6 @@ static void __init kirkwood_dt_init(void)
 	kirkwood_pm_init();
 	kirkwood_dt_eth_fixup();
 
-	if (of_machine_is_compatible("hp,t5325"))
-		t5325_init();
-
 	of_platform_populate(NULL, of_default_bus_match_table, auxdata, NULL);
 }
 
-- 
1.9.2

[PATCH 0/5] Simple-audio-card support for HP t5325

[Jason Cooper]

Mark,

On Thu, Apr 17, 2014 at 05:53:09PM +0200, Andrew Lunn wrote:
> This patchset allows the Kirkwood based HP t5325 device make use of
> simple-card to instantiate its audio hardware.
> 
> Patch 1 Adds a DT binding to the ALC5623 codec
> Patch 2 Makes use of this new binding
> Patch 3 Add fixes up the kirkwood sound node in DT
> Patch 4 Extends simple-card to support the hw_params() needed
>         when using MVEBU sound hardware.
> Patch 5 Swaps t5323 to using DT, removing all need for C code.
> 
> Andrew Lunn (5):
>   ASoC: alc5623: Add device tree binding
>   ARM: Kirkwood: Use DT to instansiate codec
>   ARM: Kirkwood: DT: Add missing #sound-dai-cells property
>   ASoC: simple-card: Support setting mclk via a fixed factor
>   ARM: Kirkwood: t5325: Use simple-card to instantiate audio
> 
>  .../devicetree/bindings/sound/alc5623.txt          | 23 ++++++++++++
>  .../devicetree/bindings/sound/simple-card.txt      |  2 ++

Are you alright with taking the above,

>  arch/arm/boot/dts/kirkwood-t5325.dts               | 33 +++++++++++++++++
>  arch/arm/boot/dts/kirkwood.dtsi                    |  1 +
>  arch/arm/mach-mvebu/board-t5325.c                  | 41 ----------------------
>  arch/arm/mach-mvebu/board.h                        |  6 ----
>  arch/arm/mach-mvebu/kirkwood.c                     |  3 --

while I take these,

>  sound/soc/codecs/alc5623.c                         | 13 +++++++
>  sound/soc/generic/simple-card.c                    | 28 +++++++++++++++

and you take these?

That should be 1 and 4 for you and 2, 3, and 5 for me before Andrew does
the patch split I asked for in my other response.

thx,

Jason.