Date:   Tue, 11 Jan 2022 12:43:12 +0100
From:   Greg KH <gregkh@linuxfoundation.org>
To:     Puma Hsu <pumahsu@google.com>
Cc:     mathias.nyman@intel.com, s.shtylyov@omp.ru,
        albertccwang@google.com, linux-usb@vger.kernel.org,
        linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH v3] xhci: re-initialize the HC during resume if HCE was
 set
Message-ID: <Yd1tUKhyZf26OVNQ@kroah.com>
References: <20211229112551.3483931-1-pumahsu@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20211229112551.3483931-1-pumahsu@google.com>
Precedence: bulk

On Wed, Dec 29, 2021 at 07:25:51PM +0800, Puma Hsu wrote:
> When HCE(Host Controller Error) is set, it means an internal
> error condition has been detected. It needs to re-initialize
> the HC too.

What is "It" in the last sentence?

> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Puma Hsu <pumahsu@google.com>

What commit id does this fix?

> ---
> v2: Follow Sergey Shtylyov <s.shtylyov@omp.ru>'s comment.
> v3: Add stable@vger.kernel.org for stable release.
> 
>  drivers/usb/host/xhci.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index dc357cabb265..ab440ce8420f 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1146,8 +1146,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
>  		temp = readl(&xhci->op_regs->status);
>  	}
>  
> -	/* If restore operation fails, re-initialize the HC during resume */
> -	if ((temp & STS_SRE) || hibernated) {
> +	/* If restore operation fails or HC error is detected, re-initialize the HC during resume */
> +	if ((temp & (STS_SRE | STS_HCE)) || hibernated) {

But if STS_HCE is set on suspend, that means the suspend was broken so
you wouldn't get here, right?

Or can the error happen between suspend and resume?

This seems like a big hammer for when the host controller throws an
error.  Why is this the only place that it should be checked for?  What
caused the error that can now allow it to be fixed?

thanks,

greg k-h