Message-ID: <5A011B30.4000507@linux.intel.com>
Date:   Tue, 7 Nov 2017 10:32:16 +0800
From:   Lu Baolu <baolu.lu@...ux.intel.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Felipe Balbi <felipe.balbi@...ux.intel.com>,
        'Mathias Nyman' <mathias.nyman@...ux.intel.com>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver

Hi,

On 11/06/2017 04:00 PM, Greg Kroah-Hartman wrote:
> On Mon, Nov 06, 2017 at 08:35:41AM +0800, Lu Baolu wrote:
>> Hi,
>>
>> On 11/03/2017 02:27 PM, Greg Kroah-Hartman wrote:
>>> On Fri, Nov 03, 2017 at 08:45:46AM +0800, Lu Baolu wrote:
>>>> Hi,
>>>>
>>>> On 11/03/2017 12:51 AM, Greg Kroah-Hartman wrote:
>>>>> On Thu, Nov 02, 2017 at 12:38:57PM +0200, Felipe Balbi wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Greg Kroah-Hartman <gregkh@...uxfoundation.org> writes:
>>>>>>>>>>> Greg Kroah-Hartman <gregkh@...uxfoundation.org> writes:
>>>>>>>>>>>>>>>>> xHCI compatible USB host controllers(i.e. super-speed USB3 controllers)
>>>>>>>>>>>>>>>>> can be implemented with the Debug Capability(DbC). It presents a debug
>>>>>>>>>>>>>>>>> device which is fully compliant with the USB framework and provides the
>>>>>>>>>>>>>>>>> equivalent of a very high performance full-duplex serial link. The debug
>>>>>>>>>>>>>>>>> capability operation model and registers interface are defined in 7.6.8
>>>>>>>>>>>>>>>>> of the xHCI specification, revision 1.1.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> The DbC debug device shares a root port with the xHCI host. By default,
>>>>>>>>>>>>>>>>> the debug capability is disabled and the root port is assigned to xHCI.
>>>>>>>>>>>>>>>>> When the DbC is enabled, the root port will be assigned to the DbC debug
>>>>>>>>>>>>>>>>> device, and the xHCI sees nothing on this port. This implementation uses
>>>>>>>>>>>>>>>>> a sysfs node named <dbc> under the xHCI device to manage the enabling
>>>>>>>>>>>>>>>>> and disabling of the debug capability.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> When the debug capability is enabled, it will present a debug device
>>>>>>>>>>>>>>>>> through the debug port. This debug device is fully compliant with the
>>>>>>>>>>>>>>>>> USB3 framework, and it can be enumerated by a debug host on the other
>>>>>>>>>>>>>>>>> end of the USB link. As soon as the debug device is configured, a TTY
>>>>>>>>>>>>>>>>> serial device named /dev/ttyDBC0 will be created.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> One use of this link is running a login service on the debug target.
>>>>>>>>>>>>>>>>> Hence it can be remote accessed by a debug host. Another use case can
>>>>>>>>>>>>>>>>> probably be found in servers. It provides a peer-to-peer USB link
>>>>>>>>>>>>>>>>> between two host-only machines. This provides a reasonable out-of-band
>>>>>>>>>>>>>>>>> communication method between two servers.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Signed-off-by: Lu Baolu <baolu.lu@...ux.intel.com>
>>>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>>>  .../ABI/testing/sysfs-bus-pci-drivers-xhci_hcd     |   25 +
>>>>>>>>>>>>>>>>>  drivers/usb/host/Kconfig                           |    9 +
>>>>>>>>>>>>>>>>>  drivers/usb/host/Makefile                          |    5 +
>>>>>>>>>>>>>>>>>  drivers/usb/host/xhci-dbgcap.c                     | 1016 ++++++++++++++++++++
>>>>>>>>>>>>>>>>>  drivers/usb/host/xhci-dbgcap.h                     |  247 +++++
>>>>>>>>>>>>>>>>>  drivers/usb/host/xhci-dbgtty.c                     |  586 +++++++++++
>>>>>>>>>>>>>>>>>  drivers/usb/host/xhci-trace.h                      |   60 ++
>>>>>>>>>>>>>>>>>  drivers/usb/host/xhci.c                            |   10 +
>>>>>>>>>>>>>>>>>  drivers/usb/host/xhci.h                            |    1 +
>>>>>>>>>>>>>>>>>  9 files changed, 1959 insertions(+)
>>>>>>>>>>>>>>>>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
>>>>>>>>>>>>>>>>>  create mode 100644 drivers/usb/host/xhci-dbgcap.c
>>>>>>>>>>>>>>>>>  create mode 100644 drivers/usb/host/xhci-dbgcap.h
>>>>>>>>>>>>>>>>>  create mode 100644 drivers/usb/host/xhci-dbgtty.c
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> [snip]
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> +#define DBC_VENDOR_ID			0x1d6b	/* Linux Foundation 0x1d6b */
>>>>>>>>>>>>>>>>> +#define DBC_PRODUCT_ID			0x0004	/* device 0004 */
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The DbC (xHCI DeBug Capability) is an optional functionality in
>>>>>>>>>>>>>>> some xHCI host controllers. It will present a super-speed debug
>>>>>>>>>>>>>>> device through the debug port after it is enabled.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The DbC register set defines an interface for system software
>>>>>>>>>>>>>>> to specify the vendor id and product id of the debug device.
>>>>>>>>>>>>>>> These two values will be presented by the debug device in its
>>>>>>>>>>>>>>> device descriptor idVendor and idProduct fields.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Microsoft Windows have a well established protocol for
>>>>>>>>>>>>>>> debugging over DbC. And it assigns below values for its use.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> USB\VID_045E&PID_062D.DeviceDesc="Microsoft USB Debug Target"
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I'm going to use 0x1d6b/0x0004 value pair for DbC use in
>>>>>>>>>>>>>>> Linux. Do you approve me to do so?
>>>>>>>>>>>>> No.  Why can't you use the same ids as Windows?  This is implementing
>>>>>>>>>>>>> the same protocol, right?
>>>>>>>>>>> the protocol running on top is 100% vendor specific. More than likely,
>>>>>>>>>>> we would just run kgdb on top of this, right? We really don't support
>>>>>>>>>>> microsoft's debug architecture.
>>>>>>>>> Ah, I didn't know about the protocol specifics here, if it is
>>>>>>>>> vendor-specific, then yes, we need our own id.
>>>>>>> Great, thanks :-)
>>>>>>>
>>>>>>> Let us know which one we're allowed to use and I'm sure Baolu can respin
>>>>>>> the patch in no time.
>>>>> Can I get a "full" description of what string this device id will
>>>>> reference?  Is it "Linux USB Debug Target" or something else?
>>>>>
>>>> Current manufacturer and product strings are set like this.
>>>>
>>>> +#define DBC_STRING_MANUFACTURER	"Linux"
>>>> +#define DBC_STRING_PRODUCT		"Remote	GDB"
>>>>
>>>> These are also place holders. We can change them to more meaningful strings.
>>> As the vendor id is assigned to "Linux Foundation" can we change that
>>> string please?
>>>
>>> And why not match what Microsoft does here, "USB Debug Target" makes
>>> more sense than "Remote GDB", right?
>>>
>>> If so, please use device id 0x0010, I've reserved that for the driver
>>> now.
>>>
>> I will change the strings and use the allocated product id.
>> Thank you very much.
>>
>> Another DbC usage is to enable DbC during early boot time,
>> and run GDB protocol data or early printk message over it.
>> The early printk implementation has been merged in 4.12-rc1.
>>
>> commit aeb9dd1 <usb/early: Add driver for xhci debug capability>
>>
>> In this code, the DbC product id is still a place holder (0x0004).
>> I am really sorry. I should apply a valid product id from you during
>> the code review time. But I forgot it since almost all attention was
>> paid to early driver itself at that time.
>>
>> We could use the same ID as you allocated here or, preferably,
>> use a different ID since we're dealing with different functionality. 
> A different one would be best, please give me a good device string to
> use and I will assign you a new ID, probably 0011.  Also send a patch
> now for this please.
>

Thanks, Greg.

How about below strings:

Manufacturer: Linux Foundation
Product: Linux USB GDB Target

I will send out the patches soon.

Best regards,
Lu Baolu

Powered by blists - more mailing lists