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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z2UtCBwofyoHlZi0@mev-dev.igk.intel.com>
Date: Fri, 20 Dec 2024 09:38:32 +0100
From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
To: Nikita Yushchenko <nikita.yoush@...entembedded.com>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Geert Uytterhoeven <geert+renesas@...der.be>,
	netdev@...r.kernel.org, linux-renesas-soc@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Michael Dege <michael.dege@...esas.com>,
	Christian Mardmoeller <christian.mardmoeller@...esas.com>,
	Dennis Ostermann <dennis.ostermann@...esas.com>
Subject: Re: [PATCH net-next 2/2] net: renesas: rswitch: request ts interrupt
 at port open

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@...entembedded.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@...ux.intel.com>

>  
> -- 
> 2.39.5