On Wed, 2015-10-14 at 15:13 +0200, dave penkler wrote:
Hi,
just a few remarks.
>
> +static int usbtmc488_ioctl_read_stb(struct usbtmc_device_data *data,
> + unsigned long arg)
> +{
> + u8 *buffer;
> + struct device *dev;
> + int rv;
> + u8 tag, stb;
> +
> + dev = &data->intf->dev;
> +
> + dev_dbg(dev, "Enter ioctl_read_stb iin_ep_present: %d\n",
> + data->iin_ep_present);
> +
> + buffer = kmalloc(8, GFP_KERNEL);
> + if (!buffer)
> + return -ENOMEM;
> +
> +
> + atomic_set(&data->iin_data_valid, 0);
> +
> + /* must issue read_stb before using poll or select */
> + atomic_set(&data->srq_asserted, 0);
> +
> + rv = usb_control_msg(data->usb_dev,
> + usb_rcvctrlpipe(data->usb_dev, 0),
> + USBTMC488_REQUEST_READ_STATUS_BYTE,
> + USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
> + data->iin_bTag,
> + data->ifnum,
> + buffer, 0x03, USBTMC_TIMEOUT);
> +
> + if (rv < 0) {
> + dev_err(dev, "stb usb_control_msg returned %d\n", rv);
> + goto exit;
> + }
> +
> + if (buffer[0] != USBTMC_STATUS_SUCCESS) {
> + dev_err(dev, "control status returned %x\n",
> + buffer[0]);
> + rv = -EIO;
> + goto exit;
> + }
> +
> + if (data->iin_ep_present) {
> +
> + rv = wait_event_interruptible_timeout(
> + data->waitq,
> + (atomic_read(&data->iin_data_valid) != 0),
> + USBTMC_TIMEOUT
> + );
> +
> + if (rv < 0) {
> + dev_dbg(dev, "wait interrupted %d\n", rv);
> + goto exit;
> + }
If you do this, yet the transfer succeeds, how do you keep the tag
between host and device consistent?
> +
> + if (rv == 0) {
> + dev_dbg(dev, "wait timed out\n");
> + rv = -ETIME;
> + goto exit;
> + }
> +
> + tag = data->bNotify1 & 0x7f;
> +
> + if (tag != data->iin_bTag) {
> + dev_err(dev, "expected bTag %x got %x\n",
> + data->iin_bTag, tag);
> + }
> +
> + stb = data->bNotify2;
> + } else {
> + stb = buffer[2];
> + }
> +
> + /* bump interrupt bTag */
> + data->iin_bTag += 1;
> + if (data->iin_bTag > 127)
> + data->iin_bTag = 2;
> +
> + rv = copy_to_user((void *)arg, &stb, sizeof(stb));
> + if (rv)
> + rv = -EFAULT;
> +
> + exit:
> + kfree(buffer);
> + return rv;
> +
> +}
> +
> +static unsigned int usbtmc_poll(struct file *file, poll_table *wait)
> +{
> + struct usbtmc_device_data *data = file->private_data;
> + unsigned int mask = 0;
> +
> + mutex_lock(&data->io_mutex);
> +
> + if (data->zombie)
> + goto no_poll;
> +
> + poll_wait(file, &data->waitq, wait);
Presumably the waiters should be woken when the device is disconnected.
> +
> + mask = data->zombie ? POLLHUP | POLLERR :
> + (atomic_read(&data->srq_asserted)) ? POLLIN | POLLRDNORM : 0;
> +
> +no_poll:
> + mutex_unlock(&data->io_mutex);
> + return mask;
> +}
Regards
Oliver
--
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
