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@xxxxxxxxxxx/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
