Re: [PATCH v14 1/4] usb: Add support for Intel LJCA device

[Oliver Neukum <oneukum@suse.com>]

On 04.09.23 07:44, Wentong Wu wrote:

> +static int ljca_send(struct ljca_adapter *adap, u8 type, u8 cmd,
> +		     const void *obuf, unsigned int obuf_len, void *ibuf,
> +		     unsigned int ibuf_len, bool ack, unsigned long timeout)
> +{
> +	unsigned int msg_len = sizeof(struct ljca_msg) + obuf_len;
> +	struct ljca_msg *header = adap->tx_buf;
> +	unsigned long flags;
> +	unsigned int actual;
> +	int ret = 0;
> +
> +	if (msg_len > adap->tx_buf_len)
> +		return -EINVAL;
> +
> +	mutex_lock(&adap->mutex);
> +
> +	spin_lock_irqsave(&adap->lock, flags);
> +
> +	header->type = type;
> +	header->cmd = cmd;
> +	header->len = obuf_len;
> +	if (obuf)
> +		memcpy(header->data, obuf, obuf_len);
> +
> +	header->flags = LJCA_CMPL_FLAG | (ack ? LJCA_ACK_FLAG : 0);
> +
> +	adap->ex_buf = ibuf;
> +	adap->ex_buf_len = ibuf_len;
> +	adap->actual_length = 0;
> +
> +	spin_unlock_irqrestore(&adap->lock, flags);
> +
> +	reinit_completion(&adap->cmd_completion);
> +
> +	usb_autopm_get_interface(adap->intf);
> +
> +	ret = usb_bulk_msg(adap->usb_dev, adap->tx_pipe, header,
> +			   msg_len, &actual, LJCA_WRITE_TIMEOUT_MS);
> +	if (!ret && ack) {
> +		ret = wait_for_completion_timeout(&adap->cmd_completion,
> +						  timeout);

Let's suppose we are here, when a device is unplugged.
timeout is arbitrary as it is passed down to us.

> +		if (ret < 0) {
> +			goto out;
> +		} if (!ret) {
> +			ret = -ETIMEDOUT;
> +			goto out;
> +		}
> +	}
> +	ret = adap->actual_length;
> +
> +out:
> +	spin_lock_irqsave(&adap->lock, flags);
> +	adap->ex_buf = NULL;
> +	adap->ex_buf_len = 0;
> +
> +	memset(header, 0, sizeof(*header));
> +	spin_unlock_irqrestore(&adap->lock, flags);
> +
> +	usb_autopm_put_interface(adap->intf);
> +
> +	mutex_unlock(&adap->mutex);
> +
> +	return ret;
> +}
> +

[..]

> +static void ljca_disconnect(struct usb_interface *interface)
> +{
> +	struct ljca_adapter *adap = usb_get_intfdata(interface);
> +	struct ljca_client *client, *next;
> +
> +	usb_kill_urb(adap->rx_urb);
> +
> +	list_for_each_entry_safe(client, next, &adap->client_list, link) {
> +		auxiliary_device_delete(&client->auxdev);
> +		auxiliary_device_uninit(&client->auxdev);
> +
> +		list_del_init(&client->link);
> +		kfree(client);
> +	}
> +
> +	usb_free_urb(adap->rx_urb);
> +
> +	usb_put_intf(adap->intf);
> +
> +	mutex_destroy(&adap->mutex);
> +}

And we execute this. rx_urb is killed and does nothing.
I see nothing that terminates the waiting if you hit the wrong window.
It seems like this needs to complete the waiting.

	Regards
		Oliver