lkml.org 
[lkml]   [2023]   [Jul]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe:Re: [PATCH] USB: add usbfs ioctl to get specific superspeedplus rates

    At 2023-07-21 19:04:29, "Greg KH" <gregkh@linuxfoundation.org> wrote:
    >On Fri, Jul 21, 2023 at 04:40:39PM +0800, Dingyan Li wrote:
    >> The usbfs interface does not provide any way to get specific
    >> superspeedplus rate, like Gen2x1, Gen1x2 or Gen2x2. Current
    >> API include an USBDEVFS_GET_SPEED ioctl, but it can only return
    >> general superspeedplus speed instead of any specific rates.
    >> Therefore we can't tell whether it's a Gen2x2(20Gbps) device.
    >>
    >> This patch introduce a new ioctl USBDEVFS_GET_SSP_RATE to fix
    >> it. Similar information is already available via sysfs, it's
    >> good to add it for usbfs too.
    >>
    >> Signed-off-by: Dingyan Li <18500469033@163.com>
    >> ---
    >> drivers/usb/core/devio.c | 3 +++
    >> include/uapi/linux/usbdevice_fs.h | 1 +
    >> 2 files changed, 4 insertions(+)
    >>
    >> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
    >> index 1a16a8bdea60..2f57eb163360 100644
    >> --- a/drivers/usb/core/devio.c
    >> +++ b/drivers/usb/core/devio.c
    >> @@ -2783,6 +2783,9 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd,
    >> case USBDEVFS_GET_SPEED:
    >> ret = ps->dev->speed;
    >> break;
    >> + case USBDEVFS_GET_SSP_RATE:
    >> + ret = ps->dev->ssp_rate;
    >> + break;
    >
    >Shouldn't this new ioctl be documented somewhere? What are the valid
    >values it can return? What if it in't a superspeed device? Who is
    >going to use this?
    >
    >And we have traditionally only been adding new information like this to
    >sysfs, which was not around when usbfs was created. Why not just use
    >that instead? Are you wanting to see all of the sysfs-provided
    >information in usbfs also?
    >
    >thanks,
    >

    >greg k-h

    1. By saying "be documented somewhere", do you mean there is extra
    documentation work which needs to be done? Sorry that I missed this
    part since it's the first time for me to work on a kernel patch.
    2. If no error, returned values are "enum usb_ssp_rate" defined in include/linux/usb/ch9.h
    3. ssp rate is only valid for superspeedplus. For other speeds, it should be
    USB_SSP_GEN_UNKNOWN.
    4. I found in libusb, there are two ways to get speed value for a device.
    One way is via sysfs, which has supported 20Gbps now. Another way is
    to use ioctl USBDEVFS_GET_SPEED. This is when I found this ioctl can only
    return USB_SPEED_SUPER_PLUS at most, it cannot determine current ssp rate
    further, no matter Gen1x2(10Gbps), Gen2x1(10Gbps) or Gen2x2(20Gbps). So I
    thought maybe it's good to provide a similar way like ioctl USBDEVFS_GET_SPEED
    in order to get ssp rates.
    5. Okay, now I get it that sysfs is a replacement for usbfs. Even in libusb, sysfs is the
    preferred way, then fall back to usbfs if sysfs doesn't exist. My intention is not to see
    all of the sysfs-provided information in usbfs also. Anyway, if you think this patch is
    really unnecessary, I'm totally fine to withdraw it too.


    Regards,
    Dingyan
    \
     
     \ /
      Last update: 2023-07-21 14:37    [W:4.611 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site