[PATCH 0/8] net: Use platform_get_irq*() variants to fetch IRQ's

[PATCH 0/8] net: Use platform_get_irq*() variants to fetch IRQ's

[Lad Prabhakar]

Hi All,

This patch series aims to drop using platform_get_resource() for IRQ types
in preparation for removal of static setup of IRQ resource from DT core
code.

Dropping usage of platform_get_resource() was agreed based on
the discussion [0].

[0] https://patchwork.kernel.org/project/linux-renesas-soc/
patch/20211209001056.29774-1-prabhakar.mahadev-lad.rj@bp.renesas.com/

Cheers,
Prabhakar

Lad Prabhakar (8):
  ethernet: netsec: Use platform_get_irq() to get the interrupt
  net: pxa168_eth: Use platform_get_irq() to get the interrupt
  fsl/fman: Use platform_get_irq() to get the interrupt
  net: ethoc: Use platform_get_irq() to get the interrupt
  net: xilinx: emaclite: Use platform_get_irq() to get the interrupt
  wcn36xx: Use platform_get_irq_byname() to get the interrupt
  ath10k: Use platform_get_irq() to get the interrupt
  net: ethernet: ti: davinci_emac: Use platform_get_irq() to get the
    interrupt

 drivers/net/ethernet/ethoc.c                  |  9 +--
 drivers/net/ethernet/freescale/fman/fman.c    | 32 ++++-----
 drivers/net/ethernet/marvell/pxa168_eth.c     |  9 +--
 drivers/net/ethernet/socionext/netsec.c       | 13 ++--
 drivers/net/ethernet/ti/davinci_emac.c        | 69 +++++++++++--------
 drivers/net/ethernet/xilinx/xilinx_emaclite.c |  9 +--
 drivers/net/wireless/ath/ath10k/snoc.c        | 15 ++--
 drivers/net/wireless/ath/wcn36xx/main.c       | 21 +++---
 8 files changed, 88 insertions(+), 89 deletions(-)

-- 
2.17.1

[PATCH 1/8] ethernet: netsec: Use platform_get_irq() to get the interrupt

[Lad Prabhakar]

platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static
allocation of IRQ resources in DT core code, this causes an issue
when using hierarchical interrupt domains using "interrupts" property
in the node as this bypasses the hierarchical setup and messes up the
irq chaining.

