Re: [PATCH v3 02/23] usb: usbtmc: Add ioctl for generic requests on control

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jul 24, 2018 at 11:05:29AM +0200, Guido Kiener wrote:
> Add USBTMC_IOCTL_CTRL_REQUEST to send arbitrary requests on the
> control pipe.  Used by specific applications of IVI Foundation,
> Inc. to implement VISA API functions: viUsbControlIn/Out.
> 
> The maximum length of control request is set to 4k.
> 
> Signed-off-by: Guido Kiener <guido.kiener@xxxxxxxxxxxxxxxxx>
> Reviewed-by: Steve Bayless <steve_bayless@xxxxxxxxxxxx>
> ---
>  drivers/usb/class/usbtmc.c   | 84 ++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/usb/tmc.h | 15 +++++++
>  2 files changed, 99 insertions(+)
> 
> diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
> index 83ffa5a14c3d..3b3b4284d04e 100644
> --- a/drivers/usb/class/usbtmc.c
> +++ b/drivers/usb/class/usbtmc.c
> @@ -5,6 +5,7 @@
>   * Copyright (C) 2007 Stefan Kopp, Gechingen, Germany
>   * Copyright (C) 2008 Novell, Inc.
>   * Copyright (C) 2008 Greg Kroah-Hartman <gregkh@xxxxxxx>
> + * Copyright (C) 2018 IVI Foundation, Inc.
>   */
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> @@ -36,6 +37,9 @@
>  /* Default USB timeout (in milliseconds) */
>  #define USBTMC_TIMEOUT		5000
>  
> +/* I/O buffer size used in generic read/write functions */
> +#define USBTMC_BUFSIZE		(4096)
> +
>  /*
>   * Maximum number of read cycles to empty bulk in endpoint during CLEAR and
>   * ABORT_BULK_IN requests. Ends the loop if (for whatever reason) a short
> @@ -129,6 +133,21 @@ struct usbtmc_file_data {
>  /* Forward declarations */
>  static struct usb_driver usbtmc_driver;
>  
> +#ifdef CONFIG_COMPAT
> +static void __user *u64_to_uptr(u64 value)
> +{
> +	if (in_compat_syscall())
> +		return compat_ptr(value);
> +	else
> +		return (void __user *)(unsigned long)value;
> +}
> +#else
> +static inline void __user *u64_to_uptr(u64 value)
> +{
> +	return (void __user *)(unsigned long)value;
> +}
> +#endif /* CONFIG_COMPAT */

For Linux, if there are functions/operations that are always needed by
all drivers (or even some), then those functions are provided in the
core kernel with header files or in the library.  So if you are writing
a single driver, and find you need to create something that looks very
"generic", either you are doing something that no other driver has ever
needed before in the history of Linux, or you are doing something wrong.

Which do you think it is here?  :)

Please fix this up and do it properly.  Your driver should only have 1
#ifdef CONFIG_COMPAT line, and that was in patch 1 for this series.
Nothing else should be needed if you create your structures correctly.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux