Zitat von Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>:
On Sat, Jul 21, 2018 at 11:11:55AM +0000, guido@xxxxxxxxxxxxxxxxxx wrote:
Zitat von Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>:
> On Wed, Jul 18, 2018 at 10:45:41AM +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 | 151 +++++++++++++++++++++++++++++++++++
> > include/uapi/linux/usb/tmc.h | 15 ++++
> > 2 files changed, 166 insertions(+)
> >
> > diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
> > index d685db78b80b..846599dd0c84 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
> > @@ -126,6 +130,17 @@ struct usbtmc_file_data {
> > bool term_char_enabled;
> > };
> >
> > +#ifdef CONFIG_COMPAT
> > +
> > +struct compat_usbtmc_ctrlrequest {
> > + struct usbtmc_request req;
> > + compat_uptr_t data;
> > +} __packed;
>
> Wait, why? You are creating a new api here, why do you need a 32bit
> compat layer? Just create it in a way that works for both layouts.
> Just make your pointer a 64bit value and cast it to the pointer when
> using it. Other ioctls do this in lots of places, making things simpler
> overall. That way you don't need an ioctl compat call at all.
>
> Please rework the code that way, don't create new apis that have to add
> "old" 32bit compatibility support to them. That's just extra work you
> don't need to do here.
>
Ok. I found some places where in_compat_syscall() or u64_to_uptr() is used
to solve the 32/64 bit pointer problem. This makes the usbtmc kernel driver
much easier, however the user needs to call something like this with
double type casts to assign a 32/64 bit pointer to a __u64 value:
request.data = (__u64)(uintptr_t)buf
We agreed to accept this drawback and I will send a v3 patch series.
Regards,
Guido
--
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
