Re: [PATCH] mailbox: pcc: Fix can't clear level interrupt of type3 in cornor case

["lihuisong (C)" <lihuisong@huawei.com>]

在 2025/3/1 0:32, Sudeep Holla 写道:
> On Thu, Feb 27, 2025 at 03:23:41PM +0800, Huisong Li wrote:
>> The mbox_chan_received_data() will call Rx callback of mbox client driver
>> using type3 to set the flag of command completion. Then driver can continue
>> to do something like sending a new command. In this case, the rest of the
>> interrupt handler function may be concurrent with pcc_send_data().
>>
> Understood and valid issue/bug.
>
>> The 'chan_in_use' flag of a channel is true after sending a command. And
>> the flag of the new command may be cleared by the running interrupt handler
>> in cornor case. As a result, the interrupt being level triggered can't be
>> cleared in pcc_mbox_irq() and it will be disabled after the number of
>> handled times exceeds the specified value. The error log is as follows:
>>
>> [519082.811553] kunpeng_hccs HISI04B2:00: PCC command executed timeout!
>     ^^^^
> These timestamps are useless, needs to be dropped.
Got it. Thanks.
>
>> [519082.828532] kunpeng_hccs HISI04B2:00: get port link status info failed, ret = -110.
>> [519082.833438] irq 13: nobody cared (try booting with the "irqpoll" option)
>> [519082.844622] CPU: 304 PID: 15206 Comm: systemd-journal Kdump: loaded Tainted: G           OE     5.10.0 #5
>> [519082.854959] Hardware name: To be filled by O.E.M. To be filled by O.E.M./To be filled by O.E.M., BIOS Nezha B800 V3.1.0 01/02/2024
> "To be filled by O.E.M." interesting. Either as silicon vendor, some prefer
> to leave it this way to ensure the O.E.M fill them correctly or the firmware
> engineers are not bothered to get these right as nothing breaks without
> these.
>
> Anyways, good example of what not to have in the products, as it is completely
> useless.
>
> [...]
Thanks for pointing it out.
Please ignore this. will drop the line.
>
>> To solve this issue, pcc_mbox_irq() clear 'chann_in_use' flag immediately
>> after clearing interrupt ack register.
>>
> This may be correct way of fixing the issue here, but I am questioning the
> existence of this flag now. I have some rework on this file. I will pick
This flag is for shared interrupt case on type3. please see:
3db174e478cb ("mailbox: pcc: Support shared interrupt for multiple 
subspaces")

We may need to fix it first before your refactoring this file. After 
all, it's an issue.
A little modification is more easily to backport and merge.
If it's ok for you, I'll update this commit log.
> this up if I think this is needed as it may conflict with my changes or
> we will drop the flag completely. Give a week or so, will post the changes
> and we can take it from there.
>
Looking forward to your rework.


/Huisong

>
> .