<div data-ntes="ntes_mail_body_root" style="line-height:1.7;color:#000000;font-size:14px;font-family:Arial"><div id="spnEditorContent"><div style="margin: 0;"><br></div><div style="margin: 0;">Hi, <br> <b> Is gmac_clk_enable() also redundant?<br></b><br>
<span style="color: rgb(17, 17, 51); font-family: -apple-system, blinkmacsystemfont, "Helvetica Neue", helvetica, "segoe ui", arial, roboto, "PingFang SC", miui, "Hiragino Sans GB", "Microsoft Yahei", sans-serif, "Source Han Serif"; font-size: 16px; letter-spacing: 0.064px; white-space-collapse: preserve;">it never gets executed because </span><code node="[object Object]" message="[object Object]" contentid="1dcf5c504cf44558a20ae7701b2c7334_0" contentstatus="finished" agentid="" style="--tw-border-spacing-x: 0; --tw-border-spacing-y: 0; --tw-translate-x: 0; --tw-translate-y: 0; --tw-rotate: 0; --tw-skew-x: 0; --tw-skew-y: 0; --tw-scale-x: 1; --tw-scale-y: 1; --tw-pan-x: ; --tw-pan-y: ; --tw-pinch-zoom: ; --tw-scroll-snap-strictness: proximity; --tw-gradient-from-position: ; --tw-gradient-via-position: ; --tw-gradient-to-position: ; --tw-ordinal: ; --tw-slashed-zero: ; --tw-numeric-figure: ; --tw-numeric-spacing: ; --tw-numeric-fraction: ; --tw-ring-inset: ; --tw-ring-offset-width: 0px; --tw-ring-offset-color: #fff; --tw-ring-color: rgba(59,130,246,0.5); --tw-ring-offset-shadow: 0 0 #0000; --tw-ring-shadow: 0 0 #0000; --tw-shadow: 0 0 #0000; --tw-shadow-colored: 0 0 #0000; --tw-blur: ; --tw-brightness: ; --tw-contrast: ; --tw-grayscale: ; --tw-hue-rotate: ; --tw-invert: ; --tw-saturate: ; --tw-sepia: ; --tw-drop-shadow: ; --tw-backdrop-blur: ; --tw-backdrop-brightness: ; --tw-backdrop-contrast: ; --tw-backdrop-grayscale: ; --tw-backdrop-hue-rotate: ; --tw-backdrop-invert: ; --tw-backdrop-opacity: ; --tw-backdrop-saturate: ; --tw-backdrop-sepia: ; --tw-contain-size: ; --tw-contain-layout: ; --tw-contain-paint: ; --tw-contain-style: ; border: 0px solid; box-sizing: border-box; font-feature-settings: normal; font-family: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Consolas, "Liberation Mono", monospace; font-size: 13.6px; font-variation-settings: normal; white-space-collapse: break-spaces; margin: 0px; background-color: rgba(175, 184, 193, 0.2); border-radius: 6px; padding: 0.2em 0.4em; color: rgb(17, 17, 51); letter-spacing: 0.064px;">phy_power_on()</code><span style="color: rgb(17, 17, 51); font-family: -apple-system, blinkmacsystemfont, "Helvetica Neue", helvetica, "segoe ui", arial, roboto, "PingFang SC", miui, "Hiragino Sans GB", "Microsoft Yahei", sans-serif, "Source Han Serif"; font-size: 16px; letter-spacing: 0.064px; white-space-collapse: preserve;"> always returns false
</span><span style="color: rgb(17, 17, 51); font-family: -apple-system, blinkmacsystemfont, "Helvetica Neue", helvetica, "segoe ui", arial, roboto, "PingFang SC", miui, "Hiragino Sans GB", "Microsoft Yahei", sans-serif, "Source Han Serif"; font-size: 16px; letter-spacing: 0.064px; white-space-collapse: preserve;">Therefore, the following approach might be a fix : </span> <span style="color: rgb(17, 17, 51); font-family: -apple-system, blinkmacsystemfont, "Helvetica Neue", helvetica, "segoe ui", arial, roboto, "PingFang SC", miui, "Hiragino Sans GB", "Microsoft Yahei", sans-serif, "Source Han Serif"; font-size: 16px; letter-spacing: 0.064px; white-space-collapse: preserve;">
</span><br> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c</div><div style="margin: 0;">index 51ea0caf16c1..085c0d669eca 100644</div><div style="margin: 0;">--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c</div><div style="margin: 0;">+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c</div><div style="margin: 0;">@@ -1461,7 +1461,7 @@ static int gmac_clk_enable(struct rk_priv_data *bsp_priv, bool enable)</div><div style="margin: 0;"> return 0;</div><div style="margin: 0;"> }</div><div style="margin: 0;"><br></div><div style="margin: 0;">-static int phy_power_on(struct rk_priv_data *bsp_priv, bool enable)</div><div style="margin: 0;">+static int rk_phy_power_set(struct rk_priv_data *bsp_priv, bool enable)</div><div style="margin: 0;"> {</div><div style="margin: 0;"> struct regulator *ldo = bsp_priv->regulator;</div><div style="margin: 0;"> struct device *dev = bsp_priv->dev;</div><div style="margin: 0;">@@ -1477,7 +1477,7 @@ static int phy_power_on(struct rk_priv_data *bsp_priv, bool enable)</div><div style="margin: 0;"> dev_err(dev, "fail to disable phy-supply\n");</div><div style="margin: 0;"> }</div><div style="margin: 0;"><br></div><div style="margin: 0;">- return 0;</div><div style="margin: 0;">+ return ret;</div><div style="margin: 0;"> }</div><div style="margin: 0;"><br></div><div style="margin: 0;"> static struct rk_priv_data *rk_gmac_setup(struct platform_device *pdev,</div><div style="margin: 0;">@@ -1655,7 +1655,7 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)</div><div style="margin: 0;"> dev_err(dev, "NO interface defined!\n");</div><div style="margin: 0;"> }</div><div style="margin: 0;"><br></div><div style="margin: 0;">- ret = phy_power_on(bsp_priv, true);</div><div style="margin: 0;">+ ret = rk_phy_power_set(bsp_priv, true);</div><div style="margin: 0;"> if (ret) {</div><div style="margin: 0;"> gmac_clk_enable(bsp_priv, false);</div><div style="margin: 0;"> return ret;</div><div style="margin: 0;">@@ -1676,7 +1676,7 @@ static void rk_gmac_powerdown(struct rk_priv_data *gmac)</div><div style="margin: 0;"><br></div><div style="margin: 0;"> pm_runtime_put_sync(gmac->dev);</div><div style="margin: 0;"><br></div><div style="margin: 0;">- phy_power_on(gmac, false);</div><div style="margin: 0;">+ rk_phy_power_set(gmac, false);</div><div style="margin: 0;"> gmac_clk_enable(gmac, false);</div><div style="margin: 0;"> }</div><div style="margin: 0;"><br></div><div style="margin: 0;">--</div><div style="margin: 0;">2.17.1</div><div style="margin: 0;"><br><br></div><div style="margin: 0;"><br></div><div style="margin: 0;"><br></div><div style="margin: 0;"><br></div></div><div style="position:relative;zoom:1"></div><div id="divNeteaseMailCard"></div><div style="margin: 0;"><br></div><pre><br>At 2025-10-15 14:17:29, "Chaoyi Chen" <chaoyi.chen@rock-chips.com> wrote:
>On 10/15/2025 12:08 PM, Lizhe wrote:
>
>> 'phy_power_on' is a local scope one within the driver, since the return
>> value of the phy_power_on() function is always 0, checking its return
>> value is redundant.
>>
>> the function name 'phy_power_on()' conflicts with the existing
>> phy_power_on() function in the PHY subsystem. a suitable alternative
>> name would be rk_phy_power_set(), particularly since when the second
>> argument is false, this function actually powers off the PHY
>>
>> Signed-off-by: Lizhe <sensor1010@163.com>
>> ---
>> drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 17 ++++-------------
>> 1 file changed, 4 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> index 51ea0caf16c1..9d296bfab013 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> @@ -1461,23 +1461,18 @@ static int gmac_clk_enable(struct rk_priv_data *bsp_priv, bool enable)
>> return 0;
>> }
>>
>> -static int phy_power_on(struct rk_priv_data *bsp_priv, bool enable)
>> +static void rk_phy_power_on(struct rk_priv_data *bsp_priv, bool enable)
>> {
>> struct regulator *ldo = bsp_priv->regulator;
>> struct device *dev = bsp_priv->dev;
>> - int ret;
>>
>> if (enable) {
>> - ret = regulator_enable(ldo);
>> - if (ret)
>> + if (regulator_enable(ldo))
>> dev_err(dev, "fail to enable phy-supply\n");
>> } else {
>> - ret = regulator_disable(ldo);
>> - if (ret)
>> + if (regulator_disable(ldo))
>> dev_err(dev, "fail to disable phy-supply\n");
>> }
>> -
>> - return 0;
>> }
>>
>> static struct rk_priv_data *rk_gmac_setup(struct platform_device *pdev,
>> @@ -1655,11 +1650,7 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
>> dev_err(dev, "NO interface defined!\n");
>> }
>>
>> - ret = phy_power_on(bsp_priv, true);
>> - if (ret) {
>> - gmac_clk_enable(bsp_priv, false);
>
>Is gmac_clk_enable() also redundant?
>
>
>> - return ret;
>> - }
>> + rk_phy_power_on(bsp_priv, true);
>>
>> pm_runtime_get_sync(dev);
>>
>
>--
>Best,
>Chaoyi
</pre></div>