usb: host: xhci-hub: drop redundant port register reads

Commit Message

Sergey Shtylyov Feb. 11, 2022, 7:05 p.m. UTC

In xhci_hub_control(), there are many port register readbacks in several
branches of the *switch* statement which get duplicated right after that
*switch* by reading back the port register once more -- which is done to
flush the posted writes. Remove the redundant reads inside that *switch*.

Found by Linux Verification Center (linuxtesting.org) with the SVACE static
analysis tool.

Signed-off-by: Sergey Shtylyov <[e-mail]>

---
This patch is against the 'usb-next' branch of Greg KH's 'usb.git' repo.

 drivers/usb/host/xhci-hub.c |    8 --------
 1 file changed, 8 deletions(-)

Comments

Greg Kroah-Hartman Feb. 12, 2022, 7:03 a.m. UTC | #1

On Fri, Feb 11, 2022 at 10:05:19PM +0300, Sergey Shtylyov wrote:
> In xhci_hub_control(), there are many port register readbacks in several
> branches of the *switch* statement which get duplicated right after that
> *switch* by reading back the port register once more -- which is done to
> flush the posted writes. Remove the redundant reads inside that *switch*.

Doing a read right after a write is good, perhaps the one after the
switch should be removed instead?

thanks,

greg k-h

Sergey Shtylyov Feb. 14, 2022, 6:44 p.m. UTC | #2

On 2/12/22 10:03 AM, Greg Kroah-Hartman wrote:

>> In xhci_hub_control(), there are many port register readbacks in several
>> branches of the *switch* statement which get duplicated right after that
>> *switch* by reading back the port register once more -- which is done to
>> flush the posted writes. Remove the redundant reads inside that *switch*.
> 
> Doing a read right after a write is good, perhaps the one after the
> switch should be removed instead?

   OK, hoping that the PORTPMSC writes don't need the PORTSC readbacks...
   Looking a the git history, the readback after *switch* was redudant from the start... 

> thanks,
> 
> greg k-h

MBR, Sergey

Patch

Index: usb/drivers/usb/host/xhci-hub.c
===================================================================
--- usb.orig/drivers/usb/host/xhci-hub.c
+++ usb/drivers/usb/host/xhci-hub.c
@@ -1323,7 +1323,6 @@  int xhci_hub_control(struct usb_hcd *hcd
 			msleep(10); /* wait device to enter */
 			spin_lock_irqsave(&xhci->lock, flags);
 
-			temp = readl(ports[wIndex]->addr);
 			bus_state->suspended_ports |= 1 << wIndex;
 			break;
 		case USB_PORT_FEAT_LINK_STATE:
@@ -1341,7 +1340,6 @@  int xhci_hub_control(struct usb_hcd *hcd
 					PORT_OCC | PORT_RC | PORT_PLC |
 					PORT_CEC;
 				writel(temp | PORT_PE, ports[wIndex]->addr);
-				temp = readl(ports[wIndex]->addr);
 				break;
 			}
 
@@ -1351,7 +1349,6 @@  int xhci_hub_control(struct usb_hcd *hcd
 					 hcd->self.busnum, wIndex + 1);
 				xhci_set_link_state(xhci, ports[wIndex],
 							link_state);
-				temp = readl(ports[wIndex]->addr);
 				break;
 			}
 
@@ -1384,8 +1381,6 @@  int xhci_hub_control(struct usb_hcd *hcd
 					 hcd->self.busnum, wIndex + 1);
 				xhci_set_link_state(xhci, ports[wIndex],
 						link_state);
-
-				temp = readl(ports[wIndex]->addr);
 				break;
 			}
 			/* Port must be enabled */
@@ -1435,7 +1430,6 @@  int xhci_hub_control(struct usb_hcd *hcd
 					xhci_dbg(xhci, "missing U0 port change event for port %d-%d\n",
 						 hcd->self.busnum, wIndex + 1);
 				spin_lock_irqsave(&xhci->lock, flags);
-				temp = readl(ports[wIndex]->addr);
 				break;
 			}
 
@@ -1460,7 +1454,6 @@  int xhci_hub_control(struct usb_hcd *hcd
 						break;
 				}
 				spin_lock_irqsave(&xhci->lock, flags);
-				temp = readl(ports[wIndex]->addr);
 				bus_state->suspended_ports |= 1 << wIndex;
 			}
 			break;
@@ -1491,7 +1484,6 @@  int xhci_hub_control(struct usb_hcd *hcd
 		case USB_PORT_FEAT_BH_PORT_RESET:
 			temp |= PORT_WR;
 			writel(temp, ports[wIndex]->addr);
-			temp = readl(ports[wIndex]->addr);
 			break;
 		case USB_PORT_FEAT_U1_TIMEOUT:
 			if (hcd->speed < HCD_USB3)