lkml.org 
[lkml]   [2023]   [Aug]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2] USB: Support 20Gbps speed for ioctl USBDEVFS_GET_SPEED
    On Sat, Aug 19, 2023 at 01:46:55PM +0800, Dingyan Li wrote:
    > Currently ioctl USBDEVFS_GET_SPEED can only return
    > USB_SPEED_SUPER_PLUS at most. However, there are also
    > ssp rates to indicate different connection speeds, which
    > we can not tell further via USBDEVFS_GET_SPEED.
    >
    > To fix it, this patch still uses USB_SPEED_SUPER_PLUS
    > to indicate USB_SSP_GEN_UNKNOWN, USB_SSP_GEN_2x1, and
    > USB_SSP_GEN_1x2. But need to #define a new value for
    > USB_SSP_GEN_2x2. Besides, move the definition of enum
    > usb_ssp_rate from include/linux/usb/ch9.h to
    > include/uapi/linux/usb/ch9.h, which is a better place
    > to hold it.
    >
    > Signed-off-by: Dingyan Li <18500469033@163.com>

    I'm not going to ACK this. It's clumsy -- having two separate
    enumerations for USB device speeds just looks ridiculous.

    We should fix the whole situation once and for all, recognizing that any
    code which depends on the speed needs to be upward compatible because
    new speeds and bus technologies may be added at any time.

    > --- a/include/uapi/linux/usb/ch9.h
    > +++ b/include/uapi/linux/usb/ch9.h
    > @@ -1185,6 +1185,14 @@ enum usb_device_speed {
    > USB_SPEED_SUPER_PLUS, /* usb 3.1 */
    > };
    >
    > +/* USB 3.2 SuperSpeed Plus phy signaling rate generation and lane count */
    > +
    > +enum usb_ssp_rate {
    > + USB_SSP_GEN_UNKNOWN = 0,
    > + USB_SSP_GEN_2x1,
    > + USB_SSP_GEN_1x2,
    > + USB_SSP_GEN_2x2,
    > +};

    This would make more sense if you kept very clear the distinction
    between the overall speed and the physical communication mechanism. In
    other words, 10000 bps is 10000 bps, no matter whether the underlying
    technology uses one lane carrying 10000 bits per second or two lanes
    each carrying 5000 bits per second.

    I'm not sure if anything in the kernel or userspace really cares about
    the number of lanes, as opposed to the total speed. If it turns out
    that nothing does, the usb_ssp_rate enumeration could be removed.
    Besides, it should named something else, like usb_ssp_gen or
    usb_sp_generation, since it isn't just a rate designation. (Whereas as
    enum usb_device_speed _is_ just a rate designation.)

    Regardless of what happens to usb_ssp_rate, usb_device_speed should be
    enlarged to encompass all possible existing speeds. That would
    immediately fix the ioctl problem. Doing this in an upward-compatible
    way might end up being a little awkward but it ought to be possible.

    Alan Stern

    \
     
     \ /
      Last update: 2023-08-20 12:13    [W:3.380 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site