Re: [PATCH v7 2/2] ethernet: eswin: Add eic7700 ethernet driver
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
Date: 2025-09-18 17:16:53
Also in:
linux-arm-kernel, linux-devicetree, lkml
On Thu, Sep 18, 2025 at 05:00:26PM +0800, weishangjuan@eswincomputing.com wrote:
+ plat_dat->clk_tx_i = stmmac_pltfr_find_clk(plat_dat, "tx");
+ plat_dat->set_clk_tx_rate = stmmac_set_clk_tx_rate;
+ plat_dat->bsp_priv = dwc_priv;
+ plat_dat->clks_config = eic7700_clks_config;
+ dwc_priv->plat_dat = plat_dat;
+
+ ret = eic7700_clks_config(dwc_priv, true);
+ if (ret)
+ return dev_err_probe(&pdev->dev,
+ ret,
+ "error enable clock\n");
+
+ ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+ if (ret) {
+ eic7700_clks_config(dwc_priv, false);
+ return dev_err_probe(&pdev->dev,
+ ret,
+ "Failed to driver probe\n");
+ }
+
+ return ret;
+}
+
+static void eic7700_dwmac_remove(struct platform_device *pdev)
+{
+ struct eic7700_qos_priv *dwc_priv = get_stmmac_bsp_priv(&pdev->dev);
+
+ stmmac_pltfr_remove(pdev);
+ eic7700_clks_config(dwc_priv, false);It would be nice to see the above code cleaned up like I did for all the other stmmac glue drivers recently. However, this is not to say this shouldn't be merged - but please consider this if you do another rework of these patches, if not as a follow-up patch. Essentially, you can use devm_stmmac_pltfm_probe(), populate the plat_dat->init() and plat_dat->exit() methods to call the clks_config function, but as you don't want these methods to be called during suspend/resume (because plat_dat->clks_config() is already called there), provide empty plat_dat->suspend() and plat_dat->resume() methods. Bonus points if you include a patch which provides this functionality as library functions in stmmac_platform.c which can be used to initialise ->init() and ->exit() for this behaviour, and check other stmmac platform glue drivers to see if they would benefit from using these. Of course, it would be nice not to have to go to the extent of adding empty functions for ->suspend() and ->resume(), but stmmac has a lot of weirdo history, and there was no easy way to maintain compatibility without doing that when I added these two new methods. Lastly, please consider using "net: stmmac: <shortened-glue-name>: blah" as the subject so there's a consistent style for stmmac patches. Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!