RE: [PATCH net v3] rswitch: Fix PHY station management clock setting

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Andrew,

> 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.
>  * @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?

Best regards,
Yoshihiro Shimoda

>       Andrew




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux