Re: [PATCH v3 1/2] usb: add find_raw_port_number callback to struct hc_driver()

2013-03-08 Thread Sarah Sharp
Hi Tianyu,

Sorry for the long long delay in reviewing this. :(

On Fri, Jan 25, 2013 at 10:29:26AM +0800, Lan Tianyu wrote:
 xhci driver divides the root hub into two logical hubs which work
 respectively for usb 2.0 and usb 3.0 devices. They are independent
 devices in the usb core. But in the ACPI table, it's one device node
 and all usb2.0 and usb3.0 ports are under it. Binding usb port with
 its acpi node needs the raw port number which is reflected in the xhci
 extended capabilities table. This patch is to add find_raw_port_number
 callback to struct hc_driver() and fill it with xhci_find_raw_port_number().
 Otherwise, call xhci_find_raw_port_number() to get real index in the HW port
 status registers instead of scanning through the xHCI roothub port array in
 the xhci_find_real_port_number().
 
 Signed-off-by: Lan Tianyu tianyu@intel.com
 ---
 Change since v1:
   Don't export usb_hcd_find_raw_port_number() symbol since there is no
 its user outside of usb core.

You say this is change, but the code still has it exported.

 Change since v2:
   combine patch usb: add find_raw_port_number callback to struct
 hc_driver() with this patch.
 
 This patchset is rebased on the usb-next tree commit 6e24777
 usb: ehci-s5p/ohci-exynos: Fix compatible strings for the device
 
 
  drivers/usb/core/hcd.c  |9 +
  drivers/usb/host/xhci-mem.c |   36 
  drivers/usb/host/xhci-pci.c |1 +
  drivers/usb/host/xhci.c |   22 ++
  drivers/usb/host/xhci.h |1 +
  include/linux/usb/hcd.h |2 ++
  6 files changed, 43 insertions(+), 28 deletions(-)
 
 diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
 index 2459896..774ac61 100644
 --- a/drivers/usb/core/hcd.c
 +++ b/drivers/usb/core/hcd.c
 @@ -2368,6 +2368,15 @@ int usb_hcd_is_primary_hcd(struct usb_hcd *hcd)
  }
  EXPORT_SYMBOL_GPL(usb_hcd_is_primary_hcd);
  
 +int usb_hcd_find_raw_port_number(struct usb_hcd *hcd, int port1)
 +{
 + if (!hcd-driver-find_raw_port_number)
 + return port1;
 +
 + return hcd-driver-find_raw_port_number(hcd, port1);
 +}
 +EXPORT_SYMBOL_GPL(usb_hcd_find_raw_port_number);
 +

