[1/6] ASoC: max98088: Document DT bindings

Commit Message

Andreas Färber Feb. 20, 2015, 12:36 a.m. UTC

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:



> 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

Comments

Sylwester Nawrocki/Kernel \(PLT\) /SRPOL/Staff Engineer/Samsung Electronics Feb. 20, 2015, 12:12 p.m. UTC | #1

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.

Javier Martinez Canillas Feb. 23, 2015, 4:46 p.m. UTC | #2

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

Patch

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);
        }