linux-kernel - Re: [PATCH 1/3] ASoC: simple-card-utils: Fix device module clock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ca540fb6-2ea7-90b0-66ad-097e99b6e585@nvidia.com>
Date:   Wed, 10 Mar 2021 20:20:28 +0530
From:   Sameer Pujar <spujar@...dia.com>
To:     Michael Walle <michael@...le.cc>
CC:     <alsa-devel@...a-project.org>, <broonie@...nel.org>,
        <devicetree@...r.kernel.org>, <jonathanh@...dia.com>,
        <kuninori.morimoto.gx@...esas.com>, <linux-kernel@...r.kernel.org>,
        <linux-tegra@...r.kernel.org>, <robh@...nel.org>,
        <sharadg@...dia.com>, <thierry.reding@...il.com>
Subject: Re: [PATCH 1/3] ASoC: simple-card-utils: Fix device module clock



On 3/10/2021 4:00 AM, Michael Walle wrote:
> Am 2021-03-09 17:27, schrieb Sameer Pujar:
>> On 3/9/2021 8:11 PM, Michael Walle wrote:
>>>> If "clocks = <&xxx>" is specified from the CPU or Codec component
>>>> device node, the clock is not getting enabled. Thus audio playback
>>>> or capture fails.
>>>>
>>>> Fix this by populating "simple_dai->clk" field when clocks property
>>>> is specified from device node as well. Also tidy up by re-organising
>>>> conditional statements of parsing logic.
>>>>
>>>> Fixes: bb6fc620c2ed ("ASoC: simple-card-utils: add
>>>> asoc_simple_card_parse_clk()")
>>>> Cc: Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>
>>>> Signed-off-by: Sameer Pujar <spujar@...dia.com>
>>>
>>> This actually breaks sound on my board
>>> (arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts).
>>> The codec on this board (wm8904) has a fixed clock input (only
>>> distinct
>>> frequencies are supported) and uses the FLL of the codec to generate
>>> the
>>> desired sample rate.
>>>
>>> It seems that after this patch the clock rate of the codecs clock
>>> (rather
>>> than the FLL) is tried to be changed. Which fails, because it doesn't
>>> support arbitrary frequencies.
>>
>> Yes, after the given change the clock will be updated if "*mclk-fs"
>> property is specified.
>>
>> DT you mentioned has property "simple-audio-card,mclk-fs = <256>",
>> which means you need a clock that is a function of sample rate. But as
>> per above you want a fixed clock for MCLK. I think if you drop this
>> property, the clock updates won't happen. Earlier for your case, this
>> property was not used at all because the clock handle was not
>> populated.
>
> You mean to drop the mclk-fs property? I can't do that because I
> actually need a frequency of 256 * sample rate. But that doesn't
> necessarily need to be the MCLK, because the codec itself has a
> FLL/PLL which can be used to generate any frequency for a given
> MCLK. So that is a valid scenario. See also commit 13409d27cb39
> ("ASoC: wm8904: configure sysclk/FLL automatically").
>

If I read this correctly below is the configuration you need,
SoC -> MCLK(fixed rate) -> PLL(wm8904) -> PLL output (256 * fs) -> sysclk

 From the doc simple-card.txt, "simple-audio-card,mclk-fs" is a scaling 
factor for MCLK and hence I am not sure if it is correct to have 
"*mclk-fs" property when MCLK is fixed. In simple cases, codec sysclk 
direclty depends on MCLK and set_sysclk() callback helps. Your case 
requires PLL configuration and set_pll() may be a better alternative. 
However simple-card does not offer this yet. But even if this is added, 
there should be a way to suggest PLL output requirement as a function of 
sample rate.