[alsa-devel] [PATCH 0/6] ASoC: samsung: Spring sound support
Hello,
This series prepares sound support for the HP Chromebook 11.
Regards, Andreas
Andreas Färber (6): ASoC: max98088: Document DT bindings ASoC: max98088: Add DT bindings ARM: dts: Add max98089 to exynos5250-spring ASoC: samsung: Document binding for max98089 based Snow driver ASoC: samsung: Extend Snow driver to support max98089 ARM: dts: Add sound support to exynos5250-spring
.../devicetree/bindings/sound/max98088.txt | 16 ++++++++++++ Documentation/devicetree/bindings/sound/snow.txt | 1 + arch/arm/boot/dts/exynos5250-spring.dts | 30 ++++++++++++++++++++++ sound/soc/codecs/max98088.c | 8 ++++++ sound/soc/samsung/Kconfig | 1 + sound/soc/samsung/snow.c | 1 + 6 files changed, 57 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/max98088.txt
Signed-off-by: Andreas Färber afaerber@suse.de --- Documentation/devicetree/bindings/sound/max98088.txt | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/max98088.txt
diff --git a/Documentation/devicetree/bindings/sound/max98088.txt b/Documentation/devicetree/bindings/sound/max98088.txt new file mode 100644 index 000000000000..6f8fe85040ee --- /dev/null +++ b/Documentation/devicetree/bindings/sound/max98088.txt @@ -0,0 +1,16 @@ +MAX98088 audio CODEC + +This device supports I2C only. + +Required properties: + +- compatible : "maxim,max98088" or "maxim,max98089". + +- reg : The I2C address of the device. + +Example: + +max98089: codec@10 { + compatible = "maxim,max98089"; + reg = <0x10>; +};
Hello Andreas,
On 02/18/2015 07:25 PM, Andreas Färber wrote:
Signed-off-by: Andreas Färber afaerber@suse.de
Documentation/devicetree/bindings/sound/max98088.txt | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/max98088.txt
diff --git a/Documentation/devicetree/bindings/sound/max98088.txt b/Documentation/devicetree/bindings/sound/max98088.txt new file mode 100644 index 000000000000..6f8fe85040ee --- /dev/null +++ b/Documentation/devicetree/bindings/sound/max98088.txt @@ -0,0 +1,16 @@ +MAX98088 audio CODEC
+This device supports I2C only.
+Required properties:
+- compatible : "maxim,max98088" or "maxim,max98089".
+- reg : The I2C address of the device.
+Example:
+max98089: codec@10 {
- compatible = "maxim,max98089";
- reg = <0x10>;
+};
I see that a master clock (mclk) is added in patch 6/6 but the max98088 codec driver does handle this clock.
If the SoC XCLKOUT provides the master clock to the max98089 codec in Spring like is the case for the max9809{0,5} codecs in the Snow and Peach Pit/Pi Chromebooks then you need to do something along the lines of the following commits:
e3048c3d2be5 ASoC: max98095: Add master clock handling b10ab7b838bd ASoC: max98090: Add master clock handling
If that's the case you also have to mention in the DT binding doc that "clocks" and "clock-names" are optional properties like Documentation/devicetree/bindings/sound/max9809{0,5}.txt.
Best regards, Javier
Hello Javier, Doug,
Am 19.02.2015 um 14:55 schrieb Javier Martinez Canillas:
On 02/18/2015 07:25 PM, Andreas Färber wrote:
Signed-off-by: Andreas Färber afaerber@suse.de
Documentation/devicetree/bindings/sound/max98088.txt | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/max98088.txt
diff --git a/Documentation/devicetree/bindings/sound/max98088.txt b/Documentation/devicetree/bindings/sound/max98088.txt new file mode 100644 index 000000000000..6f8fe85040ee --- /dev/null +++ b/Documentation/devicetree/bindings/sound/max98088.txt @@ -0,0 +1,16 @@ +MAX98088 audio CODEC
+This device supports I2C only.
+Required properties:
+- compatible : "maxim,max98088" or "maxim,max98089".
+- reg : The I2C address of the device.
+Example:
+max98089: codec@10 {
- compatible = "maxim,max98089";
- reg = <0x10>;
+};
I see that a master clock (mclk) is added in patch 6/6 but the max98088 codec driver does handle this clock.
If the SoC XCLKOUT provides the master clock to the max98089 codec in Spring like is the case for the max9809{0,5} codecs in the Snow and Peach Pit/Pi Chromebooks then you need to do something along the lines of the following commits:
e3048c3d2be5 ASoC: max98095: Add master clock handling b10ab7b838bd ASoC: max98090: Add master clock handling
If that's the case you also have to mention in the DT binding doc that "clocks" and "clock-names" are optional properties like Documentation/devicetree/bindings/sound/max9809{0,5}.txt.
When I prepared this patch, I believe it was a straight copy from max98090. Sounds like they changed since then.
My 6/6 adopted the mclk clock from your now-cancelled v2 patch for Snow, assuming it would be the same on all Chromebooks. I tested that last change by checking for errors in dmesg.
Doug, can you advise on how the clock wiring is for Spring?
Regards, Andreas
Andreas,
On Thu, Feb 19, 2015 at 6:13 AM, Andreas Färber afaerber@suse.de wrote:
I see that a master clock (mclk) is added in patch 6/6 but the max98088 codec driver does handle this clock.
If the SoC XCLKOUT provides the master clock to the max98089 codec in Spring like is the case for the max9809{0,5} codecs in the Snow and Peach Pit/Pi Chromebooks then you need to do something along the lines of the following commits:
e3048c3d2be5 ASoC: max98095: Add master clock handling b10ab7b838bd ASoC: max98090: Add master clock handling
If that's the case you also have to mention in the DT binding doc that "clocks" and "clock-names" are optional properties like Documentation/devicetree/bindings/sound/max9809{0,5}.txt.
When I prepared this patch, I believe it was a straight copy from max98090. Sounds like they changed since then.
My 6/6 adopted the mclk clock from your now-cancelled v2 patch for Snow, assuming it would be the same on all Chromebooks. I tested that last change by checking for errors in dmesg.
Doug, can you advise on how the clock wiring is for Spring?
I can confirm that XCLKOUT is connected to the codec MCLK on the Spring schematics I have.
-Doug
Am 19.02.2015 um 18:48 schrieb Doug Anderson:
On Thu, Feb 19, 2015 at 6:13 AM, Andreas Färber afaerber@suse.de wrote:
I see that a master clock (mclk) is added in patch 6/6 but the max98088 codec driver does handle this clock.
If the SoC XCLKOUT provides the master clock to the max98089 codec in Spring like is the case for the max9809{0,5} codecs in the Snow and Peach Pit/Pi Chromebooks then you need to do something along the lines of the following commits:
e3048c3d2be5 ASoC: max98095: Add master clock handling b10ab7b838bd ASoC: max98090: Add master clock handling
If that's the case you also have to mention in the DT binding doc that "clocks" and "clock-names" are optional properties like Documentation/devicetree/bindings/sound/max9809{0,5}.txt.
When I prepared this patch, I believe it was a straight copy from max98090. Sounds like they changed since then.
My 6/6 adopted the mclk clock from your now-cancelled v2 patch for Snow, assuming it would be the same on all Chromebooks. I tested that last change by checking for errors in dmesg.
Doug, can you advise on how the clock wiring is for Spring?
I can confirm that XCLKOUT is connected to the codec MCLK on the Spring schematics I have.
Thanks! I updated max98088 and had it working on first boot, but on second boot it complained about the frequency:
[ 7.896834] max98088 7-0010: revision A [ 7.912776] snow-audio sound: HiFi <-> 3830000.i2s mapping ok [ 7.919367] max98088 7-0010: Invalid master clock frequency [ 7.919429] snow-audio sound: ASoC: Spring-I2S-MAX98089 late_probe() failed: -22 [ 7.920019] snow-audio sound: snd_soc_register_card failed (-22) [ 7.920109] snow-audio: probe of sound failed with error -22
Will investigate.
Andreas
Am 19.02.2015 um 19:40 schrieb Andreas Färber:
Am 19.02.2015 um 18:48 schrieb Doug Anderson:
On Thu, Feb 19, 2015 at 6:13 AM, Andreas Färber afaerber@suse.de wrote:
I see that a master clock (mclk) is added in patch 6/6 but the max98088 codec driver does handle this clock.
If the SoC XCLKOUT provides the master clock to the max98089 codec in Spring like is the case for the max9809{0,5} codecs in the Snow and Peach Pit/Pi Chromebooks then you need to do something along the lines of the following commits:
e3048c3d2be5 ASoC: max98095: Add master clock handling b10ab7b838bd ASoC: max98090: Add master clock handling
If that's the case you also have to mention in the DT binding doc that "clocks" and "clock-names" are optional properties like Documentation/devicetree/bindings/sound/max9809{0,5}.txt.
When I prepared this patch, I believe it was a straight copy from max98090. Sounds like they changed since then.
My 6/6 adopted the mclk clock from your now-cancelled v2 patch for Snow, assuming it would be the same on all Chromebooks. I tested that last change by checking for errors in dmesg.
Doug, can you advise on how the clock wiring is for Spring?
I can confirm that XCLKOUT is connected to the codec MCLK on the Spring schematics I have.
Thanks! I updated max98088 and had it working on first boot, but on second boot it complained about the frequency:
[ 7.896834] max98088 7-0010: revision A [ 7.912776] snow-audio sound: HiFi <-> 3830000.i2s mapping ok [ 7.919367] max98088 7-0010: Invalid master clock frequency [ 7.919429] snow-audio sound: ASoC: Spring-I2S-MAX98089 late_probe() failed: -22 [ 7.920019] snow-audio sound: snd_soc_register_card failed (-22) [ 7.920109] snow-audio: probe of sound failed with error -22
Reproducible on third boot.
On a suspicion, the fourth boot I waited for the double-beep of the firmware (waiting for Ctrl+d/u), and then it did work.
So it seems the mclk is not always set up properly by the kernel, relying on firmware. Who's in charge of setting that clock up?
Regards, Andreas
Hello Andreas,
We already talked over irc but for completeness I'll comment here as well.
On 02/19/2015 07:54 PM, Andreas Färber wrote:
Am 19.02.2015 um 19:40 schrieb Andreas Färber:
Am 19.02.2015 um 18:48 schrieb Doug Anderson:
On Thu, Feb 19, 2015 at 6:13 AM, Andreas Färber afaerber@suse.de wrote:
I see that a master clock (mclk) is added in patch 6/6 but the max98088 codec driver does handle this clock.
Sorry, I wanted to say " the driver *does not* handle this clock" but fortunately you understood what I meant :)
If the SoC XCLKOUT provides the master clock to the max98089 codec in Spring like is the case for the max9809{0,5} codecs in the Snow and Peach Pit/Pi Chromebooks then you need to do something along the lines of the following commits:
e3048c3d2be5 ASoC: max98095: Add master clock handling b10ab7b838bd ASoC: max98090: Add master clock handling
If that's the case you also have to mention in the DT binding doc that "clocks" and "clock-names" are optional properties like Documentation/devicetree/bindings/sound/max9809{0,5}.txt.
When I prepared this patch, I believe it was a straight copy from max98090. Sounds like they changed since then.
My 6/6 adopted the mclk clock from your now-cancelled v2 patch for Snow, assuming it would be the same on all Chromebooks. I tested that last change by checking for errors in dmesg.
Doug, can you advise on how the clock wiring is for Spring?
I can confirm that XCLKOUT is connected to the codec MCLK on the Spring schematics I have.
Thanks! I updated max98088 and had it working on first boot, but on second boot it complained about the frequency:
[ 7.896834] max98088 7-0010: revision A [ 7.912776] snow-audio sound: HiFi <-> 3830000.i2s mapping ok [ 7.919367] max98088 7-0010: Invalid master clock frequency [ 7.919429] snow-audio sound: ASoC: Spring-I2S-MAX98089 late_probe() failed: -22 [ 7.920019] snow-audio sound: snd_soc_register_card failed (-22) [ 7.920109] snow-audio: probe of sound failed with error -22
I had the same error on Snow but even on the first boot and after doing some code archeology, I found the following commit [0] in a Samsung downstream tree that solves the issue.
The problem is that clk_round_rate(max98095->mclk, freq) returns 0 as the rounded rate if XCLOUT is not allowed to be re-parented on rate change.
With Tushar's patch I see that clk_round_rate() returns 24000000 (24MHz) so the codec driver setups the correct PLL clock.
You mentioned that it does make the error go away but still audio is not working on Spring. Let's see if someone has an idea of what could be missing.
Reproducible on third boot.
On a suspicion, the fourth boot I waited for the double-beep of the firmware (waiting for Ctrl+d/u), and then it did work.
So it seems the mclk is not always set up properly by the kernel, relying on firmware. Who's in charge of setting that clock up?
Right, it seems audio is only working due the firmware doing some previous setup. Probably it works on every boot if you have "sound init" as a part of the u-boot boot commands?
Regards, Andreas
Best regards, Javier
[0]: https://github.com/exynos-reference/kernel/commit/34ae055b32e621409a92dea6a2...
Am 19.02.2015 um 21:48 schrieb Javier Martinez Canillas:
On 02/19/2015 07:54 PM, Andreas Färber wrote:
Am 19.02.2015 um 19:40 schrieb Andreas Färber:
I updated max98088 and had it working on first boot, but on second boot it complained about the frequency:
[ 7.896834] max98088 7-0010: revision A [ 7.912776] snow-audio sound: HiFi <-> 3830000.i2s mapping ok [ 7.919367] max98088 7-0010: Invalid master clock frequency [ 7.919429] snow-audio sound: ASoC: Spring-I2S-MAX98089 late_probe() failed: -22 [ 7.920019] snow-audio sound: snd_soc_register_card failed (-22) [ 7.920109] snow-audio: probe of sound failed with error -22
I had the same error on Snow but even on the first boot and after doing some code archeology, I found the following commit [0] in a Samsung downstream tree that solves the issue.
The problem is that clk_round_rate(max98095->mclk, freq) returns 0 as the rounded rate if XCLOUT is not allowed to be re-parented on rate change.
Same on Spring:
diff --git a/sound/soc/codecs/max98088.c b/sound/soc/codecs/max98088.c index 1aa81321afba..46dc64675c26 100644 --- a/sound/soc/codecs/max98088.c +++ b/sound/soc/codecs/max98088.c @@ -1365,6 +1365,7 @@ static int max98088_dai_set_sysclk(struct snd_soc_dai *dai,
if (!IS_ERR(max98088->mclk)) { freq = clk_round_rate(max98088->mclk, freq); + dev_warn(codec->dev, "freq = %u\n", freq); clk_set_rate(max98088->mclk, freq); }
With Tushar's patch I see that clk_round_rate() returns 24000000 (24MHz) so the codec driver setups the correct PLL clock.
Ditto. With the clkout reparenting patch, clk_round_rate() returns 24MHz just like when double-beep-initialized. However when not double-beep-initialized, the driver initializes, but no audible output, so there must be another missing puzzle piece.
On a suspicion, the fourth boot I waited for the double-beep of the firmware (waiting for Ctrl+d/u), and then it did work.
So it seems the mclk is not always set up properly by the kernel, relying on firmware. Who's in charge of setting that clock up?
Right, it seems audio is only working due the firmware doing some previous setup. Probably it works on every boot if you have "sound init" as a part of the u-boot boot commands?
Indeed it does, 24 MHz without the reparenting patch, and sound working.
'sound init' code: https://github.com/afaerber/u-boot/blob/spring/drivers/sound/max98088.c
Regards, Andreas
On 20/02/15 01:36, Andreas Färber wrote:
So it seems the mclk is not always set up properly by the kernel, relying on firmware. Who's in charge of setting that clock up?
Right, it seems audio is only working due the firmware doing some previous setup. Probably it works on every boot if you have "sound init" as a part of the u-boot boot commands?
Indeed it does, 24 MHz without the reparenting patch, and sound working.
You can have parent of the CLKOUT clock set by the clk core if it is specified in device tree in the PMU (the clkout clock supplier) device node.
Similarly as we did for the Odroix U3: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/ar...
Relying on the clk_set_rate() to set the parent clock is not optimal IMO. Presumably you need to set select stable parent clock for clkout like XXTI. But I'm not very familiar with exyno5250 and that might be something different.
Hello Sylwester,
On 02/20/2015 01:12 PM, Sylwester Nawrocki wrote:
On 20/02/15 01:36, Andreas Färber wrote:
So it seems the mclk is not always set up properly by the kernel, relying on firmware. Who's in charge of setting that clock up?
Right, it seems audio is only working due the firmware doing some previous setup. Probably it works on every boot if you have "sound init" as a part of the u-boot boot commands?
Indeed it does, 24 MHz without the reparenting patch, and sound working.
You can have parent of the CLKOUT clock set by the clk core if it is specified in device tree in the PMU (the clkout clock supplier) device node.
Similarly as we did for the Odroix U3: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/ar...
Relying on the clk_set_rate() to set the parent clock is not optimal IMO. Presumably you need to set select stable parent clock for clkout like XXTI. But I'm not very familiar with exyno5250 and that might be something different.
Thanks a lot for your suggestion. I'll drop Tushar's patch to allow clkout to be reparent during set_rate then and change his DTS patch to set a default parent for CLKOUT using "assigned-clock-parents".
Best regards, Javier
"maxim,max98089" will be used for the Spring Chromebook.
Signed-off-by: Andreas Färber afaerber@suse.de --- sound/soc/codecs/max98088.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/sound/soc/codecs/max98088.c b/sound/soc/codecs/max98088.c index 805b3f8cd39d..69a21d1946e3 100644 --- a/sound/soc/codecs/max98088.c +++ b/sound/soc/codecs/max98088.c @@ -2009,10 +2009,18 @@ static const struct i2c_device_id max98088_i2c_id[] = { }; MODULE_DEVICE_TABLE(i2c, max98088_i2c_id);
+static const struct of_device_id max98088_of_match[] = { + { .compatible = "maxim,max98088" }, + { .compatible = "maxim,max98089" }, + { } +}; +MODULE_DEVICE_TABLE(of, max98088_of_match); + static struct i2c_driver max98088_i2c_driver = { .driver = { .name = "max98088", .owner = THIS_MODULE, + .of_match_table = of_match_ptr(max98088_of_match), }, .probe = max98088_i2c_probe, .remove = max98088_i2c_remove,
Hello.
On 02/18/2015 09:25 PM, Andreas Färber wrote:
"maxim,max98089" will be used for the Spring Chromebook.
Signed-off-by: Andreas Färber afaerber@suse.de
sound/soc/codecs/max98088.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/sound/soc/codecs/max98088.c b/sound/soc/codecs/max98088.c index 805b3f8cd39d..69a21d1946e3 100644 --- a/sound/soc/codecs/max98088.c +++ b/sound/soc/codecs/max98088.c @@ -2009,10 +2009,18 @@ static const struct i2c_device_id max98088_i2c_id[] = { }; MODULE_DEVICE_TABLE(i2c, max98088_i2c_id);
+static const struct of_device_id max98088_of_match[] = {
If I don't mistake, gcc will complain about this variable being unused if CONFIG_OF=n.
- { .compatible = "maxim,max98088" },
- { .compatible = "maxim,max98089" },
- { }
+}; +MODULE_DEVICE_TABLE(of, max98088_of_match);
- static struct i2c_driver max98088_i2c_driver = { .driver = { .name = "max98088", .owner = THIS_MODULE,
.of_match_table = of_match_ptr(max98088_of_match),
You probably don;t need of_match_ptr(). Either that or do something with the match table, like enclosing into #ifdef or annotating with '__maybe_unused'.
[...]
WBR, Sergei
Hi,
Am 18.02.2015 um 20:08 schrieb Sergei Shtylyov:
On 02/18/2015 09:25 PM, Andreas Färber wrote:
"maxim,max98089" will be used for the Spring Chromebook.
Signed-off-by: Andreas Färber afaerber@suse.de
sound/soc/codecs/max98088.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/sound/soc/codecs/max98088.c b/sound/soc/codecs/max98088.c index 805b3f8cd39d..69a21d1946e3 100644 --- a/sound/soc/codecs/max98088.c +++ b/sound/soc/codecs/max98088.c @@ -2009,10 +2009,18 @@ static const struct i2c_device_id max98088_i2c_id[] = { }; MODULE_DEVICE_TABLE(i2c, max98088_i2c_id);
+static const struct of_device_id max98088_of_match[] = {
If I don't mistake, gcc will complain about this variable being unused if CONFIG_OF=n.
I assume there will be no warning because of the "const". For just "static" you would be right.
- { .compatible = "maxim,max98088" },
- { .compatible = "maxim,max98089" },
- { }
+}; +MODULE_DEVICE_TABLE(of, max98088_of_match);
- static struct i2c_driver max98088_i2c_driver = { .driver = { .name = "max98088", .owner = THIS_MODULE,
.of_match_table = of_match_ptr(max98088_of_match),You probably don;t need of_match_ptr(). Either that or do something with the match table, like enclosing into #ifdef or annotating with '__maybe_unused'.
This was copied from max98090 and max98095.
Cheers, Andreas
If master clock is provided through device tree, then update the master clock frequency during set_sysclk.
Cc: Tushar Behera tushar.behera@linaro.org Signed-off-by: Andreas Färber afaerber@suse.de --- sound/soc/codecs/max98088.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
diff --git a/sound/soc/codecs/max98088.c b/sound/soc/codecs/max98088.c index 69a21d1946e3..1aa81321afba 100644 --- a/sound/soc/codecs/max98088.c +++ b/sound/soc/codecs/max98088.c @@ -16,6 +16,7 @@ #include <linux/pm.h> #include <linux/i2c.h> #include <linux/regmap.h> +#include <linux/clk.h> #include <sound/core.h> #include <sound/pcm.h> #include <sound/pcm_params.h> @@ -42,6 +43,7 @@ struct max98088_priv { struct regmap *regmap; enum max98088_type devtype; struct max98088_pdata *pdata; + struct clk *mclk; unsigned int sysclk; struct max98088_cdata dai[2]; int eq_textcnt; @@ -1361,6 +1363,11 @@ static int max98088_dai_set_sysclk(struct snd_soc_dai *dai, if (freq == max98088->sysclk) return 0;
+ if (!IS_ERR(max98088->mclk)) { + freq = clk_round_rate(max98088->mclk, freq); + clk_set_rate(max98088->mclk, freq); + } + /* Setup clocks for slave mode, and using the PLL * PSCLK = 0x01 (when master clk is 10MHz to 20MHz) * 0x02 (when master clk is 20MHz to 30MHz).. @@ -1568,6 +1575,19 @@ static int max98088_set_bias_level(struct snd_soc_codec *codec, break;
case SND_SOC_BIAS_PREPARE: + /* + * SND_SOC_BIAS_PREPARE is called while preparing for a + * transition to ON or away from ON. If current bias_level + * is SND_SOC_BIAS_ON, then it is preparing for a transition + * away from ON. Disable the clock in that case, otherwise + * enable it. + */ + if (!IS_ERR(max98088->mclk)) { + if (codec->dapm.bias_level == SND_SOC_BIAS_ON) + clk_disable_unprepare(max98088->mclk); + else + clk_prepare_enable(max98088->mclk); + } break;
case SND_SOC_BIAS_STANDBY: @@ -1900,6 +1920,10 @@ static int max98088_probe(struct snd_soc_codec *codec) max98088->sysclk = (unsigned)-1; max98088->eq_textcnt = 0;
+ max98088->mclk = devm_clk_get(codec->dev, "mclk"); + if (PTR_ERR(max98088->mclk) == -EPROBE_DEFER) + return -EPROBE_DEFER; + cdata = &max98088->dai[0]; cdata->rate = (unsigned)-1; cdata->fmt = (unsigned)-1;
On 02/20/2015 12:48 AM, Andreas Färber wrote:
If master clock is provided through device tree, then update the master clock frequency during set_sysclk.
Cc: Tushar Behera tushar.behera@linaro.org Signed-off-by: Andreas Färber afaerber@suse.de
sound/soc/codecs/max98088.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
Looks good.
Acked-by: Tushar Behera trblinux@gmail.com
Hello,
On 02/20/2015 04:27 AM, Tushar Behera wrote:
On 02/20/2015 12:48 AM, Andreas Färber wrote:
If master clock is provided through device tree, then update the master clock frequency during set_sysclk.
Cc: Tushar Behera tushar.behera@linaro.org Signed-off-by: Andreas Färber afaerber@suse.de
sound/soc/codecs/max98088.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
Looks good.
Acked-by: Tushar Behera trblinux@gmail.com
Looks good to me as well.
Reviewed-by: Javier Martinez Canillas javier.martinez@collabora.co.uk
Best regards, Javier
Hi,
Am 23.02.2015 um 09:29 schrieb Javier Martinez Canillas:
On 02/20/2015 04:27 AM, Tushar Behera wrote:
On 02/20/2015 12:48 AM, Andreas Färber wrote:
If master clock is provided through device tree, then update the master clock frequency during set_sysclk.
Cc: Tushar Behera tushar.behera@linaro.org Signed-off-by: Andreas Färber afaerber@suse.de
sound/soc/codecs/max98088.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
Looks good.
Acked-by: Tushar Behera trblinux@gmail.com
Looks good to me as well.
Reviewed-by: Javier Martinez Canillas javier.martinez@collabora.co.uk
Thanks guys. One self-doubt: Is there any downside to returning -EPROBE_DEFER after regcache_mark_dirty(max98088->regmap)? I.e., should I move the last hunk some lines up to be the very first thing executed?
Cheers, Andreas
Signed-off-by: Andreas Färber afaerber@suse.de --- arch/arm/boot/dts/exynos5250-spring.dts | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/arch/arm/boot/dts/exynos5250-spring.dts b/arch/arm/boot/dts/exynos5250-spring.dts index d075a68ac078..866beb4e0597 100644 --- a/arch/arm/boot/dts/exynos5250-spring.dts +++ b/arch/arm/boot/dts/exynos5250-spring.dts @@ -400,6 +400,11 @@ samsung,i2c-sda-delay = <100>; samsung,i2c-max-bus-freq = <66000>;
+ max98089: codec@10 { + compatible = "maxim,max98089", "maxim,max98088"; + reg = <0x10>; + }; + temperature-sensor@4c { compatible = "gmt,g781"; reg = <0x4c>;
Signed-off-by: Andreas Färber afaerber@suse.de --- Documentation/devicetree/bindings/sound/snow.txt | 1 + 1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/sound/snow.txt b/Documentation/devicetree/bindings/sound/snow.txt index 6df74f15687f..1430a6bdb2ae 100644 --- a/Documentation/devicetree/bindings/sound/snow.txt +++ b/Documentation/devicetree/bindings/sound/snow.txt @@ -2,6 +2,7 @@ Audio Binding for Snow boards
Required properties: - compatible : Can be one of the following, + "google,snow-audio-max98089" or "google,snow-audio-max98090" or "google,snow-audio-max98091" or "google,snow-audio-max98095"
On 02/18/2015 11:55 PM, Andreas Färber wrote:
Signed-off-by: Andreas Färber afaerber@suse.de
Documentation/devicetree/bindings/sound/snow.txt | 1 + 1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/sound/snow.txt b/Documentation/devicetree/bindings/sound/snow.txt index 6df74f15687f..1430a6bdb2ae 100644 --- a/Documentation/devicetree/bindings/sound/snow.txt +++ b/Documentation/devicetree/bindings/sound/snow.txt @@ -2,6 +2,7 @@ Audio Binding for Snow boards
Required properties:
- compatible : Can be one of the following,
"google,snow-audio-max98089" or "google,snow-audio-max98090" or "google,snow-audio-max98091" or "google,snow-audio-max98095"
Looks good.
Acked-by: Tushar Behera trblinux@gmail.com
This is needed for Spring.
Signed-off-by: Andreas Färber afaerber@suse.de --- sound/soc/samsung/Kconfig | 1 + sound/soc/samsung/snow.c | 1 + 2 files changed, 2 insertions(+)
diff --git a/sound/soc/samsung/Kconfig b/sound/soc/samsung/Kconfig index 3cebf6ca03df..222181655e4f 100644 --- a/sound/soc/samsung/Kconfig +++ b/sound/soc/samsung/Kconfig @@ -214,6 +214,7 @@ config SND_SOC_LITTLEMILL config SND_SOC_SNOW tristate "Audio support for Google Snow boards" depends on SND_SOC_SAMSUNG && I2C + select SND_SOC_MAX98088 select SND_SOC_MAX98090 select SND_SOC_MAX98095 select SND_SAMSUNG_I2S diff --git a/sound/soc/samsung/snow.c b/sound/soc/samsung/snow.c index 7651dc924161..89d7e39b206e 100644 --- a/sound/soc/samsung/snow.c +++ b/sound/soc/samsung/snow.c @@ -105,6 +105,7 @@ static int snow_probe(struct platform_device *pdev) }
static const struct of_device_id snow_of_match[] = { + { .compatible = "google,snow-audio-max98089", }, { .compatible = "google,snow-audio-max98090", }, { .compatible = "google,snow-audio-max98091", }, { .compatible = "google,snow-audio-max98095", },
On Wed, Feb 18, 2015 at 07:25:58PM +0100, Andreas Färber wrote:
static const struct of_device_id snow_of_match[] = {
- { .compatible = "google,snow-audio-max98089", }, { .compatible = "google,snow-audio-max98090", }, { .compatible = "google,snow-audio-max98091", }, { .compatible = "google,snow-audio-max98095", },
Since we completely ignore the CODEC in the property might it not be better to just add a plain old snow-audio compatible and bind to that, that way we don't need these driver updates? Just the "snow" bit should be enough to know it's one of this class of machines.
Am 19.02.2015 um 10:44 schrieb Mark Brown:
On Wed, Feb 18, 2015 at 07:25:58PM +0100, Andreas Färber wrote:
static const struct of_device_id snow_of_match[] = {
- { .compatible = "google,snow-audio-max98089", }, { .compatible = "google,snow-audio-max98090", }, { .compatible = "google,snow-audio-max98091", }, { .compatible = "google,snow-audio-max98095", },
Since we completely ignore the CODEC in the property might it not be better to just add a plain old snow-audio compatible and bind to that, that way we don't need these driver updates? Just the "snow" bit should be enough to know it's one of this class of machines.
Personally I don't mind either way, but I'd rather leave that decision to Google/Samsung people.
With all those Tegra and Rockchip based Chromebooks popping up, are there any further Exynos based Chromebooks planned using this driver, or is it the last of its kind?
Cheers, Andreas
Mark,
On Thu, Feb 19, 2015 at 1:44 AM, Mark Brown broonie@kernel.org wrote:
On Wed, Feb 18, 2015 at 07:25:58PM +0100, Andreas Färber wrote:
static const struct of_device_id snow_of_match[] = {
{ .compatible = "google,snow-audio-max98089", }, { .compatible = "google,snow-audio-max98090", }, { .compatible = "google,snow-audio-max98091", }, { .compatible = "google,snow-audio-max98095", },Since we completely ignore the CODEC in the property might it not be better to just add a plain old snow-audio compatible and bind to that, that way we don't need these driver updates? Just the "snow" bit should be enough to know it's one of this class of machines.
I think what you're suggesting is that here we should add a new compatible string "google,snow-audio" instead of adding "google,snow-audio-max98089" here. Then the sound node in the spring DTS would look like:
compatible = "google,snow-audio-max98089", "google,snow-audio";
That would allow us to later figure out that we're on a board with max98089 in case it became important but means that any other minor tweaks like this wouldn't need anything special. I haven't tried that to make sure that the fallback compatible string actually works in this case, but it seems like the right way to go...
Sound good?
-Doug
Am 19.02.2015 um 18:44 schrieb Doug Anderson:
On Thu, Feb 19, 2015 at 1:44 AM, Mark Brown broonie@kernel.org wrote:
On Wed, Feb 18, 2015 at 07:25:58PM +0100, Andreas Färber wrote:
static const struct of_device_id snow_of_match[] = {
{ .compatible = "google,snow-audio-max98089", }, { .compatible = "google,snow-audio-max98090", }, { .compatible = "google,snow-audio-max98091", }, { .compatible = "google,snow-audio-max98095", },Since we completely ignore the CODEC in the property might it not be better to just add a plain old snow-audio compatible and bind to that, that way we don't need these driver updates? Just the "snow" bit should be enough to know it's one of this class of machines.
I think what you're suggesting is that here we should add a new compatible string "google,snow-audio" instead of adding "google,snow-audio-max98089" here. Then the sound node in the spring DTS would look like:
compatible = "google,snow-audio-max98089", "google,snow-audio";
If we want to be specific just in case, was it an active decision not to use "google,peach-pi[t]-audio-max98..."?
Again, either way works for me.
Andreas
Andreas,
On Thu, Feb 19, 2015 at 9:56 AM, Andreas Färber afaerber@suse.de wrote:
Am 19.02.2015 um 18:44 schrieb Doug Anderson:
On Thu, Feb 19, 2015 at 1:44 AM, Mark Brown broonie@kernel.org wrote:
On Wed, Feb 18, 2015 at 07:25:58PM +0100, Andreas Färber wrote:
static const struct of_device_id snow_of_match[] = {
{ .compatible = "google,snow-audio-max98089", }, { .compatible = "google,snow-audio-max98090", }, { .compatible = "google,snow-audio-max98091", }, { .compatible = "google,snow-audio-max98095", },Since we completely ignore the CODEC in the property might it not be better to just add a plain old snow-audio compatible and bind to that, that way we don't need these driver updates? Just the "snow" bit should be enough to know it's one of this class of machines.
I think what you're suggesting is that here we should add a new compatible string "google,snow-audio" instead of adding "google,snow-audio-max98089" here. Then the sound node in the spring DTS would look like:
compatible = "google,snow-audio-max98089", "google,snow-audio";
If we want to be specific just in case, was it an active decision not to use "google,peach-pi[t]-audio-max98..."?
Again, either way works for me.
I don't think it was an active decision, but I am certainly nowhere near an audio expert and I don't think I made that particular decision (I could be wrong).
One could make the argument that "snow" describes the general hookup style of the hardware and then you list the actual codec after that, but that's a pretty weak argument...
-Doug
On Thu, Feb 19, 2015 at 09:44:08AM -0800, Doug Anderson wrote:
On Thu, Feb 19, 2015 at 1:44 AM, Mark Brown broonie@kernel.org wrote:
I think what you're suggesting is that here we should add a new compatible string "google,snow-audio" instead of adding "google,snow-audio-max98089" here. Then the sound node in the spring DTS would look like:
compatible = "google,snow-audio-max98089", "google,snow-audio";
Yes.
Signed-off-by: Andreas Färber afaerber@suse.de --- arch/arm/boot/dts/exynos5250-spring.dts | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/arch/arm/boot/dts/exynos5250-spring.dts b/arch/arm/boot/dts/exynos5250-spring.dts index 866beb4e0597..142e0550e223 100644 --- a/arch/arm/boot/dts/exynos5250-spring.dts +++ b/arch/arm/boot/dts/exynos5250-spring.dts @@ -49,6 +49,17 @@ }; };
+ sound { + compatible = "google,snow-audio-max98089"; + samsung,model = "Spring-I2S-MAX98089"; + samsung,i2s-controller = <&i2s0>; + samsung,audio-codec = <&max98089>; + clocks = <&pmu_system_controller 0>; + clock-names = "mclk"; + pinctrl-names = "default"; + pinctrl-0 = <&mic_det_gpio>, <&hp_det_gpio>; + }; + usb-hub { compatible = "smsc,usb3503a"; reset-gpios = <&gpe1 0 GPIO_ACTIVE_LOW>; @@ -499,6 +510,20 @@ samsung,pin-drv = <0>; };
+ mic_det_gpio: mic-det-gpio { + samsung,pins = "gpx2-0"; + samsung,pin-function = <0>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + + hp_det_gpio: hp-det-gpio { + samsung,pins = "gpx2-2"; + samsung,pin-function = <0>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + s5m8767_ds: s5m8767-ds { samsung,pins = "gpx2-3", "gpx2-4", "gpx2-5"; samsung,pin-function = <0>;
Hello Andreas,
On 02/18/2015 07:25 PM, Andreas Färber wrote:
Signed-off-by: Andreas Färber afaerber@suse.de
arch/arm/boot/dts/exynos5250-spring.dts | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/arch/arm/boot/dts/exynos5250-spring.dts b/arch/arm/boot/dts/exynos5250-spring.dts index 866beb4e0597..142e0550e223 100644 --- a/arch/arm/boot/dts/exynos5250-spring.dts +++ b/arch/arm/boot/dts/exynos5250-spring.dts @@ -49,6 +49,17 @@ }; };
- sound {
compatible = "google,snow-audio-max98089";samsung,model = "Spring-I2S-MAX98089";samsung,i2s-controller = <&i2s0>;samsung,audio-codec = <&max98089>;clocks = <&pmu_system_controller 0>;clock-names = "mclk";
Related to my comment in patch 1/6, "mclk" should be defined in the codec device node. I know that you probably used as a reference Tushar's patch [0] I posted recently but I just figured out that his dependency patch [1] mentioned in [2] handled the master clock in the ASoC machine driver but the final version merged handled it in the codec drivers.
I asked Kukjin to drop the patch to Snow and Peach Pit/Pi and will post a correct version as a part of a series once I've sound working.
pinctrl-names = "default";pinctrl-0 = <&mic_det_gpio>, <&hp_det_gpio>;- };
The mic and headphone jack detection are still not supported in the Snow ASoC machine driver so this pinctrl are not needed but I guess it doesn't hurt to have them mux'ed too.
Best regards, Javier
[0]: https://lkml.org/lkml/2015/2/6/495 [1]: https://lkml.org/lkml/2014/5/20/4 [2]: https://patches.linaro.org/30406/
participants (7)
-
Andreas Färber -
Doug Anderson -
Javier Martinez Canillas -
Mark Brown -
Sergei Shtylyov -
Sylwester Nawrocki -
Tushar Behera