On Tue, Jul 9, 2024 at 9:27 PM Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On 7/5/24 04:43, Łukasz Bartosik wrote:
> > On Fri, Jul 5, 2024 at 11:40 AM Greg Kroah-Hartman
> > <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> >>
> >> On Fri, Jul 05, 2024 at 09:19:02AM +0000, Łukasz Bartosik wrote:
> >>> Add USB_SPEED_SUPER_PLUS as valid argument to allow
> >>> to attach USB SuperSpeed+ devices.
> >>>
> >>> Signed-off-by: Łukasz Bartosik <ukaszb@xxxxxxxxxxxx>
> >>> ---
> >>> drivers/usb/usbip/vhci_sysfs.c | 3 ++-
> >>> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c
> >>> index e2847cd3e6e3..d5865460e82d 100644
> >>> --- a/drivers/usb/usbip/vhci_sysfs.c
> >>> +++ b/drivers/usb/usbip/vhci_sysfs.c
> >>> @@ -283,6 +283,7 @@ static int valid_args(__u32 *pdev_nr, __u32 *rhport,
> >>> case USB_SPEED_HIGH:
> >>> case USB_SPEED_WIRELESS:
> >>> case USB_SPEED_SUPER:
> >>> + case USB_SPEED_SUPER_PLUS:
> >>> break;
> >>> default:
> >>> pr_err("Failed attach request for unsupported USB speed: %s\n",
> >>> @@ -349,7 +350,7 @@ static ssize_t attach_store(struct device *dev, struct device_attribute *attr,
> >>> vhci_hcd = hcd_to_vhci_hcd(hcd);
> >>> vhci = vhci_hcd->vhci;
> >>>
> >>> - if (speed == USB_SPEED_SUPER)
> >>> + if (speed >= USB_SPEED_SUPER)
> >>
> >> It's an enum, are you sure this will work?
> >>
> >
> > Gcc (gcc (Debian 13.2.0-13) 13.2.0) which I used to compile the patch
> > does not complain about this change at all:
> > make
> > ...
> > CC [M] drivers/usb/usbip/vhci_sysfs.o
> > LD [M] drivers/usb/usbip/vhci-hcd.o
> >
> >
> >
> > Without the patch I was getting the following error when trying to
> > attach a device:
> > vhci_hcd: Failed attach request for unsupported USB speed: super-speed-plus
> >
> > With the patch USB SS+ device attaches successfully:
> > [248223.654445] vhci_hcd vhci_hcd.0: pdev(0) rhport(0) sockfd(3)
> > [248223.660701] vhci_hcd vhci_hcd.0: devid(65538) speed(6)
> > speed_str(super-speed-plus)
> > [248223.668540] vhci_hcd vhci_hcd.0: Device attached
> > [248223.936363] usb 2-1: SetAddress Request (2) to port 0
> > [248223.941698] usb 2-1: new SuperSpeed USB device number 2 using vhci_hcd
> > [248224.138020] usb 2-1: LPM exit latency is zeroed, disabling LPM.
> > [248224.331984] usb 2-1: New USB device found, idVendor=18d1,
> > idProduct=0010, bcdDevice= 0.10
> > [248224.340416] usb 2-1: New USB device strings: Mfr=1, Product=2,
> > SerialNumber=3
> > [248224.347805] usb 2-1: Product: Linux USB Debug Target
> > [248224.352984] usb 2-1: Manufacturer: Linux Foundation
> > [248224.358162] usb 2-1: SerialNumber: 0001
> >
> > I hope this will resolve your doubts.
> >
>
> What about the other places that check for USB_SPEED_SUPER?
> take a look at attach_store() that checks for USB_SPEED_SUPER
> and picks the correct vdev.
>
This patch already updates attach_store() to select the correct vdev.
Please see lines:
- if (speed == USB_SPEED_SUPER)
+ if (speed >= USB_SPEED_SUPER)
Also there are two other places where USB_SPEED_SUPER is used:
1) vhci_hcd.c:1158: hcd->self.root_hub->speed = USB_SPEED_SUPER - IMHO
no need for a change as it is ok to attach USB3.1 device (taking into
account it is backwards compatible) to USB3.0 HC
2) vudc_transfer.c:34: case USB_SPEED_SUPER - this seems to be
unrelated to this patch
Thanks,
Lukasz
> This change is incomplete and will break things.
>
> thanks,
> -- Shuah
>
