[PATCH v7 2/5] net: stmmac: platform: read channels irq

Jan Petrous via B4 Relay posted 5 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v7 2/5] net: stmmac: platform: read channels irq
Posted by Jan Petrous via B4 Relay 1 month, 1 week ago
From: "Jan Petrous (OSS)" <jan.petrous@oss.nxp.com>

Read IRQ resources for all rx/tx channels, to allow Multi-IRQ mode
for platform glue drivers.

Reviewed-by: Matthias Brugger <mbrugger@suse.com>
Signed-off-by: Jan Petrous (OSS) <jan.petrous@oss.nxp.com>
---
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  | 46 +++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 5c9fd91a1db9..93bd915ab6eb 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -697,9 +697,40 @@ struct clk *stmmac_pltfr_find_clk(struct plat_stmmacenet_data *plat_dat,
 }
 EXPORT_SYMBOL_GPL(stmmac_pltfr_find_clk);
 
+static int stmmac_pltfr_get_queue_irqs(struct platform_device *pdev,
+				       struct stmmac_resources *stmmac_res,
+				       bool tx)
+{
+	int *irqs = tx ? &stmmac_res->tx_irq[0] : &stmmac_res->rx_irq[0];
+	char name[16];
+	int i;
+
+	/* RX channels irq */
+	STMMAC_FOREACH_MTL_QUEUE(i, MTL_MAX_RX_QUEUES) {
+		scnprintf(name, sizeof(name), "%cx-queue-%d",
+			  tx ? 't' : 'r', i);
+
+		irqs[i] = platform_get_irq_byname_optional(pdev, name);
+		if (irqs[i] <= 0) {
+			if (irqs[i] == -EPROBE_DEFER)
+				return -EPROBE_DEFER;
+
+			dev_dbg(&pdev->dev, "IRQ %s not found\n", name);
+
+			/* Stop on first unset rx/tx-queue-%i property member */
+			irqs[i] = 0;
+			break;
+		}
+	}
+
+	return 0;
+}
+
 int stmmac_get_platform_resources(struct platform_device *pdev,
 				  struct stmmac_resources *stmmac_res)
 {
+	int ret;
+
 	memset(stmmac_res, 0, sizeof(*stmmac_res));
 
 	/* Get IRQ information early to have an ability to ask for deferred
@@ -735,7 +766,20 @@ int stmmac_get_platform_resources(struct platform_device *pdev,
 
 	stmmac_res->addr = devm_platform_ioremap_resource(pdev, 0);
 
-	return PTR_ERR_OR_ZERO(stmmac_res->addr);
+	if (IS_ERR(stmmac_res->addr))
+		return PTR_ERR(stmmac_res->addr);
+
+	/* TX channels irq */
+	ret = stmmac_pltfr_get_queue_irqs(pdev, stmmac_res, true);
+	if (ret)
+		return ret;
+
+	/* RX channels irq */
+	ret = stmmac_pltfr_get_queue_irqs(pdev, stmmac_res, false);
+	if (ret)
+		return ret;
+
+	return 0;
 }
 EXPORT_SYMBOL_GPL(stmmac_get_platform_resources);
 

