1. Patchew
  2. linux
  3. [PATCH net-next 00/13] net: fec: cleanups, update quirk, update IRQ naming
  4. View patch

[PATCH net-next 07/13] net: fec: fec_probe(): update quirk: bring IRQs in correct order

Marc Kleine-Budde posted 13 patches 1 month, 1 week ago
[PATCH net-next 07/13] net: fec: fec_probe(): update quirk: bring IRQs in correct order
Posted by Marc Kleine-Budde 1 month, 1 week ago 
With i.MX8MQ and compatible SoCs, the order of the IRQs in the device
tree is not optimal. The driver expects the first three IRQs to match
their corresponding queue, while the last (fourth) IRQ is used for the
PPS:

- 1st IRQ: "int0": queue0 + other IRQs
- 2nd IRQ: "int1": queue1
- 3rd IRQ: "int2": queue2
- 4th IRQ: "pps": pps

However, the i.MX8MQ and compatible SoCs do not use the
"interrupt-names" property and specify the IRQs in the wrong order:

- 1st IRQ: queue1
- 2nd IRQ: queue2
- 3rd IRQ: queue0 + other IRQs
- 4th IRQ: pps

First rename the quirk from FEC_QUIRK_WAKEUP_FROM_INT2 to
FEC_QUIRK_INT2_IS_MAIN_IRQ, to better reflect it's functionality.

If the FEC_QUIRK_INT2_IS_MAIN_IRQ quirk is active, put the IRQs back
in the correct order, this is done in fec_probe().

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/ethernet/freescale/fec.h      | 24 ++++++++++++++++++++++--
 drivers/net/ethernet/freescale/fec_main.c | 18 +++++++++++-------
 2 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 63744a86752540fcede7fc4c29865b2529492526..b0f1a3e28d5c8052be3a8a0fa18303a1df2bb5bd 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -504,8 +504,28 @@ struct bufdesc_ex {
  */
 #define FEC_QUIRK_DELAYED_CLKS_SUPPORT	(1 << 21)
 
-/* i.MX8MQ SoC integration mix wakeup interrupt signal into "int2" interrupt line. */
-#define FEC_QUIRK_WAKEUP_FROM_INT2	(1 << 22)
+/* With i.MX8MQ and compatible SoCs, the order of the IRQs in the
+ * device tree is not optimal. The driver expects the first three IRQs
+ * to match their corresponding queue, while the last (fourth) IRQ is
+ * used for the PPS:
+ *
+ * - 1st IRQ: "int0": queue0 + other IRQs
+ * - 2nd IRQ: "int1": queue1
+ * - 3rd IRQ: "int2": queue2
+ * - 4th IRQ: "pps": pps
+ *
+ * However, the i.MX8MQ and compatible SoCs do not use the
+ * "interrupt-names" property and specify the IRQs in the wrong order:
+ *
+ * - 1st IRQ: queue1
+ * - 2nd IRQ: queue2
+ * - 3rd IRQ: queue0 + other IRQs
+ * - 4th IRQ: pps
+ *
+ * If the following quirk is active, put the IRQs back in the correct
+ * order, this is done in fec_probe().
+ */
+#define FEC_QUIRK_DT_IRQ2_IS_MAIN_IRQ	BIT(22)
 
 /* i.MX6Q adds pm_qos support */
 #define FEC_QUIRK_HAS_PMQOS			BIT(23)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index d948ed9810027d5fabe521dc3af2cf505dacd13e..f124ffe3619d82dc089f8494d33d2398e6f631fb 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -157,7 +157,7 @@ static const struct fec_devinfo fec_imx8mq_info = {
 		  FEC_QUIRK_ERR007885 | FEC_QUIRK_BUG_CAPTURE |
 		  FEC_QUIRK_HAS_RACC | FEC_QUIRK_HAS_COALESCE |
 		  FEC_QUIRK_CLEAR_SETUP_MII | FEC_QUIRK_HAS_MULTI_QUEUES |
-		  FEC_QUIRK_HAS_EEE | FEC_QUIRK_WAKEUP_FROM_INT2 |
+		  FEC_QUIRK_HAS_EEE | FEC_QUIRK_DT_IRQ2_IS_MAIN_IRQ |
 		  FEC_QUIRK_HAS_MDIO_C45,
 };
 
@@ -4260,10 +4260,7 @@ static void fec_enet_get_wakeup_irq(struct platform_device *pdev)
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct fec_enet_private *fep = netdev_priv(ndev);
 
-	if (fep->quirks & FEC_QUIRK_WAKEUP_FROM_INT2)
-		fep->wake_irq = fep->irq[2];
-	else
-		fep->wake_irq = fep->irq[0];
+	fep->wake_irq = fep->irq[0];
 }
 
 static int fec_enet_init_stop_mode(struct fec_enet_private *fep,
@@ -4495,10 +4492,17 @@ fec_probe(struct platform_device *pdev)
 		goto failed_init;
 
 	for (i = 0; i < irq_cnt; i++) {
-		snprintf(irq_name, sizeof(irq_name), "int%d", i);
+		int irq_num;
+
+		if (fep->quirks & FEC_QUIRK_DT_IRQ2_IS_MAIN_IRQ)
+			irq_num = (i + irq_cnt - 1) % irq_cnt;
+		else
+			irq_num = i;
+
+		snprintf(irq_name, sizeof(irq_name), "int%d", irq_num);
 		irq = platform_get_irq_byname_optional(pdev, irq_name);
 		if (irq < 0)
-			irq = platform_get_irq(pdev, i);
+			irq = platform_get_irq(pdev, irq_num);
 		if (irq < 0) {
 			ret = irq;
 			goto failed_irq;

-- 
2.45.2
Re: [PATCH net-next 07/13] net: fec: fec_probe(): update quirk: bring IRQs in correct order
Posted by Frank Li 1 month, 1 week ago 
On Wed, Oct 16, 2024 at 11:51:55PM +0200, Marc Kleine-Budde wrote:
> With i.MX8MQ and compatible SoCs, the order of the IRQs in the device
> tree is not optimal. The driver expects the first three IRQs to match
> their corresponding queue, while the last (fourth) IRQ is used for the
> PPS:
>
> - 1st IRQ: "int0": queue0 + other IRQs
> - 2nd IRQ: "int1": queue1
> - 3rd IRQ: "int2": queue2
> - 4th IRQ: "pps": pps
>
> However, the i.MX8MQ and compatible SoCs do not use the
> "interrupt-names" property and specify the IRQs in the wrong order:
>
> - 1st IRQ: queue1
> - 2nd IRQ: queue2
> - 3rd IRQ: queue0 + other IRQs
> - 4th IRQ: pps

why not fix dts?

Frank

>
> First rename the quirk from FEC_QUIRK_WAKEUP_FROM_INT2 to
> FEC_QUIRK_INT2_IS_MAIN_IRQ, to better reflect it's functionality.
>
> If the FEC_QUIRK_INT2_IS_MAIN_IRQ quirk is active, put the IRQs back
> in the correct order, this is done in fec_probe().
>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
>  drivers/net/ethernet/freescale/fec.h      | 24 ++++++++++++++++++++++--
>  drivers/net/ethernet/freescale/fec_main.c | 18 +++++++++++-------
>  2 files changed, 33 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
> index 63744a86752540fcede7fc4c29865b2529492526..b0f1a3e28d5c8052be3a8a0fa18303a1df2bb5bd 100644
> --- a/drivers/net/ethernet/freescale/fec.h
> +++ b/drivers/net/ethernet/freescale/fec.h
> @@ -504,8 +504,28 @@ struct bufdesc_ex {
>   */
>  #define FEC_QUIRK_DELAYED_CLKS_SUPPORT	(1 << 21)
>
> -/* i.MX8MQ SoC integration mix wakeup interrupt signal into "int2" interrupt line. */
> -#define FEC_QUIRK_WAKEUP_FROM_INT2	(1 << 22)
> +/* With i.MX8MQ and compatible SoCs, the order of the IRQs in the
> + * device tree is not optimal. The driver expects the first three IRQs
> + * to match their corresponding queue, while the last (fourth) IRQ is
> + * used for the PPS:
> + *
> + * - 1st IRQ: "int0": queue0 + other IRQs
> + * - 2nd IRQ: "int1": queue1
> + * - 3rd IRQ: "int2": queue2
> + * - 4th IRQ: "pps": pps
> + *
> + * However, the i.MX8MQ and compatible SoCs do not use the
> + * "interrupt-names" property and specify the IRQs in the wrong order:
> + *
> + * - 1st IRQ: queue1
> + * - 2nd IRQ: queue2
> + * - 3rd IRQ: queue0 + other IRQs
> + * - 4th IRQ: pps
> + *
> + * If the following quirk is active, put the IRQs back in the correct
> + * order, this is done in fec_probe().
> + */
> +#define FEC_QUIRK_DT_IRQ2_IS_MAIN_IRQ	BIT(22)
>
>  /* i.MX6Q adds pm_qos support */
>  #define FEC_QUIRK_HAS_PMQOS			BIT(23)
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index d948ed9810027d5fabe521dc3af2cf505dacd13e..f124ffe3619d82dc089f8494d33d2398e6f631fb 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -157,7 +157,7 @@ static const struct fec_devinfo fec_imx8mq_info = {
>  		  FEC_QUIRK_ERR007885 | FEC_QUIRK_BUG_CAPTURE |
>  		  FEC_QUIRK_HAS_RACC | FEC_QUIRK_HAS_COALESCE |
>  		  FEC_QUIRK_CLEAR_SETUP_MII | FEC_QUIRK_HAS_MULTI_QUEUES |
> -		  FEC_QUIRK_HAS_EEE | FEC_QUIRK_WAKEUP_FROM_INT2 |
> +		  FEC_QUIRK_HAS_EEE | FEC_QUIRK_DT_IRQ2_IS_MAIN_IRQ |
>  		  FEC_QUIRK_HAS_MDIO_C45,
>  };
>
> @@ -4260,10 +4260,7 @@ static void fec_enet_get_wakeup_irq(struct platform_device *pdev)
>  	struct net_device *ndev = platform_get_drvdata(pdev);
>  	struct fec_enet_private *fep = netdev_priv(ndev);
>
> -	if (fep->quirks & FEC_QUIRK_WAKEUP_FROM_INT2)
> -		fep->wake_irq = fep->irq[2];
> -	else
> -		fep->wake_irq = fep->irq[0];
> +	fep->wake_irq = fep->irq[0];
>  }
>
>  static int fec_enet_init_stop_mode(struct fec_enet_private *fep,
> @@ -4495,10 +4492,17 @@ fec_probe(struct platform_device *pdev)
>  		goto failed_init;
>
>  	for (i = 0; i < irq_cnt; i++) {
> -		snprintf(irq_name, sizeof(irq_name), "int%d", i);
> +		int irq_num;
> +
> +		if (fep->quirks & FEC_QUIRK_DT_IRQ2_IS_MAIN_IRQ)
> +			irq_num = (i + irq_cnt - 1) % irq_cnt;
> +		else
> +			irq_num = i;
> +
> +		snprintf(irq_name, sizeof(irq_name), "int%d", irq_num);
>  		irq = platform_get_irq_byname_optional(pdev, irq_name);
>  		if (irq < 0)
> -			irq = platform_get_irq(pdev, i);
> +			irq = platform_get_irq(pdev, irq_num);
>  		if (irq < 0) {
>  			ret = irq;
>  			goto failed_irq;
>
> --
> 2.45.2
>
>
Re: [PATCH net-next 07/13] net: fec: fec_probe(): update quirk: bring IRQs in correct order
Posted by Marc Kleine-Budde 1 month, 1 week ago 
On 16.10.2024 22:15:43, Frank Li wrote:
> On Wed, Oct 16, 2024 at 11:51:55PM +0200, Marc Kleine-Budde wrote:
> > With i.MX8MQ and compatible SoCs, the order of the IRQs in the device
> > tree is not optimal. The driver expects the first three IRQs to match
> > their corresponding queue, while the last (fourth) IRQ is used for the
> > PPS:
> >
> > - 1st IRQ: "int0": queue0 + other IRQs
> > - 2nd IRQ: "int1": queue1
> > - 3rd IRQ: "int2": queue2
> > - 4th IRQ: "pps": pps
> >
> > However, the i.MX8MQ and compatible SoCs do not use the
> > "interrupt-names" property and specify the IRQs in the wrong order:
> >
> > - 1st IRQ: queue1
> > - 2nd IRQ: queue2
> > - 3rd IRQ: queue0 + other IRQs
> > - 4th IRQ: pps
> 
> why not fix dts?

Because of compatibility. You could update the fec driver and try to
detect if the IRQs in the DTS are in the "correct" order, but the new
DTS will be incompatible with the old driver.

I'm working on a patch series that updates the fec driver to "per queue
IRQ and NAPI" handling. For this approach it's crucial that the IRQs
match the queue number.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.