Messages in this thread | | | Date | Wed, 29 Mar 2023 10:57:00 +0800 | Subject | Re: [net-next v9 5/6] net: stmmac: Add glue layer for StarFive JH7110 SoC | From | Guo Samin <> |
| |
Re: [net-next v9 5/6] net: stmmac: Add glue layer for StarFive JH7110 SoC From: Jakub Kicinski <kuba@kernel.org> to: Samin Guo <samin.guo@starfivetech.com> data: 2023/3/29
> On Tue, 28 Mar 2023 14:20:08 +0800 Samin Guo wrote: >> This adds StarFive dwmac driver support on the StarFive JH7110 SoC. >> >> Tested-by: Tommaso Merciai <tomm.merciai@gmail.com> >> Co-developed-by: Emil Renner Berthing <kernel@esmil.dk> >> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> >> Signed-off-by: Samin Guo <samin.guo@starfivetech.com> > > Excellent, now it applies cleanly :) > > Our clang build with W=1 complains that: > > drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c:37:2: warning: variable 'rate' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized] > default: > ^~~~~~~ > drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c:42:36: note: uninitialized use occurs here > err = clk_set_rate(dwmac->clk_tx, rate); > ^~~~ > drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c:24:20: note: initialize the variable 'rate' to silence this warning > unsigned long rate; > ^ > = 0 > > > not sure how you prefer to fix this. Maybe return early?
Hi Jakub,
Sorry, gcc I used does not report this error (:, and clang compile checks are more stringent. Also, if return early at default node, Arun Ramadoss doesn't think it's a good idea because the function is a void type.
I think I can initialize the value of rate first by clk_get_rate (Intel did the same <dwmac-intel-plat.c: kmb_eth_fix_mac_speed >), like this:
static void starfive_dwmac_fix_mac_speed(void *priv, unsigned int speed) { struct starfive_dwmac *dwmac = priv; unsigned long rate; int err;
rate = clk_get_rate(dwmac->tx_clk);
switch (speed) { case SPEED_1000: rate = 125000000; break; case SPEED_100: rate = 25000000; break; case SPEED_10: rate = 2500000; break; default: dev_err(dwmac->dev, "invalid speed %u\n", speed); break; }
err = clk_set_rate(dwmac->clk_tx, rate); if (err) dev_err(dwmac->dev, "failed to set tx rate %lu\n", rate); }
What do you think?
Best regards, Samin
| |