Skip to content

Commit eaa1add

Browse files
LaurentiuM1234kartben
authored andcommitted
drivers: intc: irqstr: refactor level 1 interrupt recounting
So far, it has been assumed that only level 2 interrupts can be shared via the `CONFIG_SHARED_INTERRUPTS` option, but this is not true. In the case of i.MX95, for instance, level 1 interrupt 143 is shared among EDMA channels 30 and 31. Due to the previous assumption, the irqsteer driver currently performs reference counting for all level 2 interrupts aggregated by each dispatcher and, of course, for the level 1 interrupts the dispatchers are attached to. For instance, assuming a machine with 100 level 1 interrupts and 1 irqsteer dispatcher attached to line 50 this would mean reference counting is performed solely for line 50 (and the level 2 interrupts MUX'd into this line). Going back to i.MX95, since there's no dispatcher attached to IRQ line 143 that means there's no reference counting for it. In turn, this means that the IRQ line can be disabled accidentally on a channel release() operation while the other channel is active. To protect against such cases, refactor the level1 interrupt reference counting. Now, reference counting is performed for _all_ level 1 interrupts. Additionally, simplify the locking logic. Ideally, there would be a lock for each dispatcher protecting the level 2 interrupts and 1 global lock protecting the level 1 interrupts. Instead of this approach (which is a bit more complex), simply use a global lock for all interrupts. If finer granularity is required then it can be added later on. Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
1 parent bb7efcf commit eaa1add

File tree

1 file changed

+77
-45
lines changed

1 file changed

+77
-45
lines changed

drivers/interrupt_controller/intc_nxp_irqsteer.c

Lines changed: 77 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -291,12 +291,12 @@ struct irqsteer_dispatcher {
291291
uint32_t irq;
292292
/* reference count for all IRQs aggregated by dispatcher */
293293
uint8_t irq_refcnt[CONFIG_MAX_IRQ_PER_AGGREGATOR];
294-
/* dispatcher lock */
295-
struct k_spinlock lock;
296-
/* reference count for dispatcher */
297-
uint8_t refcnt;
298294
};
299295

296+
static struct k_spinlock irqstr_lock;
297+
298+
static uint8_t l1_irq_refcnt[CONFIG_2ND_LVL_ISR_TBL_OFFSET];
299+
300300
static struct irqsteer_dispatcher dispatchers[] = {
301301
IRQSTEER_DECLARE_DISPATCHERS(DT_NODELABEL(irqsteer))
302302
};
@@ -342,68 +342,95 @@ static int from_zephyr_irq(uint32_t regmap, uint32_t irq, uint32_t master_index)
342342
return idx + FSL_FEATURE_IRQSTEER_IRQ_START_INDEX;
343343
}
344344

345-
static void _irqstr_disp_enable_disable(struct irqsteer_dispatcher *disp,
346-
bool enable)
345+
/* note: if disp != NULL that means there's several level2 interrupts
346+
* multiplexed into this single level 1 line via irqsteer. In such cases,
347+
* the interrupt needs to also be enable/disabled at the irqsteer level.
348+
*
349+
* brief: disp == NULL => IRQ is not managed by IRQSTEER
350+
* disp != NULL => IRQ is managed by IRQSTEER
351+
*/
352+
static void irqstr_l1_irq_enable_disable(uint32_t irq,
353+
struct irqsteer_dispatcher *disp,
354+
bool enable)
347355
{
348-
uint32_t regmap = DISPATCHER_REGMAP(disp);
349-
350356
if (enable) {
351-
irqstr_l1_irq_enable_raw(disp->irq);
352-
IRQSTEER_EnableMasterInterrupt(UINT_TO_IRQSTEER(regmap), disp->irq);
357+
irqstr_l1_irq_enable_raw(irq);
358+
359+
if (disp) {
360+
IRQSTEER_EnableMasterInterrupt(UINT_TO_IRQSTEER(DISPATCHER_REGMAP(disp)),
361+
irq);
362+
}
353363
} else {
354-
IRQSTEER_DisableMasterInterrupt(UINT_TO_IRQSTEER(regmap), disp->irq);
355-
irqstr_l1_irq_disable_raw(disp->irq);
364+
if (disp) {
365+
IRQSTEER_DisableMasterInterrupt(UINT_TO_IRQSTEER(DISPATCHER_REGMAP(disp)),
366+
irq);
367+
}
368+
369+
irqstr_l1_irq_disable_raw(irq);
356370
}
357371
}
358372

