In starfive_dwmac_fix_mac_speed(), the rate gotten by clk_get_rate()
is not necessary, remove the clk_get_rate() calling.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
index b68f42795eaa..422138ef565e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
@@ -30,8 +30,6 @@ static void starfive_dwmac_fix_mac_speed(void *priv, unsigned int speed, unsigne
 	unsigned long rate;
 	int err;
 
-	rate = clk_get_rate(dwmac->clk_tx);
-
 	switch (speed) {
 	case SPEED_1000:
 		rate = 125000000;
@@ -44,7 +42,7 @@ static void starfive_dwmac_fix_mac_speed(void *priv, unsigned int speed, unsigne
 		break;
 	default:
 		dev_err(dwmac->dev, "invalid speed %u\n", speed);
-		break;
+		return;
 	}
 
 	err = clk_set_rate(dwmac->clk_tx, rate);
-- 
2.40.1

On Sun, 27 Aug 2023 at 15:53, Jisheng Zhang <jszhang@kernel.org> wrote:
>
> In starfive_dwmac_fix_mac_speed(), the rate gotten by clk_get_rate()
> is not necessary, remove the clk_get_rate() calling.

Thanks. I suggested this change during the initial review, but someone
wanted the code as it is. I must admit I don't understand why, so
Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
>  drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> index b68f42795eaa..422138ef565e 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> @@ -30,8 +30,6 @@ static void starfive_dwmac_fix_mac_speed(void *priv, unsigned int speed, unsigne
>         unsigned long rate;
>         int err;
>
> -       rate = clk_get_rate(dwmac->clk_tx);
> -
>         switch (speed) {
>         case SPEED_1000:
>                 rate = 125000000;
> @@ -44,7 +42,7 @@ static void starfive_dwmac_fix_mac_speed(void *priv, unsigned int speed, unsigne
>                 break;
>         default:
>                 dev_err(dwmac->dev, "invalid speed %u\n", speed);
> -               break;
> +               return;
>         }
>
>         err = clk_set_rate(dwmac->clk_tx, rate);
> --
> 2.40.1
>

On Sun, Aug 27, 2023 at 07:33:10PM +0200, Emil Renner Berthing wrote:
> On Sun, 27 Aug 2023 at 15:53, Jisheng Zhang <jszhang@kernel.org> wrote:
> >
> > In starfive_dwmac_fix_mac_speed(), the rate gotten by clk_get_rate()
> > is not necessary, remove the clk_get_rate() calling.
> 
> Thanks. I suggested this change during the initial review, but someone
> wanted the code as it is. I must admit I don't understand why, so
> Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>

The code in starfive_dwmac_fix_mac_speed() is a repeated pattern amongst
many drivers, and having each platform driver re-implement this is not
sane. Those who know me will know that I have a patch that cleans this
all up - moving basically the guts of this to a library function which
platform drivers can make use of.

It's not like the clock rates are somehow special - they're standard for
10M/100M/1G speeds on a GMII-family interface, and the 10M/100M also
share the clock rates with MII.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!