-- 
2.47.0
Re: [PATCH v7 2/5] net: stmmac: platform: read channels irq
Posted by Russell King (Oracle) 1 month, 1 week ago
On Thu, Feb 26, 2026 at 09:54:07AM +0100, Jan Petrous via B4 Relay wrote:
> From: "Jan Petrous (OSS)" <jan.petrous@oss.nxp.com>
> 
> Read IRQ resources for all rx/tx channels, to allow Multi-IRQ mode
> for platform glue drivers.
> 
> Reviewed-by: Matthias Brugger <mbrugger@suse.com>
> Signed-off-by: Jan Petrous (OSS) <jan.petrous@oss.nxp.com>
> ---
>  .../net/ethernet/stmicro/stmmac/stmmac_platform.c  | 46 +++++++++++++++++++++-
>  1 file changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 5c9fd91a1db9..93bd915ab6eb 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -697,9 +697,40 @@ struct clk *stmmac_pltfr_find_clk(struct plat_stmmacenet_data *plat_dat,
>  }
>  EXPORT_SYMBOL_GPL(stmmac_pltfr_find_clk);
>  
> +static int stmmac_pltfr_get_queue_irqs(struct platform_device *pdev,
> +				       struct stmmac_resources *stmmac_res,
> +				       bool tx)
> +{
> +	int *irqs = tx ? &stmmac_res->tx_irq[0] : &stmmac_res->rx_irq[0];
> +	char name[16];
> +	int i;
> +
> +	/* RX channels irq */
> +	STMMAC_FOREACH_MTL_QUEUE(i, MTL_MAX_RX_QUEUES) {

You've missed that there are two separate definitions for tx and rx
queues - while they are currently the same number, code shouldn't
make that assumption.

> +		scnprintf(name, sizeof(name), "%cx-queue-%d",
> +			  tx ? 't' : 'r', i);

I'm not happy with this method of combining the two loops.

Maybe instead:

static int stmmac_pltfr_get_irq_array(struct platform_device *pdev,
				      const char *fmt, int *irqs,
				      size_t num)
{
	char name[16];
	size_t i;

	for (i = 0; i < num; i++) {
		if (snprintf(name, sizeof(name), fmt, i) >= sizeof(name))
			return -EINVAL;

		irqs[i] = platform_get_irq_byname_optional(pdev, name);
		if (irqs[i] == -EPROBE_DEFER) {
			return irqs[i];
		} else if (irqs[i] <= 0) {
			dev_dbg(&pdev->dev, "IRQ %s not found\n", name);

			irqs[i] = 0;
			break;
		}
	}

	return 0;
}

which has the advantage that it becomes a generic helper for getting an
any array of IRQs.

>  int stmmac_get_platform_resources(struct platform_device *pdev,
>  				  struct stmmac_resources *stmmac_res)
>  {
> +	int ret;
> +
>  	memset(stmmac_res, 0, sizeof(*stmmac_res));
>  
>  	/* Get IRQ information early to have an ability to ask for deferred
> @@ -735,7 +766,20 @@ int stmmac_get_platform_resources(struct platform_device *pdev,
>  
>  	stmmac_res->addr = devm_platform_ioremap_resource(pdev, 0);
>  
> -	return PTR_ERR_OR_ZERO(stmmac_res->addr);
> +	if (IS_ERR(stmmac_res->addr))
> +		return PTR_ERR(stmmac_res->addr);
> +
> +	/* TX channels irq */
> +	ret = stmmac_pltfr_get_queue_irqs(pdev, stmmac_res, true);
> +	if (ret)
> +		return ret;
> +
> +	/* RX channels irq */
> +	ret = stmmac_pltfr_get_queue_irqs(pdev, stmmac_res, false);
> +	if (ret)
> +		return ret;

These then become:

	ret = stmmac_pltfr_get_irq_array(pdev, "tx-queue-%d", 
					 stmmac_res->tx_irq,
					 MTL_MAX_TX_QUEUES);
	if (ret)
		return ret;

	ret = stmmac_pltfr_get_irq_array(pdev, "rx-queue-%d", 
					 stmmac_res->rx_irq,
					 MTL_MAX_RX_QUEUES);
	if (ret)
		return ret;

This has the advantage that one can grep for rx-queue to find it,
and we also use the correct limit for each queue type.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v7 2/5] net: stmmac: platform: read channels irq
Posted by Jan Petrous 1 month, 1 week ago
On Thu, Feb 26, 2026 at 07:10:22PM +0000, Russell King (Oracle) wrote:
> On Thu, Feb 26, 2026 at 09:54:07AM +0100, Jan Petrous via B4 Relay wrote:
> > From: "Jan Petrous (OSS)" <jan.petrous@oss.nxp.com>
> > 
> > Read IRQ resources for all rx/tx channels, to allow Multi-IRQ mode
> > for platform glue drivers.
> > 
> > Reviewed-by: Matthias Brugger <mbrugger@suse.com>
> > Signed-off-by: Jan Petrous (OSS) <jan.petrous@oss.nxp.com>
> > ---
> >  .../net/ethernet/stmicro/stmmac/stmmac_platform.c  | 46 +++++++++++++++++++++-
> >  1 file changed, 45 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > index 5c9fd91a1db9..93bd915ab6eb 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > @@ -697,9 +697,40 @@ struct clk *stmmac_pltfr_find_clk(struct plat_stmmacenet_data *plat_dat,
> >  }
> >  EXPORT_SYMBOL_GPL(stmmac_pltfr_find_clk);
> >  
> > +static int stmmac_pltfr_get_queue_irqs(struct platform_device *pdev,
> > +				       struct stmmac_resources *stmmac_res,
> > +				       bool tx)
> > +{
> > +	int *irqs = tx ? &stmmac_res->tx_irq[0] : &stmmac_res->rx_irq[0];
> > +	char name[16];
> > +	int i;
> > +
> > +	/* RX channels irq */
> > +	STMMAC_FOREACH_MTL_QUEUE(i, MTL_MAX_RX_QUEUES) {
> 
> You've missed that there are two separate definitions for tx and rx
> queues - while they are currently the same number, code shouldn't
> make that assumption.

Oh, yes. My fault. Next time I shall review my code better.

> 
> > +		scnprintf(name, sizeof(name), "%cx-queue-%d",
> > +			  tx ? 't' : 'r', i);
> 
> I'm not happy with this method of combining the two loops.
> 
> Maybe instead:
> 
> static int stmmac_pltfr_get_irq_array(struct platform_device *pdev,
> 				      const char *fmt, int *irqs,
> 				      size_t num)
> {
> 	char name[16];
> 	size_t i;
> 
> 	for (i = 0; i < num; i++) {
> 		if (snprintf(name, sizeof(name), fmt, i) >= sizeof(name))
> 			return -EINVAL;
> 
> 		irqs[i] = platform_get_irq_byname_optional(pdev, name);
> 		if (irqs[i] == -EPROBE_DEFER) {
> 			return irqs[i];
> 		} else if (irqs[i] <= 0) {
> 			dev_dbg(&pdev->dev, "IRQ %s not found\n", name);
> 
> 			irqs[i] = 0;
> 			break;
> 		}
> 	}
> 
> 	return 0;
> }
> 
> which has the advantage that it becomes a generic helper for getting an
> any array of IRQs.
> 
> >  int stmmac_get_platform_resources(struct platform_device *pdev,
> >  				  struct stmmac_resources *stmmac_res)
> >  {
> > +	int ret;
> > +
> >  	memset(stmmac_res, 0, sizeof(*stmmac_res));
> >  
> >  	/* Get IRQ information early to have an ability to ask for deferred
> > @@ -735,7 +766,20 @@ int stmmac_get_platform_resources(struct platform_device *pdev,
> >  
> >  	stmmac_res->addr = devm_platform_ioremap_resource(pdev, 0);
> >  
> > -	return PTR_ERR_OR_ZERO(stmmac_res->addr);
> > +	if (IS_ERR(stmmac_res->addr))
> > +		return PTR_ERR(stmmac_res->addr);
> > +
> > +	/* TX channels irq */
> > +	ret = stmmac_pltfr_get_queue_irqs(pdev, stmmac_res, true);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* RX channels irq */
> > +	ret = stmmac_pltfr_get_queue_irqs(pdev, stmmac_res, false);
> > +	if (ret)
> > +		return ret;
> 
> These then become:
> 
> 	ret = stmmac_pltfr_get_irq_array(pdev, "tx-queue-%d", 
> 					 stmmac_res->tx_irq,
> 					 MTL_MAX_TX_QUEUES);
> 	if (ret)
> 		return ret;
> 
> 	ret = stmmac_pltfr_get_irq_array(pdev, "rx-queue-%d", 
> 					 stmmac_res->rx_irq,
> 					 MTL_MAX_RX_QUEUES);
> 	if (ret)
> 		return ret;
> 
> This has the advantage that one can grep for rx-queue to find it,
> and we also use the correct limit for each queue type.
> 

Agree, your code looks better. Applied in v8 :) Thanks.

BTW, I would prefer to use sizeof() instead of constant for array size,
but this is the style used in stmmac, so I reuse the same approach.

BR.
/Jan