^^^

  static int usb_hcd_request_irqs(struct usb_hcd *hcd,
   unsigned int irqnum, unsigned long irqflags)
  {
 diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
 index 35616ff..6dc238c 100644
 --- a/drivers/usb/host/xhci-mem.c
 +++ b/drivers/usb/host/xhci-mem.c
 @@ -1022,44 +1022,24 @@ void xhci_copy_ep0_dequeue_into_input_ctx(struct 
 xhci_hcd *xhci,
   * is attached to (or the roothub port its ancestor hub is attached to).  
 All we
   * know is the index of that port under either the USB 2.0 or the USB 3.0
   * roothub, but that doesn't give us the real index into the HW port status
 - * registers.  Scan through the xHCI roothub port array, looking for the Nth
 - * entry of the correct port speed.  Return the port number of that entry.
 + * registers. Call xhci_find_raw_port_number() to get real index.
   */
  static u32 xhci_find_real_port_number(struct xhci_hcd *xhci,
   struct usb_device *udev)
  {
   struct usb_device *top_dev;
 - unsigned int num_similar_speed_ports;
 - unsigned int faked_port_num;
 - int i;
 + struct usb_hcd *hcd;
 +
 + if (udev-speed == USB_SPEED_SUPER)
 + hcd = xhci-shared_hcd;
 + else
 + hcd = xhci-main_hcd;
  
   for (top_dev = udev; top_dev-parent  top_dev-parent-parent;
   top_dev = top_dev-parent)
   /* Found device below root hub */;
 - faked_port_num = top_dev-portnum;
 - for (i = 0, num_similar_speed_ports = 0;
 - i  HCS_MAX_PORTS(xhci-hcs_params1); i++) {
 - u8 port_speed = xhci-port_array[i];
 -
 - /*
 -  * Skip ports that don't have known speeds, or have duplicate
 -  * Extended Capabilities port speed entries.
 -  */
 - if (port_speed == 0 || port_speed == DUPLICATE_ENTRY)
 - continue;
  
 - /*
 -  * USB 3.0 ports are always under a USB 3.0 hub.  USB 2.0 and
 -  * 1.1 ports are under the USB 2.0 hub.  If the port speed
 -  * matches the device speed, it's a similar speed port.
 -  */
 - if ((port_speed == 0x03) == (udev-speed == USB_SPEED_SUPER))
 - num_similar_speed_ports++;
 - if (num_similar_speed_ports == faked_port_num)
 - /* Roothub ports are numbered from 1 to N */
 - return i+1;
 - }
 - return 0;
 + return  xhci_find_raw_port_number(hcd, top_dev-portnum);
  }

You've deleted a lot of code here that's designed to work around several
corner cases:
 - What if there is a root port that doesn't have an entry in the
   extended capabilities?
 - What if we don't understand the speed description for a 

[PATCH v3 1/2] usb: add find_raw_port_number callback to struct hc_driver()

2013-01-24 Thread Lan Tianyu
xhci driver divides the root hub into two logical hubs which work
respectively for usb 2.0 and usb 3.0 devices. They are independent
devices in the usb core. But in the ACPI table, it's one device node
and all usb2.0 and usb3.0 ports are under it. Binding usb port with
its acpi node needs the raw port number which is reflected in the xhci
extended capabilities table. This patch is to add find_raw_port_number
callback to struct hc_driver() and fill it with xhci_find_raw_port_number().
Otherwise, call xhci_find_raw_port_number() to get real index in the HW port
status registers instead of scanning through the xHCI roothub port array in
the xhci_find_real_port_number().

Signed-off-by: Lan Tianyu tianyu@intel.com
---
Change since v1:
Don't export usb_hcd_find_raw_port_number() symbol since there is no
its user outside of usb core.

Change since v2:
combine patch usb: add find_raw_port_number callback to struct
hc_driver() with this patch.

This patchset is rebased on the usb-next tree commit 6e24777
usb: ehci-s5p/ohci-exynos: Fix compatible strings for the device


 drivers/usb/core/hcd.c  |9 +
 drivers/usb/host/xhci-mem.c |   36 
 drivers/usb/host/xhci-pci.c |1 +
 drivers/usb/host/xhci.c |   22 ++
 drivers/usb/host/xhci.h |1 +
 include/linux/usb/hcd.h |2 ++
 6 files changed, 43 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 2459896..774ac61 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2368,6 +2368,15 @@ int usb_hcd_is_primary_hcd(struct usb_hcd *hcd)
 }
 EXPORT_SYMBOL_GPL(usb_hcd_is_primary_hcd);
 
+int usb_hcd_find_raw_port_number(struct usb_hcd *hcd, int port1)
+{
+   if (!hcd-driver-find_raw_port_number)
+   return port1;
+
+   return hcd-driver-find_raw_port_number(hcd, port1);
+}
+EXPORT_SYMBOL_GPL(usb_hcd_find_raw_port_number);
+
 static int usb_hcd_request_irqs(struct usb_hcd *hcd,
unsigned int irqnum, unsigned long irqflags)
 {
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 35616ff..6dc238c 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1022,44 +1022,24 @@ void xhci_copy_ep0_dequeue_into_input_ctx(struct 
xhci_hcd *xhci,
  * is attached to (or the roothub port its ancestor hub is attached to).  All 
we
  * know is the index of that port under either the USB 2.0 or the USB 3.0
  * roothub, but that doesn't give us the real index into the HW port status
- * registers.  Scan through the xHCI roothub port array, looking for the Nth
- * entry of the correct port speed.  Return the port number of that entry.
+ * registers. Call xhci_find_raw_port_number() to get real index.
  */
 static u32 xhci_find_real_port_number(struct xhci_hcd *xhci,
struct usb_device *udev)
 {
struct usb_device *top_dev;
-   unsigned int num_similar_speed_ports;
-   unsigned int faked_port_num;
-   int i;
+   struct usb_hcd *hcd;
+
+   if (udev-speed == USB_SPEED_SUPER)
+   hcd = xhci-shared_hcd;
+   else
+   hcd = xhci-main_hcd;
 
for (top_dev = udev; top_dev-parent  top_dev-parent-parent;
top_dev = top_dev-parent)
/* Found device below root hub */;
-   faked_port_num = top_dev-portnum;
-   for (i = 0, num_similar_speed_ports = 0;
-   i  HCS_MAX_PORTS(xhci-hcs_params1); i++) {
-   u8 port_speed = xhci-port_array[i];
-
-   /*
-* Skip ports that don't have known speeds, or have duplicate
-* Extended Capabilities port speed entries.
-*/
-   if (port_speed == 0 || port_speed == DUPLICATE_ENTRY)
-   continue;
 
-   /*
-* USB 3.0 ports are always under a USB 3.0 hub.  USB 2.0 and
-* 1.1 ports are under the USB 2.0 hub.  If the port speed
-* matches the device speed, it's a similar speed port.
-*/
-   if ((port_speed == 0x03) == (udev-speed == USB_SPEED_SUPER))
-   num_similar_speed_ports++;
-   if (num_similar_speed_ports == faked_port_num)
-   /* Roothub ports are numbered from 1 to N */
-   return i+1;
-   }
-   return 0;
+   return  xhci_find_raw_port_number(hcd, top_dev-portnum);
 }
 
 /* Setup an xHCI virtual device for a Set Address command */
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index af259e0..1a30c38 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -313,6 +313,7 @@ static const struct hc_driver xhci_pci_hc_driver = {
.set_usb2_hw_lpm =  xhci_set_usb2_hardware_lpm,
.enable_usb3_lpm_timeout =  xhci_enable_usb3_lpm_timeout,