Linux kernel and device drivers for NXP i.MX platforms
 help / color / mirror / Atom feed
* [PATCH net] net: phy: micrel: Dynamically control external clock of KSZ PHY
@ 2024-11-25  2:29 Wei Fang
  2024-11-25 17:54 ` Frank Li
  0 siblings, 1 reply; 3+ messages in thread
From: Wei Fang @ 2024-11-25  2:29 UTC (permalink / raw)
  To: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni,
	florian.fainelli, heiko.stuebner
  Cc: netdev, linux-kernel, imx

On the i.MX6ULL-14x14-EVK board, when using the 6.12 kernel, it is found
that after disabling the two network ports, the clk_enable_count of the
enet1_ref and enet2_ref clocks is not 0 (these two clocks are used as the
clock source of the RMII reference clock of the two KSZ8081 PHYs), but
there is no such problem in the 6.6 kernel.

After analysis, we found that since the commit 985329462723 ("net: phy:
micrel: use devm_clk_get_optional_enabled for the rmii-ref clock"), the
external clock of KSZ PHY has been enabled when the PHY driver probes,
and it can only be disabled when the PHY driver is removed. This causes
the clock to continue working when the system is suspended or the network
port is down.

To solve this problem, the clock is enabled when resume() of the PHY
driver is called, and the clock is disabled when suspend() is called.
Since the PHY driver's resume() and suspend() interfaces are not called
in pairs, an additional clk_enable flag is added. When suspend() is
called, the clock is disabled only if clk_enable is true. Conversely,
when resume() is called, the clock is enabled if clk_enable is false.

Fixes: 985329462723 ("net: phy: micrel: use devm_clk_get_optional_enabled for the rmii-ref clock")
Fixes: 99ac4cbcc2a5 ("net: phy: micrel: allow usage of generic ethernet-phy clock")
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/phy/micrel.c | 103 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 95 insertions(+), 8 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 3ef508840674..44577b5d48d5 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -432,10 +432,12 @@ struct kszphy_ptp_priv {
 struct kszphy_priv {
 	struct kszphy_ptp_priv ptp_priv;
 	const struct kszphy_type *type;
+	struct clk *clk;
 	int led_mode;
 	u16 vct_ctrl1000;
 	bool rmii_ref_clk_sel;
 	bool rmii_ref_clk_sel_val;
+	bool clk_enable;
 	u64 stats[ARRAY_SIZE(kszphy_hw_stats)];
 };
 
@@ -2050,8 +2052,27 @@ static void kszphy_get_stats(struct phy_device *phydev,
 		data[i] = kszphy_get_stat(phydev, i);
 }
 
+static void kszphy_enable_clk(struct kszphy_priv *priv)
+{
+	if (!priv->clk_enable && priv->clk) {
+		clk_prepare_enable(priv->clk);
+		priv->clk_enable = true;
+	}
+}
+
+static void kszphy_disable_clk(struct kszphy_priv *priv)
+{
+	if (priv->clk_enable && priv->clk) {
+		clk_disable_unprepare(priv->clk);
+		priv->clk_enable = false;
+	}
+}
+
 static int kszphy_suspend(struct phy_device *phydev)
 {
+	struct kszphy_priv *priv = phydev->priv;
+	int ret;
+
 	/* Disable PHY Interrupts */
 	if (phy_interrupt_is_valid(phydev)) {
 		phydev->interrupts = PHY_INTERRUPT_DISABLED;
@@ -2059,7 +2080,13 @@ static int kszphy_suspend(struct phy_device *phydev)
 			phydev->drv->config_intr(phydev);
 	}
 
-	return genphy_suspend(phydev);
+	ret = genphy_suspend(phydev);
+	if (ret)
+		return ret;
+
+	kszphy_disable_clk(priv);
+
+	return 0;
 }
 
 static void kszphy_parse_led_mode(struct phy_device *phydev)
@@ -2088,8 +2115,11 @@ static void kszphy_parse_led_mode(struct phy_device *phydev)
 
 static int kszphy_resume(struct phy_device *phydev)
 {
+	struct kszphy_priv *priv = phydev->priv;
 	int ret;
 
+	kszphy_enable_clk(priv);
+
 	genphy_resume(phydev);
 
 	/* After switching from power-down to normal mode, an internal global
@@ -2112,6 +2142,24 @@ static int kszphy_resume(struct phy_device *phydev)
 	return 0;
 }
 
+static int ksz8041_resume(struct phy_device *phydev)
+{
+	struct kszphy_priv *priv = phydev->priv;
+
+	kszphy_enable_clk(priv);
+
+	return 0;
+}
+
+static int ksz8041_suspend(struct phy_device *phydev)
+{
+	struct kszphy_priv *priv = phydev->priv;
+
+	kszphy_disable_clk(priv);
+
+	return 0;
+}
+
 static int ksz9477_resume(struct phy_device *phydev)
 {
 	int ret;
@@ -2150,8 +2198,11 @@ static int ksz9477_resume(struct phy_device *phydev)
 
 static int ksz8061_resume(struct phy_device *phydev)
 {
+	struct kszphy_priv *priv = phydev->priv;
 	int ret;
 
+	kszphy_enable_clk(priv);
+
 	/* This function can be called twice when the Ethernet device is on. */
 	ret = phy_read(phydev, MII_BMCR);
 	if (ret < 0)
@@ -2194,7 +2245,7 @@ static int kszphy_probe(struct phy_device *phydev)
 
 	kszphy_parse_led_mode(phydev);
 
-	clk = devm_clk_get_optional_enabled(&phydev->mdio.dev, "rmii-ref");
+	clk = devm_clk_get_optional(&phydev->mdio.dev, "rmii-ref");
 	/* NOTE: clk may be NULL if building without CONFIG_HAVE_CLK */
 	if (!IS_ERR_OR_NULL(clk)) {
 		unsigned long rate = clk_get_rate(clk);
@@ -2216,11 +2267,14 @@ static int kszphy_probe(struct phy_device *phydev)
 		}
 	} else if (!clk) {
 		/* unnamed clock from the generic ethernet-phy binding */
-		clk = devm_clk_get_optional_enabled(&phydev->mdio.dev, NULL);
+		clk = devm_clk_get_optional(&phydev->mdio.dev, NULL);
 		if (IS_ERR(clk))
 			return PTR_ERR(clk);
 	}
 
+	if (!IS_ERR_OR_NULL(clk))
+		priv->clk = clk;
+
 	if (ksz8041_fiber_mode(phydev))
 		phydev->port = PORT_FIBRE;
 
@@ -5290,15 +5344,45 @@ static int lan8841_probe(struct phy_device *phydev)
 	return 0;
 }
 
+static int lan8804_suspend(struct phy_device *phydev)
+{
+	struct kszphy_priv *priv = phydev->priv;
+	int ret;
+
+	ret = genphy_suspend(phydev);
+	if (ret)
+		return ret;
+
+	kszphy_disable_clk(priv);
+
+	return 0;
+}
+
+static int lan8841_resume(struct phy_device *phydev)
+{
+	struct kszphy_priv *priv = phydev->priv;
+
+	kszphy_enable_clk(priv);
+
+	return genphy_resume(phydev);
+}
+
 static int lan8841_suspend(struct phy_device *phydev)
 {
 	struct kszphy_priv *priv = phydev->priv;
 	struct kszphy_ptp_priv *ptp_priv = &priv->ptp_priv;
+	int ret;
 
 	if (ptp_priv->ptp_clock)
 		ptp_cancel_worker_sync(ptp_priv->ptp_clock);
 
-	return genphy_suspend(phydev);
+	ret = genphy_suspend(phydev);
+	if (ret)
+		return ret;
+
+	kszphy_disable_clk(priv);
+
+	return 0;
 }
 
 static struct phy_driver ksphy_driver[] = {
@@ -5358,9 +5442,12 @@ static struct phy_driver ksphy_driver[] = {
 	.get_sset_count = kszphy_get_sset_count,
 	.get_strings	= kszphy_get_strings,
 	.get_stats	= kszphy_get_stats,
-	/* No suspend/resume callbacks because of errata DS80000700A,
-	 * receiver error following software power down.
+	/* Because of errata DS80000700A, receiver error following software
+	 * power down. Suspend and resume callbacks only disable and enable
+	 * external rmii reference clock.
 	 */
+	.suspend	= ksz8041_suspend,
+	.resume		= ksz8041_resume,
 }, {
 	.phy_id		= PHY_ID_KSZ8041RNLI,
 	.phy_id_mask	= MICREL_PHY_ID_MASK,
@@ -5507,7 +5594,7 @@ static struct phy_driver ksphy_driver[] = {
 	.get_sset_count	= kszphy_get_sset_count,
 	.get_strings	= kszphy_get_strings,
 	.get_stats	= kszphy_get_stats,
-	.suspend	= genphy_suspend,
+	.suspend	= lan8804_suspend,
 	.resume		= kszphy_resume,
 	.config_intr	= lan8804_config_intr,
 	.handle_interrupt = lan8804_handle_interrupt,
@@ -5526,7 +5613,7 @@ static struct phy_driver ksphy_driver[] = {
 	.get_strings	= kszphy_get_strings,
 	.get_stats	= kszphy_get_stats,
 	.suspend	= lan8841_suspend,
-	.resume		= genphy_resume,
+	.resume		= lan8841_resume,
 	.cable_test_start	= lan8814_cable_test_start,
 	.cable_test_get_status	= ksz886x_cable_test_get_status,
 }, {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net] net: phy: micrel: Dynamically control external clock of KSZ PHY
  2024-11-25  2:29 [PATCH net] net: phy: micrel: Dynamically control external clock of KSZ PHY Wei Fang
@ 2024-11-25 17:54 ` Frank Li
  2024-11-26  6:43   ` Wei Fang
  0 siblings, 1 reply; 3+ messages in thread
From: Frank Li @ 2024-11-25 17:54 UTC (permalink / raw)
  To: Wei Fang
  Cc: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni,
	florian.fainelli, heiko.stuebner, netdev, linux-kernel, imx

On Mon, Nov 25, 2024 at 10:29:05AM +0800, Wei Fang wrote:
> On the i.MX6ULL-14x14-EVK board, when using the 6.12 kernel, it is found
> that after disabling the two network ports, the clk_enable_count of the
> enet1_ref and enet2_ref clocks is not 0 (these two clocks are used as the
> clock source of the RMII reference clock of the two KSZ8081 PHYs), but
> there is no such problem in the 6.6 kernel.

skip your debug progress, just descript the problem itself.

>
> After analysis, we found that since the commit 985329462723 ("net: phy:
> micrel: use devm_clk_get_optional_enabled for the rmii-ref clock"), the
> external clock of KSZ PHY has been enabled when the PHY driver probes,
> and it can only be disabled when the PHY driver is removed. This causes
> the clock to continue working when the system is suspended or the network
> port is down.
>
> To solve this problem, the clock is enabled when resume() of the PHY
> driver is called, and the clock is disabled when suspend() is called.
> Since the PHY driver's resume() and suspend() interfaces are not called
> in pairs, an additional clk_enable flag is added. When suspend() is

Why  resume() and suspend() is not call paired?


> called, the clock is disabled only if clk_enable is true. Conversely,
> when resume() is called, the clock is enabled if clk_enable is false.
>
> Fixes: 985329462723 ("net: phy: micrel: use devm_clk_get_optional_enabled for the rmii-ref clock")
> Fixes: 99ac4cbcc2a5 ("net: phy: micrel: allow usage of generic ethernet-phy clock")
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
>  drivers/net/phy/micrel.c | 103 ++++++++++++++++++++++++++++++++++++---
>  1 file changed, 95 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 3ef508840674..44577b5d48d5 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -432,10 +432,12 @@ struct kszphy_ptp_priv {
>  struct kszphy_priv {
>  	struct kszphy_ptp_priv ptp_priv;
>  	const struct kszphy_type *type;
> +	struct clk *clk;
>  	int led_mode;
>  	u16 vct_ctrl1000;
>  	bool rmii_ref_clk_sel;
>  	bool rmii_ref_clk_sel_val;
> +	bool clk_enable;
>  	u64 stats[ARRAY_SIZE(kszphy_hw_stats)];
>  };
>
> @@ -2050,8 +2052,27 @@ static void kszphy_get_stats(struct phy_device *phydev,
>  		data[i] = kszphy_get_stat(phydev, i);
>  }
>
> +static void kszphy_enable_clk(struct kszphy_priv *priv)
> +{
> +	if (!priv->clk_enable && priv->clk) {
> +		clk_prepare_enable(priv->clk);
> +		priv->clk_enable = true;
> +	}
> +}
> +
> +static void kszphy_disable_clk(struct kszphy_priv *priv)
> +{
> +	if (priv->clk_enable && priv->clk) {
> +		clk_disable_unprepare(priv->clk);
> +		priv->clk_enable = false;
> +	}
> +}

Generally, clock not check enable status, just call enable/disable pair.

Frank

> +
>  static int kszphy_suspend(struct phy_device *phydev)
>  {
> +	struct kszphy_priv *priv = phydev->priv;
> +	int ret;
> +
>  	/* Disable PHY Interrupts */
>  	if (phy_interrupt_is_valid(phydev)) {
>  		phydev->interrupts = PHY_INTERRUPT_DISABLED;
> @@ -2059,7 +2080,13 @@ static int kszphy_suspend(struct phy_device *phydev)
>  			phydev->drv->config_intr(phydev);
>  	}
>
> -	return genphy_suspend(phydev);
> +	ret = genphy_suspend(phydev);
> +	if (ret)
> +		return ret;
> +
> +	kszphy_disable_clk(priv);
> +
> +	return 0;
>  }
>
>  static void kszphy_parse_led_mode(struct phy_device *phydev)
> @@ -2088,8 +2115,11 @@ static void kszphy_parse_led_mode(struct phy_device *phydev)
>
>  static int kszphy_resume(struct phy_device *phydev)
>  {
> +	struct kszphy_priv *priv = phydev->priv;
>  	int ret;
>
> +	kszphy_enable_clk(priv);
> +
>  	genphy_resume(phydev);
>
>  	/* After switching from power-down to normal mode, an internal global
> @@ -2112,6 +2142,24 @@ static int kszphy_resume(struct phy_device *phydev)
>  	return 0;
>  }
>
> +static int ksz8041_resume(struct phy_device *phydev)
> +{
> +	struct kszphy_priv *priv = phydev->priv;
> +
> +	kszphy_enable_clk(priv);
> +
> +	return 0;
> +}
> +
> +static int ksz8041_suspend(struct phy_device *phydev)
> +{
> +	struct kszphy_priv *priv = phydev->priv;
> +
> +	kszphy_disable_clk(priv);
> +
> +	return 0;
> +}
> +
>  static int ksz9477_resume(struct phy_device *phydev)
>  {
>  	int ret;
> @@ -2150,8 +2198,11 @@ static int ksz9477_resume(struct phy_device *phydev)
>
>  static int ksz8061_resume(struct phy_device *phydev)
>  {
> +	struct kszphy_priv *priv = phydev->priv;
>  	int ret;
>
> +	kszphy_enable_clk(priv);
> +
>  	/* This function can be called twice when the Ethernet device is on. */
>  	ret = phy_read(phydev, MII_BMCR);
>  	if (ret < 0)
> @@ -2194,7 +2245,7 @@ static int kszphy_probe(struct phy_device *phydev)
>
>  	kszphy_parse_led_mode(phydev);
>
> -	clk = devm_clk_get_optional_enabled(&phydev->mdio.dev, "rmii-ref");
> +	clk = devm_clk_get_optional(&phydev->mdio.dev, "rmii-ref");
>  	/* NOTE: clk may be NULL if building without CONFIG_HAVE_CLK */
>  	if (!IS_ERR_OR_NULL(clk)) {
>  		unsigned long rate = clk_get_rate(clk);
> @@ -2216,11 +2267,14 @@ static int kszphy_probe(struct phy_device *phydev)
>  		}
>  	} else if (!clk) {
>  		/* unnamed clock from the generic ethernet-phy binding */
> -		clk = devm_clk_get_optional_enabled(&phydev->mdio.dev, NULL);
> +		clk = devm_clk_get_optional(&phydev->mdio.dev, NULL);
>  		if (IS_ERR(clk))
>  			return PTR_ERR(clk);
>  	}
>
> +	if (!IS_ERR_OR_NULL(clk))
> +		priv->clk = clk;
> +
>  	if (ksz8041_fiber_mode(phydev))
>  		phydev->port = PORT_FIBRE;
>
> @@ -5290,15 +5344,45 @@ static int lan8841_probe(struct phy_device *phydev)
>  	return 0;
>  }
>
> +static int lan8804_suspend(struct phy_device *phydev)
> +{
> +	struct kszphy_priv *priv = phydev->priv;
> +	int ret;
> +
> +	ret = genphy_suspend(phydev);
> +	if (ret)
> +		return ret;
> +
> +	kszphy_disable_clk(priv);
> +
> +	return 0;
> +}
> +
> +static int lan8841_resume(struct phy_device *phydev)
> +{
> +	struct kszphy_priv *priv = phydev->priv;
> +
> +	kszphy_enable_clk(priv);
> +
> +	return genphy_resume(phydev);
> +}
> +
>  static int lan8841_suspend(struct phy_device *phydev)
>  {
>  	struct kszphy_priv *priv = phydev->priv;
>  	struct kszphy_ptp_priv *ptp_priv = &priv->ptp_priv;
> +	int ret;
>
>  	if (ptp_priv->ptp_clock)
>  		ptp_cancel_worker_sync(ptp_priv->ptp_clock);
>
> -	return genphy_suspend(phydev);
> +	ret = genphy_suspend(phydev);
> +	if (ret)
> +		return ret;
> +
> +	kszphy_disable_clk(priv);
> +
> +	return 0;
>  }
>
>  static struct phy_driver ksphy_driver[] = {
> @@ -5358,9 +5442,12 @@ static struct phy_driver ksphy_driver[] = {
>  	.get_sset_count = kszphy_get_sset_count,
>  	.get_strings	= kszphy_get_strings,
>  	.get_stats	= kszphy_get_stats,
> -	/* No suspend/resume callbacks because of errata DS80000700A,
> -	 * receiver error following software power down.
> +	/* Because of errata DS80000700A, receiver error following software
> +	 * power down. Suspend and resume callbacks only disable and enable
> +	 * external rmii reference clock.
>  	 */
> +	.suspend	= ksz8041_suspend,
> +	.resume		= ksz8041_resume,
>  }, {
>  	.phy_id		= PHY_ID_KSZ8041RNLI,
>  	.phy_id_mask	= MICREL_PHY_ID_MASK,
> @@ -5507,7 +5594,7 @@ static struct phy_driver ksphy_driver[] = {
>  	.get_sset_count	= kszphy_get_sset_count,
>  	.get_strings	= kszphy_get_strings,
>  	.get_stats	= kszphy_get_stats,
> -	.suspend	= genphy_suspend,
> +	.suspend	= lan8804_suspend,
>  	.resume		= kszphy_resume,
>  	.config_intr	= lan8804_config_intr,
>  	.handle_interrupt = lan8804_handle_interrupt,
> @@ -5526,7 +5613,7 @@ static struct phy_driver ksphy_driver[] = {
>  	.get_strings	= kszphy_get_strings,
>  	.get_stats	= kszphy_get_stats,
>  	.suspend	= lan8841_suspend,
> -	.resume		= genphy_resume,
> +	.resume		= lan8841_resume,
>  	.cable_test_start	= lan8814_cable_test_start,
>  	.cable_test_get_status	= ksz886x_cable_test_get_status,
>  }, {
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH net] net: phy: micrel: Dynamically control external clock of KSZ PHY
  2024-11-25 17:54 ` Frank Li
