public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/2] usb: port: Don't try to peer unused USB ports based on location
       [not found] <20240222133819.4149388-1-mathias.nyman@linux.intel.com>
@ 2024-02-22 13:38 ` Mathias Nyman
  2024-02-22 14:06   ` Greg KH
  2024-02-22 15:51   ` Paul Menzel
  0 siblings, 2 replies; 4+ messages in thread
From: Mathias Nyman @ 2024-02-22 13:38 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, stern, Mathias Nyman, Paul Menzel, stable

Unused USB ports may have bogus location data in ACPI PLD tables.
This causes port peering failures as these unused USB2 and USB3 ports
location may match.

This is seen on DELL systems where all unused ports return zeroed
location data.

Don't try to peer or match ports that have connect type set to
USB_PORT_NOT_USED.

Tested-by: Paul Menzel <pmenzel@molgen.mpg.de>
Cc: stable@vger.kernel.org
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/core/port.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index c628c1abc907..4d63496f98b6 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -573,7 +573,7 @@ static int match_location(struct usb_device *peer_hdev, void *p)
 	struct usb_hub *peer_hub = usb_hub_to_struct_hub(peer_hdev);
 	struct usb_device *hdev = to_usb_device(port_dev->dev.parent->parent);
 
-	if (!peer_hub)
+	if (!peer_hub || port_dev->connect_type == USB_PORT_NOT_USED)
 		return 0;
 
 	hcd = bus_to_hcd(hdev->bus);
@@ -584,7 +584,8 @@ static int match_location(struct usb_device *peer_hdev, void *p)
 
 	for (port1 = 1; port1 <= peer_hdev->maxchild; port1++) {
 		peer = peer_hub->ports[port1 - 1];
-		if (peer && peer->location == port_dev->location) {
+		if (peer && peer->connect_type != USB_PORT_NOT_USED &&
+		    peer->location == port_dev->location) {
 			link_peers_report(port_dev, peer);
 			return 1; /* done */
 		}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] usb: port: Don't try to peer unused USB ports based on location
  2024-02-22 13:38 ` [PATCH 2/2] usb: port: Don't try to peer unused USB ports based on location Mathias Nyman
@ 2024-02-22 14:06   ` Greg KH
  2024-02-22 15:03     ` Mathias Nyman
  2024-02-22 15:51   ` Paul Menzel
  1 sibling, 1 reply; 4+ messages in thread
From: Greg KH @ 2024-02-22 14:06 UTC (permalink / raw)
  To: Mathias Nyman; +Cc: linux-usb, stern, Paul Menzel, stable

On Thu, Feb 22, 2024 at 03:38:19PM +0200, Mathias Nyman wrote:
> Unused USB ports may have bogus location data in ACPI PLD tables.
> This causes port peering failures as these unused USB2 and USB3 ports
> location may match.
> 
> This is seen on DELL systems where all unused ports return zeroed
> location data.
> 
> Don't try to peer or match ports that have connect type set to
> USB_PORT_NOT_USED.
> 
> Tested-by: Paul Menzel <pmenzel@molgen.mpg.de>
> Cc: stable@vger.kernel.org
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>

What commit does this fix?  "all" of them?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] usb: port: Don't try to peer unused USB ports based on location
  2024-02-22 14:06   ` Greg KH
@ 2024-02-22 15:03     ` Mathias Nyman
  0 siblings, 0 replies; 4+ messages in thread
From: Mathias Nyman @ 2024-02-22 15:03 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-usb, stern, Paul Menzel, stable

On 22.2.2024 16.06, Greg KH wrote:
> On Thu, Feb 22, 2024 at 03:38:19PM +0200, Mathias Nyman wrote:
>> Unused USB ports may have bogus location data in ACPI PLD tables.
>> This causes port peering failures as these unused USB2 and USB3 ports
>> location may match.
>>
>> This is seen on DELL systems where all unused ports return zeroed
>> location data.
>>
>> Don't try to peer or match ports that have connect type set to
>> USB_PORT_NOT_USED.
>>
>> Tested-by: Paul Menzel <pmenzel@molgen.mpg.de>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> 
> What commit does this fix?  "all" of them?

Right, git blame shows the code this fixes was added 10 years ago in 3.16

Fixes: 3bfd659baec8 ("usb: find internal hub tier mismatch via acpi")
Cc: stable@vger.kernel.org # v3.16+

Thanks
Mathias

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] usb: port: Don't try to peer unused USB ports based on location
  2024-02-22 13:38 ` [PATCH 2/2] usb: port: Don't try to peer unused USB ports based on location Mathias Nyman
  2024-02-22 14:06   ` Greg KH
@ 2024-02-22 15:51   ` Paul Menzel
  1 sibling, 0 replies; 4+ messages in thread
