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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aMw-dgNiXgPeqeSz@shell.armlinux.org.uk>
Date: Thu, 18 Sep 2025 18:16:38 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: weishangjuan@...incomputing.com
Cc: devicetree@...r.kernel.org, andrew+netdev@...n.ch, davem@...emloft.net,
	edumazet@...gle.com, kuba@...nel.org, robh@...nel.org,
	krzk+dt@...nel.org, conor+dt@...nel.org, netdev@...r.kernel.org,
	pabeni@...hat.com, mcoquelin.stm32@...il.com,
	alexandre.torgue@...s.st.com, vladimir.oltean@....com,
	yong.liang.choong@...ux.intel.com, anthony.l.nguyen@...el.com,
	prabhakar.mahadev-lad.rj@...renesas.com, jan.petrous@....nxp.com,
	jszhang@...nel.org, inochiama@...il.com, 0x1207@...il.com,
	boon.khai.ng@...era.com, linux-kernel@...r.kernel.org,
	linux-stm32@...md-mailman.stormreply.com,
	linux-arm-kernel@...ts.infradead.org, ningyu@...incomputing.com,
	linmin@...incomputing.com, lizhi2@...incomputing.com,
	pinkesh.vaghela@...fochips.com
Subject: Re: [PATCH v7 2/2] ethernet: eswin: Add eic7700 ethernet driver

On Thu, Sep 18, 2025 at 05:00:26PM +0800, weishangjuan@...incomputing.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!