| Message ID | 20240705091902.789643-1-ukaszb@chromium.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] usbip: Add USB_SPEED_SUPER_PLUS as valid arg | expand |
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? thanks, greg k-h
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. Thanks, Lukasz > thanks, > > greg k-h
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
On Tue, Jul 9, 2024 at 9:27 PM Shuah Khan <skhan@linuxfoundation.org> wrote: > > 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 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 >
On 7/10/24 04:50, Łukasz Bartosik wrote: > On Tue, Jul 9, 2024 at 9:27 PM Shuah Khan <skhan@linuxfoundation.org> wrote: >> >> 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 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 Let's update them all. > 2) vudc_transfer.c:34: case USB_SPEED_SUPER - this seems to be > unrelated to this patch This isn't related - however feel free to send the patch if you are bale to. thanks, -- Shuah
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) vdev = &vhci->vhci_hcd_ss->vdev[rhport]; else vdev = &vhci->vhci_hcd_hs->vdev[rhport];
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(-)