From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 32990C4338F for ; Thu, 5 Aug 2021 14:19:41 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 13D8C61154 for ; Thu, 5 Aug 2021 14:19:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240185AbhHEOTy (ORCPT ); Thu, 5 Aug 2021 10:19:54 -0400 Received: from mga01.intel.com ([192.55.52.88]:43014 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232724AbhHEOTy (ORCPT ); Thu, 5 Aug 2021 10:19:54 -0400 X-IronPort-AV: E=McAfee;i="6200,9189,10067"; a="236129680" X-IronPort-AV: E=Sophos;i="5.84,296,1620716400"; d="scan'208";a="236129680" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Aug 2021 07:19:39 -0700 X-IronPort-AV: E=Sophos;i="5.84,296,1620716400"; d="scan'208";a="480849906" Received: from lahna.fi.intel.com (HELO lahna) ([10.237.72.163]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Aug 2021 07:19:36 -0700 Received: by lahna (sSMTP sendmail emulation); Thu, 05 Aug 2021 17:19:34 +0300 Date: Thu, 5 Aug 2021 17:19:34 +0300 From: Mika Westerberg To: Sanjay R Mehta Cc: Sanjay R Mehta , andreas.noever@gmail.com, michael.jamet@intel.com, YehezkelShB@gmail.com, Basavaraj.Natikar@amd.com, linux-usb@vger.kernel.org Subject: Re: [PATCH v2 2/4] thunderbolt: Handle ring interrupt by reading intr status Message-ID: References: <1627994096-99972-1-git-send-email-Sanju.Mehta@amd.com> <1627994096-99972-3-git-send-email-Sanju.Mehta@amd.com> <36665ca4-a999-39c2-2401-8dab282145fa@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <36665ca4-a999-39c2-2401-8dab282145fa@amd.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org 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 > >> > >> 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 > >> Signed-off-by: Sanjay R Mehta > >> --- > >> 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.