Re: [PATCH v3 2/3] usb: xhci: Add DbC support in xHCI driver

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

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@linuxfoundation.org> writes:
> >>>> > >> Greg Kroah-Hartman <gregkh@linuxfoundation.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@linux.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.

thanks,

greg k-h