All of lore.kernel.org
 help / color / mirror / Atom feed
From: Olivier Dautricourt <olivierdautricourt@gmail.com>
To: "Michał Pecio" <michal.pecio@gmail.com>
Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
	linux-usb@vger.kernel.org, mathias.nyman@intel.com
Subject: Re: [PATCH] usb: xhci: xhci_setup_port_arrays: early -ENODEV if maxports is 0.
Date: Fri, 4 Oct 2024 21:14:39 +0200	[thread overview]
Message-ID: <ZwA-n56XlNkkLNXM@freebase> (raw)
In-Reply-To: <20241004125716.75c857ae@foxbook>

Hello,

On Fri, Oct 04, 2024 at 12:57:16PM +0200, Michał Pecio wrote:
> Hi,
> 
> > If the controller reports HCSPARAMS1.maxports==0 then we can skip the
> > whole function: it would fail later after doing a bunch of unnecessary
> > stuff. It can occur on a buggy hardware (the value is driven by external
> > signals).
> 
> This function runs once during HC initialization, so what's the benefit
> of bypassing it? Does it take unusually long time? Does it panic?
> 
> It seems to alreday be written to handle such abnormal cases gracefully.

That is correct, the case is handled without panic, but the 0 value gets
silently propagated until it eventually fails on line 2220:
	if (xhci->usb2_rhub.num_ports == 0 && xhci->usb3_rhub.num_ports == 0) {
		xhci_warn(xhci, "No ports on the roothubs?\n");
		return -ENODEV;
	}
The benefits are only:
  - Reporting a more precise issue
  - Avoids iterating through the capability structures of the controller
  - failsafe if future changes

This is totally a nitpick as the case is unusual, if you think it is not
worth taking it upstream i'll understand.


Kr,
Olivier

  reply	other threads:[~2024-10-04 19:14 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-30  5:23 [PATCH] usb: xhci: xhci_setup_port_arrays: early -ENODEV if maxports is 0 Olivier Dautricourt
2024-10-04  8:07 ` Greg Kroah-Hartman
2024-10-04 19:04   ` Olivier Dautricourt
2024-10-04 10:57 ` Michał Pecio
2024-10-04 19:14   ` Olivier Dautricourt [this message]
2024-10-04 21:05     ` Michał Pecio
2024-10-10 12:50     ` Mathias Nyman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZwA-n56XlNkkLNXM@freebase \
    --to=olivierdautricourt@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@intel.com \
    --cc=michal.pecio@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.