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
--
2.1.4
Signed-off-by: Andreas Färber <[email protected]>
---
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>;
+};
--
2.1.4
"maxim,max98089" will be used for the Spring Chromebook.
Signed-off-by: Andreas Färber <[email protected]>
---
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,
--
2.1.4
Signed-off-by: Andreas Färber <[email protected]>
---
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>;
--
2.1.4
Signed-off-by: Andreas Färber <[email protected]>
---
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"
--
2.1.4
This is needed for Spring.
Signed-off-by: Andreas Färber <[email protected]>
---
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", },
--
2.1.4
Signed-off-by: Andreas Färber <[email protected]>
---
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>;
--
2.1.4
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 <[email protected]>
> ---
> 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 <[email protected]>
>> ---
>> 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
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
On 02/18/2015 11:55 PM, Andreas Färber wrote:
> Signed-off-by: Andreas Färber <[email protected]>
> ---
> 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 <[email protected]>
--
Tushar Behera
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
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG N?rnberg)
Hello Andreas,
On 02/18/2015 07:25 PM, Andreas Färber wrote:
> Signed-off-by: Andreas Färber <[email protected]>
> ---
> 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 Andreas,
On 02/18/2015 07:25 PM, Andreas Färber wrote:
> Signed-off-by: Andreas Färber <[email protected]>
> ---
> 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/
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 <[email protected]>
>> ---
>> 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
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
Mark,
On Thu, Feb 19, 2015 at 1:44 AM, Mark Brown <[email protected]> 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
Andreas,
On Thu, Feb 19, 2015 at 6:13 AM, Andreas Färber <[email protected]> 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:44 schrieb Doug Anderson:
> On Thu, Feb 19, 2015 at 1:44 AM, Mark Brown <[email protected]> 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
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
Am 19.02.2015 um 18:48 schrieb Doug Anderson:
> On Thu, Feb 19, 2015 at 6:13 AM, Andreas Färber <[email protected]> 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
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
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 <[email protected]> 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
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
Andreas,
On Thu, Feb 19, 2015 at 9:56 AM, Andreas Färber <[email protected]> wrote:
> Am 19.02.2015 um 18:44 schrieb Doug Anderson:
>> On Thu, Feb 19, 2015 at 1:44 AM, Mark Brown <[email protected]> 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
If master clock is provided through device tree, then update
the master clock frequency during set_sysclk.
Cc: Tushar Behera <[email protected]>
Signed-off-by: Andreas Färber <[email protected]>
---
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;
--
2.1.4
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 <[email protected]> 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/34ae055b32e621409a92dea6a29f65b723798f33
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
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
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 <[email protected]>
> Signed-off-by: Andreas Färber <[email protected]>
> ---
> sound/soc/codecs/max98088.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
Looks good.
Acked-by: Tushar Behera <[email protected]>
--
Tushar Behera
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/arm/boot/dts/exynos4412-odroid-common.dtsi#n39
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.
--
Regards,
Sylwester
On Thu, Feb 19, 2015 at 09:44:08AM -0800, Doug Anderson wrote:
> On Thu, Feb 19, 2015 at 1:44 AM, Mark Brown <[email protected]> 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.
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 <[email protected]>
>> Signed-off-by: Andreas Färber <[email protected]>
>> ---
>> sound/soc/codecs/max98088.c | 24 ++++++++++++++++++++++++
>> 1 file changed, 24 insertions(+)
>>
>
> Looks good.
>
> Acked-by: Tushar Behera <[email protected]>
>
Looks good to me as well.
Reviewed-by: Javier Martinez Canillas <[email protected]>
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 <[email protected]>
>>> Signed-off-by: Andreas Färber <[email protected]>
>>> ---
>>> sound/soc/codecs/max98088.c | 24 ++++++++++++++++++++++++
>>> 1 file changed, 24 insertions(+)
>>>
>>
>> Looks good.
>>
>> Acked-by: Tushar Behera <[email protected]>
>>
>
> Looks good to me as well.
>
> Reviewed-by: Javier Martinez Canillas <[email protected]>
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
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
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/arm/boot/dts/exynos4412-odroid-common.dtsi#n39
>
> 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