Re: [PATCH] irqchip/mbigen: Fix mbigen node address layout

[Yipeng Zou <zouyipeng@huawei.com>]

Hi Thomas:

     Reword it in v2.

https://lore.kernel.org/all/20240730014400.1751530-1-zouyipeng@huawei.com/T/#u

在 2024/7/29 18:15, Thomas Gleixner 写道:
> On Sat, Jul 20 2024 at 09:35, Yipeng Zou wrote:
>> Mbigen chip contains several mbigen nodes, and mapped address space per
>> nodes one by one.
>>
>>                      mbigen chip
>>         |-----------------|------------|--------------|
>>     mgn_node_0         mgn_node_1     ...         mgn_node_i
>> |--------------|   |--------------|       |----------------------|
>> [0x0000, 0x1000)   [0x1000, 0x2000)       [i*0x1000, (i+1)*0x1000)
>>
>> Mbigen also defined a clear register with all other mbigen nodes in
>> uniform address space.
>>
>>                           mbigen chip
>>      |-----------|--------|--------|---------------|--------|
>> mgn_node_0  mgn_node_1  ...  mgn_clear_register  ...   mgn_node_i
>>                              |-----------------|
>>                               [0xA000, 0xB000)
>>
>> Everything is OK for now, when the mbigen nodes number less than 10,
>> there is no conflict with clear register.
>>
>> Once we defined mbigen node more than 10, it's going to touch clear
>> register in unexpected way.
>>
>> There should have a gap of 0x1000 between mgn_node9 and mgn_node10.
>>
>> The simplest solution is directly skip clear register when access to
>> more than 10 mbigen nodes.
> I see what you are trying to tell. Something like this makes it more
> clear:
>
>     The mbigen interrupt chip has its per node registers located in a
>     contiguous region of page sized chunks. The code maps them into
>     virtual address space as a contiguous region and determines the
>     address of a node by using the node ID as index.
>
>     This works correctly up to 10 nodes, but then fails because the
>     11th's array slot is used for the MGN_CLEAR registers.
>
>     Skip the MGN_CLEAR register space when calculating the offset for
>     node IDs greater or equal ten.
>
> Hmm?
>
>> Fixes: a6c2f87b8820 ("irqchip/mbigen: Implement the mbigen irq chip operation functions")
>> Signed-off-by: Yipeng Zou <zouyipeng@huawei.com>
>> ---
>>   drivers/irqchip/irq-mbigen.c | 18 ++++++++++++++++--
>>   1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
>> index 58881d313979..b600637f5cd7 100644
>> --- a/drivers/irqchip/irq-mbigen.c
>> +++ b/drivers/irqchip/irq-mbigen.c
>> @@ -64,6 +64,20 @@ struct mbigen_device {
>>   	void __iomem		*base;
>>   };
>>   
>> +static inline unsigned int get_mbigen_node_offset(unsigned int nid)
>> +{
>> +	unsigned int offset = nid * MBIGEN_NODE_OFFSET;
>> +
>> +	/**
> This is not a kernel doc comment. Please use '/*'
>
>> +	 * To avoid touched clear register in unexpected way, we need to directly
>> +	 * skip clear register when access to more than 10 mbigen nodes.
>> +	 */
>> @@ -72,7 +86,7 @@ static inline unsigned int get_mbigen_vec_reg(irq_hw_number_t hwirq)
>>   	nid = hwirq / IRQS_PER_MBIGEN_NODE + 1;
>>   	pin = hwirq % IRQS_PER_MBIGEN_NODE;
>>   
>> -	return pin * 4 + nid * MBIGEN_NODE_OFFSET
>> +	return pin * 4 + get_mbigen_node_offset(nid)
>>   			+ REG_MBIGEN_VEC_OFFSET;
> Please get rid of these pointless line breaks.
>
> Thanks,
>
>          tglx

-- 
Regards,
Yipeng Zou