lkml.org 
[lkml]   [2018]   [Feb]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v3] rtc: ac100: Fix ac100 determine rate bug
On Fri, Feb 16, 2018 at 9:07 PM, Maxime Ripard
<maxime.ripard@bootlin.com> wrote:
> On Fri, Feb 16, 2018 at 12:10:18PM +0800, Chen-Yu Tsai wrote:
>> > diff --git a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
>> > b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
>> > index 6550bf0e594b..6f56d429f17e 100644
>> > --- a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
>> > +++ b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
>> > @@ -175,11 +175,18 @@
>> > compatible = "x-powers,ac100-rtc";
>> > interrupt-parent = <&r_intc>;
>> > interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
>> > - clocks = <&ac100_codec>;
>> > + clocks = <&ac100_rtc_32k>;
>> > #clock-cells = <1>;
>> > clock-output-names = "cko1_rtc",
>> > "cko2_rtc",
>> > "cko3_rtc";
>> > +
>> > + ac100_rtc_32k: rtc-32k-oscillator {
>> > + compatible = "fixed-clock";
>> > + #clock-cells = <0>;
>> > + clock-frequency = <32768>;
>> > + clock-output-names = "ac100-rtc-32k";
>> > + };
>> > };
>> > };
>> > };
>> >
>> > What do you think about that solution?
>>
>> That's not quite right either. As I mentioned before, the
>> RTC block has two clock inputs, one 4MHz signal from the
>> codec block, and one 32.768 kHz signal from an external
>> crystal. The original device tree binding describes the
>> first one, and the 32.768 kHz clock was registered by
>> the RTC driver internally.
>>
>> If you're going to add the crystal clock, you still need
>> to keep the codec one. Note that this does not fix what
>> Maxime is asking you. I've already provided an explanation:
>>
>> The clock core allows registering clocks with not-yet-existing
>> clock parents. Parents are matches by string names. If no
>> clock by that name is registered yet, the clock core simply
>> orphans the new clock if the unregistered parent is its
>> current parent or simply ignores that parent if its not the
>> current parent. This is entirely valid and is what we are
>> counting on here, as we haven't implemented the codec-side
>> driver.
>
> So, we end up in a situation where clk_hw_get_num_parents returns the
> amount of clocks we can be parented to (orphans or not), but
> clk_hw_get_parent_by_index will not return the orphan clocks?

There is no placeholder for missing parents, unlike the regulator
subsystem that has a dummy regulator for this purpose.

> That's pretty bad :/

Yeah. I didn't expect this to happen. But to be fair, I should
have done the check on clk_hw_get_parent_by_index.

> Is there a way to test before registering that all our parents are
> actually there? clk_get?

That's probably the way to do it. However in the AC100 RTC case,
I left it open to be missing on purpose, so we could use the RTC
without waiting for the codec to be supported.

ChenYu

\
 
 \ /
  Last update: 2018-02-16 14:16    [W:0.598 / U:0.056 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site