| Message ID | 20241220041659.2985492-3-nikita.yoush@cogentembedded.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: renesas: rswitch: update irq handling | expand |
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
> 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 --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); };
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(-)