Re: [PATCH v2 2/4] thunderbolt: Handle ring interrupt by reading intr status

[Mika Westerberg <mika.westerberg@linux.intel.com>]

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;