At 2023-07-21 19:04:29, "Greg KH" <gregkh@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxx>
>> ---
>> 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