From: Paul Menzel @ 2024-02-22 15:51 UTC (permalink / raw)
  To: Mathias Nyman; +Cc: gregkh, linux-usb, stern, stable, Mike Jones

Dear Mathias,


Thank you for your patches fixing the problem.

Am 22.02.24 um 14:38 schrieb Mathias Nyman:
> Unused USB ports may have bogus location data in ACPI PLD tables.
> This causes port peering failures as these unused USB2 and USB3 ports
> location may match.

I comment here, although it should probably be in another branch of this 
thread.

If it is a firmware issue, this check should be added to FirmWare Test 
Suite (fwts) [1] too (I can report it there), and maybe some debug log 
should report this firmware error too.

> This is seen on DELL systems where all unused ports return zeroed
> location data.

As noted in the post scriptum in [2], much more systems seem to be affected.

> Don't try to peer or match ports that have connect type set to
> USB_PORT_NOT_USED.

When grepping the git history, pasting the warning message would help 
me. Maybe:

This fixes the warning below on the affected systems:

     usb: port power management may be unreliable

If you want to add add the Linux Kernel Bugzilla URLs:

Link: https://bugzilla.kernel.org/show_bug.cgi?id=218465
Link: https://bugzilla.kernel.org/show_bug.cgi?id=218486

I wasn’t able to test the other two systems yet, but maybe it is obvious 
from the ACPI tables/ASL code:

Link: https://bugzilla.kernel.org/show_bug.cgi?id=218487 (Dell OptiPlex 
5055)
Link: https://bugzilla.kernel.org/show_bug.cgi?id=218490 (Dell PowerEdge 
T440)

> Tested-by: Paul Menzel <pmenzel@molgen.mpg.de>
> Cc: stable@vger.kernel.org
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> ---
>   drivers/usb/core/port.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
> index c628c1abc907..4d63496f98b6 100644
> --- a/drivers/usb/core/port.c
> +++ b/drivers/usb/core/port.c
> @@ -573,7 +573,7 @@ static int match_location(struct usb_device *peer_hdev, void *p)
>   	struct usb_hub *peer_hub = usb_hub_to_struct_hub(peer_hdev);
>   	struct usb_device *hdev = to_usb_device(port_dev->dev.parent->parent);
>   
> -	if (!peer_hub)
> +	if (!peer_hub || port_dev->connect_type == USB_PORT_NOT_USED)
>   		return 0;
>   
>   	hcd = bus_to_hcd(hdev->bus);
> @@ -584,7 +584,8 @@ static int match_location(struct usb_device *peer_hdev, void *p)
>   
>   	for (port1 = 1; port1 <= peer_hdev->maxchild; port1++) {
>   		peer = peer_hub->ports[port1 - 1];
> -		if (peer && peer->location == port_dev->location) {
> +		if (peer && peer->connect_type != USB_PORT_NOT_USED &&
> +		    peer->location == port_dev->location) {
>   			link_peers_report(port_dev, peer);
>   			return 1; /* done */
>   		}


Thank you again and kind regards,

Paul


[1]: https://wiki.ubuntu.com/FirmwareTestSuite/
[2]: 
https://lore.kernel.org/linux-usb/5406d361-f5b7-4309-b0e6-8c94408f7d75@molgen.mpg.de/

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-02-22 15:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20240222133819.4149388-1-mathias.nyman@linux.intel.com>
2024-02-22 13:38 ` [PATCH 2/2] usb: port: Don't try to peer unused USB ports based on location Mathias Nyman
2024-02-22 14:06   ` Greg KH
2024-02-22 15:03     ` Mathias Nyman
2024-02-22 15:51   ` Paul Menzel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox