diff mbox series

[net-next,2/2] net: renesas: rswitch: request ts interrupt at port open

Message ID 20241220041659.2985492-3-nikita.yoush@cogentembedded.com (mailing list archive)
State New
Delegated to: Geert Uytterhoeven
Headers show
Series net: renesas: rswitch: update irq handling | expand

Commit Message

Nikita Yushchenko Dec. 20, 2024, 4:16 a.m. UTC
Data interrupts are now requested at port open and freed at port close.

For symmetry, do the same for ts interrupt.

Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
---
 drivers/net/ethernet/renesas/rswitch.c | 35 +++++++++++++-------------
 drivers/net/ethernet/renesas/rswitch.h |  2 +-
 2 files changed, 18 insertions(+), 19 deletions(-)

Comments

Michal Swiatkowski Dec. 20, 2024, 8:38 a.m. UTC | #1
On Fri, Dec 20, 2024 at 09:16:59AM +0500, Nikita Yushchenko wrote:
> Data interrupts are now requested at port open and freed at port close.
> 
> For symmetry, do the same for ts interrupt.
> 
> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
> ---
>  drivers/net/ethernet/renesas/rswitch.c | 35 +++++++++++++-------------
>  drivers/net/ethernet/renesas/rswitch.h |  2 +-
>  2 files changed, 18 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/ethernet/renesas/rswitch.c b/drivers/net/ethernet/renesas/rswitch.c
> index eb9dea8b16f3..cc8f2a4e3d70 100644
> --- a/drivers/net/ethernet/renesas/rswitch.c
> +++ b/drivers/net/ethernet/renesas/rswitch.c
> @@ -989,18 +989,6 @@ static irqreturn_t rswitch_gwca_ts_irq(int irq, void *dev_id)
>  	return IRQ_NONE;
>  }
>  
> -static int rswitch_gwca_ts_request_irqs(struct rswitch_private *priv)
> -{
> -	int irq;
> -
> -	irq = platform_get_irq_byname(priv->pdev, GWCA_TS_IRQ_RESOURCE_NAME);
> -	if (irq < 0)
> -		return irq;
> -
> -	return devm_request_irq(&priv->pdev->dev, irq, rswitch_gwca_ts_irq,
> -				0, GWCA_TS_IRQ_NAME, priv);
> -}
> -
>  /* Ethernet TSN Agent block (ETHA) and Ethernet MAC IP block (RMAC) */
>  static int rswitch_etha_change_mode(struct rswitch_etha *etha,
>  				    enum rswitch_etha_mode mode)
> @@ -1510,8 +1498,14 @@ static int rswitch_open(struct net_device *ndev)
>  	unsigned long flags;
>  	int ret;
>  
> -	if (bitmap_empty(rdev->priv->opened_ports, RSWITCH_NUM_PORTS))
> +	if (bitmap_empty(rdev->priv->opened_ports, RSWITCH_NUM_PORTS)) {
> +		ret = request_irq(rdev->priv->gwca.ts_irq, rswitch_gwca_ts_irq,
> +				  0, "rswitch_ts", rdev->priv);
> +		if (ret < 0)
> +			return ret;
> +
>  		iowrite32(GWCA_TS_IRQ_BIT, rdev->priv->addr + GWTSDIE);
> +	}
>  
>  	napi_enable(&rdev->napi);
>  
> @@ -1535,8 +1529,10 @@ static int rswitch_open(struct net_device *ndev)
>  err_request_irq:
>  	napi_disable(&rdev->napi);
>  
> -	if (bitmap_empty(rdev->priv->opened_ports, RSWITCH_NUM_PORTS))
> +	if (bitmap_empty(rdev->priv->opened_ports, RSWITCH_NUM_PORTS)) {
>  		iowrite32(GWCA_TS_IRQ_BIT, rdev->priv->addr + GWTSDID);
> +		free_irq(rdev->priv->gwca.ts_irq, rdev->priv);
> +	}
>  
>  	return ret;
>  };
> @@ -1562,8 +1558,10 @@ static int rswitch_stop(struct net_device *ndev)
>  
>  	napi_disable(&rdev->napi);
>  
> -	if (bitmap_empty(rdev->priv->opened_ports, RSWITCH_NUM_PORTS))
> +	if (bitmap_empty(rdev->priv->opened_ports, RSWITCH_NUM_PORTS)) {
>  		iowrite32(GWCA_TS_IRQ_BIT, rdev->priv->addr + GWTSDID);
> +		free_irq(rdev->priv->gwca.ts_irq, rdev->priv);
> +	}
>  
>  	for (tag = find_first_bit(rdev->ts_skb_used, TS_TAGS_PER_PORT);
>  	     tag < TS_TAGS_PER_PORT;
> @@ -2001,9 +1999,10 @@ static int rswitch_init(struct rswitch_private *priv)
>  	if (err < 0)
>  		goto err_ptp_register;
>  
> -	err = rswitch_gwca_ts_request_irqs(priv);
> +	err = platform_get_irq_byname(priv->pdev, GWCA_TS_IRQ_RESOURCE_NAME);
>  	if (err < 0)
> -		goto err_gwca_ts_request_irq;
> +		goto err_gwca_ts_irq;
> +	priv->gwca.ts_irq = err;
>  
>  	err = rswitch_gwca_hw_init(priv);
>  	if (err < 0)
> @@ -2035,7 +2034,7 @@ static int rswitch_init(struct rswitch_private *priv)
>  	rswitch_gwca_hw_deinit(priv);
>  
>  err_gwca_hw_init:
> -err_gwca_ts_request_irq:
> +err_gwca_ts_irq:
>  	rcar_gen4_ptp_unregister(priv->ptp_priv);
>  
>  err_ptp_register:
> diff --git a/drivers/net/ethernet/renesas/rswitch.h b/drivers/net/ethernet/renesas/rswitch.h
> index a1e62a6b3844..54b9f059707a 100644
> --- a/drivers/net/ethernet/renesas/rswitch.h
> +++ b/drivers/net/ethernet/renesas/rswitch.h
> @@ -58,7 +58,6 @@
>  #define GWRO			RSWITCH_GWCA0_OFFSET
>  
>  #define GWCA_TS_IRQ_RESOURCE_NAME	"gwca0_rxts0"
> -#define GWCA_TS_IRQ_NAME		"rswitch: gwca0_rxts0"
>  #define GWCA_TS_IRQ_BIT			BIT(0)
>  
>  #define FWRO	0
> @@ -978,6 +977,7 @@ struct rswitch_gwca {
>  	struct rswitch_gwca_queue *queues;
>  	int num_queues;
>  	struct rswitch_gwca_queue ts_queue;
> +	int ts_irq;
>  	DECLARE_BITMAP(used, RSWITCH_MAX_NUM_QUEUES);
>  };

Wasn't previous implementation more obvious? This ts irq you have one
per device, no per port, so it better fit to one time initialization
instead of checking if it is first and last port.

Anyway, it is your descision, patch looks correct:
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>

>  
> -- 
> 2.39.5
Nikita Yushchenko Dec. 20, 2024, 8:51 a.m. UTC | #2
> Wasn't previous implementation more obvious? This ts irq you have one
> per device, no per port, so it better fit to one time initialization
> instead of checking if it is first and last port.
> 
> Anyway, it is your descision, patch looks correct:
> Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>

For data interrupts, after making them per-port, it is better to request only for opened device, to 
avoid unneeded calls to shared handlers when some ports are up and some are not.

And once data interrupts are requested at open, it looks cleaner for me to request ts interrupt at open 
as well. Although I agree that this is matter of taste.

Nikita
diff mbox series

Patch

diff --git a/drivers/net/ethernet/renesas/rswitch.c b/drivers/net/ethernet/renesas/rswitch.c
index eb9dea8b16f3..cc8f2a4e3d70 100644
--- a/drivers/net/ethernet/renesas/rswitch.c
+++ b/drivers/net/ethernet/renesas/rswitch.c
@@ -989,18 +989,6 @@  static irqreturn_t rswitch_gwca_ts_irq(int irq, void *dev_id)
 	return IRQ_NONE;
 }
 
