From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
Software node created in probe function is not being cleaned up if
the probe function returns an error.
The stmmac core provides mechanism to handle this error condition
with plat->init, plat->exit helper functions.
Move glue driver's initialization code to plat->init function.
If the probe function returns an error, plat->exit function is
called. Handle any glue driver level cleanup in the plat->exit
handler.
Use devm_add_action_or_reset() to register a callback to free
irq vectors automatically, simplifying error handling in probe().
Suggested-by: Russell King (Oracle) <linux@armlinux.org.uk>
Signed-off-by: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
---
.../net/ethernet/stmicro/stmmac/dwmac-brcm.c | 164 ++++++++++--------
1 file changed, 90 insertions(+), 74 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-brcm.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-brcm.c
index 3a00452c0aa6..9103e9cf38f1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-brcm.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-brcm.c
@@ -218,6 +218,13 @@ static void brcm_config_misc_regs(struct pci_dev *pdev,
XGMAC_PCIE_MISC_MII_CTRL_LINK_UP);
}
+static void brcm_free_irq_vectors(void *data)
+{
+ struct pci_dev *pdev = data;
+
+ pci_free_irq_vectors(pdev);
+}
+
static int brcm_config_multi_msi(struct pci_dev *pdev,
struct plat_stmmacenet_data *plat,
struct stmmac_resources *res)
@@ -251,9 +258,87 @@ static int brcm_config_multi_msi(struct pci_dev *pdev,
plat->flags |= STMMAC_FLAG_MULTI_MSI_EN;
plat->flags |= STMMAC_FLAG_TSO_EN;
plat->flags |= STMMAC_FLAG_SPH_DISABLE;
+
+ return devm_add_action_or_reset(&pdev->dev,
+ brcm_free_irq_vectors, pdev);
+}
+
+static int brcm_drv_init(struct device *dev, void *bsp_priv)
+{
+ struct brcm_priv_data *brcm_priv = (struct brcm_priv_data *)bsp_priv;
+ struct pci_dev *pdev = to_pci_dev(dev);
+ int rx_offset;
+ int tx_offset;
+ int vector;
+ int ret;
+
+ /* This device is directly attached to the switch chip internal to the
+ * SoC using XGMII interface. Since no MDIO is present, register
+ * fixed-link software_node to create phylink.
+ */
+ ret = software_node_register_node_group(brcm_swnodes);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "failed to register software_node\n");
+ device_set_node(dev, software_node_fwnode(&parent_swnode));
+
+ pci_write_config_dword(pdev, XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_LOW,
+ XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_LO_VALUE);
+ pci_write_config_dword(pdev, XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_HIGH,
+ XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_HI_VALUE);
+
+ misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_LO_OFFSET,
+ XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_LO_VALUE);
+ misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_HI_OFFSET,
+ XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_HI_VALUE);
+
+ /* SBD Interrupt */
+ misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_SBD_ALL_OFFSET,
+ XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_SBD_ALL_VALUE);
+ /* EP_DOORBELL Interrupt */
+ misc_iowrite(brcm_priv,
+ XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST_DBELL_OFFSET,
+ XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST_DBELL_VALUE);
+ /* EP_H0 Interrupt */
+ misc_iowrite(brcm_priv,
+ XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST0_OFFSET,
+ XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST0_VALUE);
+ /* EP_H1 Interrupt */
+ misc_iowrite(brcm_priv,
+ XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST1_OFFSET,
+ XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST1_VALUE);
+
+ rx_offset = XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_RX0_PF0_OFFSET;
+ tx_offset = XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_TX0_PF0_OFFSET;
+ vector = BRCM_XGMAC_MSI_RX_VECTOR_START;
+ for (int i = 0; i < BRCM_MAX_DMA_CHANNEL_PAIRS; i++) {
+ /* RX Interrupt */
+ misc_iowrite(brcm_priv, rx_offset, vector++);
+ /* TX Interrupt */
+ misc_iowrite(brcm_priv, tx_offset, vector++);
+ rx_offset += 4;
+ tx_offset += 4;
+ }
+
+ /* Enable Switch Link */
+ misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MII_CTRL_OFFSET,
+ XGMAC_PCIE_MISC_MII_CTRL_PAUSE_RX |
+ XGMAC_PCIE_MISC_MII_CTRL_PAUSE_TX |
+ XGMAC_PCIE_MISC_MII_CTRL_LINK_UP);
+ /* Enable MSI-X */
+ misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_PCIESS_CTRL_OFFSET,
+ XGMAC_PCIE_MISC_PCIESS_CTRL_EN_MSI_MSIX);
return 0;
}
+static void brcm_drv_exit_cleanup(struct device *dev, void *bsp_priv)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+
+ device_set_node(&pdev->dev, NULL);
+ software_node_unregister_node_group(brcm_swnodes);
+}
+
static int brcm_pci_resume(struct device *dev, void *bsp_priv)
{
struct pci_dev *pdev = to_pci_dev(dev);
@@ -272,9 +357,6 @@ static int dwxgmac_brcm_pci_probe(struct pci_dev *pdev,
struct brcm_priv_data *brcm_priv;
struct stmmac_resources res;
struct device *dev;
- int rx_offset;
- int tx_offset;
- int vector;
int ret;
dev = &pdev->dev;
@@ -291,13 +373,6 @@ static int dwxgmac_brcm_pci_probe(struct pci_dev *pdev,
if (!plat->axi)
return -ENOMEM;
- /* This device is directly attached to the switch chip internal to the
- * SoC using XGMII interface. Since no MDIO is present, register
- * fixed-link software_node to create phylink.
- */
- software_node_register_node_group(brcm_swnodes);
- device_set_node(dev, software_node_fwnode(&parent_swnode));
-
/* Disable D3COLD as our device does not support it */
pci_d3cold_disable(pdev);
@@ -324,6 +399,8 @@ static int dwxgmac_brcm_pci_probe(struct pci_dev *pdev,
res.addr += BRCM_XGMAC_IOMEM_CFG_REG_OFFSET;
brcm_priv->xgmac_regs = res.addr;
+ plat->init = brcm_drv_init;
+ plat->exit = brcm_drv_exit_cleanup;
plat->suspend = stmmac_pci_plat_suspend;
plat->resume = brcm_pci_resume;
plat->bsp_priv = brcm_priv;
@@ -332,78 +409,17 @@ static int dwxgmac_brcm_pci_probe(struct pci_dev *pdev,
if (ret)
return ret;
- pci_write_config_dword(pdev, XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_LOW,
- XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_LO_VALUE);
- pci_write_config_dword(pdev, XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_HIGH,
- XGMAC_PCIE_CFG_MSIX_ADDR_MATCH_HI_VALUE);
-
- misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_LO_OFFSET,
- XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_LO_VALUE);
- misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_HI_OFFSET,
- XGMAC_PCIE_MISC_MSIX_ADDR_MATCH_HI_VALUE);
-
- /* SBD Interrupt */
- misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_SBD_ALL_OFFSET,
- XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_SBD_ALL_VALUE);
- /* EP_DOORBELL Interrupt */
- misc_iowrite(brcm_priv,
- XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST_DBELL_OFFSET,
- XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST_DBELL_VALUE);
- /* EP_H0 Interrupt */
- misc_iowrite(brcm_priv,
- XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST0_OFFSET,
- XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST0_VALUE);
- /* EP_H1 Interrupt */
- misc_iowrite(brcm_priv,
- XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST1_OFFSET,
- XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_EP2HOST1_VALUE);
-
- rx_offset = XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_RX0_PF0_OFFSET;
- tx_offset = XGMAC_PCIE_MISC_MSIX_VECTOR_MAP_TX0_PF0_OFFSET;
- vector = BRCM_XGMAC_MSI_RX_VECTOR_START;
- for (int i = 0; i < BRCM_MAX_DMA_CHANNEL_PAIRS; i++) {
- /* RX Interrupt */
- misc_iowrite(brcm_priv, rx_offset, vector++);
- /* TX Interrupt */
- misc_iowrite(brcm_priv, tx_offset, vector++);
- rx_offset += 4;
- tx_offset += 4;
- }
-
- /* Enable Switch Link */
- misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_MII_CTRL_OFFSET,
- XGMAC_PCIE_MISC_MII_CTRL_PAUSE_RX |
- XGMAC_PCIE_MISC_MII_CTRL_PAUSE_TX |
- XGMAC_PCIE_MISC_MII_CTRL_LINK_UP);
- /* Enable MSI-X */
- misc_iowrite(brcm_priv, XGMAC_PCIE_MISC_PCIESS_CTRL_OFFSET,
- XGMAC_PCIE_MISC_PCIESS_CTRL_EN_MSI_MSIX);
-
ret = brcm_config_multi_msi(pdev, plat, &res);
- if (ret) {
- dev_err(&pdev->dev,
- "%s: ERROR: failed to enable IRQ\n", __func__);
- goto err_disable_msi;
- }
-
- ret = stmmac_dvr_probe(&pdev->dev, plat, &res);
if (ret)
- goto err_disable_msi;
-
- return ret;
-
-err_disable_msi:
- pci_free_irq_vectors(pdev);
+ return dev_err_probe(&pdev->dev, ret,
+ "failed to configure IRQ\n");
- return ret;
+ return stmmac_dvr_probe(&pdev->dev, plat, &res);
}
static void dwxgmac_brcm_pci_remove(struct pci_dev *pdev)
{
stmmac_dvr_remove(&pdev->dev);
- pci_free_irq_vectors(pdev);
- device_set_node(&pdev->dev, NULL);
- software_node_unregister_node_group(brcm_swnodes);
}
static const struct pci_device_id dwxgmac_brcm_id_table[] = {
--
2.34.1On Fri, Mar 20, 2026 at 02:19:20PM -0700, Jitendra Vegiraju wrote: > From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com> > > Software node created in probe function is not being cleaned up if > the probe function returns an error. > The stmmac core provides mechanism to handle this error condition > with plat->init, plat->exit helper functions. > Move glue driver's initialization code to plat->init function. > If the probe function returns an error, plat->exit function is > called. Handle any glue driver level cleanup in the plat->exit > handler. > Use devm_add_action_or_reset() to register a callback to free > irq vectors automatically, simplifying error handling in probe(). > > Suggested-by: Russell King (Oracle) <linux@armlinux.org.uk> > Signed-off-by: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com> Oh, you did fix it. Please merge this into patch 4, there is no need to have this fix seperate. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Hi Russell On Thu, Mar 26, 2026 at 9:57 AM Russell King (Oracle) <linux@armlinux.org.uk> wrote: > > On Fri, Mar 20, 2026 at 02:19:20PM -0700, Jitendra Vegiraju wrote: > > From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com> > > > > Software node created in probe function is not being cleaned up if > > the probe function returns an error. > > The stmmac core provides mechanism to handle this error condition > > with plat->init, plat->exit helper functions. > > Move glue driver's initialization code to plat->init function. > > If the probe function returns an error, plat->exit function is > > called. Handle any glue driver level cleanup in the plat->exit > > handler. > > Use devm_add_action_or_reset() to register a callback to free > > irq vectors automatically, simplifying error handling in probe(). > > > > Suggested-by: Russell King (Oracle) <linux@armlinux.org.uk> > > Signed-off-by: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com> > > Oh, you did fix it. Please merge this into patch 4, there is no need > to have this fix seperate. Sure, I will merge this into patch 4. > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
© 2016 - 2026 Red Hat, Inc.