[AMD Official Use Only - General] > -----Original Message----- > From: Limonciello, Mario <Mario.Limonciello@xxxxxxx> > Sent: Monday, April 24, 2023 2:56 PM > To: Andreas Noever <andreas.noever@xxxxxxxxx>; Michael Jamet > <michael.jamet@xxxxxxxxx>; Mika Westerberg > <mika.westerberg@xxxxxxxxxxxxxxx>; Yehezkel Bernat > <YehezkelShB@xxxxxxxxx>; Limonciello, Mario > <Mario.Limonciello@xxxxxxx> > Cc: S, Sanath <Sanath.S@xxxxxxx>; Gong, Richard > <Richard.Gong@xxxxxxx>; Mehta, Sanju <Sanju.Mehta@xxxxxxx>; > Takashi Iwai <tiwai@xxxxxxx>; linux-usb@xxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx > Subject: [PATCH v2 1/2] thunderbolt: Clear registers properly when auto > clear isn't in use > > When `QUIRK_AUTO_CLEAR_INT` isn't set, interrupt masking should be > cleared by writing to Interrupt Mask Clear (IMR) and interrupt > status should be cleared properly at shutdown/init. > > This fixes an error where interrupts are left enabled during resume > from hibernation with `CONFIG_USB4=y`. > > Fixes: 468c49f44759 ("thunderbolt: Disable interrupt auto clear for rings") > Reported-by: Takashi Iwai <tiwai@xxxxxxx> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=217343 > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx> > --- > v1->v2: > * Whitespace changes > * Add new static functions for nhi_mask_interrupt() and > nhi_clear_interrupt() > > I tried to base this off thunderbolt.git/next (tag: thunderbolt-for-v6.4-rc1) > but the following 3 commits are missing from that branch but are in 6.3-rc7: > > 58cdfe6f58b3 thunderbolt: Rename shadowed variables bit to interrupt_bit > and auto_clear_bit > 468c49f44759 thunderbolt: Disable interrupt auto clear for rings > 1716efdb0793 thunderbolt: Use const qualifier for `ring_interrupt_index` > > I cherry picked them first as this patch builds on them. It's expected that > this patch should apply on top of 6.4-rc1 properly. Mika, Since 6.4-rc1 is out, you should be able to test and apply this patch now. If you need any changes I'll send a v3 dropping the second patch. Thanks! > --- > drivers/thunderbolt/nhi.c | 29 ++++++++++++++++++++++++----- > drivers/thunderbolt/nhi_regs.h | 2 ++ > 2 files changed, 26 insertions(+), 5 deletions(-) > > diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c > index d76e923fbc6a..c0aee5dc5237 100644 > --- a/drivers/thunderbolt/nhi.c > +++ b/drivers/thunderbolt/nhi.c > @@ -54,6 +54,21 @@ static int ring_interrupt_index(const struct tb_ring > *ring) > return bit; > } > > +static void nhi_mask_interrupt(struct tb_nhi *nhi, int mask, int ring) > +{ > + if (nhi->quirks & QUIRK_AUTO_CLEAR_INT) > + return; > + iowrite32(mask, nhi->iobase + > REG_RING_INTERRUPT_MASK_CLEAR_BASE + ring); > +} > + > +static void nhi_clear_interrupt(struct tb_nhi *nhi, int ring) > +{ > + if (nhi->quirks & QUIRK_AUTO_CLEAR_INT) > + ioread32(nhi->iobase + REG_RING_NOTIFY_BASE + ring); > + else > + iowrite32(~0, nhi->iobase + REG_RING_INT_CLEAR + ring); > +} > + > /* > * ring_interrupt_active() - activate/deactivate interrupts for a single ring > * > @@ -61,8 +76,8 @@ static int ring_interrupt_index(const struct tb_ring > *ring) > */ > static void ring_interrupt_active(struct tb_ring *ring, bool active) > { > - int reg = REG_RING_INTERRUPT_BASE + > - ring_interrupt_index(ring) / 32 * 4; > + int index = ring_interrupt_index(ring) / 32 * 4; > + int reg = REG_RING_INTERRUPT_BASE + index; > int interrupt_bit = ring_interrupt_index(ring) & 31; > int mask = 1 << interrupt_bit; > u32 old, new; > @@ -123,7 +138,11 @@ static void ring_interrupt_active(struct tb_ring *ring, > bool active) > "interrupt for %s %d is already > %s\n", > RING_TYPE(ring), ring->hop, > active ? "enabled" : "disabled"); > - iowrite32(new, ring->nhi->iobase + reg); > + > + if (active) > + iowrite32(new, ring->nhi->iobase + reg); > + else > + nhi_mask_interrupt(ring->nhi, mask, index); > } > > /* > @@ -136,11 +155,11 @@ static void nhi_disable_interrupts(struct tb_nhi > *nhi) > int i = 0; > /* disable interrupts */ > for (i = 0; i < RING_INTERRUPT_REG_COUNT(nhi); i++) > - iowrite32(0, nhi->iobase + REG_RING_INTERRUPT_BASE + 4 > * i); > + nhi_mask_interrupt(nhi, ~0, 4 * i); > > /* clear interrupt status bits */ > for (i = 0; i < RING_NOTIFY_REG_COUNT(nhi); i++) > - ioread32(nhi->iobase + REG_RING_NOTIFY_BASE + 4 * i); > + nhi_clear_interrupt(nhi, 4 * i); > } > > /* ring helper methods */ > diff --git a/drivers/thunderbolt/nhi_regs.h b/drivers/thunderbolt/nhi_regs.h > index faef165a919c..6ba295815477 100644 > --- a/drivers/thunderbolt/nhi_regs.h > +++ b/drivers/thunderbolt/nhi_regs.h > @@ -93,6 +93,8 @@ struct ring_desc { > #define REG_RING_INTERRUPT_BASE 0x38200 > #define RING_INTERRUPT_REG_COUNT(nhi) ((31 + 2 * nhi->hop_count) / > 32) > > +#define REG_RING_INTERRUPT_MASK_CLEAR_BASE 0x38208 > + > #define REG_INT_THROTTLING_RATE 0x38c00 > > /* Interrupt Vector Allocation */ > -- > 2.34.1