Re: [PATCH v4 0/2] Loongarch irq-redirect supprot

[Tianyang Zhang <zhangtianyang@loongson.cn>]

Hi, Huacai

在 2025/6/10 下午7:54, Huacai Chen 写道:
> Hi, Tianyang,
>
> Have you received my comments in V3?
> https://lore.kernel.org/loongarch/20250523101833.17940-1-zhangtianyang@loongson.cn/T/#m2883f379ce7eb663f3f3eb4736bf9b071c7fd8ab

After a few days of effort, I did not let that email reappear in my 
mailbox... so I am replying to the above email here

* Re: [PATCH v3 2/2] irq/irq-loongarch-ir:Add Redirect irqchip support
   2025-05-23 10:18 ` [PATCH v3 2/2] irq/irq-loongarch-ir:Add Redirect 
irqchip support Tianyang Zhang
@ 2025-05-24 14:12   ` Huacai Chen
   2025-05-25  9:06   ` Thomas Gleixner
   1 sibling, 0 replies; 7+ messages in thread
From: Huacai Chen @ 2025-05-24 14:12 UTC (permalink / raw)
   To: Tianyang Zhang
   Cc: kernel, corbet, alexs, si.yanteng, tglx, jiaxun.yang, peterz,
     wangliupu, lvjianmin, maobibo, siyanteng, gaosong, yangtiezhu,
     loongarch, linux-doc, linux-kernel

 >> +
 >> +#define REDIRECT_REG_BASE(reg, node) \
 >> +       (UNCACHE_BASE | redirect_reg_base | (u64)(node) << 
NODE_ADDRSPACE_SHIFT | (reg))
 > IO_BASE is a little better than UNCACHE_BASE.

yes, it is batter, thanks

 >> +#define        redirect_reg_queue_head(node) 
REDIRECT_REG_BASE(LOONGARCH_IOCSR_REDIRECT_CQH, (node))
 >> +#define        redirect_reg_queue_tail(node) 
REDIRECT_REG_BASE(LOONGARCH_IOCSR_REDIRECT_CQT, (node))
 >> +#define read_queue_head(node)          (*((u32 
*)(redirect_reg_queue_head(node))))
 >> +#define read_queue_tail(node)          (*((u32 
*)(redirect_reg_queue_tail(node))))
 >> +#define write_queue_tail(node, val)    (*((u32 
*)(redirect_reg_queue_tail(node))) = (val))
 >You can use readl() and writel() directly, then you can remove the 
memory barrier around write_queue_tail().

OK , It is realy a good idea.thanks

 >> +static void irde_invlid_entry_node(struct redirect_item *item)
 > s/irde_invlid_entry_node/irde_invalid_entry_node/g

OK , thanks

 >> +       avecintc_sync(adata);
 >> +
 >> +       return IRQ_SET_MASK_OK;
 >> +}
 > Have you tried to build with no SMP? This function (and maybe more) 
should be guarded by CONFIG_SMP.

I did it at the beginning... Okay, I will supplement this test in the 
current version

 >> +static int __init redirect_reg_base_init(void)
 >> +{
 >> +       acpi_status status;
 >> +       uint64_t addr = 0;
 >> +
 >> +       if (acpi_disabled)
 >> +               return 0;
 >> +
 >> +       status = acpi_evaluate_integer(NULL, "\\_SB.NO00", NULL, &addr);
 >> +       if (ACPI_FAILURE(status) || !addr)
 >> +               pr_info("redirect_iocsr_base used default 
0x1fe00000\n");
 >> +       else
 >> +               redirect_reg_base = addr;
 >> +
 >> +       return 0;
 >> +}
 >> +subsys_initcall_sync(redirect_reg_base_init);
 >Can this function be put at the end of redirect_acpi_init()? It is too
 >late in an initcall() function because the irqchip drivers begin to
 >work before that.
Ok I got it , thanks

Tianyang