[PATCH net-next v4 1/3] net: stmmac: Add generic suspend/resume helper for PCI-based controllers

Yao Zi posted 3 patches 2 months, 4 weeks ago
There is a newer version of this series
[PATCH net-next v4 1/3] net: stmmac: Add generic suspend/resume helper for PCI-based controllers
Posted by Yao Zi 2 months, 4 weeks ago
Most glue driver for PCI-based DWMAC controllers utilize similar
platform suspend/resume routines. Add a generic implementation to reduce
duplicated code.

Signed-off-by: Yao Zi <ziyao@disroot.org>
Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/stmicro/stmmac/Kconfig   |  8 ++++
 drivers/net/ethernet/stmicro/stmmac/Makefile  |  1 +
 .../ethernet/stmicro/stmmac/stmmac_libpci.c   | 48 +++++++++++++++++++
 .../ethernet/stmicro/stmmac/stmmac_libpci.h   | 12 +++++
 4 files changed, 69 insertions(+)
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_libpci.c
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_libpci.h

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 87c5bea6c2a2..1350f16f7138 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -349,6 +349,14 @@ config DWMAC_VISCONTI
 
 endif
 
+config STMMAC_LIBPCI
+	tristate "STMMAC PCI helper library"
+	depends on PCI
+	default y
+	help
+	  This selects the PCI bus helpers for the stmmac driver. If you
+	  have a controller with PCI interface, say Y or M here.
+
 config DWMAC_INTEL
 	tristate "Intel GMAC support"
 	default X86
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index 1681a8a28313..7bf528731034 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -44,6 +44,7 @@ obj-$(CONFIG_DWMAC_VISCONTI)	+= dwmac-visconti.o
 stmmac-platform-objs:= stmmac_platform.o
 dwmac-altr-socfpga-objs := dwmac-socfpga.o
 
+obj-$(CONFIG_STMMAC_LIBPCI)	+= stmmac_libpci.o
 obj-$(CONFIG_STMMAC_PCI)	+= stmmac-pci.o
 obj-$(CONFIG_DWMAC_INTEL)	+= dwmac-intel.o
 obj-$(CONFIG_DWMAC_LOONGSON)	+= dwmac-loongson.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_libpci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_libpci.c