-static int rswitch_gwca_ts_request_irqs(struct rswitch_private *priv)
-{
-	int irq;
-
-	irq = platform_get_irq_byname(priv->pdev, GWCA_TS_IRQ_RESOURCE_NAME);
-	if (irq < 0)
-		return irq;
-
-	return devm_request_irq(&priv->pdev->dev, irq, rswitch_gwca_ts_irq,
-				0, GWCA_TS_IRQ_NAME, priv);
-}
-
 /* Ethernet TSN Agent block (ETHA) and Ethernet MAC IP block (RMAC) */
 static int rswitch_etha_change_mode(struct rswitch_etha *etha,
 				    enum rswitch_etha_mode mode)
@@ -1510,8 +1498,14 @@  static int rswitch_open(struct net_device *ndev)
 	unsigned long flags;
 	int ret;
 
-	if (bitmap_empty(rdev->priv->opened_ports, RSWITCH_NUM_PORTS))
+	if (bitmap_empty(rdev->priv->opened_ports, RSWITCH_NUM_PORTS)) {
+		ret = request_irq(rdev->priv->gwca.ts_irq, rswitch_gwca_ts_irq,
+				  0, "rswitch_ts", rdev->priv);
+		if (ret < 0)
+			return ret;
+
 		iowrite32(GWCA_TS_IRQ_BIT, rdev->priv->addr + GWTSDIE);
+	}
 
 	napi_enable(&rdev->napi);
 
