On Wed, Nov 18, 2015 at 10:38 AM, Dave Penkler <dpenkler@xxxxxxxxx> wrote:
> These ioctls provide support for the USBTMC-USB488 control requests
> for REN_CONTROL, GO_TO_LOCAL and LOCAL_LOCKOUT
Couple of comments below.
> diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
> index 2358991..d416a5f 100644
> --- a/drivers/usb/class/usbtmc.c
> +++ b/drivers/usb/class/usbtmc.c
> @@ -476,6 +476,62 @@ static int usbtmc488_ioctl_read_stb(struct usbtmc_device_data *data,
> return rv;
> }
>
> +static int usbtmc488_ioctl_simple(struct usbtmc_device_data *data,
> + unsigned long arg,
> + unsigned int cmd)
> +{
> + u8 *buffer;
> + struct device *dev = &data->intf->dev;
> + int rv;
> + unsigned int val;
> + u16 wValue;
> +
> + if (!(data->usb488_caps & USBTMC488_CAPABILITY_SIMPLE))
> + return -EINVAL;
> +
> + buffer = kmalloc(8, GFP_KERNEL);
> + if (!buffer)
> + return -ENOMEM;
> +
> + if (cmd == USBTMC488_REQUEST_REN_CONTROL) {
> + rv = copy_from_user(&val, (void __user *)arg, sizeof(val));
> + if (rv) {
> + rv = -EFAULT;
> + goto exit;
> + }
> + wValue = val ? 1 : 0;
> + } else {
> + wValue = 0;
> + }
> +
> + rv = usb_control_msg(data->usb_dev,
> + usb_rcvctrlpipe(data->usb_dev, 0),
> + cmd,
> + USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
> + wValue,
> + data->ifnum,
> + buffer, 0x01, USBTMC_TIMEOUT);
> +
Redundant empty line
> + if (rv < 0) {
> + dev_err(dev, "simple usb_control_msg failed %d\n", rv);
> + goto exit;
> + } else if (rv != 1) {
> + dev_warn(dev, "simple usb_control_msg returned %d\n", rv);
Actually here what king of results could be? 0? 2+? In all cases of
error you have to provide an error code.
> + goto exit;
> + }
> +
> + if (buffer[0] != USBTMC_STATUS_SUCCESS) {
> + dev_err(dev, "simple control status returned %x\n", buffer[0]);
> + rv = -EIO;
> + goto exit;
> + }
> + rv = 0;
> +
> + exit:
> + kfree(buffer);
> + return rv;
> +}
> +
--
With Best Regards,
Andy Shevchenko
--
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
