Re: [PATCH v3] rtc: ac100: Fix ac100 determine rate bug

From: Maxime Ripard
Date: Fri Feb 16 2018 - 08:07:57 EST


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?

That's pretty bad :/

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

Maxime

--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

Attachment: signature.asc
Description: PGP signature