In preparation for removal of static setup of IRQ resource from DT core
code use platform_get_irq().

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/net/ethernet/socionext/netsec.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
index 25dcd8eda5fc..556bd353dd42 100644
--- a/drivers/net/ethernet/socionext/netsec.c
+++ b/drivers/net/ethernet/socionext/netsec.c
@@ -1977,11 +1977,12 @@ static int netsec_register_mdio(struct netsec_priv *priv, u32 phy_addr)
 
 static int netsec_probe(struct platform_device *pdev)
 {
-	struct resource *mmio_res, *eeprom_res, *irq_res;
+	struct resource *mmio_res, *eeprom_res;
 	struct netsec_priv *priv;
 	u32 hw_ver, phy_addr = 0;
 	struct net_device *ndev;
 	int ret;
+	int irq;
 
 	mmio_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!mmio_res) {
@@ -1995,11 +1996,9 @@ static int netsec_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (!irq_res) {
-		dev_err(&pdev->dev, "No IRQ resource found.\n");
-		return -ENODEV;
-	}
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
 
 	ndev = alloc_etherdev(sizeof(*priv));
 	if (!ndev)
@@ -2010,7 +2009,7 @@ static int netsec_probe(struct platform_device *pdev)
 	spin_lock_init(&priv->reglock);
 	SET_NETDEV_DEV(ndev, &pdev->dev);
 	platform_set_drvdata(pdev, priv);
-	ndev->irq = irq_res->start;
+	ndev->irq = irq;
 	priv->dev = &pdev->dev;
 	priv->ndev = ndev;
 
-- 
2.17.1

[PATCH 2/8] net: pxa168_eth: Use platform_get_irq() to get the interrupt

[Lad Prabhakar]

platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static
allocation of IRQ resources in DT core code, this causes an issue
when using hierarchical interrupt domains using "interrupts" property
in the node as this bypasses the hierarchical setup and messes up the
irq chaining.

In preparation for removal of static setup of IRQ resource from DT core
code use platform_get_irq().

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/net/ethernet/marvell/pxa168_eth.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
index 1d607bc6b59e..52bef50f5a0d 100644
--- a/drivers/net/ethernet/marvell/pxa168_eth.c
+++ b/drivers/net/ethernet/marvell/pxa168_eth.c
@@ -1388,7 +1388,6 @@ static int pxa168_eth_probe(struct platform_device *pdev)
 {
 	struct pxa168_eth_private *pep = NULL;
 	struct net_device *dev = NULL;
-	struct resource *res;
 	struct clk *clk;
 	struct device_node *np;
 	int err;
@@ -1419,9 +1418,11 @@ static int pxa168_eth_probe(struct platform_device *pdev)
 		goto err_netdev;
 	}
 
-	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	BUG_ON(!res);
-	dev->irq = res->start;
+	err = platform_get_irq(pdev, 0);
+	if (err == -EPROBE_DEFER)
+		goto err_netdev;
+	BUG_ON(dev->irq < 0);
+	dev->irq = err;
 	dev->netdev_ops = &pxa168_eth_netdev_ops;
 	dev->watchdog_timeo = 2 * HZ;
 	dev->base_addr = 0;
-- 
2.17.1

[PATCH 3/8] fsl/fman: Use platform_get_irq() to get the interrupt

[Lad Prabhakar]

platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static
allocation of IRQ resources in DT core code, this causes an issue
when using hierarchical interrupt domains using "interrupts" property
in the node as this bypasses the hierarchical setup and messes up the
irq chaining.

In preparation for removal of static setup of IRQ resource from DT core
code use platform_get_irq(). While doing so return error pointer
from read_dts_node() as platform_get_irq() may return -EPROBE_DEFER.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/net/ethernet/freescale/fman/fman.c | 32 +++++++++++-----------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fman/fman.c b/drivers/net/ethernet/freescale/fman/fman.c
index ce0a121580f6..8f0db61cb1f6 100644
--- a/drivers/net/ethernet/freescale/fman/fman.c
+++ b/drivers/net/ethernet/freescale/fman/fman.c
@@ -2727,7 +2727,7 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
 
 	fman = kzalloc(sizeof(*fman), GFP_KERNEL);
 	if (!fman)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	fm_node = of_node_get(of_dev->dev.of_node);
 
@@ -2740,26 +2740,21 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
 	fman->dts_params.id = (u8)val;
 
 	/* Get the FM interrupt */
-	res = platform_get_resource(of_dev, IORESOURCE_IRQ, 0);
-	if (!res) {
-		dev_err(&of_dev->dev, "%s: Can't get FMan IRQ resource\n",
-			__func__);
+	err = platform_get_irq(of_dev, 0);
+	if (err < 0)
 		goto fman_node_put;
-	}
-	irq = res->start;
+	irq = err;
 
 	/* Get the FM error interrupt */
-	res = platform_get_resource(of_dev, IORESOURCE_IRQ, 1);
-	if (!res) {
-		dev_err(&of_dev->dev, "%s: Can't get FMan Error IRQ resource\n",
-			__func__);
+	err = platform_get_irq(of_dev, 1);
+	if (err < 0)
 		goto fman_node_put;
-	}
-	fman->dts_params.err_irq = res->start;
+	fman->dts_params.err_irq = err;
 
 	/* Get the FM address */
 	res = platform_get_resource(of_dev, IORESOURCE_MEM, 0);
 	if (!res) {
+		err = -EINVAL;
 		dev_err(&of_dev->dev, "%s: Can't get FMan memory resource\n",
 			__func__);
 		goto fman_node_put;
@@ -2770,6 +2765,7 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
 
 	clk = of_clk_get(fm_node, 0);
 	if (IS_ERR(clk)) {
+		err = PTR_ERR(clk);
 		dev_err(&of_dev->dev, "%s: Failed to get FM%d clock structure\n",
 			__func__, fman->dts_params.id);
 		goto fman_node_put;
@@ -2777,6 +2773,7 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
 
 	clk_rate = clk_get_rate(clk);
 	if (!clk_rate) {
+		err = -EINVAL;
 		dev_err(&of_dev->dev, "%s: Failed to determine FM%d clock rate\n",
 			__func__, fman->dts_params.id);
 		goto fman_node_put;
@@ -2797,6 +2794,7 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
 	/* Get the MURAM base address and size */
 	muram_node = of_find_matching_node(fm_node, fman_muram_match);
 	if (!muram_node) {
+		err = -EINVAL;
 		dev_err(&of_dev->dev, "%s: could not find MURAM node\n",
 			__func__);
 		goto fman_free;
@@ -2836,6 +2834,7 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
 		devm_request_mem_region(&of_dev->dev, phys_base_addr,
 					mem_size, "fman");
 	if (!fman->dts_params.res) {
+		err = -EBUSY;
 		dev_err(&of_dev->dev, "%s: request_mem_region() failed\n",
 			__func__);
 		goto fman_free;
@@ -2844,6 +2843,7 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
 	fman->dts_params.base_addr =
 		devm_ioremap(&of_dev->dev, phys_base_addr, mem_size);
 	if (!fman->dts_params.base_addr) {
+		err = -ENOMEM;
 		dev_err(&of_dev->dev, "%s: devm_ioremap() failed\n", __func__);
 		goto fman_free;
 	}
@@ -2868,7 +2868,7 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
 	of_node_put(fm_node);
 fman_free:
 	kfree(fman);
-	return NULL;
+	return ERR_PTR(err);
 }
 
 static int fman_probe(struct platform_device *of_dev)
@@ -2880,8 +2880,8 @@ static int fman_probe(struct platform_device *of_dev)
 	dev = &of_dev->dev;
 
 	fman = read_dts_node(of_dev);
-	if (!fman)
-		return -EIO;
+	if (IS_ERR(fman))
+		return PTR_ERR(fman);
 
 	err = fman_config(fman);
 	if (err) {
-- 
2.17.1

[PATCH 4/8] net: ethoc: Use platform_get_irq() to get the interrupt

[Lad Prabhakar]

platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static
allocation of IRQ resources in DT core code, this causes an issue
when using hierarchical interrupt domains using "interrupts" property
in the node as this bypasses the hierarchical setup and messes up the
irq chaining.

In preparation for removal of static setup of IRQ resource from DT core
code use platform_get_irq().

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/net/ethernet/ethoc.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/ethoc.c b/drivers/net/ethernet/ethoc.c
index d618a8b785b0..437c5acfe222 100644
--- a/drivers/net/ethernet/ethoc.c
+++ b/drivers/net/ethernet/ethoc.c
@@ -1078,14 +1078,11 @@ static int ethoc_probe(struct platform_device *pdev)
 
 
 	/* obtain device IRQ number */
-	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (!res) {
-		dev_err(&pdev->dev, "cannot obtain IRQ\n");
-		ret = -ENXIO;
+	ret = platform_get_irq(pdev, 0);
+	if (ret < 0)
 		goto free;
-	}
 
-	netdev->irq = res->start;
+	netdev->irq = ret;
 
 	/* setup driver-private data */
 	priv = netdev_priv(netdev);
-- 
2.17.1

[PATCH 5/8] net: xilinx: emaclite: Use platform_get_irq() to get the interrupt

[Lad Prabhakar]

platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static
allocation of IRQ resources in DT core code, this causes an issue
when using hierarchical interrupt domains using "interrupts" property
in the node as this bypasses the hierarchical setup and messes up the
irq chaining.

In preparation for removal of static setup of IRQ resource from DT core
code use platform_get_irq().

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/net/ethernet/xilinx/xilinx_emaclite.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 0815de581c7f..519599480b15 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -1133,14 +1133,11 @@ static int xemaclite_of_probe(struct platform_device *ofdev)
 	lp->ndev = ndev;
 
 	/* Get IRQ for the device */
-	res = platform_get_resource(ofdev, IORESOURCE_IRQ, 0);
-	if (!res) {
-		dev_err(dev, "no IRQ found\n");
-		rc = -ENXIO;
+	rc = platform_get_irq(ofdev, 0);
+	if (rc < 0)
 		goto error;
-	}
 
-	ndev->irq = res->start;
+	ndev->irq = rc;
 
 	res = platform_get_resource(ofdev, IORESOURCE_MEM, 0);
 	lp->base_addr = devm_ioremap_resource(&ofdev->dev, res);
-- 
2.17.1

[PATCH 6/8] wcn36xx: Use platform_get_irq_byname() to get the interrupt

[Lad Prabhakar]

platform_get_resource_byname(pdev, IORESOURCE_IRQ, ..) relies on static
allocation of IRQ resources in DT core code, this causes an issue
when using hierarchical interrupt domains using "interrupts" property
in the node as this bypasses the hierarchical setup and messes up the
irq chaining.

In preparation for removal of static setup of IRQ resource from DT core
code use platform_get_irq_byname().

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/net/wireless/ath/wcn36xx/main.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index 9575d7373bf2..bd334a302057 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -1446,25 +1446,20 @@ static int wcn36xx_platform_get_resources(struct wcn36xx *wcn,
 {
 	struct device_node *mmio_node;
 	struct device_node *iris_node;
-	struct resource *res;
 	int index;
 	int ret;
 
 	/* Set TX IRQ */
-	res = platform_get_resource_byname(pdev, IORESOURCE_IRQ, "tx");
-	if (!res) {
-		wcn36xx_err("failed to get tx_irq\n");
-		return -ENOENT;
-	}
-	wcn->tx_irq = res->start;
+	ret = platform_get_irq_byname(pdev, "tx");
+	if (ret < 0)
+		return ret;
+	wcn->tx_irq = ret;
 
 	/* Set RX IRQ */
-	res = platform_get_resource_byname(pdev, IORESOURCE_IRQ, "rx");
-	if (!res) {
-		wcn36xx_err("failed to get rx_irq\n");
-		return -ENOENT;
-	}
-	wcn->rx_irq = res->start;
+	ret = platform_get_irq_byname(pdev, "rx");
+	if (ret < 0)
+		return ret;
+	wcn->rx_irq = ret;
 
 	/* Acquire SMSM tx enable handle */
 	wcn->tx_enable_state = qcom_smem_state_get(&pdev->dev,
-- 
2.17.1

[PATCH 7/8] ath10k: Use platform_get_irq() to get the interrupt

[Lad Prabhakar]

platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static
allocation of IRQ resources in DT core code, this causes an issue
when using hierarchical interrupt domains using "interrupts" property
in the node as this bypasses the hierarchical setup and messes up the
irq chaining.

In preparation for removal of static setup of IRQ resource from DT core
code use platform_get_irq().

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/net/wireless/ath/ath10k/snoc.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c
index 9513ab696fff..681e1abe7440 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -1306,13 +1306,10 @@ static int ath10k_snoc_resource_init(struct ath10k *ar)
 	}
 
 	for (i = 0; i < CE_COUNT; i++) {
-		res = platform_get_resource(ar_snoc->dev, IORESOURCE_IRQ, i);
-		if (!res) {
-			ath10k_err(ar, "failed to get IRQ%d\n", i);
-			ret = -ENODEV;
-			goto out;
-		}
-		ar_snoc->ce_irqs[i].irq_line = res->start;
+		ret = platform_get_irq(ar_snoc->dev, i);
+		if (ret < 0)
+			return ret;
+		ar_snoc->ce_irqs[i].irq_line = ret;
 	}
 
 	ret = device_property_read_u32(&pdev->dev, "qcom,xo-cal-data",
@@ -1323,10 +1320,8 @@ static int ath10k_snoc_resource_init(struct ath10k *ar)
 		ath10k_dbg(ar, ATH10K_DBG_SNOC, "xo cal data %x\n",
 			   ar_snoc->xo_cal_data);
 	}
-	ret = 0;
 
-out:
-	return ret;
+	return 0;
 }
 
 static void ath10k_snoc_quirks_init(struct ath10k *ar)
-- 
2.17.1

[PATCH 8/8] net: ethernet: ti: davinci_emac: Use platform_get_irq() to get the interrupt

[Lad Prabhakar]

platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static
allocation of IRQ resources in DT core code, this causes an issue
when using hierarchical interrupt domains using "interrupts" property
in the node as this bypasses the hierarchical setup and messes up the
irq chaining.

In preparation for removal of static setup of IRQ resource from DT core
code use platform_get_irq() for DT users only.

While at it propagate error code in case request_irq() fails instead of
returning -EBUSY.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/net/ethernet/ti/davinci_emac.c | 69 ++++++++++++++++----------
 1 file changed, 42 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
index d55f06120ce7..31df3267a01a 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
@@ -1454,23 +1454,33 @@ static int emac_dev_open(struct net_device *ndev)
 	}
 
 	/* Request IRQ */
-	while ((res = platform_get_resource(priv->pdev, IORESOURCE_IRQ,
-					    res_num))) {
-		for (irq_num = res->start; irq_num <= res->end; irq_num++) {
-			if (request_irq(irq_num, emac_irq, 0, ndev->name,
-					ndev)) {
-				dev_err(emac_dev,
-					"DaVinci EMAC: request_irq() failed\n");
-				ret = -EBUSY;
+	if (dev_of_node(&priv->pdev->dev)) {
+		while ((ret = platform_get_irq_optional(priv->pdev, res_num)) != -ENXIO) {
+			if (ret < 0)
+				goto rollback;
 
+			ret = request_irq(ret, emac_irq, 0, ndev->name, ndev);
+			if (ret) {
+				dev_err(emac_dev, "DaVinci EMAC: request_irq() failed\n");
 				goto rollback;
 			}
+			res_num++;
 		}
-		res_num++;
+	} else {
+		while ((res = platform_get_resource(priv->pdev, IORESOURCE_IRQ, res_num))) {
+			for (irq_num = res->start; irq_num <= res->end; irq_num++) {
+				ret = request_irq(irq_num, emac_irq, 0, ndev->name, ndev);
+				if (ret) {
+					dev_err(emac_dev, "DaVinci EMAC: request_irq() failed\n");
+					goto rollback;
+				}
+			}
+			res_num++;
+		}
+		/* prepare counters for rollback in case of an error */
+		res_num--;
+		irq_num--;
 	}
-	/* prepare counters for rollback in case of an error */
-	res_num--;
-	irq_num--;
 
 	/* Start/Enable EMAC hardware */
 	emac_hw_enable(priv);
@@ -1554,16 +1564,24 @@ static int emac_dev_open(struct net_device *ndev)
 	napi_disable(&priv->napi);
 
 rollback:
-	for (q = res_num; q >= 0; q--) {
-		res = platform_get_resource(priv->pdev, IORESOURCE_IRQ, q);
-		/* at the first iteration, irq_num is already set to the
-		 * right value
-		 */
-		if (q != res_num)
-			irq_num = res->end;
+	if (dev_of_node(&priv->pdev->dev)) {
+		for (q = res_num - 1; q >= 0; q--) {
+			irq_num = platform_get_irq(priv->pdev, q);
+			if (irq_num > 0)
+				free_irq(irq_num, ndev);
+		}
+	} else {
+		for (q = res_num; q >= 0; q--) {
+			res = platform_get_resource(priv->pdev, IORESOURCE_IRQ, q);
+			/* at the first iteration, irq_num is already set to the
+			 * right value
+			 */
+			if (q != res_num)
+				irq_num = res->end;
 
-		for (m = irq_num; m >= res->start; m--)
-			free_irq(m, ndev);
+			for (m = irq_num; m >= res->start; m--)
+				free_irq(m, ndev);
+		}
 	}
 	cpdma_ctlr_stop(priv->dma);
 	pm_runtime_put(&priv->pdev->dev);
@@ -1899,13 +1917,10 @@ static int davinci_emac_probe(struct platform_device *pdev)
 		goto err_free_txchan;
 	}
 
-	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (!res) {
-		dev_err(&pdev->dev, "error getting irq res\n");
-		rc = -ENOENT;
+	rc = platform_get_irq(pdev, 0);
+	if (rc < 0)
 		goto err_free_rxchan;
-	}
-	ndev->irq = res->start;
+	ndev->irq = rc;
 
 	rc = davinci_emac_try_get_mac(pdev, res_ctrl ? 0 : 1, priv->mac_addr);
 	if (!rc)
-- 
2.17.1

Re: [PATCH 2/8] net: pxa168_eth: Use platform_get_irq() to get the interrupt

[Lad, Prabhakar]

Hi Andy,

Thank you for the review.

On Sat, Dec 25, 2021 at 11:24 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
>
>
> On Friday, December 24, 2021, Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
>>
>> platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static
>> allocation of IRQ resources in DT core code, this causes an issue
>> when using hierarchical interrupt domains using "interrupts" property
>> in the node as this bypasses the hierarchical setup and messes up the
>> irq chaining.
>>
>> In preparation for removal of static setup of IRQ resource from DT core
>> code use platform_get_irq().
>>
>> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>> ---
>>  drivers/net/ethernet/marvell/pxa168_eth.c | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
>> index 1d607bc6b59e..52bef50f5a0d 100644
>> --- a/drivers/net/ethernet/marvell/pxa168_eth.c
>> +++ b/drivers/net/ethernet/marvell/pxa168_eth.c
>> @@ -1388,7 +1388,6 @@ static int pxa168_eth_probe(struct platform_device *pdev)
>>  {
>>         struct pxa168_eth_private *pep = NULL;
>>         struct net_device *dev = NULL;
>> -       struct resource *res;
>>         struct clk *clk;
>>         struct device_node *np;
>>         int err;
>> @@ -1419,9 +1418,11 @@ static int pxa168_eth_probe(struct platform_device *pdev)
>>                 goto err_netdev;
>>         }
>>
>> -       res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>> -       BUG_ON(!res);
>> -       dev->irq = res->start;
>> +       err = platform_get_irq(pdev, 0);
>> +       if (err == -EPROBE_DEFER)
>
>
>  What about other errors?
>
Ouch I missed it...
>
>>
>> +               goto err_netdev;
>> +       BUG_ON(dev->irq < 0);
>
>
> ??? What is this and how it supposed to work?
>
.. should have been BUG_ON(dev->irq < 0);

Cheers,
Prabhakar
>>
>> +       dev->irq = err;
>>         dev->netdev_ops = &pxa168_eth_netdev_ops;
>>         dev->watchdog_timeo = 2 * HZ;
>>         dev->base_addr = 0;
>> --
>> 2.17.1
>>
>
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Re: [PATCH 2/8] net: pxa168_eth: Use platform_get_irq() to get the interrupt

[Andrew Lunn]

> >> +               goto err_netdev;
> >> +       BUG_ON(dev->irq < 0);
> >
> >
> > ??? What is this and how it supposed to work?
> >
> .. should have been BUG_ON(dev->irq < 0);

Is this fatal to the machine as a whole, now way to recover, all that
can be done is to limit the damage while it explodes?

If not, please use WARN_ON(), not BUG_ON(). There is an email from
Linus about this, not using BUG_ON() in general.

   Andrew

Re: [PATCH 8/8] net: ethernet: ti: davinci_emac: Use platform_get_irq() to get the interrupt

[Andy Shevchenko]

On Sat, Dec 25, 2021 at 4:06 AM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
>
> platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static
> allocation of IRQ resources in DT core code, this causes an issue
> when using hierarchical interrupt domains using "interrupts" property
> in the node as this bypasses the hierarchical setup and messes up the
> irq chaining.
>
> In preparation for removal of static setup of IRQ resource from DT core
> code use platform_get_irq() for DT users only.
>
> While at it propagate error code in case request_irq() fails instead of
> returning -EBUSY.


> +       if (dev_of_node(&priv->pdev->dev)) {

Why?! What's wrong with using the same approach in all cases?

> +               while ((ret = platform_get_irq_optional(priv->pdev, res_num)) != -ENXIO) {

This is wrong.

You need to check better as I pointed several times against your patches.

> +                       if (ret < 0)
> +                               goto rollback;
>
> +                       ret = request_irq(ret, emac_irq, 0, ndev->name, ndev);
> +                       if (ret) {
> +                               dev_err(emac_dev, "DaVinci EMAC: request_irq() failed\n");
>                                 goto rollback;
>                         }
> +                       res_num++;
>                 }
> -               res_num++;
> +       } else {
> +               while ((res = platform_get_resource(priv->pdev, IORESOURCE_IRQ, res_num))) {
> +                       for (irq_num = res->start; irq_num <= res->end; irq_num++) {
> +                               ret = request_irq(irq_num, emac_irq, 0, ndev->name, ndev);
> +                               if (ret) {
> +                                       dev_err(emac_dev, "DaVinci EMAC: request_irq() failed\n");
> +                                       goto rollback;
> +                               }
> +                       }
> +                       res_num++;
> +               }
> +               /* prepare counters for rollback in case of an error */
> +               res_num--;
> +               irq_num--;
>         }

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 8/8] net: ethernet: ti: davinci_emac: Use platform_get_irq() to get the interrupt

[Lad, Prabhakar]

Hi Andy,

Thank you for the review.

On Sat, Dec 25, 2021 at 4:59 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Sat, Dec 25, 2021 at 4:06 AM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> >
> > platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static
> > allocation of IRQ resources in DT core code, this causes an issue
> > when using hierarchical interrupt domains using "interrupts" property
> > in the node as this bypasses the hierarchical setup and messes up the
> > irq chaining.
> >
> > In preparation for removal of static setup of IRQ resource from DT core
> > code use platform_get_irq() for DT users only.
> >
> > While at it propagate error code in case request_irq() fails instead of
> > returning -EBUSY.
>
>
> > +       if (dev_of_node(&priv->pdev->dev)) {
>
> Why?! What's wrong with using the same approach in all cases?
>
The earlier code looped the resource from start to end and then
requested the resources. So I believed the the IORESOURCE_IRQ was
sending the range of interrupts. But now looking at the mach-davinci
folder I can confirm its just send an single IRQ resource. So this
check can be dropped and same approach can be used for OF and non OF
users.

> > +               while ((ret = platform_get_irq_optional(priv->pdev, res_num)) != -ENXIO) {
>
> This is wrong.
>
> You need to check better as I pointed several times against your patches.
>
Right, Ill have a look.

Cheers,
Prabhakar
> > +                       if (ret < 0)
> > +                               goto rollback;
> >
> > +                       ret = request_irq(ret, emac_irq, 0, ndev->name, ndev);
> > +                       if (ret) {
> > +                               dev_err(emac_dev, "DaVinci EMAC: request_irq() failed\n");
> >                                 goto rollback;
> >                         }
> > +                       res_num++;
> >                 }
> > -               res_num++;
> > +       } else {
> > +               while ((res = platform_get_resource(priv->pdev, IORESOURCE_IRQ, res_num))) {
> > +                       for (irq_num = res->start; irq_num <= res->end; irq_num++) {
> > +                               ret = request_irq(irq_num, emac_irq, 0, ndev->name, ndev);
> > +                               if (ret) {
> > +                                       dev_err(emac_dev, "DaVinci EMAC: request_irq() failed\n");
> > +                                       goto rollback;
> > +                               }
> > +                       }
> > +                       res_num++;
> > +               }
> > +               /* prepare counters for rollback in case of an error */
> > +               res_num--;
> > +               irq_num--;
> >         }
>
> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH 2/8] net: pxa168_eth: Use platform_get_irq() to get the interrupt

[Heiner Kallweit]

On 25.12.2021 13:19, Lad, Prabhakar wrote:
> Hi Andy,
> 
> Thank you for the review.
> 
> On Sat, Dec 25, 2021 at 11:24 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>>
>>
>>
>> On Friday, December 24, 2021, Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
>>>
>>> platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static
>>> allocation of IRQ resources in DT core code, this causes an issue
>>> when using hierarchical interrupt domains using "interrupts" property
>>> in the node as this bypasses the hierarchical setup and messes up the
>>> irq chaining.
>>>
>>> In preparation for removal of static setup of IRQ resource from DT core
>>> code use platform_get_irq().
>>>
>>> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>> ---
>>>  drivers/net/ethernet/marvell/pxa168_eth.c | 9 +++++----
>>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
>>> index 1d607bc6b59e..52bef50f5a0d 100644
>>> --- a/drivers/net/ethernet/marvell/pxa168_eth.c
>>> +++ b/drivers/net/ethernet/marvell/pxa168_eth.c
>>> @@ -1388,7 +1388,6 @@ static int pxa168_eth_probe(struct platform_device *pdev)
>>>  {
>>>         struct pxa168_eth_private *pep = NULL;
>>>         struct net_device *dev = NULL;
>>> -       struct resource *res;
>>>         struct clk *clk;
>>>         struct device_node *np;
>>>         int err;
>>> @@ -1419,9 +1418,11 @@ static int pxa168_eth_probe(struct platform_device *pdev)
>>>                 goto err_netdev;
>>>         }
>>>
>>> -       res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>>> -       BUG_ON(!res);
>>> -       dev->irq = res->start;
>>> +       err = platform_get_irq(pdev, 0);
>>> +       if (err == -EPROBE_DEFER)
>>
>>
>>  What about other errors?
>>
> Ouch I missed it...
>>
>>>
>>> +               goto err_netdev;
>>> +       BUG_ON(dev->irq < 0);
>>
>>
>> ??? What is this and how it supposed to work?
>>
> 
.. should have been BUG_ON(dev->irq < 0);

Usage of BUG_ON() is discouraged. Better handle the error w/o stopping
the whole system.

> 
> Cheers,
> Prabhakar
>>>
>>> +       dev->irq = err;
>>>         dev->netdev_ops = &pxa168_eth_netdev_ops;
>>>         dev->watchdog_timeo = 2 * HZ;
>>>         dev->base_addr = 0;
>>> --
>>> 2.17.1
>>>
>>
>>
>> --
>> With Best Regards,
>> Andy Shevchenko
>>
>>

Re: [PATCH 2/8] net: pxa168_eth: Use platform_get_irq() to get the interrupt

[Lad, Prabhakar]

Hi Heiner,

Thank you for the review.

On Sun, Dec 26, 2021 at 11:15 AM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> On 25.12.2021 13:19, Lad, Prabhakar wrote:
> > Hi Andy,
> >
> > Thank you for the review.
> >
> > On Sat, Dec 25, 2021 at 11:24 AM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> >>
> >>
> >>
> >> On Friday, December 24, 2021, Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> >>>
> >>> platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static
> >>> allocation of IRQ resources in DT core code, this causes an issue
> >>> when using hierarchical interrupt domains using "interrupts" property
> >>> in the node as this bypasses the hierarchical setup and messes up the
> >>> irq chaining.
> >>>
> >>> In preparation for removal of static setup of IRQ resource from DT core
> >>> code use platform_get_irq().
> >>>
> >>> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>> ---
> >>>  drivers/net/ethernet/marvell/pxa168_eth.c | 9 +++++----
> >>>  1 file changed, 5 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
> >>> index 1d607bc6b59e..52bef50f5a0d 100644
> >>> --- a/drivers/net/ethernet/marvell/pxa168_eth.c
> >>> +++ b/drivers/net/ethernet/marvell/pxa168_eth.c
> >>> @@ -1388,7 +1388,6 @@ static int pxa168_eth_probe(struct platform_device *pdev)
> >>>  {
> >>>         struct pxa168_eth_private *pep = NULL;
> >>>         struct net_device *dev = NULL;
> >>> -       struct resource *res;
> >>>         struct clk *clk;
> >>>         struct device_node *np;
> >>>         int err;
> >>> @@ -1419,9 +1418,11 @@ static int pxa168_eth_probe(struct platform_device *pdev)
> >>>                 goto err_netdev;
> >>>         }
> >>>
> >>> -       res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> >>> -       BUG_ON(!res);
> >>> -       dev->irq = res->start;
> >>> +       err = platform_get_irq(pdev, 0);
> >>> +       if (err == -EPROBE_DEFER)
> >>
> >>
> >>  What about other errors?
> >>
> > Ouch I missed it...
> >>
> >>>
> >>> +               goto err_netdev;
> >>> +       BUG_ON(dev->irq < 0);
> >>
> >>
> >> ??? What is this and how it supposed to work?
> >>
> >
> .. should have been BUG_ON(dev->irq < 0);
>
> Usage of BUG_ON() is discouraged. Better handle the error w/o stopping
> the whole system.
>
Agreed, if everyone is OK i'll create a separate patch for removal of
BUG_ON() and the later patch for using platform_get_irq().

Cheers,
Prabhakar

Re: [PATCH 7/8] ath10k: Use platform_get_irq() to get the interrupt

[Kalle Valo]

Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:

> platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static
> allocation of IRQ resources in DT core code, this causes an issue
> when using hierarchical interrupt domains using "interrupts" property
> in the node as this bypasses the hierarchical setup and messes up the
> irq chaining.
> 
> In preparation for removal of static setup of IRQ resource from DT core
> code use platform_get_irq().
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>

Patch applied to ath-next branch of ath.git, thanks.

f14c3f4db9cb ath10k: Use platform_get_irq() to get the interrupt

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20211224192626.15843-8-prabhakar.mahadev-lad.rj@bp.renesas.com/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH 6/8] wcn36xx: Use platform_get_irq_byname() to get the interrupt

[Kalle Valo]

Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:

> platform_get_resource_byname(pdev, IORESOURCE_IRQ, ..) relies on static
> allocation of IRQ resources in DT core code, this causes an issue
> when using hierarchical interrupt domains using "interrupts" property
> in the node as this bypasses the hierarchical setup and messes up the
> irq chaining.
> 
> In preparation for removal of static setup of IRQ resource from DT core
> code use platform_get_irq_byname().
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>

Patch applied to ath-next branch of ath.git, thanks.

d17efe4f80fc wcn36xx: Use platform_get_irq_byname() to get the interrupt

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20211224192626.15843-7-prabhakar.mahadev-lad.rj@bp.renesas.com/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches