Re: [RFC PATCH] ASoC: wm8904: add CCF support

Boris BREZILLON
2014-03-24T09:44:27Z

Le 21/03/2014 12:55, Mark Brown a écrit :
> On Fri, Mar 21, 2014 at 10:37:47AM +0000, Mark Rutland wrote:
>> On Fri, Mar 21, 2014 at 02:51:02AM +0000, Bo Shen wrote:
>>> + - wlf,sysclk-from-mclk: set the sys clock is driven from mclk,
>> Why can the kernel not decide this?
> It can.
>
>>> + - wlf,mclk-use-xtal: if the mclk is generated by crystal.
>>> + if without this property, the mclk is generated from SOC.
>> Huh? What exact property do you actually are about here?
> This should just be omitted - based on the previous posting it's saying
> if this is a fixed or variable rate clock.
>
>>> + - wlf,mclk-freq: mclk's frequency
>> If you expect mclk, you should be able to query this from it. You don't
>> need a separate property.
>> Unless this is a frequency to set it to? If so, why can the kernel not
>> choose this?
> Yes, quite - and even if it needs to be set explicitly the clock API
> generic bindings should be able to support this (I *think* that is due
> to go in during the next merge window but iddn't check yet).
I guess you're talking about this: https://lkml.org/lkml/2014/3/3/324.
AFAIK this has not been accepted yet, but this is definitely the way to
go if we need to specify a default-rate from the DT.

On the whole subject, I'd like to have your opinion on how we should
implement clk handling within the wm8904 driver.

From my point of view we have 2 choices:

1) Expose the wm8904 clock tree using the CCF (and its DT bindings)

From what I read in the datasheet this would give the following DT
definition:

wm8904@xx {
compatible="wm8904";
[...]

clocks {
fll {
compatible = "wolfson,wm8904-fll"
#clock-cells = <0>;
clocks = <&pck0>;
clock-output-names = "wm8904-fll";
};

sysclk {
compatible = "wolfson,wm8904-sysclk"
clocks = <&pck0 &fll>;
clock-output-names = "wm8904-sysclk";
};

/* other clk derived from sysclk */
};
};

If we use the CCF in the wm8904 driver, we'll have to move all users
of this chip to the CCF too (in other terms, we need to move the sam9n12
SoC and boards to the CCF).

2) keep the driver as it is except for the mclk retrieval.

wm8904@xx {
compatible="wm8904";
clocks = <&pck0>;
clock-names = "mclk";
clock-rates = <32768>;
};

The first solution is obviously more complicated to implement (and
requires the
CCF), but in the other hand, it gives fine-grained control over clk
configuration
and use a standard way to expose/manipulate clks.

Best Regards,

Boris