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

[Michal Swiatkowski <michal.swiatkowski@linux.intel.com>]

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