Re: [PATCH V2] irqchip/loongson-pch-pic: Update interrupt registration policy

From: Christophe JAILLET
Date: Sat Mar 16 2024 - 10:03:41 EST


Le 16/03/2024 à 09:21, Tianyang Zhang a écrit :
From: Baoqi Zhang <zhangbaoqi@xxxxxxxxxxx>

This patch remove the fixed mapping between the 7A interrupt source
and the HT interrupt vector, and replaced it with a dynamically
allocated approach.

We introduce a mapping table in struct pch_pic, where each interrupt
source will allocate an index as a 'hwirq' from the table in the order
of application and set table value as interrupt source number. This hwirq
will be configured as its vector in the HT interrupt controller. For an
interrupt source, the validity period of the obtained hwirq will last until
the system reset

This will be more conducive to fully utilizing existing vectors to
support more devices

Signed-off-by: Baoqi Zhang <zhangbaoqi@xxxxxxxxxxx>
Signed-off-by: Biao Dong <dongbiao@xxxxxxxxxxx>
Signed-off-by: Tianyang Zhang <zhangtianyang@xxxxxxxxxxx>
---
drivers/irqchip/irq-loongson-pch-pic.c | 77 ++++++++++++++++++++------
1 file changed, 59 insertions(+), 18 deletions(-)


..

@@ -171,6 +183,27 @@ static int pch_pic_domain_translate(struct irq_domain *d,
return -EINVAL;
*hwirq = fwspec->param[0] - priv->gsi_base;
+
+ raw_spin_lock_irqsave(&priv->pic_lock, flags);
+ /* Check pic-table to confirm if the hwirq has been assigned */
+ for (i = 0; i < priv->inuse; i++) {
+ if (priv->table[i] == *hwirq) {
+ *hwirq = i;
+ break;
+ }
+ }
+ if (i == priv->inuse) {
+ /* Assign a new hwirq in pic-table */
+ if (priv->inuse >= PIC_COUNT) {
+ pr_err("pch-pic domain has no free vectors\n");
+ raw_spin_unlock_irqrestore(&priv->pic_lock, flags);
+ return -EINVAL;
+ }
+ priv->table[priv->inuse] = *hwirq;
+ *hwirq = priv->inuse++;
+ }
+ raw_spin_unlock_irqrestore(&priv->pic_lock, flags);

Hi,

not sure if it helps or if this is a hot path, but, IIUC, taking the lock could be avoided with some code reordering and 'inuse' being an atomic_t.

IIUC, the lock is only needed to protect 'inuse' and 'table' is never modified afterwards.

Somehing like:

> + int cur_inuse;
...
> + cur_inuse = atomic_read(&priv->inuse);
> + /* Check pic-table to confirm if the hwirq has been assigned */
> + for (i = 0; i < cur_inuse; i++) {
> + if (priv->table[i] == *hwirq) {
> + *hwirq = i;
> + break;
> + }
> + }
> + if (i == cur_inuse) {
> + /* Assign a new hwirq in pic-table */
> + if (cur_inuse >= PIC_COUNT) {
> + pr_err("pch-pic domain has no free vectors\n");
> + return -EINVAL;
> + }
> + priv->table[cur_inuse] = *hwirq;
> + *hwirq = atomic_inc_return(&priv->inuse);
> + }


+
if (fwspec->param_count > 1)
*type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
else
@@ -194,6 +227,9 @@ static int pch_pic_alloc(struct irq_domain *domain, unsigned int virq,
if (err)
return err;
+ /* Write vector ID */
+ writeb(priv->ht_vec_base + hwirq, priv->base + PCH_INT_HTVEC(hwirq_to_bit(priv, hwirq)));
+
parent_fwspec.fwnode = domain->parent->fwnode;
parent_fwspec.param_count = 1;
parent_fwspec.param[0] = hwirq + priv->ht_vec_base;

..