Re: [RFC PATCH 0/2] Propose critical clocks

[Marco Felsch]

Hi Marek,

sorry for the delay.

On 22-10-06, Marek Vasut wrote:
> On 10/6/22 01:06, Stephen Boyd wrote:
> > Quoting Marco Felsch (2022-10-05 01:23:48)
> > > Hi Stephen, Michael,
> > > 
> > > I know it is a busy time right now, but maybe you have a few minutes for
> > > this RFC. I know it is incomplete, but the interessting part is there
> > > and it would fix a real issue we encountered on the imx8mm-evk's.
> 
> The i.MX8M hang when using 32kHz supplied by PMIC is solved by modeling the
> clock in DT correctly, see:
> 
> https://lore.kernel.org/all/20220924174603.458956-1-marex@xxxxxxx/

Thanks for this link, but I have a few doubts about your modeling. As
you already noted in the above link, the 32K osc is critical for some
reason not listed in the RM.

My doubts about your modeling are:
 - The snvs-rtc driver will be required no matter if it is used or not
 - According the i.MX8MM RM rev.3 11/2020: The SNVS is supplied by clock
   gate 71 (CCM_CCGR71). So this would be the clock provider for the
   SNVS modul. This clock gate is enabled by the ROM loader and the
   clock driver have no support for it yet. As soon as the clock driver
   have support for it your modeling will break since the clock
   gate gets turned off since you specify an other clock source.

With my solution the modeling is still correct and the user is not
enforced to enable driver for an _maybe_ unused modul. What do you
think?

Regards,
  Marco

> > There's another approach by Marek[1]. Can you work together on a
> > solution? I think we should step away from trying to make the critical
> > flag work during clk registration, and turn on the clk during provider
> > registration instead.
> 
> So that would work like the qualcomm-specific 'protected-clock' property?
> 
> I really want to avoid such clock-driver specific hacks which are poorly or
> inconsistently supported. This critical-clock should be a generic solution
> and that should be in clock core.
> 
> > That hopefully makes it simpler. We can keep the
> > clk flag of course, so that the clk can't be turned off, but otherwise
> > we shouldn't need to make registration path check for the property.
> > 
> > [1] https://lore.kernel.org/all/20220924174517.458657-1-marex@xxxxxxx/
> 
>