Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 21 Jul 2023 13:04:29 +0200
From:   Greg KH <gregkh@linuxfoundation.org>
To:     Dingyan Li <18500469033@163.com>
Cc:     stern@rowland.harvard.edu, sebastian.reichel@collabora.com,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] USB: add usbfs ioctl to get specific superspeedplus rates
Message-ID: <2023072105-lethargic-saddling-ad97@gregkh>
References: <20230721084039.9728-1-18500469033@163.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230721084039.9728-1-18500469033@163.com>
Precedence: bulk

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