@ 2024-11-26  6:43   ` Wei Fang
  0 siblings, 0 replies; 3+ messages in thread
From: Wei Fang @ 2024-11-26  6:43 UTC (permalink / raw)
  To: Frank Li
  Cc: andrew@lunn.ch, hkallweit1@gmail.com, linux@armlinux.org.uk,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, florian.fainelli@broadcom.com,
	heiko.stuebner@cherry.de, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, imx@lists.linux.dev



> On Mon, Nov 25, 2024 at 10:29:05AM +0800, Wei Fang wrote:
> > On the i.MX6ULL-14x14-EVK board, when using the 6.12 kernel, it is
> > found that after disabling the two network ports, the clk_enable_count
> > of the enet1_ref and enet2_ref clocks is not 0 (these two clocks are
> > used as the clock source of the RMII reference clock of the two
> > KSZ8081 PHYs), but there is no such problem in the 6.6 kernel.
> 
> skip your debug progress, just descript the problem itself.
> 
> >
> > After analysis, we found that since the commit 985329462723 ("net: phy:
> > micrel: use devm_clk_get_optional_enabled for the rmii-ref clock"),
> > the external clock of KSZ PHY has been enabled when the PHY driver
> > probes, and it can only be disabled when the PHY driver is removed.
> > This causes the clock to continue working when the system is suspended
> > or the network port is down.
> >
> > To solve this problem, the clock is enabled when resume() of the PHY
> > driver is called, and the clock is disabled when suspend() is called.
> > Since the PHY driver's resume() and suspend() interfaces are not
> > called in pairs, an additional clk_enable flag is added. When
> > suspend() is
> 
> Why  resume() and suspend() is not call paired?
> 

For the MAC drivers which use PHY Library, when the network interface
is up (net_device_ops:: ndo_open() is called). The phy_driver:: resume()
will be called twice:

net_device_ops:: ndo_open()
	--> of_phy_connect()
		--> phy_attach_direct()
			--> phy_resume()
				--> phy_driver:: resume() #1
	--> phy_start()
		--> __phy_resume()
			--> phy_driver:: resume() #2

But phy_driver:: suspend() is called only once when the network port
is down (net_device_ops:: ndo_stop ()).

net_device_ops:: ndo_stop ()
	--> phy_stop()
		--> phy_suspend()
			--> phy_driver::suspend() #1

For MAC drivers which use phylink also have the same situation.

net_device_ops:: ndo_open()
	--> phylink_of_phy_connect()
		--> phy_attach_direct()
			--> phy_resume()
				--> phy_driver:: resume() #1
	--> phylink_start()
		--> phy_start()
			--> __phy_resume()
				--> phy_driver:: resume() #2

net_device_ops:: ndo_stop ()
	--> phylink_stop()
		--> phy_stop()
			--> phy_suspend()
				--> phy_driver::suspend() #1

> 
> > called, the clock is disabled only if clk_enable is true. Conversely,
> > when resume() is called, the clock is enabled if clk_enable is false.
> >
> > Fixes: 985329462723 ("net: phy: micrel: use
> > devm_clk_get_optional_enabled for the rmii-ref clock")
> > Fixes: 99ac4cbcc2a5 ("net: phy: micrel: allow usage of generic
> > ethernet-phy clock")
> > Signed-off-by: Wei Fang <wei.fang@nxp.com>
> > ---
> >  drivers/net/phy/micrel.c | 103
> > ++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 95 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c index
> > 3ef508840674..44577b5d48d5 100644
> > --- a/drivers/net/phy/micrel.c
> > +++ b/drivers/net/phy/micrel.c
> > @@ -432,10 +432,12 @@ struct kszphy_ptp_priv {  struct kszphy_priv {
> >  	struct kszphy_ptp_priv ptp_priv;
> >  	const struct kszphy_type *type;
> > +	struct clk *clk;
> >  	int led_mode;
> >  	u16 vct_ctrl1000;
> >  	bool rmii_ref_clk_sel;
> >  	bool rmii_ref_clk_sel_val;
> > +	bool clk_enable;
> >  	u64 stats[ARRAY_SIZE(kszphy_hw_stats)];
> >  };
> >
> > @@ -2050,8 +2052,27 @@ static void kszphy_get_stats(struct phy_device
> *phydev,
> >  		data[i] = kszphy_get_stat(phydev, i);  }
> >
> > +static void kszphy_enable_clk(struct kszphy_priv *priv) {
> > +	if (!priv->clk_enable && priv->clk) {
> > +		clk_prepare_enable(priv->clk);
> > +		priv->clk_enable = true;
> > +	}
> > +}
> > +
> > +static void kszphy_disable_clk(struct kszphy_priv *priv) {
> > +	if (priv->clk_enable && priv->clk) {
> > +		clk_disable_unprepare(priv->clk);
> > +		priv->clk_enable = false;
> > +	}
> > +}
> 
> Generally, clock not check enable status, just call enable/disable pair.
>

Yes, but for PHY driver, the situation is different, resume() is called once
more than suspend(). So we need to ensure clk_prepare_enable() will
not be called repeatedly.


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-11-26  6:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-25  2:29 [PATCH net] net: phy: micrel: Dynamically control external clock of KSZ PHY Wei Fang
2024-11-25 17:54 ` Frank Li
2024-11-26  6:43   ` Wei Fang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox