Re: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs

From: Stephen Boyd
Date: Fri Aug 31 2018 - 13:10:54 EST


Quoting Zhu, Yi Xin (2018-08-28 23:56:22)
>
> On 8/28/2018 3:09 AM, Stephen Boyd wrote:
> > Quoting yixin zhu (2018-08-08 01:52:20)
> >> On 8/8/2018 1:50 PM, Stephen Boyd wrote:
> >>>> +/* clock flags definition */
> >>>> +#define CLOCK_FLAG_VAL_INIT BIT(16)
> >>>> +#define GATE_CLK_HW BIT(17)
> >>>> +#define GATE_CLK_SW BIT(18)
> >>>> +#define GATE_CLK_VT BIT(19)
> >>> What does VT mean? Virtual?
> >> Yes. VT means virtual here.
> >> Will change to GATE_CLK_VIRT.
> >>
> > Is it a hardware concept? Or virtualization with hypervisor?
>
> Some peripheral drivers want to use same code cross platforms.
>
> But not all platforms provide HW gate clock. So in this case, clock
> driver creates
>
> a virtual gate clock to make it work if no HW gate clock in the SoC.

That's not how things are supposed to work. If a clk isn't there in the
hardware we don't make them up in software so that the consumer software
drivers can keep requesting clks on different platforms. On a different
platform, the driver needs to know that the clks aren't there with a
different compatible string.

>
>
> >
> >>>> +}
> >>>> +
> >>>> +CLK_OF_DECLARE(intel_grx500_cgu, "intel,grx500-cgu", grx500_clk_init);
> >>> Any reason a platform driver can't be used instead of CLK_OF_DECLARE()?
> >> It provides CPU clock which is used in early boot stage.
> >>
> > Ok. What is the CPU clock doing in early boot stage? Some sort of timer
> > frequency? If the driver can be split into two pieces, one to handle the
> > really early stuff that must be in place to get timers up and running
> > and the other to register the rest of the clks that aren't critical from
> > a regular platform driver it would be good. That's preferred model if
> > something is super critical.
>
> Yes, CPU clock is providing CPU frequency in the early boot stage.
>
> Will put the non-critical clocks in the platform driver.
>
>

Sure the CPU clock is handling frequency, but does that matter for early
boot to get going? If timers aren't involved here then it doesn't sound
like this needs CLK_OF_DECLARE.