[U-Boot] [PATCH 10/16] usb: xhci: Program 'route string' in the input slot context

Thu Jun 29 05:46:56 UTC 2017

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 at 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 at gmail.com> wrote:
>>>>
>>>> Hi Stefan,
>>>>
>>>> On Wed, Jun 28, 2017 at 7:28 PM, Stefan Roese <sr at 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 at 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 at 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 at 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