Hi Shimoda-san,
On Thu, Sep 28, 2023 at 4:13 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote:
> > From: Andrew Lunn, Sent: Wednesday, September 27, 2023 9:44 PM
> > > + /* MPIC.PSMCS = (clk [MHz] / (MDC frequency [MHz] * 2) - 1.
> > > + * Calculating PSMCS value as MDC frequency = 2.5MHz. So, multiply
> > > + * both the numerator and the denominator by 10.
> > > + */
> > > + etha->psmcs = clk_get_rate(priv->clk) / 100000 / (25 * 2) - 1;
> > > }
> > >
> > > static int rswitch_device_alloc(struct rswitch_private *priv, int index)
> > > @@ -1900,6 +1907,10 @@ static int renesas_eth_sw_probe(struct platform_device *pdev)
> > > return -ENOMEM;
> > > spin_lock_init(&priv->lock);
> > >
> > > + priv->clk = devm_clk_get(&pdev->dev, NULL);
> > > + if (IS_ERR(priv->clk))
> > > + return PTR_ERR(priv->clk);
> > > +
> >
> > /**
> > * clk_get_rate - obtain the current clock rate (in Hz) for a clock source.
> > * This is only valid once the clock source has been enabled.
Whether clk_get_rate() works when the clock is still disabled actually
depends on the clock driver implementation/hardware. It's not
guaranteed, so generic code cannot make that assumption.
It should work fine on all Renesas on-SoC clock generators.
> > * @clk: clock source
> > */
> > unsigned long clk_get_rate(struct clk *clk);
> >
> > I don't see the clock being enabled anywhere.
>
> Since GENPD_FLAG_PM_CLK is set in the drivers/pmdomain/renesas/rcar-gen4-sysc.c,
> pm_runtime_get_sync() will enable the clock. That's why this code works correctly
> without clk_enable() calling.
>
> > Also, is the clock documented in the device tree binding?
>
> Yes, but this is a "clocks" property only though. In the dt-bindings doc:
> ---
> examples:
> ...
> clocks = <&cpg CPG_MOD 1505>;
> ---
>
> And, in the drivers/clk/renesas/r8a779f0-cpg-mssr.c:
> ---
> DEF_FIXED("rsw2", R8A779F0_CLK_RSW2, CLK_PLL5_DIV2, 5, 1),
> ...
> DEF_MOD("rswitch2", 1505, R8A779F0_CLK_RSW2),
> ---
> So, the device will get the paranet clock " R8A779F0_CLK_RSW2".
> And according to the clk_summary in the debugfs:
> ---
> # grep rsw /sys/kernel/debug/clk/clk_summary
> rsw2 1 1 0 320000000 0 0 50000 Y
> rswitch2 1 1 0 320000000 0 0 50000 Y
> ---
>
> I found that i2c-rcar.c and pwm-rcar.c are also the same implementation
> which call clk_get_rate() without clk_enable(). But, perhaps, we should enable
> the clock by clk API?
>
> To Geert-san, do you have any opinion?
As the device is part of a clock domain, the clock is managed through
Runtime PM, and there is no need to enable or disable manually.
Just make sure to call pm_runtime_resume_and_get() before accessing
any register of the device.
Calling clk_get_rate() at any time is fine.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
