Hi Geert-san, > From: Geert Uytterhoeven, Sent: Thursday, September 28, 2023 4:33 PM > > 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. Thank you for the information! > > > * @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. I got it. Thank you for your reply! To Andrew, I believe this v3 patch can be acceptable for upstream. But, what do you think? Should I add some description somewhere about the clock of Renesas SoC? Best regards, Yoshihiro Shimoda > 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