Received: by 2002:ab2:7855:0:b0:1f9:5764:f03e with SMTP id m21csp1003765lqp; Thu, 23 May 2024 06:37:04 -0700 (PDT) X-Forwarded-Encrypted: i=3; AJvYcCWKC11/OqKzN4nN+VeMiWFge8G1SaaQHkFGmmtGhr1sPVm4+3CXDMRvCqjwOxMfXGiNUSCNDyr0/UKFTn40dMdxo5bp+4TLFJhHjfHgjw== X-Google-Smtp-Source: AGHT+IEUoxywBK2NVIZP4mjb8Ca8hitCZHH7gCI0DfXikvkM4UkNRgtBPcr5C7l/qRg2ASJc4xFo X-Received: by 2002:a05:690c:f07:b0:61b:e643:58c7 with SMTP id 00721157ae682-627e472b21fmr63886147b3.28.1716471424045; Thu, 23 May 2024 06:37:04 -0700 (PDT) ARC-Seal: i=2; a=rsa-sha256; t=1716471424; cv=pass; d=google.com; s=arc-20160816; b=MaW+s6fC4QC1MPLwou187hf7fnRHZe0XrO1GrsXP39fbsjNUvFJnXz7anYCQNPqiVY NrBfgqhnHRM6BjnkTZbAzdKREIvDkeWH7MtcTOinADjeAw1OZM9JlRZeK1QITqQoR7UO 3g54ZGV/+WfIfvon0GEBjrUVqj1zx6kGo5Y4RRGL92obzGx4BbhsYi/3XUaKyMmbj27a D9Ry56Ej6BVr7YuTJiIc7gBPjebG1C0YoZYff/S7e4ujNVqJr0EWLd7m/0UV7cRfrwhY ymVLGjPwfTUAahXoLwsfVWzYK7DPFBWXeddl+De6mqB4VtySVskajFB1apOkyTGMWPcb bWuA== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:in-reply-to:subject:from:references:cc:to :content-language:user-agent:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:date:message-id:dkim-signature; bh=oMHNbjcYG4rJaZTMNmajMCBNCYRIoxOaofpRtwGh6JI=; fh=R0MFU5bWH9VcGSw/xOskoP0pAbfapJiCcYWu6AkaY0M=; b=jw9zYS7/UUBUqy+yNEcgnuROsaTMTHCIqidh9uDduir7ymOW9MDKuj87wLcFWM93PT yq8wBLGsGa0pD5pw5jUm5lE+1vj/mi6N1lyZY1UrRDwcpkG2UZwAHdUYbPoMQY7uqxHd DJaOW0WyaTJA9PT1ffyEKagyMK8BAfcfUFRrgpKWullyRfJzMhH6lhtk0ymB89BNgV5T 6SZm1kduTJYxPUzB8tgUtLpDQR9QO+bQrI09JkNeBkxINtnOt0SLgIfanC0M/p/xHxbT HXpiXJbJgs8INkgMF/e6OdYyZpXvhtmjTrUgc9vLmHO9pBt3W2Ky7bJQVR5LonfcMLXb BjUg==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=hNKRsfqs; arc=pass (i=1 dkim=pass dkdomain=intel.com dmarc=pass fromdomain=linux.intel.com); spf=pass (google.com: domain of linux-kernel+bounces-187585-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-187585-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Return-Path: Received: from ny.mirrors.kernel.org (ny.mirrors.kernel.org. [2604:1380:45d1:ec00::1]) by mx.google.com with ESMTPS id ada2fe7eead31-4806ccc1b6bsi4438563137.774.2024.05.23.06.37.03 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 May 2024 06:37:04 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel+bounces-187585-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=hNKRsfqs; arc=pass (i=1 dkim=pass dkdomain=intel.com dmarc=pass fromdomain=linux.intel.com); spf=pass (google.com: domain of linux-kernel+bounces-187585-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-187585-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ny.mirrors.kernel.org (Postfix) with ESMTPS id 3786E1C21020 for ; Thu, 23 May 2024 13:37:03 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 8756F14A62F; Thu, 23 May 2024 13:36:57 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="hNKRsfqs" Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.10]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0F4D013C80E; Thu, 23 May 2024 13:36:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.175.65.10 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1716471416; cv=none; b=FUEX4tVM63V3nqq8NAWgSSnSM3iKLtjWh8c4L+APQb8T7qOaVHhLfRu8SnM8D9wHxlUTXv7XESPlRY6noeiLrj0+JL5kREGAUdjxWSX3P7gfJZKc1tfpY7s+p7GLEjnbMi+3FW0jLBdBA4UH5X1rI/JKp+EMSJ+Jy5NGQAUZ7P0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1716471416; c=relaxed/simple; bh=kRSxtYfvGaEYm+Suf72IVaC4RooK0ePrcIkOVCA7lR4=; h=Message-ID:Date:MIME-Version:To:Cc:References:From:Subject: In-Reply-To:Content-Type; b=C9EK4E1AJ7wXvf1Ru874ZlcsOlRXT2IjjCrrb/qAv5aTlMc+B9MrvJHQwnA/rYSfs8/sy9YclSY1tWeY1J+A4tpQO/5beBM4+z7MMylYqTng8Yk1JH09ct93VNyY2p+4+jNCw6Yipk4AKyzKNwBCLGSkeFCCtP1KXQKnngkEXNQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com; spf=none smtp.mailfrom=linux.intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=hNKRsfqs; arc=none smtp.client-ip=198.175.65.10 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=linux.intel.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1716471415; x=1748007415; h=message-id:date:mime-version:to:cc:references:from: subject:in-reply-to:content-transfer-encoding; bh=kRSxtYfvGaEYm+Suf72IVaC4RooK0ePrcIkOVCA7lR4=; b=hNKRsfqsypAyw1ZtwRK40RY0CzwxJMuhX6ebmAkghDtdMnFyBv3eBHvN 7cfQ5H1K8dJOjO9u3Cczzi/6xMXlPdDgETwcOvYn75nKSW1tTX4JRyRFC /FVz1oNaTUdgBrJizRenhXVd7bSm2DlbCdBHuNZU5NeqewQI42BzeGvh/ AJe5EKNG63B8u/CgzwPyZULHx8IHTnRB+SwfJr6Iv6YkwxDQQDGNQ+5Ba mDOCUUpmbix1D6wuV24Ztm7VMVGG7YaNMckp0DQSwQXgo124jo1ipYeex 2aEtlUqSrtgjDgyC+Vz0pdo+tyYPEeibhxDOtT4j0DYlUnEiM4utCq7Tl g==; X-CSE-ConnectionGUID: HegFF0DFQGGqhNni9Nh9yw== X-CSE-MsgGUID: i6UiiKdzQFa4VwvC/LUfXw== X-IronPort-AV: E=McAfee;i="6600,9927,11081"; a="30289874" X-IronPort-AV: E=Sophos;i="6.08,182,1712646000"; d="scan'208";a="30289874" Received: from orviesa010.jf.intel.com ([10.64.159.150]) by orvoesa102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 May 2024 06:36:54 -0700 X-CSE-ConnectionGUID: v5Ok4vCWR9WOxxSbY9f7ow== X-CSE-MsgGUID: KFwLKT1YSkC01sfJMEv1Dw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.08,182,1712646000"; d="scan'208";a="33515993" Received: from mattu-haswell.fi.intel.com (HELO [10.237.72.199]) ([10.237.72.199]) by orviesa010.jf.intel.com with ESMTP; 23 May 2024 06:36:53 -0700 Message-ID: Date: Thu, 23 May 2024 16:38:48 +0300 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Firefox/102.0 Thunderbird/102.13.0 Content-Language: en-US To: Jung Daehwan Cc: Mathias Nyman , Greg Kroah-Hartman , "open list:USB XHCI DRIVER" , open list , Thinh Nguyen References: <1716339839-44022-1-git-send-email-dh10.jung@samsung.com> <6a4767b5-1e2f-dbec-58ca-c44eb0fca6f1@linux.intel.com> <20240523044314.GA58326@ubuntu> From: Mathias Nyman Subject: Re: [RFC] usb: host: xhci-mem: Write high first on erst base of secondary interrupter In-Reply-To: <20240523044314.GA58326@ubuntu> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 23.5.2024 7.43, Jung Daehwan wrote: > On Wed, May 22, 2024 at 04:40:56PM +0300, Mathias Nyman wrote: >> On 22.5.2024 4.03, Daehwan Jung wrote: >>> ERSTBA_HI should be written first on secondary interrupter. >>> That's why secondary interrupter could be set while Host Controller >>> is already running. >>> >>> [Synopsys]- The host controller was design to support ERST setting >>> during the RUN state. But since there is a limitation in controller >>> in supporting separate ERSTBA_HI and ERSTBA_LO programming, >>> It is supported when the ERSTBA is programmed in 64bit, >>> or in 32 bit mode ERSTBA_HI before ERSTBA_LO >> >> xHCI specification 5.1 "Register Conventions "states that 64 bit >> registers should be written in low-high order >> >>> >>> [Synopsys]- The internal initialization of event ring fetches >>> the "Event Ring Segment Table Entry" based on the indication of >>> ERSTBA_LO written. >>> >> >> Any idea if this is a common issue with this host? >> Should other 64 bit registers also be written in reverse order. >> >>> Signed-off-by: Daehwan Jung >>> --- >>> drivers/usb/host/xhci-mem.c | 5 ++++- >>> drivers/usb/host/xhci.h | 6 ++++++ >>> 2 files changed, 10 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c >>> index 3100219..36ee704 100644 >>> --- a/drivers/usb/host/xhci-mem.c >>> +++ b/drivers/usb/host/xhci-mem.c >>> @@ -2325,7 +2325,10 @@ xhci_add_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir, >>> erst_base = xhci_read_64(xhci, &ir->ir_set->erst_base); >>> erst_base &= ERST_BASE_RSVDP; >>> erst_base |= ir->erst.erst_dma_addr & ~ERST_BASE_RSVDP; >>> - xhci_write_64(xhci, erst_base, &ir->ir_set->erst_base); >>> + if (intr_num == 0) >>> + xhci_write_64(xhci, erst_base, &ir->ir_set->erst_base); >>> + else >>> + xhci_write_64_r(xhci, erst_base, &ir->ir_set->erst_base); >> >> This may cause issues with other hosts expecting low-high order as stated >> in the specification. >> >> If all 64 bit registers should be written in high-low order for this host then >> maybe set a quirk flag and change xhci_write_64()instead. >> >> xhci_write_64(...) >> { >> if (xhci->quirks & XHCI_WRITE_64_HI_LO) >> hi_lo_writeq(val, regs); >> else >> lo_hi_writeq(val, regs); >> } >> > > Mathias, Thanks for the comment. > > I've seen this issue only writing the base address of ERST. > It's better to use a quirk flag as you said. > How about using the quirk only in xhci_add_interrupter? > > @@ -2325,7 +2325,10 @@ xhci_add_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir, > erst_base = xhci_read_64(xhci, &ir->ir_set->erst_base); > erst_base &= ERST_BASE_RSVDP; > erst_base |= ir->erst.erst_dma_addr & ~ERST_BASE_RSVDP; > xhci_write_64(xhci, erst_base, &ir->ir_set->erst_base); > if (xhci->quirks & XHCI_WRITE_64_HI_LO) > xhci_write_64_r(xhci, erst_base, &ir->ir_set->erst_base); > else > xhci_write_64(xhci, erst_base, &ir->ir_set->erst_base); > This works. Maybe even skip the xhci_write_64_r() helper and just use hi_lo_writeq() directly. Thanks Mathias