[alsa-devel] [PATCH 0/5] Simple-audio-card support for HP t5325
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
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@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;
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?
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
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.
Hi Andrew,
Please find my comment below.
On Thu, Apr 17, 2014 at 9:23 PM, Andrew Lunn andrew@lunn.ch wrote:
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@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)
of_property_read_u32 returns 0 on success, why "if (ret >= 0)" check ?
alc5623->add_ctrl = val32;ret = of_property_read_u32(np, "jack-det-ctrl",&val32);
+ if (ret >= 0)
same as above
alc5623->jack_det_ctrl = val32;} } alc5623->id = vid2;-- 1.9.2
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
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@11000 { status = "okay"; + + alc5621: alc5621@1a { + compatible = "realtek,alc5621"; + reg = <0x1a>; + #sound-dai-cells = <0>; + add-ctrl = <0x3700>; + jack-det-ctrl = <0x4810>; + }; };
serial@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); }
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.
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@a0000 { compatible = "marvell,kirkwood-audio"; + #sound-dai-cells = <0>; reg = <0xa0000 0x2210>; interrupts = <24>; clocks = <&gate_clk 9>;
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)
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.
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); }
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.
participants (4)
-
Andrew Lunn -
Anil Kumar -
Jason Cooper -
Mark Brown