On 8/4/2021 9:18 PM, Mika Westerberg wrote:
> [CAUTION: External Email]
>
> Hi,
>
> On Tue, Aug 03, 2021 at 07:34:54AM -0500, Sanjay R Mehta wrote:
>> From: Sanjay R Mehta <sanju.mehta@xxxxxxx>
>>
>> As per USB4 spec by default "Disable ISR Auto-Clear" bit is set to 0,
>> and the Tx/Rx ring interrupt status is needs to be cleared.
>>
>> Hence handling it by reading the "Interrupt status" register in the ISR.
>>
>> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@xxxxxxx>
>> Signed-off-by: Sanjay R Mehta <sanju.mehta@xxxxxxx>
>> ---
>> drivers/thunderbolt/nhi.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
>> index ef01aa6..7ad2202 100644
>> --- a/drivers/thunderbolt/nhi.c
>> +++ b/drivers/thunderbolt/nhi.c
>> @@ -373,11 +373,25 @@ void tb_ring_poll_complete(struct tb_ring *ring)
>> }
>> EXPORT_SYMBOL_GPL(tb_ring_poll_complete);
>>
>> +static void check_and_clear_intr_status(struct tb_ring *ring)
>> +{
>> + if (!(ring->nhi->pdev->vendor == PCI_VENDOR_ID_INTEL)) {
>> + if (ring->is_tx)
>> + ioread32(ring->nhi->iobase
>> + + REG_RING_NOTIFY_BASE);
>> + else
>> + ioread32(ring->nhi->iobase
>> + + REG_RING_NOTIFY_BASE
>> + + 4 * (ring->nhi->hop_count / 32));
>> + }
>> +}
>
> I'm now playing with this series on Intel hardware. I wanted to check
> from you whether the AMD controller implements the Auto-Clear feature? I
> mean if we always clear bit 17 of the Host Interface Control register do
> you still need to call the above or it is cleared automatically?
>
Yes, AMD implements Auto-Clear and a read operation is required to clear
the interrupt status.
It is explicitly described in the Spec, Section "12.6.3.4.1 -> "Table
12-27. Interrupt Status" as below
"If the Disable ISR Auto-Clear bit is set to 0b, then a read operation
returns the current value and then clears the register to 0."
> I'm hoping that we could make this work on all controllers without too
> many special cases ;-)
Will it be good idea to have a separate variable in "struct tb_nhi" as
"nhi->is_intr_autoclr" so that we can set in the
"quirk_enable_intr_auto_clr()" as required which can be used in above
check_and_clear_intr_status() function instead of vendor check.
>
