Re: [PATCH v7 2/2] ethernet: eswin: Add eic7700 ethernet driver

["Russell King (Oracle)" <linux@armlinux.org.uk>]

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!