@@ -1535,8 +1529,10 @@  static int rswitch_open(struct net_device *ndev)
 err_request_irq:
 	napi_disable(&rdev->napi);
 
-	if (bitmap_empty(rdev->priv->opened_ports, RSWITCH_NUM_PORTS))
+	if (bitmap_empty(rdev->priv->opened_ports, RSWITCH_NUM_PORTS)) {
 		iowrite32(GWCA_TS_IRQ_BIT, rdev->priv->addr + GWTSDID);
+		free_irq(rdev->priv->gwca.ts_irq, rdev->priv);
+	}
 
 	return ret;
 };
@@ -1562,8 +1558,10 @@  static int rswitch_stop(struct net_device *ndev)
 
 	napi_disable(&rdev->napi);
 
-	if (bitmap_empty(rdev->priv->opened_ports, RSWITCH_NUM_PORTS))
+	if (bitmap_empty(rdev->priv->opened_ports, RSWITCH_NUM_PORTS)) {
 		iowrite32(GWCA_TS_IRQ_BIT, rdev->priv->addr + GWTSDID);
+		free_irq(rdev->priv->gwca.ts_irq, rdev->priv);
+	}
 
 	for (tag = find_first_bit(rdev->ts_skb_used, TS_TAGS_PER_PORT);
 	     tag < TS_TAGS_PER_PORT;
@@ -2001,9 +1999,10 @@  static int rswitch_init(struct rswitch_private *priv)
 	if (err < 0)
 		goto err_ptp_register;
 
-	err = rswitch_gwca_ts_request_irqs(priv);
+	err = platform_get_irq_byname(priv->pdev, GWCA_TS_IRQ_RESOURCE_NAME);
 	if (err < 0)
-		goto err_gwca_ts_request_irq;
+		goto err_gwca_ts_irq;
+	priv->gwca.ts_irq = err;
 
 	err = rswitch_gwca_hw_init(priv);
 	if (err < 0)
@@ -2035,7 +2034,7 @@  static int rswitch_init(struct rswitch_private *priv)
 	rswitch_gwca_hw_deinit(priv);
 
 err_gwca_hw_init:
-err_gwca_ts_request_irq:
+err_gwca_ts_irq:
 	rcar_gen4_ptp_unregister(priv->ptp_priv);
 
 err_ptp_register:
diff --git a/drivers/net/ethernet/renesas/rswitch.h b/drivers/net/ethernet/renesas/rswitch.h
index a1e62a6b3844..54b9f059707a 100644
--- a/drivers/net/ethernet/renesas/rswitch.h
+++ b/drivers/net/ethernet/renesas/rswitch.h
@@ -58,7 +58,6 @@ 
 #define GWRO			RSWITCH_GWCA0_OFFSET
 
 #define GWCA_TS_IRQ_RESOURCE_NAME	"gwca0_rxts0"
-#define GWCA_TS_IRQ_NAME		"rswitch: gwca0_rxts0"
 #define GWCA_TS_IRQ_BIT			BIT(0)
 
 #define FWRO	0
@@ -978,6 +977,7 @@  struct rswitch_gwca {
 	struct rswitch_gwca_queue *queues;
 	int num_queues;
 	struct rswitch_gwca_queue ts_queue;
+	int ts_irq;
 	DECLARE_BITMAP(used, RSWITCH_MAX_NUM_QUEUES);
 };