* [PATCH V2] irqchip/loongson-pch-pic: Update interrupt registration policy
@ 2024-03-16 8:21 Tianyang Zhang
2024-03-16 14:03 ` Christophe JAILLET
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Tianyang Zhang @ 2024-03-16 8:21 UTC (permalink / raw)
To: chenhuacai, jiaxun.yang, tglx
Cc: linux-mips, linux-kernel, Baoqi Zhang, Biao Dong, Tianyang Zhang
From: Baoqi Zhang <zhangbaoqi@loongson.cn>
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@loongson.cn>
Signed-off-by: Biao Dong <dongbiao@loongson.cn>
Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn>
---
drivers/irqchip/irq-loongson-pch-pic.c | 77 ++++++++++++++++++++------
1 file changed, 59 insertions(+), 18 deletions(-)
diff --git a/drivers/irqchip/irq-loongson-pch-pic.c b/drivers/irqchip/irq-loongson-pch-pic.c
index 63db8e2172e0..f17187641154 100644
--- a/drivers/irqchip/irq-loongson-pch-pic.c
+++ b/drivers/irqchip/irq-loongson-pch-pic.c
@@ -33,7 +33,7 @@
#define PIC_COUNT (PIC_COUNT_PER_REG * PIC_REG_COUNT)
#define PIC_REG_IDX(irq_id) ((irq_id) / PIC_COUNT_PER_REG)
#define PIC_REG_BIT(irq_id) ((irq_id) % PIC_COUNT_PER_REG)
-
+#define PIC_UNDEF_VECTOR 255
static int nr_pics;
struct pch_pic {
@@ -46,12 +46,19 @@ struct pch_pic {
u32 saved_vec_en[PIC_REG_COUNT];
u32 saved_vec_pol[PIC_REG_COUNT];
u32 saved_vec_edge[PIC_REG_COUNT];
+ u8 table[PIC_COUNT];
+ int inuse;
};
static struct pch_pic *pch_pic_priv[MAX_IO_PICS];
struct fwnode_handle *pch_pic_handle[MAX_IO_PICS];
+static inline u8 hwirq_to_bit(struct pch_pic *priv, int hirq)
+{
+ return priv->table[hirq];
+}
+
static void pch_pic_bitset(struct pch_pic *priv, int offset, int bit)
{
u32 reg;
@@ -80,45 +87,47 @@ static void pch_pic_mask_irq(struct irq_data *d)
{
struct pch_pic *priv = irq_data_get_irq_chip_data(d);
- pch_pic_bitset(priv, PCH_PIC_MASK, d->hwirq);
+ pch_pic_bitset(priv, PCH_PIC_MASK, hwirq_to_bit(priv, d->hwirq));
irq_chip_mask_parent(d);
}
static void pch_pic_unmask_irq(struct irq_data *d)
{
struct pch_pic *priv = irq_data_get_irq_chip_data(d);
+ int bit = hwirq_to_bit(priv, d->hwirq);
- writel(BIT(PIC_REG_BIT(d->hwirq)),
- priv->base + PCH_PIC_CLR + PIC_REG_IDX(d->hwirq) * 4);
+ writel(BIT(PIC_REG_BIT(bit)),
+ priv->base + PCH_PIC_CLR + PIC_REG_IDX(bit) * 4);
irq_chip_unmask_parent(d);
- pch_pic_bitclr(priv, PCH_PIC_MASK, d->hwirq);
+ pch_pic_bitclr(priv, PCH_PIC_MASK, bit);
}
static int pch_pic_set_type(struct irq_data *d, unsigned int type)
{
struct pch_pic *priv = irq_data_get_irq_chip_data(d);
+ int bit = hwirq_to_bit(priv, d->hwirq);
int ret = 0;
switch (type) {
case IRQ_TYPE_EDGE_RISING:
- pch_pic_bitset(priv, PCH_PIC_EDGE, d->hwirq);
- pch_pic_bitclr(priv, PCH_PIC_POL, d->hwirq);
+ pch_pic_bitset(priv, PCH_PIC_EDGE, bit);
+ pch_pic_bitclr(priv, PCH_PIC_POL, bit);
irq_set_handler_locked(d, handle_edge_irq);
break;
case IRQ_TYPE_EDGE_FALLING:
- pch_pic_bitset(priv, PCH_PIC_EDGE, d->hwirq);
- pch_pic_bitset(priv, PCH_PIC_POL, d->hwirq);
+ pch_pic_bitset(priv, PCH_PIC_EDGE, bit);
+ pch_pic_bitset(priv, PCH_PIC_POL, bit);
irq_set_handler_locked(d, handle_edge_irq);
break;
case IRQ_TYPE_LEVEL_HIGH:
- pch_pic_bitclr(priv, PCH_PIC_EDGE, d->hwirq);
- pch_pic_bitclr(priv, PCH_PIC_POL, d->hwirq);
+ pch_pic_bitclr(priv, PCH_PIC_EDGE, bit);
+ pch_pic_bitclr(priv, PCH_PIC_POL, bit);
irq_set_handler_locked(d, handle_level_irq);
break;
case IRQ_TYPE_LEVEL_LOW:
- pch_pic_bitclr(priv, PCH_PIC_EDGE, d->hwirq);
- pch_pic_bitset(priv, PCH_PIC_POL, d->hwirq);
+ pch_pic_bitclr(priv, PCH_PIC_EDGE, bit);
+ pch_pic_bitset(priv, PCH_PIC_POL, bit);
irq_set_handler_locked(d, handle_level_irq);
break;
default:
@@ -133,11 +142,12 @@ static void pch_pic_ack_irq(struct irq_data *d)
{
unsigned int reg;
struct pch_pic *priv = irq_data_get_irq_chip_data(d);
+ int bit = hwirq_to_bit(priv, d->hwirq);
- reg = readl(priv->base + PCH_PIC_EDGE + PIC_REG_IDX(d->hwirq) * 4);
- if (reg & BIT(PIC_REG_BIT(d->hwirq))) {
- writel(BIT(PIC_REG_BIT(d->hwirq)),
- priv->base + PCH_PIC_CLR + PIC_REG_IDX(d->hwirq) * 4);
+ reg = readl(priv->base + PCH_PIC_EDGE + PIC_REG_IDX(bit) * 4);
+ if (reg & BIT(PIC_REG_BIT(bit))) {
+ writel(BIT(PIC_REG_BIT(bit)),
+ priv->base + PCH_PIC_CLR + PIC_REG_IDX(bit) * 4);
}
irq_chip_ack_parent(d);
}
@@ -159,6 +169,8 @@ static int pch_pic_domain_translate(struct irq_domain *d,
{
struct pch_pic *priv = d->host_data;
struct device_node *of_node = to_of_node(fwspec->fwnode);
+ unsigned long flags;
+ int i;
if (of_node) {
if (fwspec->param_count < 2)
@@ -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);
+
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;
@@ -222,7 +258,7 @@ static void pch_pic_reset(struct pch_pic *priv)
for (i = 0; i < PIC_COUNT; i++) {
/* Write vector ID */
- writeb(priv->ht_vec_base + i, priv->base + PCH_INT_HTVEC(i));
+ writeb(priv->ht_vec_base + i, priv->base + PCH_INT_HTVEC(hwirq_to_bit(priv, i)));
/* Hardcode route to HT0 Lo */
writeb(1, priv->base + PCH_INT_ROUTE(i));
}
@@ -284,6 +320,7 @@ static int pch_pic_init(phys_addr_t addr, unsigned long size, int vec_base,
u32 gsi_base)
{
struct pch_pic *priv;
+ int i;
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv)
@@ -294,6 +331,10 @@ static int pch_pic_init(phys_addr_t addr, unsigned long size, int vec_base,
if (!priv->base)
goto free_priv;
+ priv->inuse = 0;
+ for (i = 0; i < PIC_COUNT; i++)
+ priv->table[i] = PIC_UNDEF_VECTOR;
+
priv->ht_vec_base = vec_base;
priv->vec_count = ((readq(priv->base) >> 48) & 0xff) + 1;
priv->gsi_base = gsi_base;
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH V2] irqchip/loongson-pch-pic: Update interrupt registration policy
2024-03-16 8:21 [PATCH V2] irqchip/loongson-pch-pic: Update interrupt registration policy Tianyang Zhang
@ 2024-03-16 14:03 ` Christophe JAILLET
2024-03-18 11:18 ` Thomas Gleixner
2024-03-19 11:26 ` Tianyang Zhang
2024-03-18 14:35 ` Huacai Chen
2024-03-21 6:10 ` [PATCH v2] " Binbin Zhou
2 siblings, 2 replies; 8+ messages in thread
From: Christophe JAILLET @ 2024-03-16 14:03 UTC (permalink / raw)
To: Tianyang Zhang, chenhuacai, jiaxun.yang, tglx
Cc: linux-mips, linux-kernel, Baoqi Zhang, Biao Dong
Le 16/03/2024 à 09:21, Tianyang Zhang a écrit :
> From: Baoqi Zhang <zhangbaoqi@loongson.cn>
>
> 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@loongson.cn>
> Signed-off-by: Biao Dong <dongbiao@loongson.cn>
> Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn>
> ---
> 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;
...
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] irqchip/loongson-pch-pic: Update interrupt registration policy
2024-03-16 14:03 ` Christophe JAILLET
@ 2024-03-18 11:18 ` Thomas Gleixner
2024-03-19 11:26 ` Tianyang Zhang
1 sibling, 0 replies; 8+ messages in thread
From: Thomas Gleixner @ 2024-03-18 11:18 UTC (permalink / raw)
To: Christophe JAILLET, Tianyang Zhang, chenhuacai, jiaxun.yang
Cc: linux-mips, linux-kernel, Baoqi Zhang, Biao Dong
On Sat, Mar 16 2024 at 15:03, Christophe JAILLET wrote:
> Le 16/03/2024 à 09:21, Tianyang Zhang a écrit :
>> @@ -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.
It's the translate and setup path, so nothing to optimize here.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] irqchip/loongson-pch-pic: Update interrupt registration policy
2024-03-16 8:21 [PATCH V2] irqchip/loongson-pch-pic: Update interrupt registration policy Tianyang Zhang
2024-03-16 14:03 ` Christophe JAILLET
@ 2024-03-18 14:35 ` Huacai Chen
2024-03-19 11:18 ` Tianyang Zhang
2024-03-21 6:10 ` [PATCH v2] " Binbin Zhou
2 siblings, 1 reply; 8+ messages in thread
From: Huacai Chen @ 2024-03-18 14:35 UTC (permalink / raw)
To: Tianyang Zhang
Cc: jiaxun.yang, tglx, linux-mips, linux-kernel, Baoqi Zhang, Biao Dong
Hi, Tianyang,
On Sat, Mar 16, 2024 at 4:22 PM Tianyang Zhang
<zhangtianyang@loongson.cn> wrote:
>
> From: Baoqi Zhang <zhangbaoqi@loongson.cn>
>
> This patch remove the fixed mapping between the 7A interrupt source
> and the HT interrupt vector, and replaced it with a dynamically
> allocated approach.
Use LS7A instead of 7A here.
>
> 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
Missing a . at the end.
>
> This will be more conducive to fully utilizing existing vectors to
> support more devices
>
> Signed-off-by: Baoqi Zhang <zhangbaoqi@loongson.cn>
> Signed-off-by: Biao Dong <dongbiao@loongson.cn>
> Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn>
> ---
> drivers/irqchip/irq-loongson-pch-pic.c | 77 ++++++++++++++++++++------
> 1 file changed, 59 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/irqchip/irq-loongson-pch-pic.c b/drivers/irqchip/irq-loongson-pch-pic.c
> index 63db8e2172e0..f17187641154 100644
> --- a/drivers/irqchip/irq-loongson-pch-pic.c
> +++ b/drivers/irqchip/irq-loongson-pch-pic.c
> @@ -33,7 +33,7 @@
> #define PIC_COUNT (PIC_COUNT_PER_REG * PIC_REG_COUNT)
> #define PIC_REG_IDX(irq_id) ((irq_id) / PIC_COUNT_PER_REG)
> #define PIC_REG_BIT(irq_id) ((irq_id) % PIC_COUNT_PER_REG)
> -
> +#define PIC_UNDEF_VECTOR 255
Keep a new line after the macro, please.
Huacai
> static int nr_pics;
>
> struct pch_pic {
> @@ -46,12 +46,19 @@ struct pch_pic {
> u32 saved_vec_en[PIC_REG_COUNT];
> u32 saved_vec_pol[PIC_REG_COUNT];
> u32 saved_vec_edge[PIC_REG_COUNT];
> + u8 table[PIC_COUNT];
> + int inuse;
> };
>
> static struct pch_pic *pch_pic_priv[MAX_IO_PICS];
>
> struct fwnode_handle *pch_pic_handle[MAX_IO_PICS];
>
> +static inline u8 hwirq_to_bit(struct pch_pic *priv, int hirq)
> +{
> + return priv->table[hirq];
> +}
> +
> static void pch_pic_bitset(struct pch_pic *priv, int offset, int bit)
> {
> u32 reg;
> @@ -80,45 +87,47 @@ static void pch_pic_mask_irq(struct irq_data *d)
> {
> struct pch_pic *priv = irq_data_get_irq_chip_data(d);
>
> - pch_pic_bitset(priv, PCH_PIC_MASK, d->hwirq);
> + pch_pic_bitset(priv, PCH_PIC_MASK, hwirq_to_bit(priv, d->hwirq));
> irq_chip_mask_parent(d);
> }
>
> static void pch_pic_unmask_irq(struct irq_data *d)
> {
> struct pch_pic *priv = irq_data_get_irq_chip_data(d);
> + int bit = hwirq_to_bit(priv, d->hwirq);
>
> - writel(BIT(PIC_REG_BIT(d->hwirq)),
> - priv->base + PCH_PIC_CLR + PIC_REG_IDX(d->hwirq) * 4);
> + writel(BIT(PIC_REG_BIT(bit)),
> + priv->base + PCH_PIC_CLR + PIC_REG_IDX(bit) * 4);
>
> irq_chip_unmask_parent(d);
> - pch_pic_bitclr(priv, PCH_PIC_MASK, d->hwirq);
> + pch_pic_bitclr(priv, PCH_PIC_MASK, bit);
> }
>
> static int pch_pic_set_type(struct irq_data *d, unsigned int type)
> {
> struct pch_pic *priv = irq_data_get_irq_chip_data(d);
> + int bit = hwirq_to_bit(priv, d->hwirq);
> int ret = 0;
>
> switch (type) {
> case IRQ_TYPE_EDGE_RISING:
> - pch_pic_bitset(priv, PCH_PIC_EDGE, d->hwirq);
> - pch_pic_bitclr(priv, PCH_PIC_POL, d->hwirq);
> + pch_pic_bitset(priv, PCH_PIC_EDGE, bit);
> + pch_pic_bitclr(priv, PCH_PIC_POL, bit);
> irq_set_handler_locked(d, handle_edge_irq);
> break;
> case IRQ_TYPE_EDGE_FALLING:
> - pch_pic_bitset(priv, PCH_PIC_EDGE, d->hwirq);
> - pch_pic_bitset(priv, PCH_PIC_POL, d->hwirq);
> + pch_pic_bitset(priv, PCH_PIC_EDGE, bit);
> + pch_pic_bitset(priv, PCH_PIC_POL, bit);
> irq_set_handler_locked(d, handle_edge_irq);
> break;
> case IRQ_TYPE_LEVEL_HIGH:
> - pch_pic_bitclr(priv, PCH_PIC_EDGE, d->hwirq);
> - pch_pic_bitclr(priv, PCH_PIC_POL, d->hwirq);
> + pch_pic_bitclr(priv, PCH_PIC_EDGE, bit);
> + pch_pic_bitclr(priv, PCH_PIC_POL, bit);
> irq_set_handler_locked(d, handle_level_irq);
> break;
> case IRQ_TYPE_LEVEL_LOW:
> - pch_pic_bitclr(priv, PCH_PIC_EDGE, d->hwirq);
> - pch_pic_bitset(priv, PCH_PIC_POL, d->hwirq);
> + pch_pic_bitclr(priv, PCH_PIC_EDGE, bit);
> + pch_pic_bitset(priv, PCH_PIC_POL, bit);
> irq_set_handler_locked(d, handle_level_irq);
> break;
> default:
> @@ -133,11 +142,12 @@ static void pch_pic_ack_irq(struct irq_data *d)
> {
> unsigned int reg;
> struct pch_pic *priv = irq_data_get_irq_chip_data(d);
> + int bit = hwirq_to_bit(priv, d->hwirq);
>
> - reg = readl(priv->base + PCH_PIC_EDGE + PIC_REG_IDX(d->hwirq) * 4);
> - if (reg & BIT(PIC_REG_BIT(d->hwirq))) {
> - writel(BIT(PIC_REG_BIT(d->hwirq)),
> - priv->base + PCH_PIC_CLR + PIC_REG_IDX(d->hwirq) * 4);
> + reg = readl(priv->base + PCH_PIC_EDGE + PIC_REG_IDX(bit) * 4);
> + if (reg & BIT(PIC_REG_BIT(bit))) {
> + writel(BIT(PIC_REG_BIT(bit)),
> + priv->base + PCH_PIC_CLR + PIC_REG_IDX(bit) * 4);
> }
> irq_chip_ack_parent(d);
> }
> @@ -159,6 +169,8 @@ static int pch_pic_domain_translate(struct irq_domain *d,
> {
> struct pch_pic *priv = d->host_data;
> struct device_node *of_node = to_of_node(fwspec->fwnode);
> + unsigned long flags;
> + int i;
>
> if (of_node) {
> if (fwspec->param_count < 2)
> @@ -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);
> +
> 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;
> @@ -222,7 +258,7 @@ static void pch_pic_reset(struct pch_pic *priv)
>
> for (i = 0; i < PIC_COUNT; i++) {
> /* Write vector ID */
> - writeb(priv->ht_vec_base + i, priv->base + PCH_INT_HTVEC(i));
> + writeb(priv->ht_vec_base + i, priv->base + PCH_INT_HTVEC(hwirq_to_bit(priv, i)));
> /* Hardcode route to HT0 Lo */
> writeb(1, priv->base + PCH_INT_ROUTE(i));
> }
> @@ -284,6 +320,7 @@ static int pch_pic_init(phys_addr_t addr, unsigned long size, int vec_base,
> u32 gsi_base)
> {
> struct pch_pic *priv;
> + int i;
>
> priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> if (!priv)
> @@ -294,6 +331,10 @@ static int pch_pic_init(phys_addr_t addr, unsigned long size, int vec_base,
> if (!priv->base)
> goto free_priv;
>
> + priv->inuse = 0;
> + for (i = 0; i < PIC_COUNT; i++)
> + priv->table[i] = PIC_UNDEF_VECTOR;
> +
> priv->ht_vec_base = vec_base;
> priv->vec_count = ((readq(priv->base) >> 48) & 0xff) + 1;
> priv->gsi_base = gsi_base;
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] irqchip/loongson-pch-pic: Update interrupt registration policy
2024-03-18 14:35 ` Huacai Chen
@ 2024-03-19 11:18 ` Tianyang Zhang
0 siblings, 0 replies; 8+ messages in thread
From: Tianyang Zhang @ 2024-03-19 11:18 UTC (permalink / raw)
To: Huacai Chen
Cc: jiaxun.yang, tglx, linux-mips, linux-kernel, Baoqi Zhang, Biao Dong
Hi, Huacai
Thanks for your suggestions and corrections. I will make the
modifications as required
and provide the next version of the patch as soon as possible
Tianyang
在 2024/3/18 下午10:35, Huacai Chen 写道:
> Hi, Tianyang,
>
> On Sat, Mar 16, 2024 at 4:22 PM Tianyang Zhang
> <zhangtianyang@loongson.cn> wrote:
>> From: Baoqi Zhang <zhangbaoqi@loongson.cn>
>>
>> This patch remove the fixed mapping between the 7A interrupt source
>> and the HT interrupt vector, and replaced it with a dynamically
>> allocated approach.
> Use LS7A instead of 7A here.
>
>> 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
> Missing a . at the end.
>
>> This will be more conducive to fully utilizing existing vectors to
>> support more devices
>>
>> Signed-off-by: Baoqi Zhang <zhangbaoqi@loongson.cn>
>> Signed-off-by: Biao Dong <dongbiao@loongson.cn>
>> Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn>
>> ---
>> drivers/irqchip/irq-loongson-pch-pic.c | 77 ++++++++++++++++++++------
>> 1 file changed, 59 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-loongson-pch-pic.c b/drivers/irqchip/irq-loongson-pch-pic.c
>> index 63db8e2172e0..f17187641154 100644
>> --- a/drivers/irqchip/irq-loongson-pch-pic.c
>> +++ b/drivers/irqchip/irq-loongson-pch-pic.c
>> @@ -33,7 +33,7 @@
>> #define PIC_COUNT (PIC_COUNT_PER_REG * PIC_REG_COUNT)
>> #define PIC_REG_IDX(irq_id) ((irq_id) / PIC_COUNT_PER_REG)
>> #define PIC_REG_BIT(irq_id) ((irq_id) % PIC_COUNT_PER_REG)
>> -
>> +#define PIC_UNDEF_VECTOR 255
> Keep a new line after the macro, please.
>
> Huacai
>
>> static int nr_pics;
>>
>> struct pch_pic {
>> @@ -46,12 +46,19 @@ struct pch_pic {
>> u32 saved_vec_en[PIC_REG_COUNT];
>> u32 saved_vec_pol[PIC_REG_COUNT];
>> u32 saved_vec_edge[PIC_REG_COUNT];
>> + u8 table[PIC_COUNT];
>> + int inuse;
>> };
>>
>> static struct pch_pic *pch_pic_priv[MAX_IO_PICS];
>>
>> struct fwnode_handle *pch_pic_handle[MAX_IO_PICS];
>>
>> +static inline u8 hwirq_to_bit(struct pch_pic *priv, int hirq)
>> +{
>> + return priv->table[hirq];
>> +}
>> +
>> static void pch_pic_bitset(struct pch_pic *priv, int offset, int bit)
>> {
>> u32 reg;
>> @@ -80,45 +87,47 @@ static void pch_pic_mask_irq(struct irq_data *d)
>> {
>> struct pch_pic *priv = irq_data_get_irq_chip_data(d);
>>
>> - pch_pic_bitset(priv, PCH_PIC_MASK, d->hwirq);
>> + pch_pic_bitset(priv, PCH_PIC_MASK, hwirq_to_bit(priv, d->hwirq));
>> irq_chip_mask_parent(d);
>> }
>>
>> static void pch_pic_unmask_irq(struct irq_data *d)
>> {
>> struct pch_pic *priv = irq_data_get_irq_chip_data(d);
>> + int bit = hwirq_to_bit(priv, d->hwirq);
>>
>> - writel(BIT(PIC_REG_BIT(d->hwirq)),
>> - priv->base + PCH_PIC_CLR + PIC_REG_IDX(d->hwirq) * 4);
>> + writel(BIT(PIC_REG_BIT(bit)),
>> + priv->base + PCH_PIC_CLR + PIC_REG_IDX(bit) * 4);
>>
>> irq_chip_unmask_parent(d);
>> - pch_pic_bitclr(priv, PCH_PIC_MASK, d->hwirq);
>> + pch_pic_bitclr(priv, PCH_PIC_MASK, bit);
>> }
>>
>> static int pch_pic_set_type(struct irq_data *d, unsigned int type)
>> {
>> struct pch_pic *priv = irq_data_get_irq_chip_data(d);
>> + int bit = hwirq_to_bit(priv, d->hwirq);
>> int ret = 0;
>>
>> switch (type) {
>> case IRQ_TYPE_EDGE_RISING:
>> - pch_pic_bitset(priv, PCH_PIC_EDGE, d->hwirq);
>> - pch_pic_bitclr(priv, PCH_PIC_POL, d->hwirq);
>> + pch_pic_bitset(priv, PCH_PIC_EDGE, bit);
>> + pch_pic_bitclr(priv, PCH_PIC_POL, bit);
>> irq_set_handler_locked(d, handle_edge_irq);
>> break;
>> case IRQ_TYPE_EDGE_FALLING:
>> - pch_pic_bitset(priv, PCH_PIC_EDGE, d->hwirq);
>> - pch_pic_bitset(priv, PCH_PIC_POL, d->hwirq);
>> + pch_pic_bitset(priv, PCH_PIC_EDGE, bit);
>> + pch_pic_bitset(priv, PCH_PIC_POL, bit);
>> irq_set_handler_locked(d, handle_edge_irq);
>> break;
>> case IRQ_TYPE_LEVEL_HIGH:
>> - pch_pic_bitclr(priv, PCH_PIC_EDGE, d->hwirq);
>> - pch_pic_bitclr(priv, PCH_PIC_POL, d->hwirq);
>> + pch_pic_bitclr(priv, PCH_PIC_EDGE, bit);
>> + pch_pic_bitclr(priv, PCH_PIC_POL, bit);
>> irq_set_handler_locked(d, handle_level_irq);
>> break;
>> case IRQ_TYPE_LEVEL_LOW:
>> - pch_pic_bitclr(priv, PCH_PIC_EDGE, d->hwirq);
>> - pch_pic_bitset(priv, PCH_PIC_POL, d->hwirq);
>> + pch_pic_bitclr(priv, PCH_PIC_EDGE, bit);
>> + pch_pic_bitset(priv, PCH_PIC_POL, bit);
>> irq_set_handler_locked(d, handle_level_irq);
>> break;
>> default:
>> @@ -133,11 +142,12 @@ static void pch_pic_ack_irq(struct irq_data *d)
>> {
>> unsigned int reg;
>> struct pch_pic *priv = irq_data_get_irq_chip_data(d);
>> + int bit = hwirq_to_bit(priv, d->hwirq);
>>
>> - reg = readl(priv->base + PCH_PIC_EDGE + PIC_REG_IDX(d->hwirq) * 4);
>> - if (reg & BIT(PIC_REG_BIT(d->hwirq))) {
>> - writel(BIT(PIC_REG_BIT(d->hwirq)),
>> - priv->base + PCH_PIC_CLR + PIC_REG_IDX(d->hwirq) * 4);
>> + reg = readl(priv->base + PCH_PIC_EDGE + PIC_REG_IDX(bit) * 4);
>> + if (reg & BIT(PIC_REG_BIT(bit))) {
>> + writel(BIT(PIC_REG_BIT(bit)),
>> + priv->base + PCH_PIC_CLR + PIC_REG_IDX(bit) * 4);
>> }
>> irq_chip_ack_parent(d);
>> }
>> @@ -159,6 +169,8 @@ static int pch_pic_domain_translate(struct irq_domain *d,
>> {
>> struct pch_pic *priv = d->host_data;
>> struct device_node *of_node = to_of_node(fwspec->fwnode);
>> + unsigned long flags;
>> + int i;
>>
>> if (of_node) {
>> if (fwspec->param_count < 2)
>> @@ -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);
>> +
>> 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;
>> @@ -222,7 +258,7 @@ static void pch_pic_reset(struct pch_pic *priv)
>>
>> for (i = 0; i < PIC_COUNT; i++) {
>> /* Write vector ID */
>> - writeb(priv->ht_vec_base + i, priv->base + PCH_INT_HTVEC(i));
>> + writeb(priv->ht_vec_base + i, priv->base + PCH_INT_HTVEC(hwirq_to_bit(priv, i)));
>> /* Hardcode route to HT0 Lo */
>> writeb(1, priv->base + PCH_INT_ROUTE(i));
>> }
>> @@ -284,6 +320,7 @@ static int pch_pic_init(phys_addr_t addr, unsigned long size, int vec_base,
>> u32 gsi_base)
>> {
>> struct pch_pic *priv;
>> + int i;
>>
>> priv = kzalloc(sizeof(*priv), GFP_KERNEL);
>> if (!priv)
>> @@ -294,6 +331,10 @@ static int pch_pic_init(phys_addr_t addr, unsigned long size, int vec_base,
>> if (!priv->base)
>> goto free_priv;
>>
>> + priv->inuse = 0;
>> + for (i = 0; i < PIC_COUNT; i++)
>> + priv->table[i] = PIC_UNDEF_VECTOR;
>> +
>> priv->ht_vec_base = vec_base;
>> priv->vec_count = ((readq(priv->base) >> 48) & 0xff) + 1;
>> priv->gsi_base = gsi_base;
>> --
>> 2.20.1
>>
>>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] irqchip/loongson-pch-pic: Update interrupt registration policy
2024-03-16 14:03 ` Christophe JAILLET
2024-03-18 11:18 ` Thomas Gleixner
@ 2024-03-19 11:26 ` Tianyang Zhang
1 sibling, 0 replies; 8+ messages in thread
From: Tianyang Zhang @ 2024-03-19 11:26 UTC (permalink / raw)
To: Christophe JAILLET, chenhuacai, jiaxun.yang, tglx
Cc: linux-mips, linux-kernel, Baoqi Zhang, Biao Dong
Hi Christophe
This part of the code is not a hotspot path. In fact, it may only be
executed once at startup,
so some optimizations that we consider extreme can be omitted
Tianyang
在 2024/3/16 下午10:03, Christophe JAILLET 写道:
> Le 16/03/2024 à 09:21, Tianyang Zhang a écrit :
>> From: Baoqi Zhang <zhangbaoqi@loongson.cn>
>>
>> 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@loongson.cn>
>> Signed-off-by: Biao Dong <dongbiao@loongson.cn>
>> Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn>
>> ---
>> 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;
>
> ...
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] irqchip/loongson-pch-pic: Update interrupt registration policy
2024-03-16 8:21 [PATCH V2] irqchip/loongson-pch-pic: Update interrupt registration policy Tianyang Zhang
2024-03-16 14:03 ` Christophe JAILLET
2024-03-18 14:35 ` Huacai Chen
@ 2024-03-21 6:10 ` Binbin Zhou
2024-03-22 10:16 ` Tianyang Zhang
2 siblings, 1 reply; 8+ messages in thread
From: Binbin Zhou @ 2024-03-21 6:10 UTC (permalink / raw)
To: Tianyang Zhang, chenhuacai, jiaxun.yang, tglx
Cc: linux-mips, linux-kernel, Baoqi Zhang, Biao Dong, zhoubb.aaron
Hi Tianyang:
On 2024/3/16 16:21, Tianyang Zhang wrote:
> From: Baoqi Zhang <zhangbaoqi@loongson.cn>
>
> 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@loongson.cn>
> Signed-off-by: Biao Dong <dongbiao@loongson.cn>
> Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn>
> ---
> drivers/irqchip/irq-loongson-pch-pic.c | 77 ++++++++++++++++++++------
> 1 file changed, 59 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/irqchip/irq-loongson-pch-pic.c b/drivers/irqchip/irq-loongson-pch-pic.c
> index 63db8e2172e0..f17187641154 100644
> --- a/drivers/irqchip/irq-loongson-pch-pic.c
> +++ b/drivers/irqchip/irq-loongson-pch-pic.c
> @@ -33,7 +33,7 @@
> #define PIC_COUNT (PIC_COUNT_PER_REG * PIC_REG_COUNT)
> #define PIC_REG_IDX(irq_id) ((irq_id) / PIC_COUNT_PER_REG)
> #define PIC_REG_BIT(irq_id) ((irq_id) % PIC_COUNT_PER_REG)
> -
> +#define PIC_UNDEF_VECTOR 255
> static int nr_pics;
>
> struct pch_pic {
> @@ -46,12 +46,19 @@ struct pch_pic {
> u32 saved_vec_en[PIC_REG_COUNT];
> u32 saved_vec_pol[PIC_REG_COUNT];
> u32 saved_vec_edge[PIC_REG_COUNT];
> + u8 table[PIC_COUNT];
> + int inuse;
> };
>
> static struct pch_pic *pch_pic_priv[MAX_IO_PICS];
>
> struct fwnode_handle *pch_pic_handle[MAX_IO_PICS];
>
> +static inline u8 hwirq_to_bit(struct pch_pic *priv, int hirq)
> +{
> + return priv->table[hirq];
> +}
> +
> static void pch_pic_bitset(struct pch_pic *priv, int offset, int bit)
> {
> u32 reg;
> @@ -80,45 +87,47 @@ static void pch_pic_mask_irq(struct irq_data *d)
> {
> struct pch_pic *priv = irq_data_get_irq_chip_data(d);
>
> - pch_pic_bitset(priv, PCH_PIC_MASK, d->hwirq);
> + pch_pic_bitset(priv, PCH_PIC_MASK, hwirq_to_bit(priv, d->hwirq));
> irq_chip_mask_parent(d);
> }
>
> static void pch_pic_unmask_irq(struct irq_data *d)
> {
> struct pch_pic *priv = irq_data_get_irq_chip_data(d);
> + int bit = hwirq_to_bit(priv, d->hwirq);
>
> - writel(BIT(PIC_REG_BIT(d->hwirq)),
> - priv->base + PCH_PIC_CLR + PIC_REG_IDX(d->hwirq) * 4);
> + writel(BIT(PIC_REG_BIT(bit)),
> + priv->base + PCH_PIC_CLR + PIC_REG_IDX(bit) * 4);
>
> irq_chip_unmask_parent(d);
> - pch_pic_bitclr(priv, PCH_PIC_MASK, d->hwirq);
> + pch_pic_bitclr(priv, PCH_PIC_MASK, bit);
> }
>
> static int pch_pic_set_type(struct irq_data *d, unsigned int type)
> {
> struct pch_pic *priv = irq_data_get_irq_chip_data(d);
> + int bit = hwirq_to_bit(priv, d->hwirq);
> int ret = 0;
>
> switch (type) {
> case IRQ_TYPE_EDGE_RISING:
> - pch_pic_bitset(priv, PCH_PIC_EDGE, d->hwirq);
> - pch_pic_bitclr(priv, PCH_PIC_POL, d->hwirq);
> + pch_pic_bitset(priv, PCH_PIC_EDGE, bit);
> + pch_pic_bitclr(priv, PCH_PIC_POL, bit);
> irq_set_handler_locked(d, handle_edge_irq);
> break;
> case IRQ_TYPE_EDGE_FALLING:
> - pch_pic_bitset(priv, PCH_PIC_EDGE, d->hwirq);
> - pch_pic_bitset(priv, PCH_PIC_POL, d->hwirq);
> + pch_pic_bitset(priv, PCH_PIC_EDGE, bit);
> + pch_pic_bitset(priv, PCH_PIC_POL, bit);
> irq_set_handler_locked(d, handle_edge_irq);
> break;
> case IRQ_TYPE_LEVEL_HIGH:
> - pch_pic_bitclr(priv, PCH_PIC_EDGE, d->hwirq);
> - pch_pic_bitclr(priv, PCH_PIC_POL, d->hwirq);
> + pch_pic_bitclr(priv, PCH_PIC_EDGE, bit);
> + pch_pic_bitclr(priv, PCH_PIC_POL, bit);
> irq_set_handler_locked(d, handle_level_irq);
> break;
> case IRQ_TYPE_LEVEL_LOW:
> - pch_pic_bitclr(priv, PCH_PIC_EDGE, d->hwirq);
> - pch_pic_bitset(priv, PCH_PIC_POL, d->hwirq);
> + pch_pic_bitclr(priv, PCH_PIC_EDGE, bit);
> + pch_pic_bitset(priv, PCH_PIC_POL, bit);
> irq_set_handler_locked(d, handle_level_irq);
> break;
> default:
> @@ -133,11 +142,12 @@ static void pch_pic_ack_irq(struct irq_data *d)
> {
> unsigned int reg;
> struct pch_pic *priv = irq_data_get_irq_chip_data(d);
> + int bit = hwirq_to_bit(priv, d->hwirq);
>
> - reg = readl(priv->base + PCH_PIC_EDGE + PIC_REG_IDX(d->hwirq) * 4);
> - if (reg & BIT(PIC_REG_BIT(d->hwirq))) {
> - writel(BIT(PIC_REG_BIT(d->hwirq)),
> - priv->base + PCH_PIC_CLR + PIC_REG_IDX(d->hwirq) * 4);
> + reg = readl(priv->base + PCH_PIC_EDGE + PIC_REG_IDX(bit) * 4);
> + if (reg & BIT(PIC_REG_BIT(bit))) {
> + writel(BIT(PIC_REG_BIT(bit)),
> + priv->base + PCH_PIC_CLR + PIC_REG_IDX(bit) * 4);
> }
> irq_chip_ack_parent(d);
> }
> @@ -159,6 +169,8 @@ static int pch_pic_domain_translate(struct irq_domain *d,
> {
> struct pch_pic *priv = d->host_data;
> struct device_node *of_node = to_of_node(fwspec->fwnode);
> + unsigned long flags;
> + int i;
>
> if (of_node) {
> if (fwspec->param_count < 2)
> @@ -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);
> +
The patch fails the test on LS2K2000+FDT and does not boot the system
properly.
The reason the test fails is that this part of the priv->table[]
initialization is needed for FDT as well, so I think it needs to be put
after the whole judgment, at the end of the function.
Thanks.
Binbin
> 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;
> @@ -222,7 +258,7 @@ static void pch_pic_reset(struct pch_pic *priv)
>
> for (i = 0; i < PIC_COUNT; i++) {
> /* Write vector ID */
> - writeb(priv->ht_vec_base + i, priv->base + PCH_INT_HTVEC(i));
> + writeb(priv->ht_vec_base + i, priv->base + PCH_INT_HTVEC(hwirq_to_bit(priv, i)));
> /* Hardcode route to HT0 Lo */
> writeb(1, priv->base + PCH_INT_ROUTE(i));
> }
> @@ -284,6 +320,7 @@ static int pch_pic_init(phys_addr_t addr, unsigned long size, int vec_base,
> u32 gsi_base)
> {
> struct pch_pic *priv;
> + int i;
>
> priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> if (!priv)
> @@ -294,6 +331,10 @@ static int pch_pic_init(phys_addr_t addr, unsigned long size, int vec_base,
> if (!priv->base)
> goto free_priv;
>
> + priv->inuse = 0;
> + for (i = 0; i < PIC_COUNT; i++)
> + priv->table[i] = PIC_UNDEF_VECTOR;
> +
> priv->ht_vec_base = vec_base;
> priv->vec_count = ((readq(priv->base) >> 48) & 0xff) + 1;
> priv->gsi_base = gsi_base;
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] irqchip/loongson-pch-pic: Update interrupt registration policy
2024-03-21 6:10 ` [PATCH v2] " Binbin Zhou
@ 2024-03-22 10:16 ` Tianyang Zhang
0 siblings, 0 replies; 8+ messages in thread
From: Tianyang Zhang @ 2024-03-22 10:16 UTC (permalink / raw)
To: Binbin Zhou, chenhuacai, jiaxun.yang, tglx
Cc: linux-mips, linux-kernel, Baoqi Zhang, Biao Dong, zhoubb.aaron
Hi,Binbin
Thank you for pointing out the problems in this patch. We will adjust
the scheme after the test
Thanks again
在 2024/3/21 下午2:10, Binbin Zhou 写道:
> Hi Tianyang:
>
> On 2024/3/16 16:21, Tianyang Zhang wrote:
>> From: Baoqi Zhang <zhangbaoqi@loongson.cn>
>>
>> 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@loongson.cn>
>> Signed-off-by: Biao Dong <dongbiao@loongson.cn>
>> Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn>
>> ---
>> drivers/irqchip/irq-loongson-pch-pic.c | 77 ++++++++++++++++++++------
>> 1 file changed, 59 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-loongson-pch-pic.c
>> b/drivers/irqchip/irq-loongson-pch-pic.c
>> index 63db8e2172e0..f17187641154 100644
>> --- a/drivers/irqchip/irq-loongson-pch-pic.c
>> +++ b/drivers/irqchip/irq-loongson-pch-pic.c
>> @@ -33,7 +33,7 @@
>> #define PIC_COUNT (PIC_COUNT_PER_REG * PIC_REG_COUNT)
>> #define PIC_REG_IDX(irq_id) ((irq_id) / PIC_COUNT_PER_REG)
>> #define PIC_REG_BIT(irq_id) ((irq_id) % PIC_COUNT_PER_REG)
>> -
>> +#define PIC_UNDEF_VECTOR 255
>> static int nr_pics;
>> struct pch_pic {
>> @@ -46,12 +46,19 @@ struct pch_pic {
>> u32 saved_vec_en[PIC_REG_COUNT];
>> u32 saved_vec_pol[PIC_REG_COUNT];
>> u32 saved_vec_edge[PIC_REG_COUNT];
>> + u8 table[PIC_COUNT];
>> + int inuse;
>> };
>> static struct pch_pic *pch_pic_priv[MAX_IO_PICS];
>> struct fwnode_handle *pch_pic_handle[MAX_IO_PICS];
>> +static inline u8 hwirq_to_bit(struct pch_pic *priv, int hirq)
>> +{
>> + return priv->table[hirq];
>> +}
>> +
>> static void pch_pic_bitset(struct pch_pic *priv, int offset, int bit)
>> {
>> u32 reg;
>> @@ -80,45 +87,47 @@ static void pch_pic_mask_irq(struct irq_data *d)
>> {
>> struct pch_pic *priv = irq_data_get_irq_chip_data(d);
>> - pch_pic_bitset(priv, PCH_PIC_MASK, d->hwirq);
>> + pch_pic_bitset(priv, PCH_PIC_MASK, hwirq_to_bit(priv, d->hwirq));
>> irq_chip_mask_parent(d);
>> }
>> static void pch_pic_unmask_irq(struct irq_data *d)
>> {
>> struct pch_pic *priv = irq_data_get_irq_chip_data(d);
>> + int bit = hwirq_to_bit(priv, d->hwirq);
>> - writel(BIT(PIC_REG_BIT(d->hwirq)),
>> - priv->base + PCH_PIC_CLR + PIC_REG_IDX(d->hwirq) * 4);
>> + writel(BIT(PIC_REG_BIT(bit)),
>> + priv->base + PCH_PIC_CLR + PIC_REG_IDX(bit) * 4);
>> irq_chip_unmask_parent(d);
>> - pch_pic_bitclr(priv, PCH_PIC_MASK, d->hwirq);
>> + pch_pic_bitclr(priv, PCH_PIC_MASK, bit);
>> }
>> static int pch_pic_set_type(struct irq_data *d, unsigned int type)
>> {
>> struct pch_pic *priv = irq_data_get_irq_chip_data(d);
>> + int bit = hwirq_to_bit(priv, d->hwirq);
>> int ret = 0;
>> switch (type) {
>> case IRQ_TYPE_EDGE_RISING:
>> - pch_pic_bitset(priv, PCH_PIC_EDGE, d->hwirq);
>> - pch_pic_bitclr(priv, PCH_PIC_POL, d->hwirq);
>> + pch_pic_bitset(priv, PCH_PIC_EDGE, bit);
>> + pch_pic_bitclr(priv, PCH_PIC_POL, bit);
>> irq_set_handler_locked(d, handle_edge_irq);
>> break;
>> case IRQ_TYPE_EDGE_FALLING:
>> - pch_pic_bitset(priv, PCH_PIC_EDGE, d->hwirq);
>> - pch_pic_bitset(priv, PCH_PIC_POL, d->hwirq);
>> + pch_pic_bitset(priv, PCH_PIC_EDGE, bit);
>> + pch_pic_bitset(priv, PCH_PIC_POL, bit);
>> irq_set_handler_locked(d, handle_edge_irq);
>> break;
>> case IRQ_TYPE_LEVEL_HIGH:
>> - pch_pic_bitclr(priv, PCH_PIC_EDGE, d->hwirq);
>> - pch_pic_bitclr(priv, PCH_PIC_POL, d->hwirq);
>> + pch_pic_bitclr(priv, PCH_PIC_EDGE, bit);
>> + pch_pic_bitclr(priv, PCH_PIC_POL, bit);
>> irq_set_handler_locked(d, handle_level_irq);
>> break;
>> case IRQ_TYPE_LEVEL_LOW:
>> - pch_pic_bitclr(priv, PCH_PIC_EDGE, d->hwirq);
>> - pch_pic_bitset(priv, PCH_PIC_POL, d->hwirq);
>> + pch_pic_bitclr(priv, PCH_PIC_EDGE, bit);
>> + pch_pic_bitset(priv, PCH_PIC_POL, bit);
>> irq_set_handler_locked(d, handle_level_irq);
>> break;
>> default:
>> @@ -133,11 +142,12 @@ static void pch_pic_ack_irq(struct irq_data *d)
>> {
>> unsigned int reg;
>> struct pch_pic *priv = irq_data_get_irq_chip_data(d);
>> + int bit = hwirq_to_bit(priv, d->hwirq);
>> - reg = readl(priv->base + PCH_PIC_EDGE + PIC_REG_IDX(d->hwirq)
>> * 4);
>> - if (reg & BIT(PIC_REG_BIT(d->hwirq))) {
>> - writel(BIT(PIC_REG_BIT(d->hwirq)),
>> - priv->base + PCH_PIC_CLR + PIC_REG_IDX(d->hwirq) * 4);
>> + reg = readl(priv->base + PCH_PIC_EDGE + PIC_REG_IDX(bit) * 4);
>> + if (reg & BIT(PIC_REG_BIT(bit))) {
>> + writel(BIT(PIC_REG_BIT(bit)),
>> + priv->base + PCH_PIC_CLR + PIC_REG_IDX(bit) * 4);
>> }
>> irq_chip_ack_parent(d);
>> }
>> @@ -159,6 +169,8 @@ static int pch_pic_domain_translate(struct
>> irq_domain *d,
>> {
>> struct pch_pic *priv = d->host_data;
>> struct device_node *of_node = to_of_node(fwspec->fwnode);
>> + unsigned long flags;
>> + int i;
>> if (of_node) {
>> if (fwspec->param_count < 2)
>> @@ -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);
>> +
>
> The patch fails the test on LS2K2000+FDT and does not boot the system
> properly.
>
> The reason the test fails is that this part of the priv->table[]
> initialization is needed for FDT as well, so I think it needs to be
> put after the whole judgment, at the end of the function.
>
>
> Thanks.
>
> Binbin
>
>> 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;
>> @@ -222,7 +258,7 @@ static void pch_pic_reset(struct pch_pic *priv)
>> for (i = 0; i < PIC_COUNT; i++) {
>> /* Write vector ID */
>> - writeb(priv->ht_vec_base + i, priv->base + PCH_INT_HTVEC(i));
>> + writeb(priv->ht_vec_base + i, priv->base +
>> PCH_INT_HTVEC(hwirq_to_bit(priv, i)));
>> /* Hardcode route to HT0 Lo */
>> writeb(1, priv->base + PCH_INT_ROUTE(i));
>> }
>> @@ -284,6 +320,7 @@ static int pch_pic_init(phys_addr_t addr,
>> unsigned long size, int vec_base,
>> u32 gsi_base)
>> {
>> struct pch_pic *priv;
>> + int i;
>> priv = kzalloc(sizeof(*priv), GFP_KERNEL);
>> if (!priv)
>> @@ -294,6 +331,10 @@ static int pch_pic_init(phys_addr_t addr,
>> unsigned long size, int vec_base,
>> if (!priv->base)
>> goto free_priv;
>> + priv->inuse = 0;
>> + for (i = 0; i < PIC_COUNT; i++)
>> + priv->table[i] = PIC_UNDEF_VECTOR;
>> +
>> priv->ht_vec_base = vec_base;
>> priv->vec_count = ((readq(priv->base) >> 48) & 0xff) + 1;
>> priv->gsi_base = gsi_base;
>>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-03-22 10:16 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-16 8:21 [PATCH V2] irqchip/loongson-pch-pic: Update interrupt registration policy Tianyang Zhang
2024-03-16 14:03 ` Christophe JAILLET
2024-03-18 11:18 ` Thomas Gleixner
2024-03-19 11:26 ` Tianyang Zhang
2024-03-18 14:35 ` Huacai Chen
2024-03-19 11:18 ` Tianyang Zhang
2024-03-21 6:10 ` [PATCH v2] " Binbin Zhou
2024-03-22 10:16 ` Tianyang Zhang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.