On Mon, Apr 11, 2022 at 02:20:22PM +0200, Lucas Stach wrote:
> Hi Maxime,
>
> Am Montag, dem 11.04.2022 um 13:59 +0200 schrieb Maxime Ripard:
> > Hi Lucas,
> >
> > On Wed, Apr 06, 2022 at 06:01:20PM +0200, Lucas Stach wrote:
> > > +static unsigned long phy_clk_recalc_rate(struct clk_hw *hw,
> > > + unsigned long parent_rate)
> > > +{
> > > + struct fsl_samsung_hdmi_phy *phy = to_fsl_samsung_hdmi_phy(hw);
> > > +
> > > + if (!phy->cur_cfg)
> > > + return 0;
> > > +
> > > + return phy->cur_cfg->clk_rate;
> > > +}
> >
> > This means that the clock will return 0 at initialization, which will
> > throw the rate accounting in the CCF off.
> >
> > Returning 0 here isn't valid. Surely that phy has a default
> > configuration at boot that you could use to initialize a rate?
> >
> > See https://lore.kernel.org/linux-clk/20220408091037.2041955-1-maxime@xxxxxxxxxx/
>
> Thanks for the hint. I don't know the full history of this and surely I
> can use the register reset defaults to initialize the clock rate, but
> it still seems odd. A powered down clock generator, like the PLL in
> this PHY, is not actually putting out a clock at any rate, so 0 for the
> rate seems natural, so it's kind of a bad pitfall if the CCF can't deal
> with that.
Yeah, it's what that whole discussion has been about, but my
understanding is that clk_get_rate() (and thus .recalc_rate) don't
necessarily return the actual output rate in hardware, but the rate that
would be output if the clock was enabled.
Anyway, I've ping'd Stephen on IRC to see if he can clarifies this, we
won't have an issue on this before then I'm afraid :)
Maxime
Attachment:
signature.asc
Description: PGP signature
