Re: [PATCH v1] usbip: Add USB_SPEED_SUPER_PLUS as valid arg

[Shuah Khan <skhan@linuxfoundation.org>]

On 7/5/24 04:43, Łukasz Bartosik wrote:
> On Fri, Jul 5, 2024 at 11:40 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> 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@chromium.org>
>>> ---
>>>   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 change is incomplete and will break things.

thanks,
-- Shuah