[alsa-devel] [PATCH 0/3] ARM: gr8: evb: Enable the i2s codec
The GR8-EVB comes with a wm8978 codec connected to the SoC through an I2S bus.
A few patches were needed for it to work with simple-card, those are the very first patches.
The last one will add that card to the relevant device tree so that we can use it.
Let me know what you think, Maxime
Maxime Ripard (3): ASoC: sunxi: i2s: Implement set_sysclk ASoC: wm8978: Adjust clock indices so that simple card works ARM: gr8: evb: Add i2s codec
arch/arm/boot/dts/ntc-gr8-evb.dts | 14 ++++++++- sound/soc/codecs/wm8978.h | 2 +- sound/soc/sunxi/sun4i-i2s.c | 53 ++++++++++++++++++++++---------- 3 files changed, 53 insertions(+), 16 deletions(-)
base-commit: ff9ed64af60bd87cc1c1225e595a33fe248b02bf
In our i2s driver, we were previously trying to guess which oversample the user wanted to use by looking at the rate and trying to max it.
However, the cards, and especially simple-card with its mclk-fs property will already provide the expected oversample ratio by using the set_sysclk callback.
We can thus implement it and remove the logic to deal with the runtime guess.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com --- sound/soc/sunxi/sun4i-i2s.c | 53 +++++++++++++++++++++++++++----------- 1 file changed, 38 insertions(+), 15 deletions(-)
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c index a7653114e895..f24d19526603 100644 --- a/sound/soc/sunxi/sun4i-i2s.c +++ b/sound/soc/sunxi/sun4i-i2s.c @@ -93,6 +93,8 @@ struct sun4i_i2s { struct clk *mod_clk; struct regmap *regmap;
+ unsigned int mclk_freq; + struct snd_dmaengine_dai_dma_data capture_dma_data; struct snd_dmaengine_dai_dma_data playback_dma_data; }; @@ -158,14 +160,24 @@ static int sun4i_i2s_get_mclk_div(struct sun4i_i2s *i2s, }
static int sun4i_i2s_oversample_rates[] = { 128, 192, 256, 384, 512, 768 }; +static bool sun4i_i2s_oversample_is_valid(unsigned int oversample) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(sun4i_i2s_oversample_rates); i++) + if (sun4i_i2s_oversample_rates[i] == oversample) + return true; + + return false; +}
static int sun4i_i2s_set_clk_rate(struct sun4i_i2s *i2s, unsigned int rate, unsigned int word_size) { - unsigned int clk_rate; + unsigned int oversample_rate, clk_rate; int bclk_div, mclk_div; - int ret, i; + int ret;
switch (rate) { case 176400: @@ -197,21 +209,18 @@ static int sun4i_i2s_set_clk_rate(struct sun4i_i2s *i2s, if (ret) return ret;
- /* Always favor the highest oversampling rate */ - for (i = (ARRAY_SIZE(sun4i_i2s_oversample_rates) - 1); i >= 0; i--) { - unsigned int oversample_rate = sun4i_i2s_oversample_rates[i]; - - bclk_div = sun4i_i2s_get_bclk_div(i2s, oversample_rate, - word_size); - mclk_div = sun4i_i2s_get_mclk_div(i2s, oversample_rate, - clk_rate, - rate); + oversample_rate = i2s->mclk_freq / rate; + if (!sun4i_i2s_oversample_is_valid(oversample_rate)) + return -EINVAL;
- if ((bclk_div >= 0) && (mclk_div >= 0)) - break; - } + bclk_div = sun4i_i2s_get_bclk_div(i2s, oversample_rate, + word_size); + if (bclk_div < 0) + return -EINVAL;
- if ((bclk_div < 0) || (mclk_div < 0)) + mclk_div = sun4i_i2s_get_mclk_div(i2s, oversample_rate, + clk_rate, rate); + if (mclk_div < 0) return -EINVAL;
regmap_write(i2s->regmap, SUN4I_I2S_CLK_DIV_REG, @@ -481,9 +490,23 @@ static void sun4i_i2s_shutdown(struct snd_pcm_substream *substream, regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG, 0); }
+static int sun4i_i2s_set_sysclk(struct snd_soc_dai *dai, int clk_id, + unsigned int freq, int dir) +{ + struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai); + + if (clk_id != 0) + return -EINVAL; + + i2s->mclk_freq = freq; + + return 0; +} + static const struct snd_soc_dai_ops sun4i_i2s_dai_ops = { .hw_params = sun4i_i2s_hw_params, .set_fmt = sun4i_i2s_set_fmt, + .set_sysclk = sun4i_i2s_set_sysclk, .shutdown = sun4i_i2s_shutdown, .startup = sun4i_i2s_startup, .trigger = sun4i_i2s_trigger,
The patch
ASoC: sunxi: i2s: Implement set_sysclk
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From b2b7b56f713ab833413548b119c53bbe2a9a9f8f Mon Sep 17 00:00:00 2001
From: Maxime Ripard maxime.ripard@free-electrons.com Date: Mon, 7 Nov 2016 14:08:19 +0100 Subject: [PATCH] ASoC: sunxi: i2s: Implement set_sysclk
In our i2s driver, we were previously trying to guess which oversample the user wanted to use by looking at the rate and trying to max it.
However, the cards, and especially simple-card with its mclk-fs property will already provide the expected oversample ratio by using the set_sysclk callback.
We can thus implement it and remove the logic to deal with the runtime guess.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/sunxi/sun4i-i2s.c | 53 ++++++++++++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 15 deletions(-)
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c index a7653114e895..f24d19526603 100644 --- a/sound/soc/sunxi/sun4i-i2s.c +++ b/sound/soc/sunxi/sun4i-i2s.c @@ -93,6 +93,8 @@ struct sun4i_i2s { struct clk *mod_clk; struct regmap *regmap;
+ unsigned int mclk_freq; + struct snd_dmaengine_dai_dma_data capture_dma_data; struct snd_dmaengine_dai_dma_data playback_dma_data; }; @@ -158,14 +160,24 @@ static int sun4i_i2s_get_mclk_div(struct sun4i_i2s *i2s, }
static int sun4i_i2s_oversample_rates[] = { 128, 192, 256, 384, 512, 768 }; +static bool sun4i_i2s_oversample_is_valid(unsigned int oversample) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(sun4i_i2s_oversample_rates); i++) + if (sun4i_i2s_oversample_rates[i] == oversample) + return true; + + return false; +}
static int sun4i_i2s_set_clk_rate(struct sun4i_i2s *i2s, unsigned int rate, unsigned int word_size) { - unsigned int clk_rate; + unsigned int oversample_rate, clk_rate; int bclk_div, mclk_div; - int ret, i; + int ret;
switch (rate) { case 176400: @@ -197,21 +209,18 @@ static int sun4i_i2s_set_clk_rate(struct sun4i_i2s *i2s, if (ret) return ret;
- /* Always favor the highest oversampling rate */ - for (i = (ARRAY_SIZE(sun4i_i2s_oversample_rates) - 1); i >= 0; i--) { - unsigned int oversample_rate = sun4i_i2s_oversample_rates[i]; - - bclk_div = sun4i_i2s_get_bclk_div(i2s, oversample_rate, - word_size); - mclk_div = sun4i_i2s_get_mclk_div(i2s, oversample_rate, - clk_rate, - rate); + oversample_rate = i2s->mclk_freq / rate; + if (!sun4i_i2s_oversample_is_valid(oversample_rate)) + return -EINVAL;
- if ((bclk_div >= 0) && (mclk_div >= 0)) - break; - } + bclk_div = sun4i_i2s_get_bclk_div(i2s, oversample_rate, + word_size); + if (bclk_div < 0) + return -EINVAL;
- if ((bclk_div < 0) || (mclk_div < 0)) + mclk_div = sun4i_i2s_get_mclk_div(i2s, oversample_rate, + clk_rate, rate); + if (mclk_div < 0) return -EINVAL;
regmap_write(i2s->regmap, SUN4I_I2S_CLK_DIV_REG, @@ -481,9 +490,23 @@ static void sun4i_i2s_shutdown(struct snd_pcm_substream *substream, regmap_write(i2s->regmap, SUN4I_I2S_CTRL_REG, 0); }
+static int sun4i_i2s_set_sysclk(struct snd_soc_dai *dai, int clk_id, + unsigned int freq, int dir) +{ + struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai); + + if (clk_id != 0) + return -EINVAL; + + i2s->mclk_freq = freq; + + return 0; +} + static const struct snd_soc_dai_ops sun4i_i2s_dai_ops = { .hw_params = sun4i_i2s_hw_params, .set_fmt = sun4i_i2s_set_fmt, + .set_sysclk = sun4i_i2s_set_sysclk, .shutdown = sun4i_i2s_shutdown, .startup = sun4i_i2s_startup, .trigger = sun4i_i2s_trigger,
Using simple-card with the wm8978 doesn't work because simple card calls set_sysclk on the clock index 0, which is not the MCLK in the WM8978.
Adjust the clock definition so that the clock 0 is the MCLK.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com --- sound/soc/codecs/wm8978.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/wm8978.h b/sound/soc/codecs/wm8978.h index 6ae43495b7cf..0dcf6868dff6 100644 --- a/sound/soc/codecs/wm8978.h +++ b/sound/soc/codecs/wm8978.h @@ -78,8 +78,8 @@ enum wm8978_clk_id { };
enum wm8978_sysclk_src { + WM8978_MCLK = 0, WM8978_PLL, - WM8978_MCLK };
#endif /* __WM8978_H__ */
The patch
ASoC: wm8978: Adjust clock indices so that simple card works
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From fbd972d7f4a60677f6fbe558dc23e4029dc2d45d Mon Sep 17 00:00:00 2001
From: Maxime Ripard maxime.ripard@free-electrons.com Date: Mon, 7 Nov 2016 14:08:20 +0100 Subject: [PATCH] ASoC: wm8978: Adjust clock indices so that simple card works
Using simple-card with the wm8978 doesn't work because simple card calls set_sysclk on the clock index 0, which is not the MCLK in the WM8978.
Adjust the clock definition so that the clock 0 is the MCLK.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/codecs/wm8978.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/wm8978.h b/sound/soc/codecs/wm8978.h index 6ae43495b7cf..0dcf6868dff6 100644 --- a/sound/soc/codecs/wm8978.h +++ b/sound/soc/codecs/wm8978.h @@ -78,8 +78,8 @@ enum wm8978_clk_id { };
enum wm8978_sysclk_src { + WM8978_MCLK = 0, WM8978_PLL, - WM8978_MCLK };
#endif /* __WM8978_H__ */
The GR8-EVB comes with a wm8978 codec connected to the i2s bus.
Add a card in order to have it working
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com --- arch/arm/boot/dts/ntc-gr8-evb.dts | 14 ++++++++++++++ 1 file changed, 14 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/ntc-gr8-evb.dts b/arch/arm/boot/dts/ntc-gr8-evb.dts index 12b4317a4383..5291e425caf9 100644 --- a/arch/arm/boot/dts/ntc-gr8-evb.dts +++ b/arch/arm/boot/dts/ntc-gr8-evb.dts @@ -76,6 +76,20 @@ default-brightness-level = <8>; };
+ i2s { + compatible = "simple-audio-card"; + simple-audio-card,name = "gr8-evb-wm8978"; + simple-audio-card,format = "i2s"; + simple-audio-card,mclk-fs = <512>; + + simple-audio-card,cpu { + sound-dai = <&i2s0>; + }; + + simple-audio-card,codec { + sound-dai = <&wm8978>; + }; + };
panel { compatible = "allwinner,sun4i-a10-sub-evb-5-lcd";
On Mon, Nov 7, 2016 at 9:08 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
The GR8-EVB comes with a wm8978 codec connected to the i2s bus.
Add a card in order to have it working
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
arch/arm/boot/dts/ntc-gr8-evb.dts | 14 ++++++++++++++ 1 file changed, 14 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/ntc-gr8-evb.dts b/arch/arm/boot/dts/ntc-gr8-evb.dts index 12b4317a4383..5291e425caf9 100644 --- a/arch/arm/boot/dts/ntc-gr8-evb.dts +++ b/arch/arm/boot/dts/ntc-gr8-evb.dts @@ -76,6 +76,20 @@ default-brightness-level = <8>; };
i2s {
"sound" might be a better node name? The I2S controllers are also named "i2s".
Otherwise,
Acked-by: Chen-Yu Tsai wens@csie.org
compatible = "simple-audio-card";simple-audio-card,name = "gr8-evb-wm8978";simple-audio-card,format = "i2s";simple-audio-card,mclk-fs = <512>;simple-audio-card,cpu {sound-dai = <&i2s0>;};simple-audio-card,codec {sound-dai = <&wm8978>;};}; panel { compatible = "allwinner,sun4i-a10-sub-evb-5-lcd";-- git-series 0.8.11
On Mon, Nov 07, 2016 at 10:11:45PM +0800, Chen-Yu Tsai wrote:
On Mon, Nov 7, 2016 at 9:08 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
The GR8-EVB comes with a wm8978 codec connected to the i2s bus.
Add a card in order to have it working
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
arch/arm/boot/dts/ntc-gr8-evb.dts | 14 ++++++++++++++ 1 file changed, 14 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/ntc-gr8-evb.dts b/arch/arm/boot/dts/ntc-gr8-evb.dts index 12b4317a4383..5291e425caf9 100644 --- a/arch/arm/boot/dts/ntc-gr8-evb.dts +++ b/arch/arm/boot/dts/ntc-gr8-evb.dts @@ -76,6 +76,20 @@ default-brightness-level = <8>; };
i2s {"sound" might be a better node name? The I2S controllers are also named "i2s".
I know, but we also had the codec and SPDIF on this board, so sound was too generic to be meaningful I guess. I don't really care about the name though, if you have any suggestion...
Otherwise,
Acked-by: Chen-Yu Tsai wens@csie.org
Thanks! Maxime
On Tue, Nov 8, 2016 at 3:44 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
On Mon, Nov 07, 2016 at 10:11:45PM +0800, Chen-Yu Tsai wrote:
On Mon, Nov 7, 2016 at 9:08 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
The GR8-EVB comes with a wm8978 codec connected to the i2s bus.
Add a card in order to have it working
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
arch/arm/boot/dts/ntc-gr8-evb.dts | 14 ++++++++++++++ 1 file changed, 14 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/ntc-gr8-evb.dts b/arch/arm/boot/dts/ntc-gr8-evb.dts index 12b4317a4383..5291e425caf9 100644 --- a/arch/arm/boot/dts/ntc-gr8-evb.dts +++ b/arch/arm/boot/dts/ntc-gr8-evb.dts @@ -76,6 +76,20 @@ default-brightness-level = <8>; };
i2s {"sound" might be a better node name? The I2S controllers are also named "i2s".
I know, but we also had the codec and SPDIF on this board, so sound was too generic to be meaningful I guess. I don't really care about the name though, if you have any suggestion...
Well people seem to use "sound" for the sound card nodes...
What about "sound-analog" for this one, and "sound-spdif" for the SPDIF simple card? Or "analog-sound" and "spdif-sound" if that looks better.
ChenYu
Otherwise,
Acked-by: Chen-Yu Tsai wens@csie.org
Thanks! Maxime
-- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
On Tue, Nov 08, 2016 at 03:57:48PM +0800, Chen-Yu Tsai wrote:
On Tue, Nov 8, 2016 at 3:44 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
On Mon, Nov 07, 2016 at 10:11:45PM +0800, Chen-Yu Tsai wrote:
On Mon, Nov 7, 2016 at 9:08 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
The GR8-EVB comes with a wm8978 codec connected to the i2s bus.
Add a card in order to have it working
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
arch/arm/boot/dts/ntc-gr8-evb.dts | 14 ++++++++++++++ 1 file changed, 14 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/ntc-gr8-evb.dts b/arch/arm/boot/dts/ntc-gr8-evb.dts index 12b4317a4383..5291e425caf9 100644 --- a/arch/arm/boot/dts/ntc-gr8-evb.dts +++ b/arch/arm/boot/dts/ntc-gr8-evb.dts @@ -76,6 +76,20 @@ default-brightness-level = <8>; };
i2s {"sound" might be a better node name? The I2S controllers are also named "i2s".
I know, but we also had the codec and SPDIF on this board, so sound was too generic to be meaningful I guess. I don't really care about the name though, if you have any suggestion...
Well people seem to use "sound" for the sound card nodes...
What about "sound-analog" for this one, and "sound-spdif" for the SPDIF simple card? Or "analog-sound" and "spdif-sound" if that looks better.
sound-analog works for me. I'll either fix it in the v2, or while applying.
Thanks! Maxime
1;4600;0c On Tue, Nov 08, 2016 at 03:57:48PM +0800, Chen-Yu Tsai wrote:
On Tue, Nov 8, 2016 at 3:44 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
On Mon, Nov 07, 2016 at 10:11:45PM +0800, Chen-Yu Tsai wrote:
On Mon, Nov 7, 2016 at 9:08 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
The GR8-EVB comes with a wm8978 codec connected to the i2s bus.
Add a card in order to have it working
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
arch/arm/boot/dts/ntc-gr8-evb.dts | 14 ++++++++++++++ 1 file changed, 14 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/ntc-gr8-evb.dts b/arch/arm/boot/dts/ntc-gr8-evb.dts index 12b4317a4383..5291e425caf9 100644 --- a/arch/arm/boot/dts/ntc-gr8-evb.dts +++ b/arch/arm/boot/dts/ntc-gr8-evb.dts @@ -76,6 +76,20 @@ default-brightness-level = <8>; };
i2s {"sound" might be a better node name? The I2S controllers are also named "i2s".
I know, but we also had the codec and SPDIF on this board, so sound was too generic to be meaningful I guess. I don't really care about the name though, if you have any suggestion...
Well people seem to use "sound" for the sound card nodes...
What about "sound-analog" for this one, and "sound-spdif" for the SPDIF simple card? Or "analog-sound" and "spdif-sound" if that looks better.
I fixed it and applied.
Thanks! Maxime
participants (3)
-
Chen-Yu Tsai -
Mark Brown -
Maxime Ripard