| Message ID | 1627994096-99972-3-git-send-email-Sanju.Mehta@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Added some bug fixes for USB4 | expand |
Hi, On Tue, Aug 03, 2021 at 07:34:54AM -0500, Sanjay R Mehta wrote: > From: Sanjay R Mehta <sanju.mehta@amd.com> > > 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@amd.com> > Signed-off-by: Sanjay R Mehta <sanju.mehta@amd.com> > --- > 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? I'm hoping that we could make this work on all controllers without too many special cases ;-)
On Wed, Aug 04, 2021 at 06:48:45PM +0300, Mika Westerberg wrote: > Hi, > > On Tue, Aug 03, 2021 at 07:34:54AM -0500, Sanjay R Mehta wrote: > > From: Sanjay R Mehta <sanju.mehta@amd.com> > > > > 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@amd.com> > > Signed-off-by: Sanjay R Mehta <sanju.mehta@amd.com> > > --- > > 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? > > I'm hoping that we could make this work on all controllers without too > many special cases ;-) I mean if you replace patches 1 and 2 in this series with the below, does it work with the AMD controller too? diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c index fa44332845a1..8a5656fb956f 100644 --- a/drivers/thunderbolt/nhi.c +++ b/drivers/thunderbolt/nhi.c @@ -71,10 +71,14 @@ static void ring_interrupt_active(struct tb_ring *ring, bool active) * since we already know which interrupt was triggered. */ misc = ioread32(ring->nhi->iobase + REG_DMA_MISC); - if (!(misc & REG_DMA_MISC_INT_AUTO_CLEAR)) { + /* Special bit for Intel */ + if (ring->nhi->pdev->vendor == PCI_VENDOR_ID_INTEL && + !(misc & REG_DMA_MISC_INT_AUTO_CLEAR)) misc |= REG_DMA_MISC_INT_AUTO_CLEAR; - iowrite32(misc, ring->nhi->iobase + REG_DMA_MISC); - } + /* USB4 clear the disable auto-clear bit */ + if (misc & BIT(17)) + misc &= ~BIT(17); + iowrite32(misc, ring->nhi->iobase + REG_DMA_MISC); ivr_base = ring->nhi->iobase + REG_INT_VEC_ALLOC_BASE; step = index / REG_INT_VEC_ALLOC_REGS * REG_INT_VEC_ALLOC_BITS;
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@amd.com> >> >> 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@amd.com> >> Signed-off-by: Sanjay R Mehta <sanju.mehta@amd.com> >> --- >> 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. >
On 8/5/2021 6:26 PM, Mika Westerberg wrote: > [CAUTION: External Email] > > On Wed, Aug 04, 2021 at 06:48:45PM +0300, Mika Westerberg wrote: >> Hi, >> >> On Tue, Aug 03, 2021 at 07:34:54AM -0500, Sanjay R Mehta wrote: >>> From: Sanjay R Mehta <sanju.mehta@amd.com> >>> >>> 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@amd.com> >>> Signed-off-by: Sanjay R Mehta <sanju.mehta@amd.com> >>> --- >>> 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? >> >> I'm hoping that we could make this work on all controllers without too >> many special cases ;-) > > I mean if you replace patches 1 and 2 in this series with the below, > does it work with the AMD controller too? > Actually, it wont work on AMD controller because explicit read operation of interrupt status is required to clear it. > diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c > index fa44332845a1..8a5656fb956f 100644 > --- a/drivers/thunderbolt/nhi.c > +++ b/drivers/thunderbolt/nhi.c > @@ -71,10 +71,14 @@ static void ring_interrupt_active(struct tb_ring *ring, bool active) > * since we already know which interrupt was triggered. > */ > misc = ioread32(ring->nhi->iobase + REG_DMA_MISC); > - if (!(misc & REG_DMA_MISC_INT_AUTO_CLEAR)) { > + /* Special bit for Intel */ > + if (ring->nhi->pdev->vendor == PCI_VENDOR_ID_INTEL && > + !(misc & REG_DMA_MISC_INT_AUTO_CLEAR)) > misc |= REG_DMA_MISC_INT_AUTO_CLEAR; > - iowrite32(misc, ring->nhi->iobase + REG_DMA_MISC); > - } > + /* USB4 clear the disable auto-clear bit */ > + if (misc & BIT(17)) > + misc &= ~BIT(17); > + iowrite32(misc, ring->nhi->iobase + REG_DMA_MISC); > > ivr_base = ring->nhi->iobase + REG_INT_VEC_ALLOC_BASE; > step = index / REG_INT_VEC_ALLOC_REGS * REG_INT_VEC_ALLOC_BITS; >
On Thu, Aug 05, 2021 at 06:29:36PM +0530, Sanjay R Mehta wrote: > > > 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@amd.com> > >> > >> 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@amd.com> > >> Signed-off-by: Sanjay R Mehta <sanju.mehta@amd.com> > >> --- > >> 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." D'oh, right. It is about auto clear of the ISS register on read or not. I misunderstood the whole bit. > > > 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. Probably that would work better. Let me try to figure out if we can somehow do the same in Intel controller too so we would only have single path here.
On Thu, Aug 05, 2021 at 06:36:17PM +0530, Sanjay R Mehta wrote: > > > On 8/5/2021 6:26 PM, Mika Westerberg wrote: > > [CAUTION: External Email] > > > > On Wed, Aug 04, 2021 at 06:48:45PM +0300, Mika Westerberg wrote: > >> Hi, > >> > >> On Tue, Aug 03, 2021 at 07:34:54AM -0500, Sanjay R Mehta wrote: > >>> From: Sanjay R Mehta <sanju.mehta@amd.com> > >>> > >>> 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@amd.com> > >>> Signed-off-by: Sanjay R Mehta <sanju.mehta@amd.com> > >>> --- > >>> 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? > >> > >> I'm hoping that we could make this work on all controllers without too > >> many special cases ;-) > > > > I mean if you replace patches 1 and 2 in this series with the below, > > does it work with the AMD controller too? > > > Actually, it wont work on AMD controller because explicit read operation > of interrupt status is required to clear it. Indeed, I misunderstood the bit. Please ignore this.
On Thu, Aug 05, 2021 at 05:19:39PM +0300, Mika Westerberg wrote: > On Thu, Aug 05, 2021 at 06:29:36PM +0530, Sanjay R Mehta wrote: > > > > > > 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@amd.com> > > >> > > >> 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@amd.com> > > >> Signed-off-by: Sanjay R Mehta <sanju.mehta@amd.com> > > >> --- > > >> 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." > > D'oh, right. It is about auto clear of the ISS register on read or not. > I misunderstood the whole bit. > > > > > > 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. > > Probably that would work better. Let me try to figure out if we can > somehow do the same in Intel controller too so we would only have single > path here. Looks like we cannot get it working without quirk of some kind :( I think we can do this: 1. Add nhi_check_quirks() to nhi.c and that one checks for PCI_VENDOR_ID_INTEL and sets nhi->quirks |= DMA_MISC_INT_AUTO_CLEAR. 2. Then check it in both ring_interrupt_active() and in ring_clear_msix(ring) and if set handle the case accordingly. Let's add nhi->quirks directly now because I have a feeling that we may need additional flags in the future ;-) Would you like to update this series with the above changes or you want me to do that?
On 8/5/2021 8:16 PM, Mika Westerberg wrote: > [CAUTION: External Email] > > On Thu, Aug 05, 2021 at 05:19:39PM +0300, Mika Westerberg wrote: >> On Thu, Aug 05, 2021 at 06:29:36PM +0530, Sanjay R Mehta wrote: >>> >>> >>> 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@amd.com> >>>>> >>>>> 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@amd.com> >>>>> Signed-off-by: Sanjay R Mehta <sanju.mehta@amd.com> >>>>> --- >>>>> 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." >> >> D'oh, right. It is about auto clear of the ISS register on read or not. >> I misunderstood the whole bit. >> >>> >>>> 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. >> >> Probably that would work better. Let me try to figure out if we can >> somehow do the same in Intel controller too so we would only have single >> path here. > > Looks like we cannot get it working without quirk of some kind :( I > think we can do this: > > 1. Add nhi_check_quirks() to nhi.c and that one checks for > PCI_VENDOR_ID_INTEL and sets nhi->quirks |= DMA_MISC_INT_AUTO_CLEAR. > 2. Then check it in both ring_interrupt_active() and in > ring_clear_msix(ring) and if set handle the case accordingly. > > Let's add nhi->quirks directly now because I have a feeling that we may > need additional flags in the future ;-) > > Would you like to update this series with the above changes or you want > me to do that? Sounds good. Sure, I will update this series as per your suggestion. Thank you :) >
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)); + } +} + static irqreturn_t ring_msix(int irq, void *data) { struct tb_ring *ring = data; spin_lock(&ring->nhi->lock); + check_and_clear_intr_status(ring); spin_lock(&ring->lock); __ring_interrupt(ring); spin_unlock(&ring->lock);