| Message ID | 20230827134150.2918-3-jszhang@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: stmmac: dwmac-starfive: some improvements | expand |
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.
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);
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(-)