[U-Boot] [PATCH 00/16] usb: hub: Support USB 3.0 hubs
This series is a follow-up series to previous series [1] that adds support for USB 3.0 hubs.
USB 3.0 hubs have slightly different hub descriptor format, as well as different port status bit positions. These needs to be properly handled by U-Boot USB core stack codes. xHCI driver has also been updated to correctly set up context structures that are required for devices behind a 3.0 hub to work.
Besides USB 3.0 hubs support, this series also adds support for low speed/full speed devices connected behind a high speed hub with the xHC controller. It turns out the 'Transaction Translator' part in the xHC data structure is missing.
Note there is one error during testing USB keyboard when CONFIG_USB_KEYBOARD is on.
"Failed to get keyboard state from device 413c:2105"
The enumeration process did pass and the error was thrown in the USB keyboard driver. This was due to the interrupt transfer is still not supported by xHCI driver, as of today.
This series is available at u-boot-x86/xhci-working2 for testing.
[1] https://lists.denx.de/pipermail/u-boot/2017-June/296166.html
Bin Meng (16): usb: xhci-pci: Drop non-DM version of xhci-pci driver usb: xhci-pci: Clean up the driver a little bit usb: Remove unnecessary work in usb_setup_descriptor() usb: hub: Use 'struct usb_hub_device' as hub device's uclass_priv usb: hub: Add a new API to test if a hub device is root hub usb: xhci: Change to use usb_hub_is_root_hub() API usb: hub: Translate USB 3.0 hub port status into old version usb: hub: Support 'set hub depth' request for USB 3.0 hubs usb: xhci: Change xhci_setup_addressable_virt_dev() signature usb: xhci: Program 'route string' in the input slot context usb: hub: Parse and save TT details from device descriptor dm: usb: Add a new USB controller operation 'update_hub_device' usb: hub: Call usb_update_hub_device() after hub descriptor is fetched usb: xhci: Implement update_hub_device() operation usb: xhci: Correct TT_SLOT and TT_PORT macros usb: xhci: Enable TT to support LS/FS devices behind a HS hub
common/usb.c | 40 +++++----- common/usb_hub.c | 166 +++++++++++++++++++++++++++++++++++++++++- drivers/usb/host/usb-uclass.c | 13 +++- drivers/usb/host/xhci-mem.c | 40 +++++++++- drivers/usb/host/xhci-pci.c | 68 +---------------- drivers/usb/host/xhci.c | 85 +++++++++++++++------ drivers/usb/host/xhci.h | 8 +- include/usb.h | 46 +++++++++++- include/usb_defs.h | 15 ++++ 9 files changed, 357 insertions(+), 124 deletions(-)
As there is no board that currently uses xhci-pci driver without DM USB, drop its support and leave only DM support.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
drivers/usb/host/xhci-pci.c | 52 --------------------------------------------- 1 file changed, 52 deletions(-)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 63daaa6..5ad8452 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -14,56 +14,6 @@
#include "xhci.h"
-#ifndef CONFIG_DM_USB - -/* - * Create the appropriate control structures to manage a new XHCI host - * controller. - */ -int xhci_hcd_init(int index, struct xhci_hccr **ret_hccr, - struct xhci_hcor **ret_hcor) -{ - struct xhci_hccr *hccr; - struct xhci_hcor *hcor; - pci_dev_t pdev; - uint32_t cmd; - int len; - - pdev = pci_find_class(PCI_CLASS_SERIAL_USB_XHCI, index); - if (pdev < 0) { - printf("XHCI host controller not found\n"); - return -1; - } - - hccr = (struct xhci_hccr *)pci_map_bar(pdev, - PCI_BASE_ADDRESS_0, PCI_REGION_MEM); - len = HC_LENGTH(xhci_readl(&hccr->cr_capbase)); - hcor = (struct xhci_hcor *)((uint32_t)hccr + len); - - debug("XHCI-PCI init hccr 0x%x and hcor 0x%x hc_length %d\n", - (uint32_t)hccr, (uint32_t)hcor, len); - - *ret_hccr = hccr; - *ret_hcor = hcor; - - /* enable busmaster */ - pci_read_config_dword(pdev, PCI_COMMAND, &cmd); - cmd |= PCI_COMMAND_MASTER; - pci_write_config_dword(pdev, PCI_COMMAND, cmd); - - return 0; -} - -/* - * Destroy the appropriate control structures corresponding * to the XHCI host - * controller - */ -void xhci_hcd_stop(int index) -{ -} - -#else - struct xhci_pci_priv { struct xhci_ctrl ctrl; /* Needs to come first in this struct! */ }; @@ -137,5 +87,3 @@ static struct pci_device_id xhci_pci_supported[] = { };
U_BOOT_PCI_DEVICE(xhci_pci, xhci_pci_supported); - -#endif /* CONFIG_DM_USB */
On 06/23/2017 11:54 AM, Bin Meng wrote:
As there is no board that currently uses xhci-pci driver without DM USB, drop its support and leave only DM support.
You should add something into the Kconfig to make this driver depend on DM_USB ; unless it's already there.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/usb/host/xhci-pci.c | 52 --------------------------------------------- 1 file changed, 52 deletions(-)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 63daaa6..5ad8452 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -14,56 +14,6 @@
#include "xhci.h"
-#ifndef CONFIG_DM_USB
-/*
- Create the appropriate control structures to manage a new XHCI host
- controller.
- */
-int xhci_hcd_init(int index, struct xhci_hccr **ret_hccr,
struct xhci_hcor **ret_hcor)-{
- struct xhci_hccr *hccr;
- struct xhci_hcor *hcor;
- pci_dev_t pdev;
- uint32_t cmd;
- int len;
- pdev = pci_find_class(PCI_CLASS_SERIAL_USB_XHCI, index);
- if (pdev < 0) {
printf("XHCI host controller not found\n");return -1;- }
- hccr = (struct xhci_hccr *)pci_map_bar(pdev,
PCI_BASE_ADDRESS_0, PCI_REGION_MEM);- len = HC_LENGTH(xhci_readl(&hccr->cr_capbase));
- hcor = (struct xhci_hcor *)((uint32_t)hccr + len);
- debug("XHCI-PCI init hccr 0x%x and hcor 0x%x hc_length %d\n",
(uint32_t)hccr, (uint32_t)hcor, len);- *ret_hccr = hccr;
- *ret_hcor = hcor;
- /* enable busmaster */
- pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
- cmd |= PCI_COMMAND_MASTER;
- pci_write_config_dword(pdev, PCI_COMMAND, cmd);
- return 0;
-}
-/*
- Destroy the appropriate control structures corresponding * to the XHCI host
- controller
- */
-void xhci_hcd_stop(int index) -{ -}
-#else
struct xhci_pci_priv { struct xhci_ctrl ctrl; /* Needs to come first in this struct! */ }; @@ -137,5 +87,3 @@ static struct pci_device_id xhci_pci_supported[] = { };
U_BOOT_PCI_DEVICE(xhci_pci, xhci_pci_supported);
-#endif /* CONFIG_DM_USB */
On 23 June 2017 at 03:54, Bin Meng bmeng.cn@gmail.com wrote:
As there is no board that currently uses xhci-pci driver without DM USB, drop its support and leave only DM support.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/usb/host/xhci-pci.c | 52 --------------------------------------------- 1 file changed, 52 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Note for Marek: CONFIG_USB_XHCI_PCI is not in Kconfig yet.
This cleans up the driver a little bit.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
drivers/usb/host/xhci-pci.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 5ad8452..56fd650 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -8,16 +8,10 @@
#include <common.h> #include <dm.h> -#include <errno.h> #include <pci.h> #include <usb.h> - #include "xhci.h"
-struct xhci_pci_priv { - struct xhci_ctrl ctrl; /* Needs to come first in this struct! */ -}; - static void xhci_pci_init(struct udevice *dev, struct xhci_hccr **ret_hccr, struct xhci_hcor **ret_hcor) { @@ -55,13 +49,7 @@ static int xhci_pci_probe(struct udevice *dev)
static int xhci_pci_remove(struct udevice *dev) { - int ret; - - ret = xhci_deregister(dev); - if (ret) - return ret; - - return 0; + return xhci_deregister(dev); }
static const struct udevice_id xhci_pci_ids[] = { @@ -77,7 +65,7 @@ U_BOOT_DRIVER(xhci_pci) = { .of_match = xhci_pci_ids, .ops = &xhci_usb_ops, .platdata_auto_alloc_size = sizeof(struct usb_platdata), - .priv_auto_alloc_size = sizeof(struct xhci_pci_priv), + .priv_auto_alloc_size = sizeof(struct xhci_ctrl), .flags = DM_FLAG_ALLOC_PRIV_DMA, };
On 06/23/2017 11:54 AM, Bin Meng wrote:
This cleans up the driver a little bit.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/usb/host/xhci-pci.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 5ad8452..56fd650 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -8,16 +8,10 @@
#include <common.h> #include <dm.h> -#include <errno.h> #include <pci.h> #include <usb.h>
#include "xhci.h"
-struct xhci_pci_priv {
- struct xhci_ctrl ctrl; /* Needs to come first in this struct! */
-};
static void xhci_pci_init(struct udevice *dev, struct xhci_hccr **ret_hccr, struct xhci_hcor **ret_hcor) { @@ -55,13 +49,7 @@ static int xhci_pci_probe(struct udevice *dev)
static int xhci_pci_remove(struct udevice *dev) {
- int ret;
- ret = xhci_deregister(dev);
- if (ret)
return ret;- return 0;
- return xhci_deregister(dev);
Can you insert xhci_deregister directly into the callbacks structure and nuke xhci_pci_remove() altogether ?
}
static const struct udevice_id xhci_pci_ids[] = { @@ -77,7 +65,7 @@ U_BOOT_DRIVER(xhci_pci) = { .of_match = xhci_pci_ids, .ops = &xhci_usb_ops, .platdata_auto_alloc_size = sizeof(struct usb_platdata),
- .priv_auto_alloc_size = sizeof(struct xhci_pci_priv),
- .priv_auto_alloc_size = sizeof(struct xhci_ctrl), .flags = DM_FLAG_ALLOC_PRIV_DMA,
};
On 23 June 2017 at 11:52, Marek Vasut marex@denx.de wrote:
On 06/23/2017 11:54 AM, Bin Meng wrote:
This cleans up the driver a little bit.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/usb/host/xhci-pci.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 5ad8452..56fd650 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -8,16 +8,10 @@
#include <common.h> #include <dm.h> -#include <errno.h> #include <pci.h> #include <usb.h>
#include "xhci.h"
-struct xhci_pci_priv {
struct xhci_ctrl ctrl; /* Needs to come first in this struct! */-};
static void xhci_pci_init(struct udevice *dev, struct xhci_hccr **ret_hccr, struct xhci_hcor **ret_hcor) { @@ -55,13 +49,7 @@ static int xhci_pci_probe(struct udevice *dev)
static int xhci_pci_remove(struct udevice *dev) {
int ret;ret = xhci_deregister(dev);if (ret)return ret;return 0;
return xhci_deregister(dev);Can you insert xhci_deregister directly into the callbacks structure and nuke xhci_pci_remove() altogether ?
Either way:
Reviewed-by: Simon Glass sjg@chromium.org
}
static const struct udevice_id xhci_pci_ids[] = { @@ -77,7 +65,7 @@ U_BOOT_DRIVER(xhci_pci) = { .of_match = xhci_pci_ids, .ops = &xhci_usb_ops, .platdata_auto_alloc_size = sizeof(struct usb_platdata),
.priv_auto_alloc_size = sizeof(struct xhci_pci_priv),
.priv_auto_alloc_size = sizeof(struct xhci_ctrl), .flags = DM_FLAG_ALLOC_PRIV_DMA,};
-- Best regards, Marek Vasut
The only work we need do in usb_setup_descriptor() is to initialize dev->descriptor.bMaxPacketSize0, when do_read is false. Other steps are the same as do_read being true.
While we are here, update the comment block to be within 80 cols.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
common/usb.c | 40 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 22 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 15e1e4c..293d968 100644 --- a/common/usb.c +++ b/common/usb.c @@ -962,37 +962,33 @@ static int usb_setup_descriptor(struct usb_device *dev, bool do_read) * some more, or keeps on retransmitting the 8 byte header. */
- if (dev->speed == USB_SPEED_LOW) { - dev->descriptor.bMaxPacketSize0 = 8; - dev->maxpacketsize = PACKET_SIZE_8; - } else { - dev->descriptor.bMaxPacketSize0 = 64; - dev->maxpacketsize = PACKET_SIZE_64; - } - dev->epmaxpacketin[0] = dev->descriptor.bMaxPacketSize0; - dev->epmaxpacketout[0] = dev->descriptor.bMaxPacketSize0; - if (do_read) { int err;
/* - * Validate we've received only at least 8 bytes, not that we've - * received the entire descriptor. The reasoning is: - * - The code only uses fields in the first 8 bytes, so that's all we - * need to have fetched at this stage. - * - The smallest maxpacket size is 8 bytes. Before we know the actual - * maxpacket the device uses, the USB controller may only accept a - * single packet. Consequently we are only guaranteed to receive 1 - * packet (at least 8 bytes) even in a non-error case. + * Validate we've received only at least 8 bytes, not that + * we've received the entire descriptor. The reasoning is: + * - The code only uses fields in the first 8 bytes, so that's + * all we need to have fetched at this stage. + * - The smallest maxpacket size is 8 bytes. Before we know + * the actual maxpacket the device uses, the USB controller + * may only accept a single packet. Consequently we are only + * guaranteed to receive 1 packet (at least 8 bytes) even in + * a non-error case. * - * At least the DWC2 controller needs to be programmed with the number - * of packets in addition to the number of bytes. A request for 64 - * bytes of data with the maxpacket guessed as 64 (above) yields a - * request for 1 packet. + * At least the DWC2 controller needs to be programmed with + * the number of packets in addition to the number of bytes. + * A request for 64 bytes of data with the maxpacket guessed + * as 64 (above) yields a request for 1 packet. */ err = get_descriptor_len(dev, 64, 8); if (err) return err; + } else { + if (dev->speed == USB_SPEED_LOW) + dev->descriptor.bMaxPacketSize0 = 8; + else + dev->descriptor.bMaxPacketSize0 = 64; }
dev->epmaxpacketin[0] = dev->descriptor.bMaxPacketSize0;
Hi,
On Fri, Jun 23, 2017 at 5:54 PM, Bin Meng bmeng.cn@gmail.com wrote:
The only work we need do in usb_setup_descriptor() is to initialize dev->descriptor.bMaxPacketSize0, when do_read is false. Other steps are the same as do_read being true.
While we are here, update the comment block to be within 80 cols.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
common/usb.c | 40 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 22 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 15e1e4c..293d968 100644 --- a/common/usb.c +++ b/common/usb.c @@ -962,37 +962,33 @@ static int usb_setup_descriptor(struct usb_device *dev, bool do_read) * some more, or keeps on retransmitting the 8 byte header. */
if (dev->speed == USB_SPEED_LOW) {dev->descriptor.bMaxPacketSize0 = 8;dev->maxpacketsize = PACKET_SIZE_8;} else {dev->descriptor.bMaxPacketSize0 = 64;dev->maxpacketsize = PACKET_SIZE_64;}dev->epmaxpacketin[0] = dev->descriptor.bMaxPacketSize0;dev->epmaxpacketout[0] = dev->descriptor.bMaxPacketSize0;if (do_read) { int err; /** Validate we've received only at least 8 bytes, not that we've* received the entire descriptor. The reasoning is:* - The code only uses fields in the first 8 bytes, so that's all we* need to have fetched at this stage.* - The smallest maxpacket size is 8 bytes. Before we know the actual* maxpacket the device uses, the USB controller may only accept a* single packet. Consequently we are only guaranteed to receive 1* packet (at least 8 bytes) even in a non-error case.
* Validate we've received only at least 8 bytes, not that* we've received the entire descriptor. The reasoning is:* - The code only uses fields in the first 8 bytes, so that's* all we need to have fetched at this stage.* - The smallest maxpacket size is 8 bytes. Before we know* the actual maxpacket the device uses, the USB controller* may only accept a single packet. Consequently we are only* guaranteed to receive 1 packet (at least 8 bytes) even in* a non-error case. *
* At least the DWC2 controller needs to be programmed with the number* of packets in addition to the number of bytes. A request for 64* bytes of data with the maxpacket guessed as 64 (above) yields a* request for 1 packet.
* At least the DWC2 controller needs to be programmed with* the number of packets in addition to the number of bytes.* A request for 64 bytes of data with the maxpacket guessed* as 64 (above) yields a request for 1 packet. */ err = get_descriptor_len(dev, 64, 8); if (err) return err;} else {if (dev->speed == USB_SPEED_LOW)dev->descriptor.bMaxPacketSize0 = 8;elsedev->descriptor.bMaxPacketSize0 = 64; } dev->epmaxpacketin[0] = dev->descriptor.bMaxPacketSize0;--
For some reason that I don't understand, this patch breaks EHCI. dev->maxpacketsize is equal to zero with this patch, which leads to a 'divide error' exception in ehci_submit_async(). Not sure if anyone has some hints?
For now, I will just drop this patch in v2. This needs be further revisited.
Regards, Bin
Hi,
On Fri, 30 Jun 2017 11:49:56 +0800 Bin Meng wrote:
Hi,
On Fri, Jun 23, 2017 at 5:54 PM, Bin Meng bmeng.cn@gmail.com wrote:
The only work we need do in usb_setup_descriptor() is to initialize dev->descriptor.bMaxPacketSize0, when do_read is false. Other steps are the same as do_read being true.
While we are here, update the comment block to be within 80 cols.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
common/usb.c | 40 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 22 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 15e1e4c..293d968 100644 --- a/common/usb.c +++ b/common/usb.c @@ -962,37 +962,33 @@ static int usb_setup_descriptor(struct usb_device *dev, bool do_read) * some more, or keeps on retransmitting the 8 byte header. */
if (dev->speed == USB_SPEED_LOW) {dev->descriptor.bMaxPacketSize0 = 8;dev->maxpacketsize = PACKET_SIZE_8;} else {dev->descriptor.bMaxPacketSize0 = 64;dev->maxpacketsize = PACKET_SIZE_64;}dev->epmaxpacketin[0] = dev->descriptor.bMaxPacketSize0;dev->epmaxpacketout[0] = dev->descriptor.bMaxPacketSize0;if (do_read) { int err; /** Validate we've received only at least 8 bytes, not that we've* received the entire descriptor. The reasoning is:* - The code only uses fields in the first 8 bytes, so that's all we* need to have fetched at this stage.* - The smallest maxpacket size is 8 bytes. Before we know the actual* maxpacket the device uses, the USB controller may only accept a* single packet. Consequently we are only guaranteed to receive 1* packet (at least 8 bytes) even in a non-error case.
* Validate we've received only at least 8 bytes, not that* we've received the entire descriptor. The reasoning is:* - The code only uses fields in the first 8 bytes, so that's* all we need to have fetched at this stage.* - The smallest maxpacket size is 8 bytes. Before we know* the actual maxpacket the device uses, the USB controller* may only accept a single packet. Consequently we are only* guaranteed to receive 1 packet (at least 8 bytes) even in* a non-error case. *
* At least the DWC2 controller needs to be programmed with the number* of packets in addition to the number of bytes. A request for 64* bytes of data with the maxpacket guessed as 64 (above) yields a* request for 1 packet.
* At least the DWC2 controller needs to be programmed with* the number of packets in addition to the number of bytes.* A request for 64 bytes of data with the maxpacket guessed* as 64 (above) yields a request for 1 packet. */ err = get_descriptor_len(dev, 64, 8); if (err) return err;} else {if (dev->speed == USB_SPEED_LOW)dev->descriptor.bMaxPacketSize0 = 8;elsedev->descriptor.bMaxPacketSize0 = 64; } dev->epmaxpacketin[0] = dev->descriptor.bMaxPacketSize0;--
For some reason that I don't understand, this patch breaks EHCI. dev->maxpacketsize is equal to zero with this patch, which leads to a 'divide error' exception in ehci_submit_async(). Not sure if anyone has some hints?
In the do_read case the dev->descriptor.bMaxPacketSize0 is not set up.
Lothar Waßmann
Hi Lothar,
On Fri, Jun 30, 2017 at 2:15 PM, Lothar Waßmann LW@karo-electronics.de wrote:
Hi,
On Fri, 30 Jun 2017 11:49:56 +0800 Bin Meng wrote:
Hi,
On Fri, Jun 23, 2017 at 5:54 PM, Bin Meng bmeng.cn@gmail.com wrote:
The only work we need do in usb_setup_descriptor() is to initialize dev->descriptor.bMaxPacketSize0, when do_read is false. Other steps are the same as do_read being true.
While we are here, update the comment block to be within 80 cols.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
common/usb.c | 40 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 22 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 15e1e4c..293d968 100644 --- a/common/usb.c +++ b/common/usb.c @@ -962,37 +962,33 @@ static int usb_setup_descriptor(struct usb_device *dev, bool do_read) * some more, or keeps on retransmitting the 8 byte header. */
if (dev->speed == USB_SPEED_LOW) {dev->descriptor.bMaxPacketSize0 = 8;dev->maxpacketsize = PACKET_SIZE_8;} else {dev->descriptor.bMaxPacketSize0 = 64;dev->maxpacketsize = PACKET_SIZE_64;}dev->epmaxpacketin[0] = dev->descriptor.bMaxPacketSize0;dev->epmaxpacketout[0] = dev->descriptor.bMaxPacketSize0;if (do_read) { int err; /** Validate we've received only at least 8 bytes, not that we've* received the entire descriptor. The reasoning is:* - The code only uses fields in the first 8 bytes, so that's all we* need to have fetched at this stage.* - The smallest maxpacket size is 8 bytes. Before we know the actual* maxpacket the device uses, the USB controller may only accept a* single packet. Consequently we are only guaranteed to receive 1* packet (at least 8 bytes) even in a non-error case.
* Validate we've received only at least 8 bytes, not that* we've received the entire descriptor. The reasoning is:* - The code only uses fields in the first 8 bytes, so that's* all we need to have fetched at this stage.* - The smallest maxpacket size is 8 bytes. Before we know* the actual maxpacket the device uses, the USB controller* may only accept a single packet. Consequently we are only* guaranteed to receive 1 packet (at least 8 bytes) even in* a non-error case. *
* At least the DWC2 controller needs to be programmed with the number* of packets in addition to the number of bytes. A request for 64* bytes of data with the maxpacket guessed as 64 (above) yields a* request for 1 packet.
* At least the DWC2 controller needs to be programmed with* the number of packets in addition to the number of bytes.* A request for 64 bytes of data with the maxpacket guessed* as 64 (above) yields a request for 1 packet. */ err = get_descriptor_len(dev, 64, 8); if (err) return err;} else {if (dev->speed == USB_SPEED_LOW)dev->descriptor.bMaxPacketSize0 = 8;elsedev->descriptor.bMaxPacketSize0 = 64; } dev->epmaxpacketin[0] = dev->descriptor.bMaxPacketSize0;--
For some reason that I don't understand, this patch breaks EHCI. dev->maxpacketsize is equal to zero with this patch, which leads to a 'divide error' exception in ehci_submit_async(). Not sure if anyone has some hints?
In the do_read case the dev->descriptor.bMaxPacketSize0 is not set up.
In that case, dev->descriptor.bMaxPacketSize0 was read from device. Am I missing anything?
Regards, Bin
On Freitag, 30. Juni 2017 05:49:56 CEST Bin Meng wrote:
Hi,
On Fri, Jun 23, 2017 at 5:54 PM, Bin Meng bmeng.cn@gmail.com wrote:
The only work we need do in usb_setup_descriptor() is to initialize dev->descriptor.bMaxPacketSize0, when do_read is false. Other steps are the same as do_read being true.
While we are here, update the comment block to be within 80 cols.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
common/usb.c | 40 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 22 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 15e1e4c..293d968 100644 --- a/common/usb.c +++ b/common/usb.c @@ -962,37 +962,33 @@ static int usb_setup_descriptor(struct usb_device *dev, bool do_read)> * some more, or keeps on retransmitting the 8 byte header. */
if (dev->speed == USB_SPEED_LOW) {dev->descriptor.bMaxPacketSize0 = 8;dev->maxpacketsize = PACKET_SIZE_8;} else {dev->descriptor.bMaxPacketSize0 = 64;dev->maxpacketsize = PACKET_SIZE_64;}dev->epmaxpacketin[0] = dev->descriptor.bMaxPacketSize0;dev->epmaxpacketout[0] = dev->descriptor.bMaxPacketSize0;
You remove the initialization of dev->maxpacketsize here ...
if (do_read) {
[...]
*/ err = get_descriptor_len(dev, 64, 8); if (err) return err;
... and probably return early here. Can you add some debug output to get_descriptor_len(...) (len, expected len, return code)?
} else {if (dev->speed == USB_SPEED_LOW)dev->descriptor.bMaxPacketSize0 = 8;elsedev->descriptor.bMaxPacketSize0 = 64; } dev->epmaxpacketin[0] = dev->descriptor.bMaxPacketSize0;--
For some reason that I don't understand, this patch breaks EHCI. dev->maxpacketsize is equal to zero with this patch, which leads to a 'divide error' exception in ehci_submit_async(). Not sure if anyone has some hints?
For now, I will just drop this patch in v2. This needs be further revisited.
Kind regards,
Stefan
Hi Stefan,
On Sun, Jul 2, 2017 at 1:29 AM, stefan.bruens@rwth-aachen.de wrote:
On Freitag, 30. Juni 2017 05:49:56 CEST Bin Meng wrote:
Hi,
On Fri, Jun 23, 2017 at 5:54 PM, Bin Meng bmeng.cn@gmail.com wrote:
The only work we need do in usb_setup_descriptor() is to initialize dev->descriptor.bMaxPacketSize0, when do_read is false. Other steps are the same as do_read being true.
While we are here, update the comment block to be within 80 cols.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
common/usb.c | 40 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 22 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 15e1e4c..293d968 100644 --- a/common/usb.c +++ b/common/usb.c @@ -962,37 +962,33 @@ static int usb_setup_descriptor(struct usb_device *dev, bool do_read)> * some more, or keeps on retransmitting the 8 byte header. */
if (dev->speed == USB_SPEED_LOW) {dev->descriptor.bMaxPacketSize0 = 8;dev->maxpacketsize = PACKET_SIZE_8;} else {dev->descriptor.bMaxPacketSize0 = 64;dev->maxpacketsize = PACKET_SIZE_64;}dev->epmaxpacketin[0] = dev->descriptor.bMaxPacketSize0;dev->epmaxpacketout[0] = dev->descriptor.bMaxPacketSize0;You remove the initialization of dev->maxpacketsize here ...
if (do_read) {[...]
*/ err = get_descriptor_len(dev, 64, 8); if (err) return err;... and probably return early here. Can you add some debug output to get_descriptor_len(...) (len, expected len, return code)?
The get_descriptor_len() was successful, so it is not caused by the "if (err)" branch.
} else {if (dev->speed == USB_SPEED_LOW)dev->descriptor.bMaxPacketSize0 = 8;elsedev->descriptor.bMaxPacketSize0 = 64; } dev->epmaxpacketin[0] = dev->descriptor.bMaxPacketSize0;--
For some reason that I don't understand, this patch breaks EHCI. dev->maxpacketsize is equal to zero with this patch, which leads to a 'divide error' exception in ehci_submit_async(). Not sure if anyone has some hints?
For now, I will just drop this patch in v2. This needs be further revisited.
Regards, Bin
Use USB hub device's dev->uclass_priv to point to 'usb_hub_device' so that with driver model usb_hub_reset() and usb_hub_allocate() are no longer needed.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
common/usb_hub.c | 10 +++++++++- drivers/usb/host/usb-uclass.c | 2 -- 2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/common/usb_hub.c b/common/usb_hub.c index 090966b..18bd827 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -58,9 +58,10 @@ struct usb_device_scan { struct list_head list; };
-/* TODO(sjg@chromium.org): Remove this when CONFIG_DM_USB is defined */ +#ifndef CONFIG_DM_USB static struct usb_hub_device hub_dev[USB_MAX_HUB]; static int usb_hub_index; +#endif static LIST_HEAD(usb_scan_list);
__weak void usb_hub_reset_devices(int port) @@ -167,6 +168,7 @@ static void usb_hub_power_on(struct usb_hub_device *hub) max(100, (int)pgood_delay) + 1000); }
+#ifndef CONFIG_DM_USB void usb_hub_reset(void) { usb_hub_index = 0; @@ -183,6 +185,7 @@ static struct usb_hub_device *usb_hub_allocate(void) printf("ERROR: USB_MAX_HUB (%d) reached\n", USB_MAX_HUB); return NULL; } +#endif
#define MAX_TRIES 5
@@ -557,10 +560,14 @@ static int usb_hub_configure(struct usb_device *dev) __maybe_unused struct usb_hub_status *hubsts; int ret;
+#ifndef CONFIG_DM_USB /* "allocate" Hub device */ hub = usb_hub_allocate(); if (hub == NULL) return -ENOMEM; +#else + hub = dev_get_uclass_priv(dev->dev); +#endif hub->pusb_dev = dev; /* Get the the hub descriptor */ ret = usb_get_hub_descriptor(dev, buffer, 4); @@ -795,6 +802,7 @@ UCLASS_DRIVER(usb_hub) = { .child_pre_probe = usb_child_pre_probe, .per_child_auto_alloc_size = sizeof(struct usb_device), .per_child_platdata_auto_alloc_size = sizeof(struct usb_dev_platdata), + .per_device_auto_alloc_size = sizeof(struct usb_hub_device), };
static const struct usb_device_id hub_id_table[] = { diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index 110ddc9..9bb8477 100644 --- a/drivers/usb/host/usb-uclass.c +++ b/drivers/usb/host/usb-uclass.c @@ -177,7 +177,6 @@ int usb_stop(void) #ifdef CONFIG_USB_STORAGE usb_stor_reset(); #endif - usb_hub_reset(); uc_priv->companion_device_count = 0; usb_started = 0;
@@ -230,7 +229,6 @@ int usb_init(void) int ret;
asynch_allowed = 1; - usb_hub_reset();
ret = uclass_get(UCLASS_USB, &uc); if (ret)
On 06/23/2017 11:54 AM, Bin Meng wrote:
Use USB hub device's dev->uclass_priv to point to 'usb_hub_device' so that with driver model usb_hub_reset() and usb_hub_allocate() are no longer needed.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Can you trim down the ifdeffery somehow or keep it more contained ? I don't like having the code polluted by ifdefs all over the place, so maybe factor out the specialties into function and put ifdef around that or somesuch.
common/usb_hub.c | 10 +++++++++- drivers/usb/host/usb-uclass.c | 2 -- 2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/common/usb_hub.c b/common/usb_hub.c index 090966b..18bd827 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -58,9 +58,10 @@ struct usb_device_scan { struct list_head list; };
-/* TODO(sjg@chromium.org): Remove this when CONFIG_DM_USB is defined */ +#ifndef CONFIG_DM_USB static struct usb_hub_device hub_dev[USB_MAX_HUB]; static int usb_hub_index; +#endif static LIST_HEAD(usb_scan_list);
__weak void usb_hub_reset_devices(int port) @@ -167,6 +168,7 @@ static void usb_hub_power_on(struct usb_hub_device *hub) max(100, (int)pgood_delay) + 1000); }
+#ifndef CONFIG_DM_USB void usb_hub_reset(void) { usb_hub_index = 0; @@ -183,6 +185,7 @@ static struct usb_hub_device *usb_hub_allocate(void) printf("ERROR: USB_MAX_HUB (%d) reached\n", USB_MAX_HUB); return NULL; } +#endif
#define MAX_TRIES 5
@@ -557,10 +560,14 @@ static int usb_hub_configure(struct usb_device *dev) __maybe_unused struct usb_hub_status *hubsts; int ret;
+#ifndef CONFIG_DM_USB /* "allocate" Hub device */ hub = usb_hub_allocate(); if (hub == NULL) return -ENOMEM; +#else
- hub = dev_get_uclass_priv(dev->dev);
+#endif hub->pusb_dev = dev; /* Get the the hub descriptor */ ret = usb_get_hub_descriptor(dev, buffer, 4); @@ -795,6 +802,7 @@ UCLASS_DRIVER(usb_hub) = { .child_pre_probe = usb_child_pre_probe, .per_child_auto_alloc_size = sizeof(struct usb_device), .per_child_platdata_auto_alloc_size = sizeof(struct usb_dev_platdata),
- .per_device_auto_alloc_size = sizeof(struct usb_hub_device),
};
static const struct usb_device_id hub_id_table[] = { diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index 110ddc9..9bb8477 100644 --- a/drivers/usb/host/usb-uclass.c +++ b/drivers/usb/host/usb-uclass.c @@ -177,7 +177,6 @@ int usb_stop(void) #ifdef CONFIG_USB_STORAGE usb_stor_reset(); #endif
- usb_hub_reset(); uc_priv->companion_device_count = 0; usb_started = 0;
@@ -230,7 +229,6 @@ int usb_init(void) int ret;
asynch_allowed = 1;
usb_hub_reset();
ret = uclass_get(UCLASS_USB, &uc); if (ret)
On 23 June 2017 at 03:54, Bin Meng bmeng.cn@gmail.com wrote:
Use USB hub device's dev->uclass_priv to point to 'usb_hub_device' so that with driver model usb_hub_reset() and usb_hub_allocate() are no longer needed.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
common/usb_hub.c | 10 +++++++++- drivers/usb/host/usb-uclass.c | 2 -- 2 files changed, 9 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Sometimes we need know if a given hub device is root hub or not. Add a new API to test this.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
common/usb_hub.c | 10 ++++++++++ include/usb.h | 8 ++++++++ 2 files changed, 18 insertions(+)
diff --git a/common/usb_hub.c b/common/usb_hub.c index 18bd827..d780251 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -74,6 +74,16 @@ static inline bool usb_hub_is_superspeed(struct usb_device *hdev) return hdev->descriptor.bDeviceProtocol == 3; }
+#ifdef CONFIG_DM_USB +bool usb_hub_is_root_hub(struct udevice *hub) +{ + if (device_get_uclass_id(hub->parent) != UCLASS_USB_HUB) + return true; + + return false; +} +#endif + static int usb_get_hub_descriptor(struct usb_device *dev, void *data, int size) { unsigned short dtype = USB_DT_HUB; diff --git a/include/usb.h b/include/usb.h index eb82cc2..c504d71 100644 --- a/include/usb.h +++ b/include/usb.h @@ -776,6 +776,14 @@ int usb_setup_device(struct usb_device *dev, bool do_read, struct usb_device *parent);
/** + * usb_hub_is_root_hub() - Test whether a hub device is root hub or not + * + * @hub: USB hub device to test + * @return: true if the hub device is root hub, false otherwise. + */ +bool usb_hub_is_root_hub(struct udevice *hub); + +/** * usb_hub_scan() - Scan a hub and find its devices * * @hub: Hub device to scan
On 06/23/2017 11:54 AM, Bin Meng wrote:
Sometimes we need know if a given hub device is root hub or not. Add a new API to test this.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
common/usb_hub.c | 10 ++++++++++ include/usb.h | 8 ++++++++ 2 files changed, 18 insertions(+)
diff --git a/common/usb_hub.c b/common/usb_hub.c index 18bd827..d780251 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -74,6 +74,16 @@ static inline bool usb_hub_is_superspeed(struct usb_device *hdev) return hdev->descriptor.bDeviceProtocol == 3; }
+#ifdef CONFIG_DM_USB +bool usb_hub_is_root_hub(struct udevice *hub) +{
- if (device_get_uclass_id(hub->parent) != UCLASS_USB_HUB)
Can this call fail ?
return true;- return false;
+} +#endif
static int usb_get_hub_descriptor(struct usb_device *dev, void *data, int size) { unsigned short dtype = USB_DT_HUB; diff --git a/include/usb.h b/include/usb.h index eb82cc2..c504d71 100644 --- a/include/usb.h +++ b/include/usb.h @@ -776,6 +776,14 @@ int usb_setup_device(struct usb_device *dev, bool do_read, struct usb_device *parent);
/**
- usb_hub_is_root_hub() - Test whether a hub device is root hub or not
- @hub: USB hub device to test
- @return: true if the hub device is root hub, false otherwise.
- */
+bool usb_hub_is_root_hub(struct udevice *hub);
+/**
- usb_hub_scan() - Scan a hub and find its devices
- @hub: Hub device to scan
Hi Marek,
On Sat, Jun 24, 2017 at 1:55 AM, Marek Vasut marex@denx.de wrote:
On 06/23/2017 11:54 AM, Bin Meng wrote:
Sometimes we need know if a given hub device is root hub or not. Add a new API to test this.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
common/usb_hub.c | 10 ++++++++++ include/usb.h | 8 ++++++++ 2 files changed, 18 insertions(+)
diff --git a/common/usb_hub.c b/common/usb_hub.c index 18bd827..d780251 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -74,6 +74,16 @@ static inline bool usb_hub_is_superspeed(struct usb_device *hdev) return hdev->descriptor.bDeviceProtocol == 3; }
+#ifdef CONFIG_DM_USB +bool usb_hub_is_root_hub(struct udevice *hub) +{
if (device_get_uclass_id(hub->parent) != UCLASS_USB_HUB)Can this call fail ?
No,
Regards, Bin
On 06/23/2017 11:54 AM, Bin Meng wrote:
Sometimes we need know if a given hub device is root hub or not. Add a new API to test this.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
common/usb_hub.c | 10 ++++++++++ include/usb.h | 8 ++++++++ 2 files changed, 18 insertions(+)
diff --git a/common/usb_hub.c b/common/usb_hub.c index 18bd827..d780251 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -74,6 +74,16 @@ static inline bool usb_hub_is_superspeed(struct usb_device *hdev) return hdev->descriptor.bDeviceProtocol == 3; }
+#ifdef CONFIG_DM_USB +bool usb_hub_is_root_hub(struct udevice *hub)
Actually , this is the is_root_hub() from the 6/16 , right , not a new API. If you want to factor out stuff , just do that , but also remove the is_root_hub() and do the conversion in the same patch.
+{
- if (device_get_uclass_id(hub->parent) != UCLASS_USB_HUB)
return true;- return false;
+} +#endif
static int usb_get_hub_descriptor(struct usb_device *dev, void *data, int size) { unsigned short dtype = USB_DT_HUB; diff --git a/include/usb.h b/include/usb.h index eb82cc2..c504d71 100644 --- a/include/usb.h +++ b/include/usb.h @@ -776,6 +776,14 @@ int usb_setup_device(struct usb_device *dev, bool do_read, struct usb_device *parent);
/**
- usb_hub_is_root_hub() - Test whether a hub device is root hub or not
- @hub: USB hub device to test
- @return: true if the hub device is root hub, false otherwise.
- */
+bool usb_hub_is_root_hub(struct udevice *hub);
+/**
- usb_hub_scan() - Scan a hub and find its devices
- @hub: Hub device to scan
Hi Marek,
On Sat, Jun 24, 2017 at 1:57 AM, Marek Vasut marex@denx.de wrote:
On 06/23/2017 11:54 AM, Bin Meng wrote:
Sometimes we need know if a given hub device is root hub or not. Add a new API to test this.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
common/usb_hub.c | 10 ++++++++++ include/usb.h | 8 ++++++++ 2 files changed, 18 insertions(+)
diff --git a/common/usb_hub.c b/common/usb_hub.c index 18bd827..d780251 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -74,6 +74,16 @@ static inline bool usb_hub_is_superspeed(struct usb_device *hdev) return hdev->descriptor.bDeviceProtocol == 3; }
+#ifdef CONFIG_DM_USB +bool usb_hub_is_root_hub(struct udevice *hub)
Actually , this is the is_root_hub() from the 6/16 , right , not a new API. If you want to factor out stuff , just do that , but also remove the is_root_hub() and do the conversion in the same patch.
Correct, is_root_hub() is static within xhci.c and only used by part of the xHCI driver. To other USB codes, this is a new API. The two patches (5/16, 6/16) are still self-contained, as each is against a single module. But if you would like to do the two in one patch, let me know and I will do in v2.
[snip]
Regards, Bin
On 06/24/2017 03:41 AM, Bin Meng wrote:
Hi Marek,
On Sat, Jun 24, 2017 at 1:57 AM, Marek Vasut marex@denx.de wrote:
On 06/23/2017 11:54 AM, Bin Meng wrote:
Sometimes we need know if a given hub device is root hub or not. Add a new API to test this.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
common/usb_hub.c | 10 ++++++++++ include/usb.h | 8 ++++++++ 2 files changed, 18 insertions(+)
diff --git a/common/usb_hub.c b/common/usb_hub.c index 18bd827..d780251 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -74,6 +74,16 @@ static inline bool usb_hub_is_superspeed(struct usb_device *hdev) return hdev->descriptor.bDeviceProtocol == 3; }
+#ifdef CONFIG_DM_USB +bool usb_hub_is_root_hub(struct udevice *hub)
Actually , this is the is_root_hub() from the 6/16 , right , not a new API. If you want to factor out stuff , just do that , but also remove the is_root_hub() and do the conversion in the same patch.
Correct, is_root_hub() is static within xhci.c and only used by part of the xHCI driver. To other USB codes, this is a new API. The two patches (5/16, 6/16) are still self-contained, as each is against a single module. But if you would like to do the two in one patch, let me know and I will do in v2.
I'd like a patch which pulls this out of xhci driver, yes.
[snip]
Regards, Bin
Now that we have a generic public API, remove the xHCI driver's own version is_root_hub() and use the new API.
While we are here, remove the unused/commented out get_usb_device().
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
drivers/usb/host/xhci.c | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 8631e27..a5b888a 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1116,26 +1116,6 @@ int usb_lowlevel_stop(int index) #endif /* CONFIG_DM_USB */
#ifdef CONFIG_DM_USB -/* -static struct usb_device *get_usb_device(struct udevice *dev) -{ - struct usb_device *udev; - - if (device_get_uclass_id(dev) == UCLASS_USB) - udev = dev_get_uclass_priv(dev); - else - udev = dev_get_parent_priv(dev); - - return udev; -} -*/ -static bool is_root_hub(struct udevice *dev) -{ - if (device_get_uclass_id(dev->parent) != UCLASS_USB_HUB) - return true; - - return false; -}
static int xhci_submit_control_msg(struct udevice *dev, struct usb_device *udev, unsigned long pipe, void *buffer, int length, @@ -1150,10 +1130,10 @@ static int xhci_submit_control_msg(struct udevice *dev, struct usb_device *udev, hub = udev->dev; if (device_get_uclass_id(hub) == UCLASS_USB_HUB) { /* Figure out our port number on the root hub */ - if (is_root_hub(hub)) { + if (usb_hub_is_root_hub(hub)) { root_portnr = udev->portnr; } else { - while (!is_root_hub(hub->parent)) + while (!usb_hub_is_root_hub(hub->parent)) hub = hub->parent; uhop = dev_get_parent_priv(hub); root_portnr = uhop->portnr;
On 23 June 2017 at 03:54, Bin Meng bmeng.cn@gmail.com wrote:
Now that we have a generic public API, remove the xHCI driver's own version is_root_hub() and use the new API.
While we are here, remove the unused/commented out get_usb_device().
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/usb/host/xhci.c | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
USB 3.0 hub port status field has different bit positions from 2.0 hubs. Since U-Boot only understands the old version, translate the new one into the old one.
Since we are going to add USB 3.0 hub support, this feature is only available with driver model USB.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
common/usb_hub.c | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/common/usb_hub.c b/common/usb_hub.c index d780251..835fac9 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -119,9 +119,40 @@ static int usb_get_hub_status(struct usb_device *dev, void *data)
int usb_get_port_status(struct usb_device *dev, int port, void *data) { - return usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), + int ret; + + ret = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), USB_REQ_GET_STATUS, USB_DIR_IN | USB_RT_PORT, 0, port, data, sizeof(struct usb_port_status), USB_CNTL_TIMEOUT); + +#ifdef CONFIG_DM_USB + if (ret < 0) + return ret; + + /* + * Translate the USB 3.0 hub port status field into the old version + * that U-Boot understands. Do this only when the hub is not root hub. + * For root hub, the port status field has already been translated + * in the host controller driver (see xhci_submit_root() in xhci.c). + * + * Note: this only supports driver model. + */ + + if (!usb_hub_is_root_hub(dev->dev) && usb_hub_is_superspeed(dev)) { + struct usb_port_status *status = (struct usb_port_status *)data; + u16 tmp = (status->wPortStatus) & USB_SS_PORT_STAT_MASK; + + if (status->wPortStatus & USB_SS_PORT_STAT_POWER) + tmp |= USB_PORT_STAT_POWER; + if ((status->wPortStatus & USB_SS_PORT_STAT_SPEED) == + USB_SS_PORT_STAT_SPEED_5GBPS) + tmp |= USB_PORT_STAT_SUPER_SPEED; + + status->wPortStatus = tmp; + } +#endif + + return ret; }
On 06/23/2017 11:54 AM, Bin Meng wrote:
USB 3.0 hub port status field has different bit positions from 2.0 hubs. Since U-Boot only understands the old version, translate the new one into the old one.
This is quite hairy. I'd rather see some protocol version agnostic flag field rather than patching the wPortStatus and co.
Since we are going to add USB 3.0 hub support, this feature is only available with driver model USB.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
common/usb_hub.c | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/common/usb_hub.c b/common/usb_hub.c index d780251..835fac9 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -119,9 +119,40 @@ static int usb_get_hub_status(struct usb_device *dev, void *data)
int usb_get_port_status(struct usb_device *dev, int port, void *data) {
- return usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
- int ret;
- ret = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), USB_REQ_GET_STATUS, USB_DIR_IN | USB_RT_PORT, 0, port, data, sizeof(struct usb_port_status), USB_CNTL_TIMEOUT);
+#ifdef CONFIG_DM_USB
- if (ret < 0)
return ret;- /*
* Translate the USB 3.0 hub port status field into the old version* that U-Boot understands. Do this only when the hub is not root hub.* For root hub, the port status field has already been translated* in the host controller driver (see xhci_submit_root() in xhci.c).** Note: this only supports driver model.*/- if (!usb_hub_is_root_hub(dev->dev) && usb_hub_is_superspeed(dev)) {
struct usb_port_status *status = (struct usb_port_status *)data;u16 tmp = (status->wPortStatus) & USB_SS_PORT_STAT_MASK;if (status->wPortStatus & USB_SS_PORT_STAT_POWER)tmp |= USB_PORT_STAT_POWER;if ((status->wPortStatus & USB_SS_PORT_STAT_SPEED) ==USB_SS_PORT_STAT_SPEED_5GBPS)tmp |= USB_PORT_STAT_SUPER_SPEED;status->wPortStatus = tmp;- }
+#endif
- return ret;
}
Hi Marek,
On Sat, Jun 24, 2017 at 1:59 AM, Marek Vasut marex@denx.de wrote:
On 06/23/2017 11:54 AM, Bin Meng wrote:
USB 3.0 hub port status field has different bit positions from 2.0 hubs. Since U-Boot only understands the old version, translate the new one into the old one.
This is quite hairy. I'd rather see some protocol version agnostic flag field rather than patching the wPortStatus and co.
I am not sure how do do that in a clean way. If we return the raw 3.0 hub port status data, that means we need change lot of hub codes here and there to do different parsing.
Since we are going to add USB 3.0 hub support, this feature is only available with driver model USB.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
common/usb_hub.c | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-)
[snip]
Regards, Bin
On 06/24/2017 03:53 AM, Bin Meng wrote:
Hi Marek,
On Sat, Jun 24, 2017 at 1:59 AM, Marek Vasut marex@denx.de wrote:
On 06/23/2017 11:54 AM, Bin Meng wrote:
USB 3.0 hub port status field has different bit positions from 2.0 hubs. Since U-Boot only understands the old version, translate the new one into the old one.
This is quite hairy. I'd rather see some protocol version agnostic flag field rather than patching the wPortStatus and co.
I am not sure how do do that in a clean way. If we return the raw 3.0 hub port status data, that means we need change lot of hub codes here and there to do different parsing.
How does Linux handle this?
Hi Marek,
On Tue, Jun 27, 2017 at 2:06 AM, Marek Vasut marex@denx.de wrote:
On 06/24/2017 03:53 AM, Bin Meng wrote:
Hi Marek,
On Sat, Jun 24, 2017 at 1:59 AM, Marek Vasut marex@denx.de wrote:
On 06/23/2017 11:54 AM, Bin Meng wrote:
USB 3.0 hub port status field has different bit positions from 2.0 hubs. Since U-Boot only understands the old version, translate the new one into the old one.
This is quite hairy. I'd rather see some protocol version agnostic flag field rather than patching the wPortStatus and co.
I am not sure how do do that in a clean way. If we return the raw 3.0 hub port status data, that means we need change lot of hub codes here and there to do different parsing.
How does Linux handle this?
Looks Linux is doing different parsing depending on hub is 3.0 or 2.0, at least for the power bit that I was just looking at. Do you want to do that?
Regards, Bin
Hi Marek,
On Tue, Jun 27, 2017 at 7:57 AM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Marek,
On Tue, Jun 27, 2017 at 2:06 AM, Marek Vasut marex@denx.de wrote:
On 06/24/2017 03:53 AM, Bin Meng wrote:
Hi Marek,
On Sat, Jun 24, 2017 at 1:59 AM, Marek Vasut marex@denx.de wrote:
On 06/23/2017 11:54 AM, Bin Meng wrote:
USB 3.0 hub port status field has different bit positions from 2.0 hubs. Since U-Boot only understands the old version, translate the new one into the old one.
This is quite hairy. I'd rather see some protocol version agnostic flag field rather than patching the wPortStatus and co.
I am not sure how do do that in a clean way. If we return the raw 3.0 hub port status data, that means we need change lot of hub codes here and there to do different parsing.
How does Linux handle this?
Looks Linux is doing different parsing depending on hub is 3.0 or 2.0, at least for the power bit that I was just looking at. Do you want to do that?
OK, I will do something similar like Linux in v2.
Regards, Bin
On 06/28/2017 10:28 AM, Bin Meng wrote:
Hi Marek,
On Tue, Jun 27, 2017 at 7:57 AM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Marek,
On Tue, Jun 27, 2017 at 2:06 AM, Marek Vasut marex@denx.de wrote:
On 06/24/2017 03:53 AM, Bin Meng wrote:
Hi Marek,
On Sat, Jun 24, 2017 at 1:59 AM, Marek Vasut marex@denx.de wrote:
On 06/23/2017 11:54 AM, Bin Meng wrote:
USB 3.0 hub port status field has different bit positions from 2.0 hubs. Since U-Boot only understands the old version, translate the new one into the old one.
This is quite hairy. I'd rather see some protocol version agnostic flag field rather than patching the wPortStatus and co.
I am not sure how do do that in a clean way. If we return the raw 3.0 hub port status data, that means we need change lot of hub codes here and there to do different parsing.
How does Linux handle this?
Looks Linux is doing different parsing depending on hub is 3.0 or 2.0, at least for the power bit that I was just looking at. Do you want to do that?
OK, I will do something similar like Linux in v2.
Thanks
Hi Marek,
On Thu, Jun 29, 2017 at 1:25 AM, Marek Vasut marex@denx.de wrote:
On 06/28/2017 10:28 AM, Bin Meng wrote:
Hi Marek,
On Tue, Jun 27, 2017 at 7:57 AM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Marek,
On Tue, Jun 27, 2017 at 2:06 AM, Marek Vasut marex@denx.de wrote:
On 06/24/2017 03:53 AM, Bin Meng wrote:
Hi Marek,
On Sat, Jun 24, 2017 at 1:59 AM, Marek Vasut marex@denx.de wrote:
On 06/23/2017 11:54 AM, Bin Meng wrote: > USB 3.0 hub port status field has different bit positions from 2.0 > hubs. Since U-Boot only understands the old version, translate the > new one into the old one.
This is quite hairy. I'd rather see some protocol version agnostic flag field rather than patching the wPortStatus and co.
I am not sure how do do that in a clean way. If we return the raw 3.0 hub port status data, that means we need change lot of hub codes here and there to do different parsing.
How does Linux handle this?
Looks Linux is doing different parsing depending on hub is 3.0 or 2.0, at least for the power bit that I was just looking at. Do you want to do that?
OK, I will do something similar like Linux in v2.
Thanks
After I checked Linux codes in depth, it looks current way of handling 3.0 port status translation in this patch is the minimum way to fix the 3.0 hub port status problem in U-Boot. What Linux does is, one xHCI controller registers two buses, that one is USB 3.0 and the other is USB 2.0. All 3.0 devices will only show on the 3.0 bus, while all 1.0/2.0 devices will show on the 2.0 bus. We can certainly do similar thing in U-Boot, but I am afraid that will lead to a overhaul to current USB stack and xHCI driver. And most important, that support will be only under DM, which is something you don't like to see.
Regards, Bin
On 5 July 2017 at 23:56, Bin Meng bmeng.cn@gmail.com wrote:
Hi Marek,
On Thu, Jun 29, 2017 at 1:25 AM, Marek Vasut marex@denx.de wrote:
On 06/28/2017 10:28 AM, Bin Meng wrote:
Hi Marek,
On Tue, Jun 27, 2017 at 7:57 AM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Marek,
On Tue, Jun 27, 2017 at 2:06 AM, Marek Vasut marex@denx.de wrote:
On 06/24/2017 03:53 AM, Bin Meng wrote:
Hi Marek,
On Sat, Jun 24, 2017 at 1:59 AM, Marek Vasut marex@denx.de wrote: > On 06/23/2017 11:54 AM, Bin Meng wrote: >> USB 3.0 hub port status field has different bit positions from 2.0 >> hubs. Since U-Boot only understands the old version, translate the >> new one into the old one. > > This is quite hairy. I'd rather see some protocol version agnostic flag > field rather than patching the wPortStatus and co. >
I am not sure how do do that in a clean way. If we return the raw 3.0 hub port status data, that means we need change lot of hub codes here and there to do different parsing.
How does Linux handle this?
Looks Linux is doing different parsing depending on hub is 3.0 or 2.0, at least for the power bit that I was just looking at. Do you want to do that?
OK, I will do something similar like Linux in v2.
Thanks
After I checked Linux codes in depth, it looks current way of handling 3.0 port status translation in this patch is the minimum way to fix the 3.0 hub port status problem in U-Boot. What Linux does is, one xHCI controller registers two buses, that one is USB 3.0 and the other is USB 2.0. All 3.0 devices will only show on the 3.0 bus, while all 1.0/2.0 devices will show on the 2.0 bus. We can certainly do similar thing in U-Boot, but I am afraid that will lead to a overhaul to current USB stack and xHCI driver. And most important, that support will be only under DM, which is something you don't like to see.
IMO we should not be adding features to non-DM USB.
Anyway:
Reviewed-by: Simon Glass sjg@chromium.org
USB 3.0 hub uses a hub depth value multiplied by four as an offset into the 'route string' to locate the bits it uses to determine the downstream port number. We shall set the hub depth value of a USB 3.0 hub after it is configured.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
common/usb_hub.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ include/usb.h | 1 + include/usb_defs.h | 3 +++ 3 files changed, 56 insertions(+)
diff --git a/common/usb_hub.c b/common/usb_hub.c index 835fac9..63358cd 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -82,6 +82,16 @@ bool usb_hub_is_root_hub(struct udevice *hub)
return false; } + +static int usb_set_hub_depth(struct usb_device *dev, int depth) +{ + if (depth < 0 || depth > 4) + return -EINVAL; + + return usb_control_msg(dev, usb_sndctrlpipe(dev, 0), + USB_REQ_SET_HUB_DEPTH, USB_DIR_OUT | USB_RT_HUB, + depth, 0, NULL, 0, USB_CNTL_TIMEOUT); +} #endif
static int usb_get_hub_descriptor(struct usb_device *dev, void *data, int size) @@ -719,6 +729,48 @@ static int usb_hub_configure(struct usb_device *dev) debug("%sover-current condition exists\n", (le16_to_cpu(hubsts->wHubStatus) & HUB_STATUS_OVERCURRENT) ? \ "" : "no "); + +#ifdef CONFIG_DM_USB + /* + * A maximum of seven tiers are allowed in a USB topology, and the + * root hub occupies the first tier. The last tier ends with a normal + * USB device. USB 3.0 hubs use a 20-bit field called 'route string' + * to route packet to the designated downstream port. The hub uses a + * hub depth value multiplied by four as an offset into the 'route + * string' to locate the bits it uses to determine the downstream + * port number. + */ + if (usb_hub_is_root_hub(dev->dev)) { + hub->hub_depth = -1; + } else { + struct udevice *hdev; + int depth = 0; + + hdev = dev->dev->parent; + while (!usb_hub_is_root_hub(hdev)) { + depth++; + hdev = hdev->parent; + } + + hub->hub_depth = depth; + + if (usb_hub_is_superspeed(dev)) { + debug("set hub (%p) depth to %d\n", dev, depth); + /* + * This request sets the value that the hub uses to + * determine the index into the 'route string index' + * for this hub. + */ + ret = usb_set_hub_depth(dev, depth); + if (ret < 0) { + debug("%s: failed to set hub depth (%lX)\n", + __func__, dev->status); + return ret; + } + } + } +#endif + usb_hub_power_on(hub);
/* diff --git a/include/usb.h b/include/usb.h index c504d71..40e44f4 100644 --- a/include/usb.h +++ b/include/usb.h @@ -570,6 +570,7 @@ struct usb_hub_device { ulong connect_timeout; /* Device connection timeout in ms */ ulong query_delay; /* Device query delay in ms */ int overcurrent_count[USB_MAXCHILDREN]; /* Over-current counter */ + int hub_depth; /* USB 3.0 hub depth */ };
#ifdef CONFIG_DM_USB diff --git a/include/usb_defs.h b/include/usb_defs.h index 6b4385a..273337f 100644 --- a/include/usb_defs.h +++ b/include/usb_defs.h @@ -306,6 +306,9 @@ /* Mask for wIndex in get/set port feature */ #define USB_HUB_PORT_MASK 0xf
+/* Hub class request codes */ +#define USB_REQ_SET_HUB_DEPTH 0x0c + /* * CBI style */
On 23 June 2017 at 03:54, Bin Meng bmeng.cn@gmail.com wrote:
USB 3.0 hub uses a hub depth value multiplied by four as an offset into the 'route string' to locate the bits it uses to determine the downstream port number. We shall set the hub depth value of a USB 3.0 hub after it is configured.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
common/usb_hub.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ include/usb.h | 1 + include/usb_defs.h | 3 +++ 3 files changed, 56 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
nit below
diff --git a/common/usb_hub.c b/common/usb_hub.c index 835fac9..63358cd 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -82,6 +82,16 @@ bool usb_hub_is_root_hub(struct udevice *hub)
return false;}
+static int usb_set_hub_depth(struct usb_device *dev, int depth) +{
if (depth < 0 || depth > 4)return -EINVAL;return usb_control_msg(dev, usb_sndctrlpipe(dev, 0),USB_REQ_SET_HUB_DEPTH, USB_DIR_OUT | USB_RT_HUB,depth, 0, NULL, 0, USB_CNTL_TIMEOUT);+} #endif
static int usb_get_hub_descriptor(struct usb_device *dev, void *data, int size) @@ -719,6 +729,48 @@ static int usb_hub_configure(struct usb_device *dev) debug("%sover-current condition exists\n", (le16_to_cpu(hubsts->wHubStatus) & HUB_STATUS_OVERCURRENT) ? \ "" : "no ");
+#ifdef CONFIG_DM_USB
/** A maximum of seven tiers are allowed in a USB topology, and the* root hub occupies the first tier. The last tier ends with a normal* USB device. USB 3.0 hubs use a 20-bit field called 'route string'* to route packet to the designated downstream port. The hub uses a
should this be 'packets'?
[..]
For future extension, change xhci_setup_addressable_virt_dev() signature to accept a pointer to 'struct usb_device', instead of its members slot_id & speed, as the struct already contains these two plus some other useful information of the device.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
drivers/usb/host/xhci-mem.c | 6 ++++-- drivers/usb/host/xhci.c | 3 +-- drivers/usb/host/xhci.h | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 12e277a..9aa3092 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -713,14 +713,16 @@ void xhci_slot_copy(struct xhci_ctrl *ctrl, struct xhci_container_ctx *in_ctx, * @param udev pointer to the Device Data Structure * @return returns negative value on failure else 0 on success */ -void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl, int slot_id, - int speed, int hop_portnr) +void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl, + struct usb_device *udev, int hop_portnr) { struct xhci_virt_device *virt_dev; struct xhci_ep_ctx *ep0_ctx; struct xhci_slot_ctx *slot_ctx; u32 port_num = 0; u64 trb_64 = 0; + int slot_id = udev->slot_id; + int speed = udev->speed;
virt_dev = ctrl->devs[slot_id];
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index a5b888a..1148127 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -415,8 +415,7 @@ static int xhci_address_device(struct usb_device *udev, int root_portnr) * so setting up the slot context. */ debug("Setting up addressable devices %p\n", ctrl->dcbaa); - xhci_setup_addressable_virt_dev(ctrl, udev->slot_id, udev->speed, - root_portnr); + xhci_setup_addressable_virt_dev(ctrl, udev, root_portnr);
ctrl_ctx = xhci_get_input_control_ctx(virt_dev->in_ctx); ctrl_ctx->add_flags = cpu_to_le32(SLOT_FLAG | EP0_FLAG); diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index b9602ba..cdce67c 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1247,8 +1247,8 @@ void xhci_endpoint_copy(struct xhci_ctrl *ctrl, void xhci_slot_copy(struct xhci_ctrl *ctrl, struct xhci_container_ctx *in_ctx, struct xhci_container_ctx *out_ctx); -void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl, int slot_id, - int speed, int hop_portnr); +void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl, + struct usb_device *udev, int hop_portnr); void xhci_queue_command(struct xhci_ctrl *ctrl, u8 *ptr, u32 slot_id, u32 ep_index, trb_type cmd); void xhci_acknowledge_event(struct xhci_ctrl *ctrl);
On 23 June 2017 at 03:54, Bin Meng bmeng.cn@gmail.com wrote:
For future extension, change xhci_setup_addressable_virt_dev() signature to accept a pointer to 'struct usb_device', instead of its members slot_id & speed, as the struct already contains these two plus some other useful information of the device.
I am guessing this is because you need the other info in a subsequent patch.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/usb/host/xhci-mem.c | 6 ++++-- drivers/usb/host/xhci.c | 3 +-- drivers/usb/host/xhci.h | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
xHCI spec says: the values of the 'route string' field shall be initialized by the first 'Address Device' command issued to a device slot, and shall not be modified by any other command.
So far U-Boot does not program this field, and it does not prevent SS device directly attached to root port, or HS device behind an HS hub, from working, due to the fact that 'route string' is used by the xHC to target SS packets. But in order to enumerate devices behind an SS hub, this field must be programmed.
With this commit and along with previous commits, now SS & HS devices attached to a USB 3.0 hub can be enumerated by U-Boot.
As usual, this new feature is only available when DM is on.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
--- Test logs: two USB 3.0 hubs (one tier2, one tier3) => usb tree USB device tree: 1 Hub (5 Gb/s, 0mA) | U-Boot XHCI Host Controller | +-2 Hub (5 Gb/s, 0mA) | | GenesysLogic USB3.0 Hub | | | +-4 Hub (5 Gb/s, 0mA) | | | VIA Labs, Inc. USB3.0 Hub | | | | | +-7 Mass Storage (5 Gb/s, 76mA) | | JetFlash Mass Storage Device 16Q6ZPH20GF3E8UQ | | | +-8 Vendor specific (5 Gb/s, 36mA) | Realtek USB 10/100/1000 LAN 00E04C680977 | +-3 Hub (480 Mb/s, 100mA) | GenesysLogic USB2.0 Hub | +-5 Mass Storage (480 Mb/s, 200mA) | Netac OnlyDisk FF00ECB60800FFFF1526 | +-6 Hub (480 Mb/s, 0mA) VIA Labs, Inc. USB2.0 Hub
drivers/usb/host/xhci-mem.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 9aa3092..03874db 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -723,6 +723,9 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl, u64 trb_64 = 0; int slot_id = udev->slot_id; int speed = udev->speed; + int route = 0; + struct usb_device *dev = udev; + struct usb_hub_device *hub;
virt_dev = ctrl->devs[slot_id];
@@ -733,7 +736,22 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl, slot_ctx = xhci_get_slot_ctx(ctrl, virt_dev->in_ctx);
/* Only the control endpoint is valid - one endpoint context */ - slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(1) | 0); + slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(1)); + +#ifdef CONFIG_DM_USB + /* Calculate the route string for this device */ + port_num = dev->portnr; + while (!usb_hub_is_root_hub(dev->dev)) { + hub = dev_get_uclass_priv(dev->dev); + route |= port_num << (hub->hub_depth * 4); + dev = dev_get_parent_priv(dev->dev); + port_num = dev->portnr; + dev = dev_get_parent_priv(dev->dev->parent); + } + + debug("hub (%p) route string %x\n", udev, route); +#endif + slot_ctx->dev_info |= route;
switch (speed) { case USB_SPEED_SUPER:
On 06/23/2017 11:54 AM, Bin Meng wrote:
xHCI spec says: the values of the 'route string' field shall be initialized by the first 'Address Device' command issued to a device slot, and shall not be modified by any other command.
So far U-Boot does not program this field, and it does not prevent SS device directly attached to root port, or HS device behind an HS hub, from working, due to the fact that 'route string' is used by the xHC to target SS packets. But in order to enumerate devices behind an SS hub, this field must be programmed.
With this commit and along with previous commits, now SS & HS devices attached to a USB 3.0 hub can be enumerated by U-Boot.
As usual, this new feature is only available when DM is on.
Great, but I really dislike the ifdef pollution, so this needs to be sorted out.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Test logs: two USB 3.0 hubs (one tier2, one tier3) => usb tree USB device tree: 1 Hub (5 Gb/s, 0mA) | U-Boot XHCI Host Controller | +-2 Hub (5 Gb/s, 0mA) | | GenesysLogic USB3.0 Hub | | | +-4 Hub (5 Gb/s, 0mA) | | | VIA Labs, Inc. USB3.0 Hub | | | | | +-7 Mass Storage (5 Gb/s, 76mA) | | JetFlash Mass Storage Device 16Q6ZPH20GF3E8UQ | | | +-8 Vendor specific (5 Gb/s, 36mA) | Realtek USB 10/100/1000 LAN 00E04C680977 | +-3 Hub (480 Mb/s, 100mA) | GenesysLogic USB2.0 Hub | +-5 Mass Storage (480 Mb/s, 200mA) | Netac OnlyDisk FF00ECB60800FFFF1526 | +-6 Hub (480 Mb/s, 0mA) VIA Labs, Inc. USB2.0 Hub
drivers/usb/host/xhci-mem.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 9aa3092..03874db 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -723,6 +723,9 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl, u64 trb_64 = 0; int slot_id = udev->slot_id; int speed = udev->speed;
int route = 0;
struct usb_device *dev = udev;
struct usb_hub_device *hub;
virt_dev = ctrl->devs[slot_id];
@@ -733,7 +736,22 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl, slot_ctx = xhci_get_slot_ctx(ctrl, virt_dev->in_ctx);
/* Only the control endpoint is valid - one endpoint context */
- slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(1) | 0);
- slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(1));
+#ifdef CONFIG_DM_USB
- /* Calculate the route string for this device */
- port_num = dev->portnr;
- while (!usb_hub_is_root_hub(dev->dev)) {
hub = dev_get_uclass_priv(dev->dev);route |= port_num << (hub->hub_depth * 4);dev = dev_get_parent_priv(dev->dev);port_num = dev->portnr;dev = dev_get_parent_priv(dev->dev->parent);- }
- debug("hub (%p) route string %x\n", udev, route);
+#endif
slot_ctx->dev_info |= route;
switch (speed) { case USB_SPEED_SUPER:
Hi Marek,
On Sat, Jun 24, 2017 at 2:02 AM, Marek Vasut marex@denx.de wrote:
On 06/23/2017 11:54 AM, Bin Meng wrote:
xHCI spec says: the values of the 'route string' field shall be initialized by the first 'Address Device' command issued to a device slot, and shall not be modified by any other command.
So far U-Boot does not program this field, and it does not prevent SS device directly attached to root port, or HS device behind an HS hub, from working, due to the fact that 'route string' is used by the xHC to target SS packets. But in order to enumerate devices behind an SS hub, this field must be programmed.
With this commit and along with previous commits, now SS & HS devices attached to a USB 3.0 hub can be enumerated by U-Boot.
As usual, this new feature is only available when DM is on.
Great, but I really dislike the ifdef pollution, so this needs to be sorted out.
The ifdef was needed due to it calls DM APIs or access DM udevice. I have no intention to add a new feature to the non-DM driver. Eventually we can get rid of all these ifdefs when all boards are converted to use DM USB.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Test logs: two USB 3.0 hubs (one tier2, one tier3) => usb tree USB device tree: 1 Hub (5 Gb/s, 0mA) | U-Boot XHCI Host Controller | +-2 Hub (5 Gb/s, 0mA) | | GenesysLogic USB3.0 Hub | | | +-4 Hub (5 Gb/s, 0mA) | | | VIA Labs, Inc. USB3.0 Hub | | | | | +-7 Mass Storage (5 Gb/s, 76mA) | | JetFlash Mass Storage Device 16Q6ZPH20GF3E8UQ | | | +-8 Vendor specific (5 Gb/s, 36mA) | Realtek USB 10/100/1000 LAN 00E04C680977 | +-3 Hub (480 Mb/s, 100mA) | GenesysLogic USB2.0 Hub | +-5 Mass Storage (480 Mb/s, 200mA) | Netac OnlyDisk FF00ECB60800FFFF1526 | +-6 Hub (480 Mb/s, 0mA) VIA Labs, Inc. USB2.0 Hub
[snip]
Regards, Bin
On 06/24/2017 03:57 AM, Bin Meng wrote:
Hi Marek,
On Sat, Jun 24, 2017 at 2:02 AM, Marek Vasut marex@denx.de wrote:
On 06/23/2017 11:54 AM, Bin Meng wrote:
xHCI spec says: the values of the 'route string' field shall be initialized by the first 'Address Device' command issued to a device slot, and shall not be modified by any other command.
So far U-Boot does not program this field, and it does not prevent SS device directly attached to root port, or HS device behind an HS hub, from working, due to the fact that 'route string' is used by the xHC to target SS packets. But in order to enumerate devices behind an SS hub, this field must be programmed.
With this commit and along with previous commits, now SS & HS devices attached to a USB 3.0 hub can be enumerated by U-Boot.
As usual, this new feature is only available when DM is on.
Great, but I really dislike the ifdef pollution, so this needs to be sorted out.
The ifdef was needed due to it calls DM APIs or access DM udevice. I have no intention to add a new feature to the non-DM driver.
But then this creates a massive disparity, it's like we're growing two USB stacks.
Eventually we can get rid of all these ifdefs when all boards are converted to use DM USB.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Test logs: two USB 3.0 hubs (one tier2, one tier3) => usb tree USB device tree: 1 Hub (5 Gb/s, 0mA) | U-Boot XHCI Host Controller | +-2 Hub (5 Gb/s, 0mA) | | GenesysLogic USB3.0 Hub | | | +-4 Hub (5 Gb/s, 0mA) | | | VIA Labs, Inc. USB3.0 Hub | | | | | +-7 Mass Storage (5 Gb/s, 76mA) | | JetFlash Mass Storage Device 16Q6ZPH20GF3E8UQ | | | +-8 Vendor specific (5 Gb/s, 36mA) | Realtek USB 10/100/1000 LAN 00E04C680977 | +-3 Hub (480 Mb/s, 100mA) | GenesysLogic USB2.0 Hub | +-5 Mass Storage (480 Mb/s, 200mA) | Netac OnlyDisk FF00ECB60800FFFF1526 | +-6 Hub (480 Mb/s, 0mA) VIA Labs, Inc. USB2.0 Hub
[snip]
Regards, Bin
Hi Marek,
On Tue, Jun 27, 2017 at 2:07 AM, Marek Vasut marex@denx.de wrote:
On 06/24/2017 03:57 AM, Bin Meng wrote:
Hi Marek,
On Sat, Jun 24, 2017 at 2:02 AM, Marek Vasut marex@denx.de wrote:
On 06/23/2017 11:54 AM, Bin Meng wrote:
xHCI spec says: the values of the 'route string' field shall be initialized by the first 'Address Device' command issued to a device slot, and shall not be modified by any other command.
So far U-Boot does not program this field, and it does not prevent SS device directly attached to root port, or HS device behind an HS hub, from working, due to the fact that 'route string' is used by the xHC to target SS packets. But in order to enumerate devices behind an SS hub, this field must be programmed.
With this commit and along with previous commits, now SS & HS devices attached to a USB 3.0 hub can be enumerated by U-Boot.
As usual, this new feature is only available when DM is on.
Great, but I really dislike the ifdef pollution, so this needs to be sorted out.
The ifdef was needed due to it calls DM APIs or access DM udevice. I have no intention to add a new feature to the non-DM driver.
But then this creates a massive disparity, it's like we're growing two USB stacks.
Yep, unfortunately. But if we continue adding new features/fixes to the old non-DM stuff, I am not sure how this can encourage people to switch to DM. Maybe I can check all boards that use xHCI to see if they are switched to DM?
[snip]
Regards, Bin
Hi Bin,
On 27.06.2017 02:01, Bin Meng wrote:
On Tue, Jun 27, 2017 at 2:07 AM, Marek Vasut marex@denx.de wrote:
On 06/24/2017 03:57 AM, Bin Meng wrote:
Hi Marek,
On Sat, Jun 24, 2017 at 2:02 AM, Marek Vasut marex@denx.de wrote:
On 06/23/2017 11:54 AM, Bin Meng wrote:
xHCI spec says: the values of the 'route string' field shall be initialized by the first 'Address Device' command issued to a device slot, and shall not be modified by any other command.
So far U-Boot does not program this field, and it does not prevent SS device directly attached to root port, or HS device behind an HS hub, from working, due to the fact that 'route string' is used by the xHC to target SS packets. But in order to enumerate devices behind an SS hub, this field must be programmed.
With this commit and along with previous commits, now SS & HS devices attached to a USB 3.0 hub can be enumerated by U-Boot.
As usual, this new feature is only available when DM is on.
Great, but I really dislike the ifdef pollution, so this needs to be sorted out.
The ifdef was needed due to it calls DM APIs or access DM udevice. I have no intention to add a new feature to the non-DM driver.
But then this creates a massive disparity, it's like we're growing two USB stacks.
Yep, unfortunately. But if we continue adding new features/fixes to the old non-DM stuff, I am not sure how this can encourage people to switch to DM.
Correct. We definitely don't want to add new features to non-DM drivers / IF, if this is non-trivial.
Maybe I can check all boards that use xHCI to see if they are switched to DM?
xHCI is still quite new in U-Boot, so I would expect that all platforms using it, are using DM or at least not far away from using it. Yes, please check all xHCI "users", if they use DM. Then we know if and which users need some "persuasion" to switch over to DM soon. ;)
Thanks, Stefan
Hi Stefan,
On Tue, Jun 27, 2017 at 1:23 PM, Stefan Roese sr@denx.de wrote:
Hi Bin,
On 27.06.2017 02:01, Bin Meng wrote:
On Tue, Jun 27, 2017 at 2:07 AM, Marek Vasut marex@denx.de wrote:
On 06/24/2017 03:57 AM, Bin Meng wrote:
Hi Marek,
On Sat, Jun 24, 2017 at 2:02 AM, Marek Vasut marex@denx.de wrote:
On 06/23/2017 11:54 AM, Bin Meng wrote:
xHCI spec says: the values of the 'route string' field shall be initialized by the first 'Address Device' command issued to a device slot, and shall not be modified by any other command.
So far U-Boot does not program this field, and it does not prevent SS device directly attached to root port, or HS device behind an HS hub, from working, due to the fact that 'route string' is used by the xHC to target SS packets. But in order to enumerate devices behind an SS hub, this field must be programmed.
With this commit and along with previous commits, now SS & HS devices attached to a USB 3.0 hub can be enumerated by U-Boot.
As usual, this new feature is only available when DM is on.
Great, but I really dislike the ifdef pollution, so this needs to be sorted out.
The ifdef was needed due to it calls DM APIs or access DM udevice. I have no intention to add a new feature to the non-DM driver.
But then this creates a massive disparity, it's like we're growing two USB stacks.
Yep, unfortunately. But if we continue adding new features/fixes to the old non-DM stuff, I am not sure how this can encourage people to switch to DM.
Correct. We definitely don't want to add new features to non-DM drivers / IF, if this is non-trivial.
Maybe I can check all boards that use xHCI to see if they are switched to DM?
xHCI is still quite new in U-Boot, so I would expect that all platforms using it, are using DM or at least not far away from using it. Yes, please check all xHCI "users", if they use DM. Then we know if and which users need some "persuasion" to switch over to DM soon. ;)
I checked all boards that have CONFIG_USB_XHCI_HCD defined but without CONFIG_DM_USB. Here is the list.
configs/uniphier_v8_defconfig:36:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zc1751_xm015_dc1_defconfig:62:CONFIG_USB_XHCI_HCD=y configs/am57xx_evm_nodt_defconfig:53:CONFIG_USB_XHCI_HCD=y configs/evb-rk3328_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_lpuart_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/uniphier_pxs2_ld6b_defconfig:44:CONFIG_USB_XHCI_HCD=y configs/ls1012ardb_qspi_SECURE_BOOT_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig:57:CONFIG_USB_XHCI_HCD=y configs/k2e_hs_evm_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_sdcard_qspi_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_ethboot_defconfig:48:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_ep_defconfig:70:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_nand_defconfig:57:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_qspi_defconfig:50:CONFIG_USB_XHCI_HCD=y configs/k2g_evm_defconfig:45:CONFIG_USB_XHCI_HCD=y configs/am57xx_evm_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/am43xx_hs_evm_defconfig:49:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_defconfig:39:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_defconfig:42:CONFIG_USB_XHCI_HCD=y configs/firefly-rk3399_defconfig:59:CONFIG_USB_XHCI_HCD=y configs/puma-rk3399_defconfig:78:CONFIG_USB_XHCI_HCD=y configs/cl-som-am57x_defconfig:55:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_nor_SECURE_BOOT_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/uniphier_pro4_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zc1751_xm016_dc2_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zcu102_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_SECURE_BOOT_defconfig:42:CONFIG_USB_XHCI_HCD=y configs/cm_t43_defconfig:67:CONFIG_USB_XHCI_HCD=y configs/k2g_hs_evm_defconfig:36:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_qspiboot_defconfig:45:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_qspi_defconfig:50:CONFIG_USB_XHCI_HCD=y configs/am57xx_hs_evm_defconfig:67:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zcu102_revB_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_sdcard_ifc_defconfig:55:CONFIG_USB_XHCI_HCD=y configs/uniphier_ld20_defconfig:39:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_usbhost_boot_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_sdcard_qspi_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/evb-rk3399_defconfig:60:CONFIG_USB_XHCI_HCD=y configs/k2hk_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/k2hk_hs_evm_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/k2e_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_sdcard_ifc_defconfig:54:CONFIG_USB_XHCI_HCD=y configs/k2l_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y
So it looks we have lots of conversion work to be done by many board maintainers. I am not sure how to proceed on this.
Regards, Bin
On 06/27/2017 10:27 AM, Bin Meng wrote:
Hi Stefan,
On Tue, Jun 27, 2017 at 1:23 PM, Stefan Roese sr@denx.de wrote:
Hi Bin,
On 27.06.2017 02:01, Bin Meng wrote:
On Tue, Jun 27, 2017 at 2:07 AM, Marek Vasut marex@denx.de wrote:
On 06/24/2017 03:57 AM, Bin Meng wrote:
Hi Marek,
On Sat, Jun 24, 2017 at 2:02 AM, Marek Vasut marex@denx.de wrote:
On 06/23/2017 11:54 AM, Bin Meng wrote: > > xHCI spec says: the values of the 'route string' field shall be > initialized by the first 'Address Device' command issued to a > device slot, and shall not be modified by any other command. > > So far U-Boot does not program this field, and it does not prevent > SS device directly attached to root port, or HS device behind an HS > hub, from working, due to the fact that 'route string' is used by > the xHC to target SS packets. But in order to enumerate devices > behind an SS hub, this field must be programmed. > > With this commit and along with previous commits, now SS & HS devices > attached to a USB 3.0 hub can be enumerated by U-Boot. > > As usual, this new feature is only available when DM is on.
Great, but I really dislike the ifdef pollution, so this needs to be sorted out.
The ifdef was needed due to it calls DM APIs or access DM udevice. I have no intention to add a new feature to the non-DM driver.
But then this creates a massive disparity, it's like we're growing two USB stacks.
Yep, unfortunately. But if we continue adding new features/fixes to the old non-DM stuff, I am not sure how this can encourage people to switch to DM.
Correct. We definitely don't want to add new features to non-DM drivers / IF, if this is non-trivial.
Maybe I can check all boards that use xHCI to see if they are switched to DM?
xHCI is still quite new in U-Boot, so I would expect that all platforms using it, are using DM or at least not far away from using it. Yes, please check all xHCI "users", if they use DM. Then we know if and which users need some "persuasion" to switch over to DM soon. ;)
I checked all boards that have CONFIG_USB_XHCI_HCD defined but without CONFIG_DM_USB. Here is the list.
configs/uniphier_v8_defconfig:36:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zc1751_xm015_dc1_defconfig:62:CONFIG_USB_XHCI_HCD=y configs/am57xx_evm_nodt_defconfig:53:CONFIG_USB_XHCI_HCD=y configs/evb-rk3328_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_lpuart_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/uniphier_pxs2_ld6b_defconfig:44:CONFIG_USB_XHCI_HCD=y configs/ls1012ardb_qspi_SECURE_BOOT_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig:57:CONFIG_USB_XHCI_HCD=y configs/k2e_hs_evm_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_sdcard_qspi_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_ethboot_defconfig:48:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_ep_defconfig:70:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_nand_defconfig:57:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_qspi_defconfig:50:CONFIG_USB_XHCI_HCD=y configs/k2g_evm_defconfig:45:CONFIG_USB_XHCI_HCD=y configs/am57xx_evm_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/am43xx_hs_evm_defconfig:49:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_defconfig:39:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_defconfig:42:CONFIG_USB_XHCI_HCD=y configs/firefly-rk3399_defconfig:59:CONFIG_USB_XHCI_HCD=y configs/puma-rk3399_defconfig:78:CONFIG_USB_XHCI_HCD=y configs/cl-som-am57x_defconfig:55:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_nor_SECURE_BOOT_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/uniphier_pro4_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zc1751_xm016_dc2_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zcu102_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_SECURE_BOOT_defconfig:42:CONFIG_USB_XHCI_HCD=y configs/cm_t43_defconfig:67:CONFIG_USB_XHCI_HCD=y configs/k2g_hs_evm_defconfig:36:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_qspiboot_defconfig:45:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_qspi_defconfig:50:CONFIG_USB_XHCI_HCD=y configs/am57xx_hs_evm_defconfig:67:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zcu102_revB_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_sdcard_ifc_defconfig:55:CONFIG_USB_XHCI_HCD=y configs/uniphier_ld20_defconfig:39:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_usbhost_boot_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_sdcard_qspi_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/evb-rk3399_defconfig:60:CONFIG_USB_XHCI_HCD=y configs/k2hk_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/k2hk_hs_evm_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/k2e_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_sdcard_ifc_defconfig:54:CONFIG_USB_XHCI_HCD=y configs/k2l_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y
So it looks we have lots of conversion work to be done by many board maintainers. I am not sure how to proceed on this.
Could the USB_DM be implied on these boards ?
Hi Bin,
On 27.06.2017 10:27, Bin Meng wrote:
Hi Stefan,
On Tue, Jun 27, 2017 at 1:23 PM, Stefan Roese sr@denx.de wrote:
Hi Bin,
On 27.06.2017 02:01, Bin Meng wrote:
On Tue, Jun 27, 2017 at 2:07 AM, Marek Vasut marex@denx.de wrote:
On 06/24/2017 03:57 AM, Bin Meng wrote:
Hi Marek,
On Sat, Jun 24, 2017 at 2:02 AM, Marek Vasut marex@denx.de wrote:
On 06/23/2017 11:54 AM, Bin Meng wrote: > > xHCI spec says: the values of the 'route string' field shall be > initialized by the first 'Address Device' command issued to a > device slot, and shall not be modified by any other command. > > So far U-Boot does not program this field, and it does not prevent > SS device directly attached to root port, or HS device behind an HS > hub, from working, due to the fact that 'route string' is used by > the xHC to target SS packets. But in order to enumerate devices > behind an SS hub, this field must be programmed. > > With this commit and along with previous commits, now SS & HS devices > attached to a USB 3.0 hub can be enumerated by U-Boot. > > As usual, this new feature is only available when DM is on.
Great, but I really dislike the ifdef pollution, so this needs to be sorted out.
The ifdef was needed due to it calls DM APIs or access DM udevice. I have no intention to add a new feature to the non-DM driver.
But then this creates a massive disparity, it's like we're growing two USB stacks.
Yep, unfortunately. But if we continue adding new features/fixes to the old non-DM stuff, I am not sure how this can encourage people to switch to DM.
Correct. We definitely don't want to add new features to non-DM drivers / IF, if this is non-trivial.
Maybe I can check all boards that use xHCI to see if they are switched to DM?
xHCI is still quite new in U-Boot, so I would expect that all platforms using it, are using DM or at least not far away from using it. Yes, please check all xHCI "users", if they use DM. Then we know if and which users need some "persuasion" to switch over to DM soon. ;)
I checked all boards that have CONFIG_USB_XHCI_HCD defined but without CONFIG_DM_USB. Here is the list.
configs/uniphier_v8_defconfig:36:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zc1751_xm015_dc1_defconfig:62:CONFIG_USB_XHCI_HCD=y configs/am57xx_evm_nodt_defconfig:53:CONFIG_USB_XHCI_HCD=y configs/evb-rk3328_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_lpuart_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/uniphier_pxs2_ld6b_defconfig:44:CONFIG_USB_XHCI_HCD=y configs/ls1012ardb_qspi_SECURE_BOOT_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig:57:CONFIG_USB_XHCI_HCD=y configs/k2e_hs_evm_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_sdcard_qspi_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_ethboot_defconfig:48:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_ep_defconfig:70:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_nand_defconfig:57:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_qspi_defconfig:50:CONFIG_USB_XHCI_HCD=y configs/k2g_evm_defconfig:45:CONFIG_USB_XHCI_HCD=y configs/am57xx_evm_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/am43xx_hs_evm_defconfig:49:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_defconfig:39:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_defconfig:42:CONFIG_USB_XHCI_HCD=y configs/firefly-rk3399_defconfig:59:CONFIG_USB_XHCI_HCD=y configs/puma-rk3399_defconfig:78:CONFIG_USB_XHCI_HCD=y configs/cl-som-am57x_defconfig:55:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_nor_SECURE_BOOT_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/uniphier_pro4_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zc1751_xm016_dc2_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zcu102_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_SECURE_BOOT_defconfig:42:CONFIG_USB_XHCI_HCD=y configs/cm_t43_defconfig:67:CONFIG_USB_XHCI_HCD=y configs/k2g_hs_evm_defconfig:36:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_qspiboot_defconfig:45:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_qspi_defconfig:50:CONFIG_USB_XHCI_HCD=y configs/am57xx_hs_evm_defconfig:67:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zcu102_revB_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_sdcard_ifc_defconfig:55:CONFIG_USB_XHCI_HCD=y configs/uniphier_ld20_defconfig:39:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_usbhost_boot_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_sdcard_qspi_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/evb-rk3399_defconfig:60:CONFIG_USB_XHCI_HCD=y configs/k2hk_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/k2hk_hs_evm_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/k2e_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_sdcard_ifc_defconfig:54:CONFIG_USB_XHCI_HCD=y configs/k2l_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y
So it looks we have lots of conversion work to be done by many board maintainers. I am not sure how to proceed on this.
Marek reminded me, that he thinks that most of these platforms above will most likely select DM_USB implicitly via Kconfig.
I did a quick check and it seems that at least these platforms have DM_USB enabled:
ARCH_ZYNQ ARCH_ZYNQMP ARCH_UNIPHIER ARCH_ROCKCHIP
and other from above very likely as well.
Before you invest more time and effort into implementing the xHCI additions in a "non-DM cleaner way", I suggest to find out which targets really use xHCI without USB_DM. An easy check would be to add some #error to the non-DM part and run this commit through buildman / travis.
What do you think?
Thanks, Stefan
Hi Stefan,
On Wed, Jun 28, 2017 at 7:28 PM, Stefan Roese sr@denx.de wrote:
Hi Bin,
On 27.06.2017 10:27, Bin Meng wrote:
Hi Stefan,
On Tue, Jun 27, 2017 at 1:23 PM, Stefan Roese sr@denx.de wrote:
Hi Bin,
On 27.06.2017 02:01, Bin Meng wrote:
On Tue, Jun 27, 2017 at 2:07 AM, Marek Vasut marex@denx.de wrote:
On 06/24/2017 03:57 AM, Bin Meng wrote:
Hi Marek,
On Sat, Jun 24, 2017 at 2:02 AM, Marek Vasut marex@denx.de wrote: > > > On 06/23/2017 11:54 AM, Bin Meng wrote: >> >> >> xHCI spec says: the values of the 'route string' field shall be >> initialized by the first 'Address Device' command issued to a >> device slot, and shall not be modified by any other command. >> >> So far U-Boot does not program this field, and it does not prevent >> SS device directly attached to root port, or HS device behind an HS >> hub, from working, due to the fact that 'route string' is used by >> the xHC to target SS packets. But in order to enumerate devices >> behind an SS hub, this field must be programmed. >> >> With this commit and along with previous commits, now SS & HS >> devices >> attached to a USB 3.0 hub can be enumerated by U-Boot. >> >> As usual, this new feature is only available when DM is on. > > > > Great, but I really dislike the ifdef pollution, so this needs to be > sorted out.
The ifdef was needed due to it calls DM APIs or access DM udevice. I have no intention to add a new feature to the non-DM driver.
But then this creates a massive disparity, it's like we're growing two USB stacks.
Yep, unfortunately. But if we continue adding new features/fixes to the old non-DM stuff, I am not sure how this can encourage people to switch to DM.
Correct. We definitely don't want to add new features to non-DM drivers / IF, if this is non-trivial.
Maybe I can check all boards that use xHCI to see if they are switched to DM?
xHCI is still quite new in U-Boot, so I would expect that all platforms using it, are using DM or at least not far away from using it. Yes, please check all xHCI "users", if they use DM. Then we know if and which users need some "persuasion" to switch over to DM soon. ;)
I checked all boards that have CONFIG_USB_XHCI_HCD defined but without CONFIG_DM_USB. Here is the list.
configs/uniphier_v8_defconfig:36:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zc1751_xm015_dc1_defconfig:62:CONFIG_USB_XHCI_HCD=y configs/am57xx_evm_nodt_defconfig:53:CONFIG_USB_XHCI_HCD=y configs/evb-rk3328_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_lpuart_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/uniphier_pxs2_ld6b_defconfig:44:CONFIG_USB_XHCI_HCD=y configs/ls1012ardb_qspi_SECURE_BOOT_defconfig:43:CONFIG_USB_XHCI_HCD=y
configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig:57:CONFIG_USB_XHCI_HCD=y configs/k2e_hs_evm_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_sdcard_qspi_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_ethboot_defconfig:48:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_ep_defconfig:70:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_nand_defconfig:57:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_qspi_defconfig:50:CONFIG_USB_XHCI_HCD=y configs/k2g_evm_defconfig:45:CONFIG_USB_XHCI_HCD=y configs/am57xx_evm_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/am43xx_hs_evm_defconfig:49:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_defconfig:39:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_defconfig:42:CONFIG_USB_XHCI_HCD=y configs/firefly-rk3399_defconfig:59:CONFIG_USB_XHCI_HCD=y configs/puma-rk3399_defconfig:78:CONFIG_USB_XHCI_HCD=y configs/cl-som-am57x_defconfig:55:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_nor_SECURE_BOOT_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/uniphier_pro4_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zc1751_xm016_dc2_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zcu102_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_SECURE_BOOT_defconfig:42:CONFIG_USB_XHCI_HCD=y configs/cm_t43_defconfig:67:CONFIG_USB_XHCI_HCD=y configs/k2g_hs_evm_defconfig:36:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_qspiboot_defconfig:45:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_qspi_defconfig:50:CONFIG_USB_XHCI_HCD=y configs/am57xx_hs_evm_defconfig:67:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zcu102_revB_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_sdcard_ifc_defconfig:55:CONFIG_USB_XHCI_HCD=y configs/uniphier_ld20_defconfig:39:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_usbhost_boot_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_sdcard_qspi_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/evb-rk3399_defconfig:60:CONFIG_USB_XHCI_HCD=y configs/k2hk_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/k2hk_hs_evm_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/k2e_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_sdcard_ifc_defconfig:54:CONFIG_USB_XHCI_HCD=y configs/k2l_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y
So it looks we have lots of conversion work to be done by many board maintainers. I am not sure how to proceed on this.
Marek reminded me, that he thinks that most of these platforms above will most likely select DM_USB implicitly via Kconfig.
I did a quick check and it seems that at least these platforms have DM_USB enabled:
ARCH_ZYNQ ARCH_ZYNQMP ARCH_UNIPHIER ARCH_ROCKCHIP
and other from above very likely as well.
Before you invest more time and effort into implementing the xHCI additions in a "non-DM cleaner way", I suggest to find out which targets really use xHCI without USB_DM. An easy check would be to add some #error to the non-DM part and run this commit through buildman / travis.
What do you think?
Ah, that's really a good idea. Thanks for the hints! I will launch a buildman testing soon.
Regards, Bin
Hi Stefan,
On Wed, Jun 28, 2017 at 8:27 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Stefan,
On Wed, Jun 28, 2017 at 7:28 PM, Stefan Roese sr@denx.de wrote:
Hi Bin,
On 27.06.2017 10:27, Bin Meng wrote:
Hi Stefan,
On Tue, Jun 27, 2017 at 1:23 PM, Stefan Roese sr@denx.de wrote:
Hi Bin,
On 27.06.2017 02:01, Bin Meng wrote:
On Tue, Jun 27, 2017 at 2:07 AM, Marek Vasut marex@denx.de wrote:
On 06/24/2017 03:57 AM, Bin Meng wrote: > > > Hi Marek, > > On Sat, Jun 24, 2017 at 2:02 AM, Marek Vasut marex@denx.de wrote: >> >> >> On 06/23/2017 11:54 AM, Bin Meng wrote: >>> >>> >>> xHCI spec says: the values of the 'route string' field shall be >>> initialized by the first 'Address Device' command issued to a >>> device slot, and shall not be modified by any other command. >>> >>> So far U-Boot does not program this field, and it does not prevent >>> SS device directly attached to root port, or HS device behind an HS >>> hub, from working, due to the fact that 'route string' is used by >>> the xHC to target SS packets. But in order to enumerate devices >>> behind an SS hub, this field must be programmed. >>> >>> With this commit and along with previous commits, now SS & HS >>> devices >>> attached to a USB 3.0 hub can be enumerated by U-Boot. >>> >>> As usual, this new feature is only available when DM is on. >> >> >> >> Great, but I really dislike the ifdef pollution, so this needs to be >> sorted out. > > > > The ifdef was needed due to it calls DM APIs or access DM udevice. I > have no intention to add a new feature to the non-DM driver.
But then this creates a massive disparity, it's like we're growing two USB stacks.
Yep, unfortunately. But if we continue adding new features/fixes to the old non-DM stuff, I am not sure how this can encourage people to switch to DM.
Correct. We definitely don't want to add new features to non-DM drivers / IF, if this is non-trivial.
Maybe I can check all boards that use xHCI to see if they are switched to DM?
xHCI is still quite new in U-Boot, so I would expect that all platforms using it, are using DM or at least not far away from using it. Yes, please check all xHCI "users", if they use DM. Then we know if and which users need some "persuasion" to switch over to DM soon. ;)
I checked all boards that have CONFIG_USB_XHCI_HCD defined but without CONFIG_DM_USB. Here is the list.
configs/uniphier_v8_defconfig:36:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zc1751_xm015_dc1_defconfig:62:CONFIG_USB_XHCI_HCD=y configs/am57xx_evm_nodt_defconfig:53:CONFIG_USB_XHCI_HCD=y configs/evb-rk3328_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_lpuart_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/uniphier_pxs2_ld6b_defconfig:44:CONFIG_USB_XHCI_HCD=y configs/ls1012ardb_qspi_SECURE_BOOT_defconfig:43:CONFIG_USB_XHCI_HCD=y
configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig:57:CONFIG_USB_XHCI_HCD=y configs/k2e_hs_evm_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_sdcard_qspi_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_ethboot_defconfig:48:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_ep_defconfig:70:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_nand_defconfig:57:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_qspi_defconfig:50:CONFIG_USB_XHCI_HCD=y configs/k2g_evm_defconfig:45:CONFIG_USB_XHCI_HCD=y configs/am57xx_evm_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/am43xx_hs_evm_defconfig:49:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_defconfig:39:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_defconfig:42:CONFIG_USB_XHCI_HCD=y configs/firefly-rk3399_defconfig:59:CONFIG_USB_XHCI_HCD=y configs/puma-rk3399_defconfig:78:CONFIG_USB_XHCI_HCD=y configs/cl-som-am57x_defconfig:55:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_nor_SECURE_BOOT_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/uniphier_pro4_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zc1751_xm016_dc2_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zcu102_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_SECURE_BOOT_defconfig:42:CONFIG_USB_XHCI_HCD=y configs/cm_t43_defconfig:67:CONFIG_USB_XHCI_HCD=y configs/k2g_hs_evm_defconfig:36:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_qspiboot_defconfig:45:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_qspi_defconfig:50:CONFIG_USB_XHCI_HCD=y configs/am57xx_hs_evm_defconfig:67:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zcu102_revB_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_sdcard_ifc_defconfig:55:CONFIG_USB_XHCI_HCD=y configs/uniphier_ld20_defconfig:39:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_usbhost_boot_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_sdcard_qspi_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/evb-rk3399_defconfig:60:CONFIG_USB_XHCI_HCD=y configs/k2hk_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/k2hk_hs_evm_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/k2e_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_sdcard_ifc_defconfig:54:CONFIG_USB_XHCI_HCD=y configs/k2l_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y
So it looks we have lots of conversion work to be done by many board maintainers. I am not sure how to proceed on this.
Marek reminded me, that he thinks that most of these platforms above will most likely select DM_USB implicitly via Kconfig.
I did a quick check and it seems that at least these platforms have DM_USB enabled:
ARCH_ZYNQ ARCH_ZYNQMP ARCH_UNIPHIER ARCH_ROCKCHIP
and other from above very likely as well.
Before you invest more time and effort into implementing the xHCI additions in a "non-DM cleaner way", I suggest to find out which targets really use xHCI without USB_DM. An easy check would be to add some #error to the non-DM part and run this commit through buildman / travis.
What do you think?
Ah, that's really a good idea. Thanks for the hints! I will launch a buildman testing soon.
Looks we have a smaller list indeed. Here is the buildman results:
ls1012ardb_qspi_SECURE_BOOT ls1021atwr_nor_SECURE_BOOT am43xx_hs_evm am57xx_hs_evm ls1021aqds_nand ls1021atwr_nor ls1021atwr_qspi cm_t43 ls1021atwr_nor_lpuart ls1021aqds_sdcard_qspi k2hk_hs_evm am43xx_evm ls1021aqds_qspi am57xx_evm_nodt k2g_hs_evm ls1021atwr_sdcard_qspi am43xx_evm_ethboot ls1021aqds_sdcard_ifc k2l_evm am43xx_evm_usbhost_boot am43xx_evm_qspiboot k2g_evm am57xx_evm ls1021atwr_sdcard_ifc cl-som-am57x k2hk_evm k2e_evm ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021aqds_nor_SECURE_BOOT k2e_hs_evm
Regards, Bin
Hi Bin,
On 29.06.2017 01:00, Bin Meng wrote:
Hi Stefan,
On Wed, Jun 28, 2017 at 8:27 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Stefan,
On Wed, Jun 28, 2017 at 7:28 PM, Stefan Roese sr@denx.de wrote:
Hi Bin,
On 27.06.2017 10:27, Bin Meng wrote:
Hi Stefan,
On Tue, Jun 27, 2017 at 1:23 PM, Stefan Roese sr@denx.de wrote:
Hi Bin,
On 27.06.2017 02:01, Bin Meng wrote:
On Tue, Jun 27, 2017 at 2:07 AM, Marek Vasut marex@denx.de wrote: > > > On 06/24/2017 03:57 AM, Bin Meng wrote: >> >> >> Hi Marek, >> >> On Sat, Jun 24, 2017 at 2:02 AM, Marek Vasut marex@denx.de wrote: >>> >>> >>> On 06/23/2017 11:54 AM, Bin Meng wrote: >>>> >>>> >>>> xHCI spec says: the values of the 'route string' field shall be >>>> initialized by the first 'Address Device' command issued to a >>>> device slot, and shall not be modified by any other command. >>>> >>>> So far U-Boot does not program this field, and it does not prevent >>>> SS device directly attached to root port, or HS device behind an HS >>>> hub, from working, due to the fact that 'route string' is used by >>>> the xHC to target SS packets. But in order to enumerate devices >>>> behind an SS hub, this field must be programmed. >>>> >>>> With this commit and along with previous commits, now SS & HS >>>> devices >>>> attached to a USB 3.0 hub can be enumerated by U-Boot. >>>> >>>> As usual, this new feature is only available when DM is on. >>> >>> >>> >>> Great, but I really dislike the ifdef pollution, so this needs to be >>> sorted out. >> >> >> >> The ifdef was needed due to it calls DM APIs or access DM udevice. I >> have no intention to add a new feature to the non-DM driver. > > > > But then this creates a massive disparity, it's like we're growing two > USB stacks. >
Yep, unfortunately. But if we continue adding new features/fixes to the old non-DM stuff, I am not sure how this can encourage people to switch to DM.
Correct. We definitely don't want to add new features to non-DM drivers / IF, if this is non-trivial.
Maybe I can check all boards that use xHCI to see if they are switched to DM?
xHCI is still quite new in U-Boot, so I would expect that all platforms using it, are using DM or at least not far away from using it. Yes, please check all xHCI "users", if they use DM. Then we know if and which users need some "persuasion" to switch over to DM soon. ;)
I checked all boards that have CONFIG_USB_XHCI_HCD defined but without CONFIG_DM_USB. Here is the list.
configs/uniphier_v8_defconfig:36:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zc1751_xm015_dc1_defconfig:62:CONFIG_USB_XHCI_HCD=y configs/am57xx_evm_nodt_defconfig:53:CONFIG_USB_XHCI_HCD=y configs/evb-rk3328_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_lpuart_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/uniphier_pxs2_ld6b_defconfig:44:CONFIG_USB_XHCI_HCD=y configs/ls1012ardb_qspi_SECURE_BOOT_defconfig:43:CONFIG_USB_XHCI_HCD=y
configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig:57:CONFIG_USB_XHCI_HCD=y configs/k2e_hs_evm_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_sdcard_qspi_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_ethboot_defconfig:48:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_ep_defconfig:70:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_nand_defconfig:57:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_qspi_defconfig:50:CONFIG_USB_XHCI_HCD=y configs/k2g_evm_defconfig:45:CONFIG_USB_XHCI_HCD=y configs/am57xx_evm_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/am43xx_hs_evm_defconfig:49:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_defconfig:39:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_defconfig:42:CONFIG_USB_XHCI_HCD=y configs/firefly-rk3399_defconfig:59:CONFIG_USB_XHCI_HCD=y configs/puma-rk3399_defconfig:78:CONFIG_USB_XHCI_HCD=y configs/cl-som-am57x_defconfig:55:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_nor_SECURE_BOOT_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/uniphier_pro4_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zc1751_xm016_dc2_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zcu102_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_SECURE_BOOT_defconfig:42:CONFIG_USB_XHCI_HCD=y configs/cm_t43_defconfig:67:CONFIG_USB_XHCI_HCD=y configs/k2g_hs_evm_defconfig:36:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_qspiboot_defconfig:45:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_qspi_defconfig:50:CONFIG_USB_XHCI_HCD=y configs/am57xx_hs_evm_defconfig:67:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zcu102_revB_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_sdcard_ifc_defconfig:55:CONFIG_USB_XHCI_HCD=y configs/uniphier_ld20_defconfig:39:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_usbhost_boot_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_sdcard_qspi_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/evb-rk3399_defconfig:60:CONFIG_USB_XHCI_HCD=y configs/k2hk_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/k2hk_hs_evm_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/k2e_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_sdcard_ifc_defconfig:54:CONFIG_USB_XHCI_HCD=y configs/k2l_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y
So it looks we have lots of conversion work to be done by many board maintainers. I am not sure how to proceed on this.
Marek reminded me, that he thinks that most of these platforms above will most likely select DM_USB implicitly via Kconfig.
I did a quick check and it seems that at least these platforms have DM_USB enabled:
ARCH_ZYNQ ARCH_ZYNQMP ARCH_UNIPHIER ARCH_ROCKCHIP
and other from above very likely as well.
Before you invest more time and effort into implementing the xHCI additions in a "non-DM cleaner way", I suggest to find out which targets really use xHCI without USB_DM. An easy check would be to add some #error to the non-DM part and run this commit through buildman / travis.
What do you think?
Ah, that's really a good idea. Thanks for the hints! I will launch a buildman testing soon.
Looks we have a smaller list indeed. Here is the buildman results:
ls1012ardb_qspi_SECURE_BOOT ls1021atwr_nor_SECURE_BOOT am43xx_hs_evm am57xx_hs_evm ls1021aqds_nand ls1021atwr_nor ls1021atwr_qspi cm_t43 ls1021atwr_nor_lpuart ls1021aqds_sdcard_qspi k2hk_hs_evm am43xx_evm ls1021aqds_qspi am57xx_evm_nodt k2g_hs_evm ls1021atwr_sdcard_qspi am43xx_evm_ethboot ls1021aqds_sdcard_ifc k2l_evm am43xx_evm_usbhost_boot am43xx_evm_qspiboot k2g_evm am57xx_evm ls1021atwr_sdcard_ifc cl-som-am57x k2hk_evm k2e_evm ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021aqds_nor_SECURE_BOOT k2e_hs_evm
Thanks. So this leaves only some platforms using xHCI without DM_USB enabled indeed. I suspect that most of them don't have DM_USB enabled just because of oversight. Let me write a short mail to the maintainers, to see if they can enable DM_USB to make the way free for a complete DM based xHCI support.
Thanks, Stefan
Hi Stefan,
On Thu, Jun 29, 2017 at 1:29 PM, Stefan Roese sr@denx.de wrote:
Hi Bin,
On 29.06.2017 01:00, Bin Meng wrote:
Hi Stefan,
On Wed, Jun 28, 2017 at 8:27 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Stefan,
On Wed, Jun 28, 2017 at 7:28 PM, Stefan Roese sr@denx.de wrote:
Hi Bin,
On 27.06.2017 10:27, Bin Meng wrote:
Hi Stefan,
On Tue, Jun 27, 2017 at 1:23 PM, Stefan Roese sr@denx.de wrote:
Hi Bin,
On 27.06.2017 02:01, Bin Meng wrote: > > > > On Tue, Jun 27, 2017 at 2:07 AM, Marek Vasut marex@denx.de wrote: >> >> >> >> On 06/24/2017 03:57 AM, Bin Meng wrote: >>> >>> >>> >>> Hi Marek, >>> >>> On Sat, Jun 24, 2017 at 2:02 AM, Marek Vasut marex@denx.de wrote: >>>> >>>> >>>> >>>> On 06/23/2017 11:54 AM, Bin Meng wrote: >>>>> >>>>> >>>>> >>>>> xHCI spec says: the values of the 'route string' field shall be >>>>> initialized by the first 'Address Device' command issued to a >>>>> device slot, and shall not be modified by any other command. >>>>> >>>>> So far U-Boot does not program this field, and it does not >>>>> prevent >>>>> SS device directly attached to root port, or HS device behind an >>>>> HS >>>>> hub, from working, due to the fact that 'route string' is used by >>>>> the xHC to target SS packets. But in order to enumerate devices >>>>> behind an SS hub, this field must be programmed. >>>>> >>>>> With this commit and along with previous commits, now SS & HS >>>>> devices >>>>> attached to a USB 3.0 hub can be enumerated by U-Boot. >>>>> >>>>> As usual, this new feature is only available when DM is on. >>>> >>>> >>>> >>>> >>>> Great, but I really dislike the ifdef pollution, so this needs to >>>> be >>>> sorted out. >>> >>> >>> >>> >>> The ifdef was needed due to it calls DM APIs or access DM udevice. >>> I >>> have no intention to add a new feature to the non-DM driver. >> >> >> >> >> But then this creates a massive disparity, it's like we're growing >> two >> USB stacks. >> > > Yep, unfortunately. But if we continue adding new features/fixes to > the old non-DM stuff, I am not sure how this can encourage people to > switch to DM.
Correct. We definitely don't want to add new features to non-DM drivers / IF, if this is non-trivial.
> Maybe I can check all boards that use xHCI to see if > they are switched to DM?
xHCI is still quite new in U-Boot, so I would expect that all platforms using it, are using DM or at least not far away from using it. Yes, please check all xHCI "users", if they use DM. Then we know if and which users need some "persuasion" to switch over to DM soon. ;)
I checked all boards that have CONFIG_USB_XHCI_HCD defined but without CONFIG_DM_USB. Here is the list.
configs/uniphier_v8_defconfig:36:CONFIG_USB_XHCI_HCD=y
configs/xilinx_zynqmp_zc1751_xm015_dc1_defconfig:62:CONFIG_USB_XHCI_HCD=y configs/am57xx_evm_nodt_defconfig:53:CONFIG_USB_XHCI_HCD=y configs/evb-rk3328_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_lpuart_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/uniphier_pxs2_ld6b_defconfig:44:CONFIG_USB_XHCI_HCD=y configs/ls1012ardb_qspi_SECURE_BOOT_defconfig:43:CONFIG_USB_XHCI_HCD=y
configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig:57:CONFIG_USB_XHCI_HCD=y configs/k2e_hs_evm_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_sdcard_qspi_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_ethboot_defconfig:48:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_ep_defconfig:70:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_nand_defconfig:57:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_qspi_defconfig:50:CONFIG_USB_XHCI_HCD=y configs/k2g_evm_defconfig:45:CONFIG_USB_XHCI_HCD=y configs/am57xx_evm_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/am43xx_hs_evm_defconfig:49:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_defconfig:39:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_defconfig:42:CONFIG_USB_XHCI_HCD=y configs/firefly-rk3399_defconfig:59:CONFIG_USB_XHCI_HCD=y configs/puma-rk3399_defconfig:78:CONFIG_USB_XHCI_HCD=y configs/cl-som-am57x_defconfig:55:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_nor_SECURE_BOOT_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/uniphier_pro4_defconfig:43:CONFIG_USB_XHCI_HCD=y
configs/xilinx_zynqmp_zc1751_xm016_dc2_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zcu102_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_SECURE_BOOT_defconfig:42:CONFIG_USB_XHCI_HCD=y configs/cm_t43_defconfig:67:CONFIG_USB_XHCI_HCD=y configs/k2g_hs_evm_defconfig:36:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_qspiboot_defconfig:45:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_qspi_defconfig:50:CONFIG_USB_XHCI_HCD=y configs/am57xx_hs_evm_defconfig:67:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zcu102_revB_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_sdcard_ifc_defconfig:55:CONFIG_USB_XHCI_HCD=y configs/uniphier_ld20_defconfig:39:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_usbhost_boot_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_sdcard_qspi_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/evb-rk3399_defconfig:60:CONFIG_USB_XHCI_HCD=y configs/k2hk_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/k2hk_hs_evm_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/k2e_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_sdcard_ifc_defconfig:54:CONFIG_USB_XHCI_HCD=y configs/k2l_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y
So it looks we have lots of conversion work to be done by many board maintainers. I am not sure how to proceed on this.
Marek reminded me, that he thinks that most of these platforms above will most likely select DM_USB implicitly via Kconfig.
I did a quick check and it seems that at least these platforms have DM_USB enabled:
ARCH_ZYNQ ARCH_ZYNQMP ARCH_UNIPHIER ARCH_ROCKCHIP
and other from above very likely as well.
Before you invest more time and effort into implementing the xHCI additions in a "non-DM cleaner way", I suggest to find out which targets really use xHCI without USB_DM. An easy check would be to add some #error to the non-DM part and run this commit through buildman / travis.
What do you think?
Ah, that's really a good idea. Thanks for the hints! I will launch a buildman testing soon.
Looks we have a smaller list indeed. Here is the buildman results:
ls1012ardb_qspi_SECURE_BOOT ls1021atwr_nor_SECURE_BOOT am43xx_hs_evm am57xx_hs_evm ls1021aqds_nand ls1021atwr_nor ls1021atwr_qspi cm_t43 ls1021atwr_nor_lpuart ls1021aqds_sdcard_qspi k2hk_hs_evm am43xx_evm ls1021aqds_qspi am57xx_evm_nodt k2g_hs_evm ls1021atwr_sdcard_qspi am43xx_evm_ethboot ls1021aqds_sdcard_ifc k2l_evm am43xx_evm_usbhost_boot am43xx_evm_qspiboot k2g_evm am57xx_evm ls1021atwr_sdcard_ifc cl-som-am57x k2hk_evm k2e_evm ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021aqds_nor_SECURE_BOOT k2e_hs_evm
Thanks. So this leaves only some platforms using xHCI without DM_USB enabled indeed. I suspect that most of them don't have DM_USB enabled just because of oversight. Let me write a short mail to the maintainers, to see if they can enable DM_USB to make the way free for a complete DM based xHCI support.
Thank you for your help. I see at least ls1021a_xxx boards are because of oversight as some other ls1021a boards have DM_USB.
Regards, Bin
Hi Bin,
On 29.06.2017 07:42, Bin Meng wrote:
Hi Stefan,
On Thu, Jun 29, 2017 at 1:29 PM, Stefan Roese sr@denx.de wrote:
Hi Bin,
On 29.06.2017 01:00, Bin Meng wrote:
Hi Stefan,
On Wed, Jun 28, 2017 at 8:27 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Stefan,
On Wed, Jun 28, 2017 at 7:28 PM, Stefan Roese sr@denx.de wrote:
Hi Bin,
On 27.06.2017 10:27, Bin Meng wrote:
Hi Stefan,
On Tue, Jun 27, 2017 at 1:23 PM, Stefan Roese sr@denx.de wrote: > > > Hi Bin, > > > On 27.06.2017 02:01, Bin Meng wrote: >> >> >> >> On Tue, Jun 27, 2017 at 2:07 AM, Marek Vasut marex@denx.de wrote: >>> >>> >>> >>> On 06/24/2017 03:57 AM, Bin Meng wrote: >>>> >>>> >>>> >>>> Hi Marek, >>>> >>>> On Sat, Jun 24, 2017 at 2:02 AM, Marek Vasut marex@denx.de wrote: >>>>> >>>>> >>>>> >>>>> On 06/23/2017 11:54 AM, Bin Meng wrote: >>>>>> >>>>>> >>>>>> >>>>>> xHCI spec says: the values of the 'route string' field shall be >>>>>> initialized by the first 'Address Device' command issued to a >>>>>> device slot, and shall not be modified by any other command. >>>>>> >>>>>> So far U-Boot does not program this field, and it does not >>>>>> prevent >>>>>> SS device directly attached to root port, or HS device behind an >>>>>> HS >>>>>> hub, from working, due to the fact that 'route string' is used by >>>>>> the xHC to target SS packets. But in order to enumerate devices >>>>>> behind an SS hub, this field must be programmed. >>>>>> >>>>>> With this commit and along with previous commits, now SS & HS >>>>>> devices >>>>>> attached to a USB 3.0 hub can be enumerated by U-Boot. >>>>>> >>>>>> As usual, this new feature is only available when DM is on. >>>>> >>>>> >>>>> >>>>> >>>>> Great, but I really dislike the ifdef pollution, so this needs to >>>>> be >>>>> sorted out. >>>> >>>> >>>> >>>> >>>> The ifdef was needed due to it calls DM APIs or access DM udevice. >>>> I >>>> have no intention to add a new feature to the non-DM driver. >>> >>> >>> >>> >>> But then this creates a massive disparity, it's like we're growing >>> two >>> USB stacks. >>> >> >> Yep, unfortunately. But if we continue adding new features/fixes to >> the old non-DM stuff, I am not sure how this can encourage people to >> switch to DM. > > > > > Correct. We definitely don't want to add new features to non-DM > drivers / IF, if this is non-trivial. > >> Maybe I can check all boards that use xHCI to see if >> they are switched to DM? > > > > > xHCI is still quite new in U-Boot, so I would expect that all > platforms using it, are using DM or at least not far away from using > it. Yes, please check all xHCI "users", if they use DM. Then we > know if and which users need some "persuasion" to switch over to > DM soon. ;)
I checked all boards that have CONFIG_USB_XHCI_HCD defined but without CONFIG_DM_USB. Here is the list.
configs/uniphier_v8_defconfig:36:CONFIG_USB_XHCI_HCD=y
configs/xilinx_zynqmp_zc1751_xm015_dc1_defconfig:62:CONFIG_USB_XHCI_HCD=y configs/am57xx_evm_nodt_defconfig:53:CONFIG_USB_XHCI_HCD=y configs/evb-rk3328_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_lpuart_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/uniphier_pxs2_ld6b_defconfig:44:CONFIG_USB_XHCI_HCD=y configs/ls1012ardb_qspi_SECURE_BOOT_defconfig:43:CONFIG_USB_XHCI_HCD=y
configs/ls1021atwr_sdcard_ifc_SECURE_BOOT_defconfig:57:CONFIG_USB_XHCI_HCD=y configs/k2e_hs_evm_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_sdcard_qspi_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_ethboot_defconfig:48:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_ep_defconfig:70:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_nand_defconfig:57:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_qspi_defconfig:50:CONFIG_USB_XHCI_HCD=y configs/k2g_evm_defconfig:45:CONFIG_USB_XHCI_HCD=y configs/am57xx_evm_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/am43xx_hs_evm_defconfig:49:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_defconfig:39:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_defconfig:42:CONFIG_USB_XHCI_HCD=y configs/firefly-rk3399_defconfig:59:CONFIG_USB_XHCI_HCD=y configs/puma-rk3399_defconfig:78:CONFIG_USB_XHCI_HCD=y configs/cl-som-am57x_defconfig:55:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_nor_SECURE_BOOT_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/uniphier_pro4_defconfig:43:CONFIG_USB_XHCI_HCD=y
configs/xilinx_zynqmp_zc1751_xm016_dc2_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zcu102_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_nor_SECURE_BOOT_defconfig:42:CONFIG_USB_XHCI_HCD=y configs/cm_t43_defconfig:67:CONFIG_USB_XHCI_HCD=y configs/k2g_hs_evm_defconfig:36:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_qspiboot_defconfig:45:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_qspi_defconfig:50:CONFIG_USB_XHCI_HCD=y configs/am57xx_hs_evm_defconfig:67:CONFIG_USB_XHCI_HCD=y configs/xilinx_zynqmp_zcu102_revB_defconfig:63:CONFIG_USB_XHCI_HCD=y configs/ls1021aqds_sdcard_ifc_defconfig:55:CONFIG_USB_XHCI_HCD=y configs/uniphier_ld20_defconfig:39:CONFIG_USB_XHCI_HCD=y configs/am43xx_evm_usbhost_boot_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_sdcard_qspi_defconfig:61:CONFIG_USB_XHCI_HCD=y configs/evb-rk3399_defconfig:60:CONFIG_USB_XHCI_HCD=y configs/k2hk_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/k2hk_hs_evm_defconfig:34:CONFIG_USB_XHCI_HCD=y configs/k2e_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y configs/ls1021atwr_sdcard_ifc_defconfig:54:CONFIG_USB_XHCI_HCD=y configs/k2l_evm_defconfig:43:CONFIG_USB_XHCI_HCD=y
So it looks we have lots of conversion work to be done by many board maintainers. I am not sure how to proceed on this.
Marek reminded me, that he thinks that most of these platforms above will most likely select DM_USB implicitly via Kconfig.
I did a quick check and it seems that at least these platforms have DM_USB enabled:
ARCH_ZYNQ ARCH_ZYNQMP ARCH_UNIPHIER ARCH_ROCKCHIP
and other from above very likely as well.
Before you invest more time and effort into implementing the xHCI additions in a "non-DM cleaner way", I suggest to find out which targets really use xHCI without USB_DM. An easy check would be to add some #error to the non-DM part and run this commit through buildman / travis.
What do you think?
Ah, that's really a good idea. Thanks for the hints! I will launch a buildman testing soon.
Looks we have a smaller list indeed. Here is the buildman results:
ls1012ardb_qspi_SECURE_BOOT ls1021atwr_nor_SECURE_BOOT am43xx_hs_evm am57xx_hs_evm ls1021aqds_nand ls1021atwr_nor ls1021atwr_qspi cm_t43 ls1021atwr_nor_lpuart ls1021aqds_sdcard_qspi k2hk_hs_evm am43xx_evm ls1021aqds_qspi am57xx_evm_nodt k2g_hs_evm ls1021atwr_sdcard_qspi am43xx_evm_ethboot ls1021aqds_sdcard_ifc k2l_evm am43xx_evm_usbhost_boot am43xx_evm_qspiboot k2g_evm am57xx_evm ls1021atwr_sdcard_ifc cl-som-am57x k2hk_evm k2e_evm ls1021atwr_sdcard_ifc_SECURE_BOOT ls1021aqds_nor_SECURE_BOOT k2e_hs_evm
Thanks. So this leaves only some platforms using xHCI without DM_USB enabled indeed. I suspect that most of them don't have DM_USB enabled just because of oversight. Let me write a short mail to the maintainers, to see if they can enable DM_USB to make the way free for a complete DM based xHCI support.
Thank you for your help. I see at least ls1021a_xxx boards are because of oversight as some other ls1021a boards have DM_USB.
Yes, I noticed this as well. I just sent a mail to all the maintainers of those boards. Lets see, what they respond... ;)
Thanks, Stefan
On 06/27/2017 07:23 AM, Stefan Roese wrote:
Hi Bin,
On 27.06.2017 02:01, Bin Meng wrote:
On Tue, Jun 27, 2017 at 2:07 AM, Marek Vasut marex@denx.de wrote:
On 06/24/2017 03:57 AM, Bin Meng wrote:
Hi Marek,
On Sat, Jun 24, 2017 at 2:02 AM, Marek Vasut marex@denx.de wrote:
On 06/23/2017 11:54 AM, Bin Meng wrote:
xHCI spec says: the values of the 'route string' field shall be initialized by the first 'Address Device' command issued to a device slot, and shall not be modified by any other command.
So far U-Boot does not program this field, and it does not prevent SS device directly attached to root port, or HS device behind an HS hub, from working, due to the fact that 'route string' is used by the xHC to target SS packets. But in order to enumerate devices behind an SS hub, this field must be programmed.
With this commit and along with previous commits, now SS & HS devices attached to a USB 3.0 hub can be enumerated by U-Boot.
As usual, this new feature is only available when DM is on.
Great, but I really dislike the ifdef pollution, so this needs to be sorted out.
The ifdef was needed due to it calls DM APIs or access DM udevice. I have no intention to add a new feature to the non-DM driver.
But then this creates a massive disparity, it's like we're growing two USB stacks.
Yep, unfortunately. But if we continue adding new features/fixes to the old non-DM stuff, I am not sure how this can encourage people to switch to DM.
Correct. We definitely don't want to add new features to non-DM drivers / IF, if this is non-trivial.
Fine, but we also don't want to grow two distinct stacks with a boatload of ifdefs all over the place. That's a nightmare to maintain. I think I mentioned that already, but I'd be far more accepting to this solution if we could at least keep the added ifdefs to minimum and somehow keep them in one place instead of adding them all over the code.
Maybe I can check all boards that use xHCI to see if they are switched to DM?
xHCI is still quite new in U-Boot, so I would expect that all platforms using it, are using DM or at least not far away from using it. Yes, please check all xHCI "users", if they use DM. Then we know if and which users need some "persuasion" to switch over to DM soon. ;)
Thanks, Stefan
Hi Marek,
On Tue, Jun 27, 2017 at 5:31 PM, Marek Vasut marex@denx.de wrote:
On 06/27/2017 07:23 AM, Stefan Roese wrote:
Hi Bin,
On 27.06.2017 02:01, Bin Meng wrote:
On Tue, Jun 27, 2017 at 2:07 AM, Marek Vasut marex@denx.de wrote:
On 06/24/2017 03:57 AM, Bin Meng wrote:
Hi Marek,
On Sat, Jun 24, 2017 at 2:02 AM, Marek Vasut marex@denx.de wrote:
On 06/23/2017 11:54 AM, Bin Meng wrote: > xHCI spec says: the values of the 'route string' field shall be > initialized by the first 'Address Device' command issued to a > device slot, and shall not be modified by any other command. > > So far U-Boot does not program this field, and it does not prevent > SS device directly attached to root port, or HS device behind an HS > hub, from working, due to the fact that 'route string' is used by > the xHC to target SS packets. But in order to enumerate devices > behind an SS hub, this field must be programmed. > > With this commit and along with previous commits, now SS & HS devices > attached to a USB 3.0 hub can be enumerated by U-Boot. > > As usual, this new feature is only available when DM is on.
Great, but I really dislike the ifdef pollution, so this needs to be sorted out.
The ifdef was needed due to it calls DM APIs or access DM udevice. I have no intention to add a new feature to the non-DM driver.
But then this creates a massive disparity, it's like we're growing two USB stacks.
Yep, unfortunately. But if we continue adding new features/fixes to the old non-DM stuff, I am not sure how this can encourage people to switch to DM.
Correct. We definitely don't want to add new features to non-DM drivers / IF, if this is non-trivial.
Fine, but we also don't want to grow two distinct stacks with a boatload of ifdefs all over the place. That's a nightmare to maintain. I think I mentioned that already, but I'd be far more accepting to this solution if we could at least keep the added ifdefs to minimum and somehow keep them in one place instead of adding them all over the code.
OK, I will see if I can do some additional work to remove the #ifdefs or limit them in a minimum way, even if that means I have to bring (part of) this feature to the non-DM driver.
Maybe I can check all boards that use xHCI to see if they are switched to DM?
xHCI is still quite new in U-Boot, so I would expect that all platforms using it, are using DM or at least not far away from using it. Yes, please check all xHCI "users", if they use DM. Then we know if and which users need some "persuasion" to switch over to DM soon. ;)
[snip]
Regards, Bin
Hi Marek,
On 27 June 2017 at 03:31, Marek Vasut marex@denx.de wrote:
On 06/27/2017 07:23 AM, Stefan Roese wrote:
Hi Bin,
On 27.06.2017 02:01, Bin Meng wrote:
On Tue, Jun 27, 2017 at 2:07 AM, Marek Vasut marex@denx.de wrote:
On 06/24/2017 03:57 AM, Bin Meng wrote:
Hi Marek,
On Sat, Jun 24, 2017 at 2:02 AM, Marek Vasut marex@denx.de wrote:
On 06/23/2017 11:54 AM, Bin Meng wrote: > xHCI spec says: the values of the 'route string' field shall be > initialized by the first 'Address Device' command issued to a > device slot, and shall not be modified by any other command. > > So far U-Boot does not program this field, and it does not prevent > SS device directly attached to root port, or HS device behind an HS > hub, from working, due to the fact that 'route string' is used by > the xHC to target SS packets. But in order to enumerate devices > behind an SS hub, this field must be programmed. > > With this commit and along with previous commits, now SS & HS devices > attached to a USB 3.0 hub can be enumerated by U-Boot. > > As usual, this new feature is only available when DM is on.
Great, but I really dislike the ifdef pollution, so this needs to be sorted out.
The ifdef was needed due to it calls DM APIs or access DM udevice. I have no intention to add a new feature to the non-DM driver.
But then this creates a massive disparity, it's like we're growing two USB stacks.
Yep, unfortunately. But if we continue adding new features/fixes to the old non-DM stuff, I am not sure how this can encourage people to switch to DM.
Correct. We definitely don't want to add new features to non-DM drivers / IF, if this is non-trivial.
Fine, but we also don't want to grow two distinct stacks with a boatload of ifdefs all over the place. That's a nightmare to maintain. I think I mentioned that already, but I'd be far more accepting to this solution if we could at least keep the added ifdefs to minimum and somehow keep them in one place instead of adding them all over the code.
One idea is to set a deadline (e.g. end of year) for all boards to move to DM_USB, and turn off USB for those that don't make it? In some areas the old code is quite a significant impediment to progress / productivity.
Maybe I can check all boards that use xHCI to see if they are switched to DM?
xHCI is still quite new in U-Boot, so I would expect that all platforms using it, are using DM or at least not far away from using it. Yes, please check all xHCI "users", if they use DM. Then we know if and which users need some "persuasion" to switch over to DM soon. ;)
Regards, Simon
A high speed hub has a special responsibility to handle full speed/ low speed devices connected on downstream ports. In this case, the hub must isolate the high speed signaling environment from the full speed/low speed signaling environment with the help of Transaction Translator (TT). TT details are provided by hub descriptors and we parse and save it to hub uclass_priv for later use.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
common/usb_hub.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/usb.h | 16 ++++++++++++++++ include/usb_defs.h | 12 ++++++++++++ 3 files changed, 78 insertions(+)
diff --git a/common/usb_hub.c b/common/usb_hub.c index 63358cd..4911981 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -693,6 +693,56 @@ static int usb_hub_configure(struct usb_device *dev) break; }
+ switch (dev->descriptor.bDeviceProtocol) { + case USB_HUB_PR_FS: + break; + case USB_HUB_PR_HS_SINGLE_TT: + debug("Single TT\n"); + break; + case USB_HUB_PR_HS_MULTI_TT: + ret = usb_set_interface(dev, 0, 1); + if (ret == 0) { + debug("TT per port\n"); + hub->tt.multi = true; + } else { + debug("Using single TT (err %d)\n", ret); + } + break; + case USB_HUB_PR_SS: + /* USB 3.0 hubs don't have a TT */ + break; + default: + debug("Unrecognized hub protocol %d\n", + dev->descriptor.bDeviceProtocol); + break; + } + + /* Note 8 FS bit times == (8 bits / 12000000 bps) ~= 666ns */ + switch (hubCharacteristics & HUB_CHAR_TTTT) { + case HUB_TTTT_8_BITS: + if (dev->descriptor.bDeviceProtocol != 0) { + hub->tt.think_time = 666; + debug("TT requires at most %d FS bit times (%d ns)\n", + 8, hub->tt.think_time); + } + break; + case HUB_TTTT_16_BITS: + hub->tt.think_time = 666 * 2; + debug("TT requires at most %d FS bit times (%d ns)\n", + 16, hub->tt.think_time); + break; + case HUB_TTTT_24_BITS: + hub->tt.think_time = 666 * 3; + debug("TT requires at most %d FS bit times (%d ns)\n", + 24, hub->tt.think_time); + break; + case HUB_TTTT_32_BITS: + hub->tt.think_time = 666 * 4; + debug("TT requires at most %d FS bit times (%d ns)\n", + 32, hub->tt.think_time); + break; + } + debug("power on to power good time: %dms\n", descriptor->bPwrOn2PwrGood * 2); debug("hub controller current requirement: %dmA\n", diff --git a/include/usb.h b/include/usb.h index 40e44f4..6e42be5 100644 --- a/include/usb.h +++ b/include/usb.h @@ -537,6 +537,21 @@ struct usb_hub_status { unsigned short wHubChange; } __attribute__ ((packed));
+/* + * Hub Device descriptor + * USB Hub class device protocols + */ +#define USB_HUB_PR_FS 0 /* Full speed hub */ +#define USB_HUB_PR_HS_NO_TT 0 /* Hi-speed hub without TT */ +#define USB_HUB_PR_HS_SINGLE_TT 1 /* Hi-speed hub with single TT */ +#define USB_HUB_PR_HS_MULTI_TT 2 /* Hi-speed hub with multiple TT */ +#define USB_HUB_PR_SS 3 /* Super speed hub */ + +/* Transaction Translator Think Times, in bits */ +#define HUB_TTTT_8_BITS 0x00 +#define HUB_TTTT_16_BITS 0x20 +#define HUB_TTTT_24_BITS 0x40 +#define HUB_TTTT_32_BITS 0x60
/* Hub descriptor */ struct usb_hub_descriptor { @@ -571,6 +586,7 @@ struct usb_hub_device { ulong query_delay; /* Device query delay in ms */ int overcurrent_count[USB_MAXCHILDREN]; /* Over-current counter */ int hub_depth; /* USB 3.0 hub depth */ + struct usb_tt tt; /* Transaction Translator */ };
#ifdef CONFIG_DM_USB diff --git a/include/usb_defs.h b/include/usb_defs.h index 273337f..b7f2ead 100644 --- a/include/usb_defs.h +++ b/include/usb_defs.h @@ -293,6 +293,7 @@ #define HUB_CHAR_LPSM 0x0003 #define HUB_CHAR_COMPOUND 0x0004 #define HUB_CHAR_OCPM 0x0018 +#define HUB_CHAR_TTTT 0x0060 /* TT Think Time mask */
/* * Hub Status & Hub Change bit masks @@ -310,6 +311,17 @@ #define USB_REQ_SET_HUB_DEPTH 0x0c
/* + * As of USB 2.0, full/low speed devices are segregated into trees. + * One type grows from USB 1.1 host controllers (OHCI, UHCI etc). + * The other type grows from high speed hubs when they connect to + * full/low speed devices using "Transaction Translators" (TTs). + */ +struct usb_tt { + bool multi; /* true means one TT per port */ + unsigned think_time; /* think time in ns */ +}; + +/* * CBI style */
On 23 June 2017 at 03:54, Bin Meng bmeng.cn@gmail.com wrote:
A high speed hub has a special responsibility to handle full speed/ low speed devices connected on downstream ports. In this case, the hub must isolate the high speed signaling environment from the full speed/low speed signaling environment with the help of Transaction Translator (TT). TT details are provided by hub descriptors and we parse and save it to hub uclass_priv for later use.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
common/usb_hub.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/usb.h | 16 ++++++++++++++++ include/usb_defs.h | 12 ++++++++++++ 3 files changed, 78 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
For USB host controllers like xHC, its internal representation of hub needs to be updated after the hub descriptor is fetched. This adds a new op that does this.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
drivers/usb/host/usb-uclass.c | 11 +++++++++++ include/usb.h | 21 ++++++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c index 9bb8477..1078b8c 100644 --- a/drivers/usb/host/usb-uclass.c +++ b/drivers/usb/host/usb-uclass.c @@ -139,6 +139,17 @@ int usb_reset_root_port(struct usb_device *udev) return ops->reset_root_port(bus, udev); }
+int usb_update_hub_device(struct usb_device *udev) +{ + struct udevice *bus = udev->controller_dev; + struct dm_usb_ops *ops = usb_get_ops(bus); + + if (!ops->update_hub_device) + return -ENOSYS; + + return ops->update_hub_device(bus, udev); +} + int usb_stop(void) { struct udevice *bus; diff --git a/include/usb.h b/include/usb.h index 6e42be5..d43be7e 100644 --- a/include/usb.h +++ b/include/usb.h @@ -758,6 +758,14 @@ struct dm_usb_ops { * reset_root_port() - Reset usb root port */ int (*reset_root_port)(struct udevice *bus, struct usb_device *udev); + + /** + * update_hub_device() - Update HCD's internal representation of hub + * + * After a hub descriptor is fetched, notify HCD so that its internal + * representation of this hub can be updated (xHCI) + */ + int (*update_hub_device)(struct udevice *bus, struct usb_device *udev); };
#define usb_get_ops(dev) ((struct dm_usb_ops *)(dev)->driver->ops) @@ -949,6 +957,17 @@ int usb_new_device(struct usb_device *dev); int usb_alloc_device(struct usb_device *dev);
/** + * update_hub_device() - Update HCD's internal representation of hub + * + * After a hub descriptor is fetched, notify HCD so that its internal + * representation of this hub can be updated. + * + * @dev: Hub device + * @return 0 if OK, -ve on error + */ +int usb_update_hub_device(struct usb_device *dev); + +/** * usb_emul_setup_device() - Set up a new USB device emulation * * This is normally called when a new emulation device is bound. It tells @@ -961,7 +980,7 @@ int usb_alloc_device(struct usb_device *dev); * @desc_list: List of points or USB descriptors, terminated by NULL. * The first entry must be struct usb_device_descriptor, * and others follow on after that. - * @return 0 if OK, -ve on error + * @return 0 if OK, -ENOSYS if not implemented, other -ve on error */ int usb_emul_setup_device(struct udevice *dev, int maxpacketsize, struct usb_string *strings, void **desc_list);
On 23 June 2017 at 03:54, Bin Meng bmeng.cn@gmail.com wrote:
For USB host controllers like xHC, its internal representation of hub needs to be updated after the hub descriptor is fetched. This adds a new op that does this.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/usb/host/usb-uclass.c | 11 +++++++++++ include/usb.h | 21 ++++++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
After fetching hub descriptor, we need call USB uclass operation update_hub_device() to notify HCD to do some preparation work.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
common/usb_hub.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/common/usb_hub.c b/common/usb_hub.c index 4911981..2fc544e 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -782,6 +782,17 @@ static int usb_hub_configure(struct usb_device *dev)
#ifdef CONFIG_DM_USB /* + * Update USB host controller's internal representation of this hub + * after the hub descriptor is fetched. + */ + ret = usb_update_hub_device(dev); + if (ret < 0 && ret != -ENOSYS) { + debug("%s: failed to update hub device for HCD (%x)\n", + __func__, ret); + return ret; + } + + /* * A maximum of seven tiers are allowed in a USB topology, and the * root hub occupies the first tier. The last tier ends with a normal * USB device. USB 3.0 hubs use a 20-bit field called 'route string'
On 23 June 2017 at 03:54, Bin Meng bmeng.cn@gmail.com wrote:
After fetching hub descriptor, we need call USB uclass operation
need to call
update_hub_device() to notify HCD to do some preparation work.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
common/usb_hub.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
There is no way to know whether the attached device is a hub or not in advance before device's descriptor is fetched. But once we know it's a high speed hub, per xHCI spec, we need tell xHC it's a hub device by initializing hub related fields in the input slot context.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
drivers/usb/host/xhci.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 1148127..a82502c 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1170,6 +1170,63 @@ static int xhci_alloc_device(struct udevice *dev, struct usb_device *udev) return _xhci_alloc_device(udev); }
+static int xhci_update_hub_device(struct udevice *dev, struct usb_device *udev) +{ + struct xhci_ctrl *ctrl = dev_get_priv(dev); + struct usb_hub_device *hub = dev_get_uclass_priv(udev->dev); + struct xhci_virt_device *virt_dev; + struct xhci_input_control_ctx *ctrl_ctx; + struct xhci_container_ctx *out_ctx; + struct xhci_container_ctx *in_ctx; + struct xhci_slot_ctx *slot_ctx; + int slot_id = udev->slot_id; + unsigned think_time; + + debug("%s: dev='%s', udev=%p\n", __func__, dev->name, udev); + + /* Ignore root hubs */ + if (usb_hub_is_root_hub(udev->dev)) + return 0; + + virt_dev = ctrl->devs[slot_id]; + BUG_ON(!virt_dev); + + out_ctx = virt_dev->out_ctx; + in_ctx = virt_dev->in_ctx; + + ctrl_ctx = xhci_get_input_control_ctx(in_ctx); + /* Initialize the input context control */ + ctrl_ctx->add_flags |= cpu_to_le32(SLOT_FLAG); + ctrl_ctx->drop_flags = 0; + + xhci_inval_cache((uintptr_t)out_ctx->bytes, out_ctx->size); + + /* slot context */ + xhci_slot_copy(ctrl, in_ctx, out_ctx); + slot_ctx = xhci_get_slot_ctx(ctrl, in_ctx); + + /* Update hub related fields */ + slot_ctx->dev_info |= cpu_to_le32(DEV_HUB); + if (hub->tt.multi && udev->speed == USB_SPEED_HIGH) + slot_ctx->dev_info |= cpu_to_le32(DEV_MTT); + slot_ctx->dev_info2 |= cpu_to_le32(XHCI_MAX_PORTS(udev->maxchild)); + /* + * Set TT think time - convert from ns to FS bit times. + * + * 0 = 8 FS bit times, 1 = 16 FS bit times, + * 2 = 24 FS bit times, 3 = 32 FS bit times. + * + * This field shall be 0 if the device is not a high-spped hub. + */ + think_time = hub->tt.think_time; + if (think_time != 0) + think_time = (think_time / 666) - 1; + if (udev->speed == USB_SPEED_HIGH) + slot_ctx->tt_info |= cpu_to_le32(TT_THINK_TIME(think_time)); + + return xhci_configure_endpoints(udev, false); +} + int xhci_register(struct udevice *dev, struct xhci_hccr *hccr, struct xhci_hcor *hcor) { @@ -1222,6 +1279,7 @@ struct dm_usb_ops xhci_usb_ops = { .bulk = xhci_submit_bulk_msg, .interrupt = xhci_submit_int_msg, .alloc_device = xhci_alloc_device, + .update_hub_device = xhci_update_hub_device, };
#endif
Hi Bin,
On 23 June 2017 at 03:54, Bin Meng bmeng.cn@gmail.com wrote:
There is no way to know whether the attached device is a hub or not in advance before device's descriptor is fetched. But once
nits:
before the device's
we know it's a high speed hub, per xHCI spec, we need tell xHC
per the xHCI
we need to tell
it's a hub device by initializing hub related fields in the
hub-related
input slot context.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/usb/host/xhci.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 1148127..a82502c 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1170,6 +1170,63 @@ static int xhci_alloc_device(struct udevice *dev, struct usb_device *udev) return _xhci_alloc_device(udev); }
+static int xhci_update_hub_device(struct udevice *dev, struct usb_device *udev) +{
struct xhci_ctrl *ctrl = dev_get_priv(dev);struct usb_hub_device *hub = dev_get_uclass_priv(udev->dev);struct xhci_virt_device *virt_dev;struct xhci_input_control_ctx *ctrl_ctx;struct xhci_container_ctx *out_ctx;struct xhci_container_ctx *in_ctx;struct xhci_slot_ctx *slot_ctx;int slot_id = udev->slot_id;unsigned think_time;debug("%s: dev='%s', udev=%p\n", __func__, dev->name, udev);/* Ignore root hubs */if (usb_hub_is_root_hub(udev->dev))return 0;virt_dev = ctrl->devs[slot_id];BUG_ON(!virt_dev);out_ctx = virt_dev->out_ctx;in_ctx = virt_dev->in_ctx;ctrl_ctx = xhci_get_input_control_ctx(in_ctx);/* Initialize the input context control */ctrl_ctx->add_flags |= cpu_to_le32(SLOT_FLAG);ctrl_ctx->drop_flags = 0;xhci_inval_cache((uintptr_t)out_ctx->bytes, out_ctx->size);/* slot context */xhci_slot_copy(ctrl, in_ctx, out_ctx);slot_ctx = xhci_get_slot_ctx(ctrl, in_ctx);/* Update hub related fields */slot_ctx->dev_info |= cpu_to_le32(DEV_HUB);if (hub->tt.multi && udev->speed == USB_SPEED_HIGH)slot_ctx->dev_info |= cpu_to_le32(DEV_MTT);slot_ctx->dev_info2 |= cpu_to_le32(XHCI_MAX_PORTS(udev->maxchild));/** Set TT think time - convert from ns to FS bit times.** 0 = 8 FS bit times, 1 = 16 FS bit times,* 2 = 24 FS bit times, 3 = 32 FS bit times.** This field shall be 0 if the device is not a high-spped hub.*/think_time = hub->tt.think_time;if (think_time != 0)think_time = (think_time / 666) - 1;
What is 666? Can you add a comment?
if (udev->speed == USB_SPEED_HIGH)slot_ctx->tt_info |= cpu_to_le32(TT_THINK_TIME(think_time));return xhci_configure_endpoints(udev, false);+}
int xhci_register(struct udevice *dev, struct xhci_hccr *hccr, struct xhci_hcor *hcor) { @@ -1222,6 +1279,7 @@ struct dm_usb_ops xhci_usb_ops = { .bulk = xhci_submit_bulk_msg, .interrupt = xhci_submit_int_msg, .alloc_device = xhci_alloc_device,
.update_hub_device = xhci_update_hub_device,};
#endif
2.9.2
Regards, Simon
These two macros really need a parameter to make them useful.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
drivers/usb/host/xhci.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index cdce67c..a497d9d 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -548,12 +548,12 @@ struct xhci_slot_ctx { * The Slot ID of the hub that isolates the high speed signaling from * this low or full-speed device. '0' if attached to root hub port. */ -#define TT_SLOT (0xff) +#define TT_SLOT(p) (((p) & 0xff) << 0) /* * The number of the downstream facing port of the high-speed hub * '0' if the device is not low or full speed. */ -#define TT_PORT (0xff << 8) +#define TT_PORT(p) (((p) & 0xff) << 8) #define TT_THINK_TIME(p) (((p) & 0x3) << 16)
/* dev_state bitmasks */
On 23 June 2017 at 03:54, Bin Meng bmeng.cn@gmail.com wrote:
These two macros really need a parameter to make them useful.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/usb/host/xhci.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
So far LS/FS devices directly attached to xHC root port can be successfully enumerated by xHCI driver, but if they are connected behind a hub, the enumeration process fails to address the device.
It turns out xHCI driver still misses a part that in the device's input slot context, all Transaction Translator (TT) related fields are not programmed. The xHCI spec defines how to enable TT.
Now LS/FS devices like USB keyboard/mouse can be enumerated behind a high speed hub.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
drivers/usb/host/xhci-mem.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 03874db..bac2cee 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -771,6 +771,20 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl, BUG(); }
+#ifdef CONFIG_DM_USB + /* Set up TT fields to support FS/LS devices */ + if (speed == USB_SPEED_LOW || speed == USB_SPEED_FULL) { + dev = dev_get_parent_priv(udev->dev); + if (dev->speed == USB_SPEED_HIGH) { + hub = dev_get_uclass_priv(udev->dev); + if (hub->tt.multi) + slot_ctx->dev_info |= cpu_to_le32(DEV_MTT); + slot_ctx->tt_info |= cpu_to_le32(TT_PORT(udev->portnr)); + slot_ctx->tt_info |= cpu_to_le32(TT_SLOT(dev->slot_id)); + } + } +#endif + port_num = hop_portnr; debug("port_num = %d\n", port_num);
On 23 June 2017 at 03:54, Bin Meng bmeng.cn@gmail.com wrote:
So far LS/FS devices directly attached to xHC root port can be successfully enumerated by xHCI driver, but if they are connected behind a hub, the enumeration process fails to address the device.
It turns out xHCI driver still misses a part that in the device's input slot context, all Transaction Translator (TT) related fields are not programmed. The xHCI spec defines how to enable TT.
Now LS/FS devices like USB keyboard/mouse can be enumerated behind a high speed hub.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/usb/host/xhci-mem.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
participants (6)
-
Bin Meng -
Lothar Waßmann -
Marek Vasut -
Simon Glass -
Stefan Roese -
stefan.bruens＠rwth-aachen.de