lkml.org 
[lkml]   [2017]   [Sep]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] sound: soc: fsl: Do not set DAI sysclk when it is equal to system freq
From
Date
Hi Nicolin,

> On Tue, Sep 05, 2017 at 11:13:40PM +0200, Łukasz Majewski wrote:
>
>> They key point here is the asoc_simple_card_parse_clk() function
>> from simple-card-utils.c
>>
>> Please look how the clock is assigned; It first checks for cpu
>> clock, then for "system-clock-frequency" DTS node and _finally_
>> looks for another "child" clock [1], which is the codec attached to
>> I2C.
>
> Here is the routine that I understood from the code:
> 1) asoc_simple_card_parse_clk_cpu(dev, cpu, dai_link, cpu_dai);
> => asoc_simple_card_parse_clk(dev, cpu, // cpu node in sound{} [1]
> dai_link->cpu_of_node, // node ssi2 [2]
> cpu_dai, dai_link->cpu_dai_name);
> ==> 1.1) devm_get_clk_from_child(dev, node, NULL); // [1]
> ==> 1.2) of_property_read_u32(node, "system-clock-frequency", &val)// [1]
> ==> 1.3) devm_get_clk_from_child(dev, dai_of_node, NULL); // [2]
>
> 2) asoc_simple_card_parse_clk_codec(dev, codec, dai_link, codec_dai);
> => asoc_simple_card_parse_clk(dev, codec, // codec node in sound{} [3]
> dai_link->codec_of_node,// node tfa9879 [4]
> codec_dai, dai_link->codec_dai_name);
> ==> 2.1) devm_get_clk_from_child(dev, node, NULL); // [3]
> ==> 2.2) of_property_read_u32(node, "system-clock-frequency", &val)// [3]
> ==> 2.3) devm_get_clk_from_child(dev, dai_of_node, NULL); // [4]
>
> For the cpu routine, it first checks for clock property under cpu
> node of simple-card, then for "system-clock-frequency" in the cpu
> node of simple-card, and finally looks for clock property in ssi2
> node.

I've double checked this and the
simple_dai->sysclk is assigned in the

asoc_simple_card_parse_clk()

-----> dev: sound
-----> clk node: /soc/aips-bus@02000000/spba-bus@02000000/ssi@0202c000
-----> Clk asignment

And this clock is taken from this node. It looks like ipg clock for ssi...

>
> For codec routine, it first checks for clock property under codec
> node of simple-card, then for "system-clock-frequency" in codec
> node of simple-card, and finally looks for clock property in the
> tfa9879 node.
>
> The cpu and codec are totally separate, aren't they? And I don't
> think it's right if not.

Yes, they should be separated.

>
>>>> which has clock from I2C (66 MHz).
>>>
>>> You mean I2C scl or I2S sclk?
>>
>> I2C scl.
>
> A couple of things here.....
> 1) I don't think I2C scl can run at 66MHz...The 66MHz is probably
> the ipg clock of I2C controller.

I agree. I2C scl runs with 400 kHz (as recommended by I2C high speed spec).

And considering above, the 66 MHz clock seems to be from ssi IP block.

> 2) Even if the scl does run at 66MHz, there is no point in passing
> the clock of I2C scl into an I2S dai-link.

Yes.

>
> So I feel something must be wrong. I suggest you to add some printk
> to check the names of those nodes and x_of_node in the simple card
> driver.

The problem is with the "lack" of clock nodes/properties at

dailink_master: cpu {
sound-dai = <&ssi2>;
clock = <&SSSS>;
system-clock-frequency = <XXXX>;
};


and as a result we take some "default" clock.

>
>> My DTS [1] (it is different than other in-tree supported codecs - at
>> least I did not find similar setup in DTSes):
>>
>> sound {
>> compatible = "simple-audio-card";
>> label = "tfa9879-mono";
>>
>> simple-audio-card,dai-link {
>> /* DAC */
>> format = "i2s";
>> bitclock-master = <&dailink_master>;
>> frame-master = <&dailink_master>;
>>
>> dailink_master: cpu {
>> sound-dai = <&ssi2>;
>> };
>> codec {
>> sound-dai = <&codec>;
>> };
>> };
>
> I remember there is another style for a single dai-link. Could you
> please try that one? There is an example in:
> Documentation/devicetree/bindings/sound/simple-card.txt

Yes, the

simple-audio-card,cpu
simple-audio-card,format,
....

etc.

I've omitted it since in the code it is regarded as "old":
(simple-card.c line 384).


>


I think that the proper solution would be to add check for:

freq < sysclk/5 in fsl_ssi_set_dai_sysclk() and return -ENOTSUPP to make
the simple-audo-card driver happy (and not introducing regressions).


--
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de

\
 
 \ /
  Last update: 2017-09-06 11:23    [W:0.095 / U:0.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site