new file mode 100644
index 000000000000..5c5dd502f79a
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_libpci.c
@@ -0,0 +1,48 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PCI bus helpers for STMMAC driver
+ * Copyright (C) 2025 Yao Zi <ziyao@disroot.org>
+ */
+
+#include <linux/device.h>
+#include <linux/pci.h>
+
+#include "stmmac_libpci.h"
+
+int stmmac_pci_plat_suspend(struct device *dev, void *bsp_priv)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	int ret;
+
+	ret = pci_save_state(pdev);
+	if (ret)
+		return ret;
+
+	pci_disable_device(pdev);
+	pci_wake_from_d3(pdev, true);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(stmmac_pci_plat_suspend);
+
+int stmmac_pci_plat_resume(struct device *dev, void *bsp_priv)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	int ret;
+
+	pci_restore_state(pdev);
+	pci_set_power_state(pdev, PCI_D0);
+
+	ret = pci_enable_device(pdev);
+	if (ret)
+		return ret;
+
+	pci_set_master(pdev);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(stmmac_pci_plat_resume);
+
+MODULE_DESCRIPTION("STMMAC PCI helper library");
+MODULE_AUTHOR("Yao Zi <ziyao@disroot.org>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_libpci.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_libpci.h
new file mode 100644
index 000000000000..71553184f982
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_libpci.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2025 Yao Zi <ziyao@disroot.org>
+ */
+
+#ifndef __STMMAC_LIBPCI_H__
+#define __STMMAC_LIBPCI_H__
+
+int stmmac_pci_plat_suspend(struct device *dev, void *bsp_priv);
+int stmmac_pci_plat_resume(struct device *dev, void *bsp_priv);
+
+#endif /* __STMMAC_LIBPCI_H__ */
-- 
2.51.2
Re: [PATCH net-next v4 1/3] net: stmmac: Add generic suspend/resume helper for PCI-based controllers
Posted by Jakub Kicinski 2 months, 4 weeks ago
On Tue, 11 Nov 2025 10:07:26 +0000 Yao Zi wrote:
> +config STMMAC_LIBPCI
> +	tristate "STMMAC PCI helper library"
> +	depends on PCI
> +	default y
> +	help
> +	  This selects the PCI bus helpers for the stmmac driver. If you
> +	  have a controller with PCI interface, say Y or M here.

I didn't pay enough attention to the discussion on v2, sorry.
I understand that there's precedent for a library symbol hiding
real symbols in this driver but it really makes for a poor user
experience.

The symbol should be hidden, and select'ed by what needs it.
With the PCI dependency on the real symbol, not here.

The "default y" may draw the attention of the Superior Penguin.
He may have quite a lot to criticize in this area, so let's
not risk it..
Re: [PATCH net-next v4 1/3] net: stmmac: Add generic suspend/resume helper for PCI-based controllers
Posted by Russell King (Oracle) 2 months, 3 weeks ago
On Wed, Nov 12, 2025 at 06:57:20AM -0800, Jakub Kicinski wrote:
> On Tue, 11 Nov 2025 10:07:26 +0000 Yao Zi wrote:
> > +config STMMAC_LIBPCI
> > +	tristate "STMMAC PCI helper library"
> > +	depends on PCI
> > +	default y
> > +	help
> > +	  This selects the PCI bus helpers for the stmmac driver. If you
> > +	  have a controller with PCI interface, say Y or M here.
> 
> I didn't pay enough attention to the discussion on v2, sorry.
> I understand that there's precedent for a library symbol hiding
> real symbols in this driver but it really makes for a poor user
> experience.
> 
> The symbol should be hidden, and select'ed by what needs it.
> With the PCI dependency on the real symbol, not here.
> 
> The "default y" may draw the attention of the Superior Penguin.
> He may have quite a lot to criticize in this area, so let's
> not risk it..

Okay, should we also convert STMMAC_PLATFORM to behave the same way,
because it's odd to have one bus type acting one way and the other
differently.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next v4 1/3] net: stmmac: Add generic suspend/resume helper for PCI-based controllers
Posted by Yao Zi 2 months, 3 weeks ago
On Wed, Nov 12, 2025 at 03:15:45PM +0000, Russell King (Oracle) wrote:
> On Wed, Nov 12, 2025 at 06:57:20AM -0800, Jakub Kicinski wrote:
> > On Tue, 11 Nov 2025 10:07:26 +0000 Yao Zi wrote:
> > > +config STMMAC_LIBPCI
> > > +	tristate "STMMAC PCI helper library"
> > > +	depends on PCI
> > > +	default y
> > > +	help
> > > +	  This selects the PCI bus helpers for the stmmac driver. If you
> > > +	  have a controller with PCI interface, say Y or M here.
> > 
> > I didn't pay enough attention to the discussion on v2, sorry.
> > I understand that there's precedent for a library symbol hiding
> > real symbols in this driver but it really makes for a poor user
> > experience.
> > 
> > The symbol should be hidden, and select'ed by what needs it.
> > With the PCI dependency on the real symbol, not here.
> > 
> > The "default y" may draw the attention of the Superior Penguin.
> > He may have quite a lot to criticize in this area, so let's
> > not risk it..
> 
> Okay, should we also convert STMMAC_PLATFORM to behave the same way,
> because it's odd to have one bus type acting one way and the other
> differently.

I don't have a strong opinion about the Kconfig style, so should I go
back to make drivers select STMMAC_LIBPCI as suggested by Maxime in
v2[1], and drop the "default y" statement from STMMAC_LIBPCI?

Best regards,
Yao Zi

[1]: https://lore.kernel.org/netdev/da8d9585-d464-4611-98c0-a10d84874297@bootlin.com/

> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next v4 1/3] net: stmmac: Add generic suspend/resume helper for PCI-based controllers
Posted by Yanteng Si 2 months, 4 weeks ago
在 2025/11/11 18:07, Yao Zi 写道:
> Most glue driver for PCI-based DWMAC controllers utilize similar
> platform suspend/resume routines. Add a generic implementation to reduce
> duplicated code.
>
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Yanteng Si <siyanteng@cqsoftware.com.cn


Thanks,

Yanteng

> ---
>   drivers/net/ethernet/stmicro/stmmac/Kconfig   |  8 ++++
>   drivers/net/ethernet/stmicro/stmmac/Makefile  |  1 +
>   .../ethernet/stmicro/stmmac/stmmac_libpci.c   | 48 +++++++++++++++++++
>   .../ethernet/stmicro/stmmac/stmmac_libpci.h   | 12 +++++
>   4 files changed, 69 insertions(+)
>   create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_libpci.c
>   create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_libpci.h
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> index 87c5bea6c2a2..1350f16f7138 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> @@ -349,6 +349,14 @@ config DWMAC_VISCONTI
>   
>   endif
>   
> +config STMMAC_LIBPCI
> +	tristate "STMMAC PCI helper library"
> +	depends on PCI
> +	default y
> +	help
> +	  This selects the PCI bus helpers for the stmmac driver. If you
> +	  have a controller with PCI interface, say Y or M here.
> +
>   config DWMAC_INTEL
>   	tristate "Intel GMAC support"
>   	default X86
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> index 1681a8a28313..7bf528731034 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> @@ -44,6 +44,7 @@ obj-$(CONFIG_DWMAC_VISCONTI)	+= dwmac-visconti.o
>   stmmac-platform-objs:= stmmac_platform.o
>   dwmac-altr-socfpga-objs := dwmac-socfpga.o
>   
> +obj-$(CONFIG_STMMAC_LIBPCI)	+= stmmac_libpci.o
>   obj-$(CONFIG_STMMAC_PCI)	+= stmmac-pci.o
>   obj-$(CONFIG_DWMAC_INTEL)	+= dwmac-intel.o
>   obj-$(CONFIG_DWMAC_LOONGSON)	+= dwmac-loongson.o
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_libpci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_libpci.c
> new file mode 100644
> index 000000000000..5c5dd502f79a
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_libpci.c
> @@ -0,0 +1,48 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * PCI bus helpers for STMMAC driver
> + * Copyright (C) 2025 Yao Zi <ziyao@disroot.org>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/pci.h>
> +
> +#include "stmmac_libpci.h"
> +
> +int stmmac_pci_plat_suspend(struct device *dev, void *bsp_priv)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	int ret;
> +
> +	ret = pci_save_state(pdev);
> +	if (ret)
> +		return ret;
> +
> +	pci_disable_device(pdev);
> +	pci_wake_from_d3(pdev, true);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(stmmac_pci_plat_suspend);
> +
> +int stmmac_pci_plat_resume(struct device *dev, void *bsp_priv)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	int ret;
> +
> +	pci_restore_state(pdev);
> +	pci_set_power_state(pdev, PCI_D0);
> +
> +	ret = pci_enable_device(pdev);
> +	if (ret)
> +		return ret;
> +
> +	pci_set_master(pdev);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(stmmac_pci_plat_resume);
> +
> +MODULE_DESCRIPTION("STMMAC PCI helper library");
> +MODULE_AUTHOR("Yao Zi <ziyao@disroot.org>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_libpci.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_libpci.h
> new file mode 100644
> index 000000000000..71553184f982
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_libpci.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2025 Yao Zi <ziyao@disroot.org>
> + */
> +
> +#ifndef __STMMAC_LIBPCI_H__
> +#define __STMMAC_LIBPCI_H__
> +
> +int stmmac_pci_plat_suspend(struct device *dev, void *bsp_priv);
> +int stmmac_pci_plat_resume(struct device *dev, void *bsp_priv);
> +
> +#endif /* __STMMAC_LIBPCI_H__ */