* [PATCH net-next 0/2] net: stmmac: dwmac-starfive: some improvements @ 2023-08-27 13:41 Jisheng Zhang 2023-08-27 13:41 ` [PATCH net-next 1/2] net: stmmac: dwmac-starfive: improve error handling during probe Jisheng Zhang 2023-08-27 13:41 ` [PATCH net-next 2/2] net: stmmac: dwmac-starfive: remove unnecessary clk_get_rate() Jisheng Zhang 0 siblings, 2 replies; 7+ messages in thread From: Jisheng Zhang @ 2023-08-27 13:41 UTC (permalink / raw) To: Emil Renner Berthing, Samin Guo, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel This series is to improve the dwmac-starfive driver by correcting error handling and removing unnecessary clk_get_rate(). Jisheng Zhang (2): net: stmmac: dwmac-starfive: improve error handling during probe net: stmmac: dwmac-starfive: remove unnecessary clk_get_rate() .../ethernet/stmicro/stmmac/dwmac-starfive.c | 38 +++++++++++-------- 1 file changed, 22 insertions(+), 16 deletions(-) -- 2.40.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net-next 1/2] net: stmmac: dwmac-starfive: improve error handling during probe 2023-08-27 13:41 [PATCH net-next 0/2] net: stmmac: dwmac-starfive: some improvements Jisheng Zhang @ 2023-08-27 13:41 ` Jisheng Zhang 2023-08-27 13:58 ` Russell King (Oracle) 2023-08-27 13:41 ` [PATCH net-next 2/2] net: stmmac: dwmac-starfive: remove unnecessary clk_get_rate() Jisheng Zhang 1 sibling, 1 reply; 7+ messages in thread From: Jisheng Zhang @ 2023-08-27 13:41 UTC (permalink / raw) To: Emil Renner Berthing, Samin Guo, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel After stmmac_probe_config_dt() succeeds, when error happens later, stmmac_remove_config_dt() needs to be called for proper error handling. Signed-off-by: Jisheng Zhang <jszhang@kernel.org> --- .../ethernet/stmicro/stmmac/dwmac-starfive.c | 34 ++++++++++++------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c index 892612564694..b68f42795eaa 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c @@ -111,18 +111,24 @@ static int starfive_dwmac_probe(struct platform_device *pdev) "dt configuration failed\n"); dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL); - if (!dwmac) - return -ENOMEM; + if (!dwmac) { + err = -ENOMEM; + goto err_remove_config_dt; + } dwmac->clk_tx = devm_clk_get_enabled(&pdev->dev, "tx"); - if (IS_ERR(dwmac->clk_tx)) - return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_tx), - "error getting tx clock\n"); + if (IS_ERR(dwmac->clk_tx)) { + err = dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_tx), + "error getting tx clock\n"); + goto err_remove_config_dt; + } clk_gtx = devm_clk_get_enabled(&pdev->dev, "gtx"); - if (IS_ERR(clk_gtx)) - return dev_err_probe(&pdev->dev, PTR_ERR(clk_gtx), - "error getting gtx clock\n"); + if (IS_ERR(clk_gtx)) { + err = dev_err_probe(&pdev->dev, PTR_ERR(clk_gtx), + "error getting gtx clock\n"); + goto err_remove_config_dt; + } /* Generally, the rgmii_tx clock is provided by the internal clock, * which needs to match the corresponding clock frequency according @@ -139,15 +145,17 @@ static int starfive_dwmac_probe(struct platform_device *pdev) err = starfive_dwmac_set_mode(plat_dat); if (err) - return err; + goto err_remove_config_dt; err = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res); - if (err) { - stmmac_remove_config_dt(pdev, plat_dat); - return err; - } + if (err) + goto err_remove_config_dt; return 0; + +err_remove_config_dt: + stmmac_remove_config_dt(pdev, plat_dat); + return err; } static const struct of_device_id starfive_dwmac_match[] = { -- 2.40.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next 1/2] net: stmmac: dwmac-starfive: improve error handling during probe 2023-08-27 13:41 ` [PATCH net-next 1/2] net: stmmac: dwmac-starfive: improve error handling during probe Jisheng Zhang @ 2023-08-27 13:58 ` Russell King (Oracle) 2023-08-27 17:35 ` Emil Renner Berthing 0 siblings, 1 reply; 7+ messages in thread From: Russell King (Oracle) @ 2023-08-27 13:58 UTC (permalink / raw) To: Jisheng Zhang Cc: Emil Renner Berthing, Samin Guo, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, netdev, linux-stm32, linux-arm-kernel, linux-kernel On Sun, Aug 27, 2023 at 09:41:49PM +0800, Jisheng Zhang wrote: > After stmmac_probe_config_dt() succeeds, when error happens later, > stmmac_remove_config_dt() needs to be called for proper error handling. Have you thought about converting to use devm_stmmac_probe_config_dt() which will call stmmac_remove_config_dt() if the probe fails or when the device is unbound? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next 1/2] net: stmmac: dwmac-starfive: improve error handling during probe 2023-08-27 13:58 ` Russell King (Oracle) @ 2023-08-27 17:35 ` Emil Renner Berthing 0 siblings, 0 replies; 7+ messages in thread From: Emil Renner Berthing @ 2023-08-27 17:35 UTC (permalink / raw) To: Russell King (Oracle) Cc: Jisheng Zhang, Samin Guo, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, netdev, linux-stm32, linux-arm-kernel, linux-kernel On Sun, 27 Aug 2023 at 15:59, Russell King (Oracle) <linux@armlinux.org.uk> wrote: > On Sun, Aug 27, 2023 at 09:41:49PM +0800, Jisheng Zhang wrote: > > After stmmac_probe_config_dt() succeeds, when error happens later, > > stmmac_remove_config_dt() needs to be called for proper error handling. > > Have you thought about converting to use devm_stmmac_probe_config_dt() > which will call stmmac_remove_config_dt() if the probe fails or when > the device is unbound? +1. Using devm_stmmac_probe_config_dt() seems like a better solution. > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net-next 2/2] net: stmmac: dwmac-starfive: remove unnecessary clk_get_rate() 2023-08-27 13:41 [PATCH net-next 0/2] net: stmmac: dwmac-starfive: some improvements Jisheng Zhang 2023-08-27 13:41 ` [PATCH net-next 1/2] net: stmmac: dwmac-starfive: improve error handling during probe Jisheng Zhang @ 2023-08-27 13:41 ` Jisheng Zhang 2023-08-27 17:33 ` Emil Renner Berthing 1 sibling, 1 reply; 7+ messages in thread From: Jisheng Zhang @ 2023-08-27 13:41 UTC (permalink / raw) To: Emil Renner Berthing, Samin Guo, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next 2/2] net: stmmac: dwmac-starfive: remove unnecessary clk_get_rate() 2023-08-27 13:41 ` [PATCH net-next 2/2] net: stmmac: dwmac-starfive: remove unnecessary clk_get_rate() Jisheng Zhang @ 2023-08-27 17:33 ` Emil Renner Berthing 2023-08-27 17:57 ` Russell King (Oracle) 0 siblings, 1 reply; 7+ messages in thread From: Emil Renner Berthing @ 2023-08-27 17:33 UTC (permalink / raw) To: Jisheng Zhang Cc: Samin Guo, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, netdev, linux-stm32, linux-arm-kernel, linux-kernel 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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next 2/2] net: stmmac: dwmac-starfive: remove unnecessary clk_get_rate() 2023-08-27 17:33 ` Emil Renner Berthing @ 2023-08-27 17:57 ` Russell King (Oracle) 0 siblings, 0 replies; 7+ messages in thread From: Russell King (Oracle) @ 2023-08-27 17:57 UTC (permalink / raw) To: Emil Renner Berthing Cc: Jisheng Zhang, Samin Guo, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, netdev, linux-stm32, linux-arm-kernel, linux-kernel 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! ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-08-27 18:00 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-08-27 13:41 [PATCH net-next 0/2] net: stmmac: dwmac-starfive: some improvements Jisheng Zhang 2023-08-27 13:41 ` [PATCH net-next 1/2] net: stmmac: dwmac-starfive: improve error handling during probe Jisheng Zhang 2023-08-27 13:58 ` Russell King (Oracle) 2023-08-27 17:35 ` Emil Renner Berthing 2023-08-27 13:41 ` [PATCH net-next 2/2] net: stmmac: dwmac-starfive: remove unnecessary clk_get_rate() Jisheng Zhang 2023-08-27 17:33 ` Emil Renner Berthing 2023-08-27 17:57 ` Russell King (Oracle)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).