Skip to content

Commit 4d5159b

Browse files
scosumarckleinebudde
authored andcommitted
can: m_can: Do not cancel timer from within timer
On setups without interrupts, the interrupt handler is called from a timer callback. For non-peripheral receives napi is scheduled, interrupts are disabled and the timer is canceled with a blocking call. In case of an error this can happen as well. Check if napi is scheduled in the timer callback after the interrupt handler executed. If napi is scheduled, the timer is disabled. It will be reenabled by m_can_poll(). Return error values from the interrupt handler so that interrupt threads and timer callback can deal differently with it. In case of the timer we only disable the timer. The rest will be done when stopping the interface. Fixes: b382380 ("can: m_can: Add hrtimer to generate software interrupt") Fixes: a163c57 ("can: m_can: Start/Cancel polling timer together with interrupts") Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> Link: https://lore.kernel.org/all/20240805183047.305630-5-msp@baylibre.com Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
1 parent 40e4552 commit 4d5159b

1 file changed

Lines changed: 42 additions & 15 deletions

File tree

drivers/net/can/m_can/m_can.c

Lines changed: 42 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,7 @@ static inline void m_can_disable_all_interrupts(struct m_can_classdev *cdev)
487487

488488
if (!cdev->net->irq) {
489489
dev_dbg(cdev->dev, "Stop hrtimer\n");
490-
hrtimer_cancel(&cdev->hrtimer);
490+
hrtimer_try_to_cancel(&cdev->hrtimer);
491491
}
492492
}
493493

@@ -1201,11 +1201,15 @@ static void m_can_coalescing_update(struct m_can_classdev *cdev, u32 ir)
12011201
HRTIMER_MODE_REL);
12021202
}
12031203

1204-
static irqreturn_t m_can_isr(int irq, void *dev_id)
1204+
/* This interrupt handler is called either from the interrupt thread or a
1205+
* hrtimer. This has implications like cancelling a timer won't be possible
1206+
* blocking.
1207+
*/
1208+
static int m_can_interrupt_handler(struct m_can_classdev *cdev)
12051209
{
1206-
struct net_device *dev = (struct net_device *)dev_id;
1207-
struct m_can_classdev *cdev = netdev_priv(dev);
1210+
struct net_device *dev = cdev->net;
12081211
u32 ir;
1212+
int ret;
12091213

12101214
if (pm_runtime_suspended(cdev->dev))
12111215
return IRQ_NONE;
@@ -1232,11 +1236,9 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
12321236
m_can_disable_all_interrupts(cdev);
12331237
napi_schedule(&cdev->napi);
12341238
} else {
1235-
int pkts;
1236-
1237-
pkts = m_can_rx_handler(dev, NAPI_POLL_WEIGHT, ir);
1238-
if (pkts < 0)
1239-
goto out_fail;
1239+
ret = m_can_rx_handler(dev, NAPI_POLL_WEIGHT, ir);
1240+
if (ret < 0)
1241+
return ret;
12401242
}
12411243
}
12421244

@@ -1254,25 +1256,41 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
12541256
} else {
12551257
if (ir & (IR_TEFN | IR_TEFW)) {
12561258
/* New TX FIFO Element arrived */
1257-
if (m_can_echo_tx_event(dev) != 0)
1258-
goto out_fail;
1259+
ret = m_can_echo_tx_event(dev);
1260+
if (ret != 0)
1261+
return ret;
12591262
}
12601263
}
12611264

12621265
if (cdev->is_peripheral)
12631266
can_rx_offload_threaded_irq_finish(&cdev->offload);
12641267

12651268
return IRQ_HANDLED;
1269+
}
12661270

1267-
out_fail:
1268-
m_can_disable_all_interrupts(cdev);
1269-
return IRQ_HANDLED;
1271+
static irqreturn_t m_can_isr(int irq, void *dev_id)
1272+
{
1273+
struct net_device *dev = (struct net_device *)dev_id;
1274+
struct m_can_classdev *cdev = netdev_priv(dev);
1275+
int ret;
1276+
1277+
ret = m_can_interrupt_handler(cdev);
1278+
if (ret < 0) {
1279+
m_can_disable_all_interrupts(cdev);
1280+
return IRQ_HANDLED;
1281+
}
1282+
1283+
return ret;
12701284
}
12711285

12721286
static enum hrtimer_restart m_can_coalescing_timer(struct hrtimer *timer)
12731287
{
12741288
struct m_can_classdev *cdev = container_of(timer, struct m_can_classdev, hrtimer);
12751289

1290+
if (cdev->can.state == CAN_STATE_BUS_OFF ||
1291+
cdev->can.state == CAN_STATE_STOPPED)
1292+
return HRTIMER_NORESTART;
1293+
12761294
irq_wake_thread(cdev->net->irq, cdev->net);
12771295

12781296
return HRTIMER_NORESTART;
@@ -1973,8 +1991,17 @@ static enum hrtimer_restart hrtimer_callback(struct hrtimer *timer)
19731991
{
19741992
struct m_can_classdev *cdev = container_of(timer, struct
19751993
m_can_classdev, hrtimer);
1994+
int ret;
1995+
1996+
if (cdev->can.state == CAN_STATE_BUS_OFF ||
1997+
cdev->can.state == CAN_STATE_STOPPED)
1998+
return HRTIMER_NORESTART;
1999+
2000+
ret = m_can_interrupt_handler(cdev);
19762001

1977-
m_can_isr(0, cdev->net);
2002+
/* On error or if napi is scheduled to read, stop the timer */
2003+
if (ret < 0 || napi_is_scheduled(&cdev->napi))
2004+
return HRTIMER_NORESTART;
19782005

19792006
hrtimer_forward_now(timer, ms_to_ktime(HRTIMER_POLL_INTERVAL_MS));
19802007

0 commit comments

Comments
 (0)