359-
static void _irqstr_disp_get_unlocked(struct irqsteer_dispatcher *disp)
373+
static void irqstr_request_l1_irq_unlocked(uint32_t irq,
374+
struct irqsteer_dispatcher *disp)
360375
{
361376
int ret;
362377

363-
if (disp->refcnt == UINT8_MAX) {
364-
LOG_WRN("disp for irq %d reference count reached limit", disp->irq);
378+
#ifndef CONFIG_SHARED_INTERRUPTS
379+
if (l1_irq_refcnt[irq]) {
380+
LOG_WRN("L1 IRQ %d already requested", irq);
381+
return;
382+
}
383+
#endif /* CONFIG_SHARED_INTERRUPTS */
384+
385+
if (l1_irq_refcnt[irq] == UINT8_MAX) {
386+
LOG_WRN("L1 IRQ %d reference count reached limit", irq);
365387
return;
366388
}
367389

368-
if (!disp->refcnt) {
369-
ret = pm_device_runtime_get(disp->dev);
370-
if (ret < 0) {
371-
LOG_ERR("failed to enable PM resources: %d", ret);
372-
return;
390+
if (!l1_irq_refcnt[irq]) {
391+
if (disp) {
392+
ret = pm_device_runtime_get(disp->dev);
393+
if (ret < 0) {
394+
LOG_ERR("failed to enable PM resources: %d", ret);
395+
return;
396+
}
373397
}
374398

375-
_irqstr_disp_enable_disable(disp, true);
399+
irqstr_l1_irq_enable_disable(irq, disp, true);
376400
}
377401

378-
disp->refcnt++;
402+
l1_irq_refcnt[irq]++;
379403

380-
LOG_DBG("get on disp for irq %d results in refcnt: %d",
381-
disp->irq, disp->refcnt);
404+
LOG_DBG("request for L1 IRQ %d results in refcnt: %d",
405+
irq, l1_irq_refcnt[irq]);
382406
}
383407

384-
static void _irqstr_disp_put_unlocked(struct irqsteer_dispatcher *disp)
408+
static void irqstr_release_l1_irq_unlocked(uint32_t irq,
409+
struct irqsteer_dispatcher *disp)
385410
{
386411
int ret;
387412

388-
if (!disp->refcnt) {
389-
LOG_WRN("disp for irq %d already put", disp->irq);
413+
if (!l1_irq_refcnt[irq]) {
414+
LOG_WRN("L1 IRQ %d already released", irq);
390415
return;
391416
}
392417

393-
disp->refcnt--;
418+
l1_irq_refcnt[irq]--;
394419

395-
if (!disp->refcnt) {
396-
_irqstr_disp_enable_disable(disp, false);
420+
if (!l1_irq_refcnt[irq]) {
421+
irqstr_l1_irq_enable_disable(irq, disp, false);
397422

398-
ret = pm_device_runtime_put(disp->dev);
399-
if (ret < 0) {
400-
LOG_ERR("failed to disable PM resources: %d", ret);
401-
return;
423+
if (disp) {
424+
ret = pm_device_runtime_put(disp->dev);
425+
if (ret < 0) {
426+
LOG_ERR("failed to disable PM resources: %d", ret);
427+
return;
428+
}
402429
}
403430
}
404431

405-
LOG_DBG("put on disp for irq %d results in refcnt: %d",
406-
disp->irq, disp->refcnt);
432+
LOG_DBG("release on L1 IRQ %d results in refcnt: %d",
433+
irq, l1_irq_refcnt[irq]);
407434
}
408435

409436
static void _irqstr_enable_disable_irq(struct irqsteer_dispatcher *disp,
@@ -437,7 +464,7 @@ static void irqstr_request_irq_unlocked(struct irqsteer_dispatcher *disp,
437464
}
438465

439466
if (!disp->irq_refcnt[zephyr_irq]) {
440-
_irqstr_disp_get_unlocked(disp);
467+
irqstr_request_l1_irq_unlocked(disp->irq, disp);
441468
_irqstr_enable_disable_irq(disp, system_irq, true);
442469
}
443470

@@ -462,7 +489,7 @@ static void irqstr_release_irq_unlocked(struct irqsteer_dispatcher *disp,
462489

463490
if (!disp->irq_refcnt[zephyr_irq]) {
464491
_irqstr_enable_disable_irq(disp, system_irq, false);
465-
_irqstr_disp_put_unlocked(disp);
492+
irqstr_release_l1_irq_unlocked(disp->irq, disp);
466493
}
467494

468495
LOG_DBG("released irq %d has refcount %d",
@@ -476,10 +503,12 @@ void z_soc_irq_enable_disable(uint32_t irq, bool enable)
476503

477504
if (irq_get_level(irq) == 1) {
478505
/* LEVEL 1 interrupts are DSP direct */
479-
if (enable) {
480-
irqstr_l1_irq_enable_raw(irq);
481-
} else {
482-
irqstr_l1_irq_disable_raw(irq);
506+
K_SPINLOCK(&irqstr_lock) {
507+
if (enable) {
508+
irqstr_request_l1_irq_unlocked(irq, NULL);
509+
} else {
510+
irqstr_release_l1_irq_unlocked(irq, NULL);
511+
}
483512
}
484513
return;
485514
}
@@ -493,7 +522,7 @@ void z_soc_irq_enable_disable(uint32_t irq, bool enable)
493522
continue;
494523
}
495524

496-
K_SPINLOCK(&dispatchers[i].lock) {
525+
K_SPINLOCK(&irqstr_lock) {
497526
if (enable) {
498527
irqstr_request_irq_unlocked(&dispatchers[i], level2_irq);
499528
} else {
@@ -523,7 +552,10 @@ int z_soc_irq_is_enabled(unsigned int irq)
523552
bool enabled;
524553

525554
if (irq_get_level(irq) == 1) {
526-
return irqsteer_level1_irq_is_enabled(irq);
555+
K_SPINLOCK(&irqstr_lock) {
556+
enabled = l1_irq_refcnt[irq];
557+
}
558+
return enabled;
527559
}
528560

529561
parent_irq = irq_parent_level_2(irq);
@@ -537,7 +569,7 @@ int z_soc_irq_is_enabled(unsigned int irq)
537569

538570
cfg = dispatchers[i].dev->config;
539571

540-
K_SPINLOCK(&dispatchers[i].lock) {
572+
K_SPINLOCK(&irqstr_lock) {
541573
enabled = dispatchers[i].irq_refcnt[irq_from_level_2(irq)];
542574
}
543575

0 commit comments

Comments
 (0)