Re: [PATCHv2 8/9] hackrf: add support for transmitter

[Hans Verkuil <hverkuil@xs4all.nl>]

On 07/16/2015 09:04 AM, Antti Palosaari wrote:
> HackRF SDR device has both receiver and transmitter. There is limitation
> that receiver and transmitter cannot be used at the same time
> (half-duplex operation). That patch implements transmitter support to
> existing receiver only driver.
> 
> Cc: Hans Verkuil <hverkuil@xs4all.nl>
> Signed-off-by: Antti Palosaari <crope@iki.fi>
> ---
>  drivers/media/usb/hackrf/hackrf.c | 787 +++++++++++++++++++++++++++-----------
>  1 file changed, 572 insertions(+), 215 deletions(-)
> 


> @@ -611,8 +751,15 @@ static int hackrf_queue_setup(struct vb2_queue *vq,
>  		unsigned int *nplanes, unsigned int sizes[], void *alloc_ctxs[])
>  {
>  	struct hackrf_dev *dev = vb2_get_drv_priv(vq);
> +	struct usb_interface *intf = dev->intf;
> +	int ret;
>  
> -	dev_dbg(dev->dev, "nbuffers=%d\n", *nbuffers);
> +	dev_dbg(&intf->dev, "nbuffers=%d\n", *nbuffers);
> +
> +	if (test_and_set_bit(QUEUE_SETUP, &dev->flags)) {
> +		ret = -EBUSY;
> +		goto err;
> +	}

This doesn't work. The bit is only cleared when start_streaming fails or
stop_streaming is called. But the application can also call REQBUFS again
or just close the file handle, and then QUEUE_SETUP should also be cleared.

But why is this here in the first place? It doesn't seem to do anything
useful (except mess up the v4l2-compliance tests).

I've removed it and it now seems to work OK.

Regards,

	Hans

>  
>  	/* Need at least 8 buffers */
>  	if (vq->num_buffers + *nbuffers < 8)
> @@ -620,8 +767,11 @@ static int hackrf_queue_setup(struct vb2_queue *vq,
>  	*nplanes = 1;
>  	sizes[0] = PAGE_ALIGN(dev->buffersize);
>  
> -	dev_dbg(dev->dev, "nbuffers=%d sizes[0]=%d\n", *nbuffers, sizes[0]);
> +	dev_dbg(&intf->dev, "nbuffers=%d sizes[0]=%d\n", *nbuffers, sizes[0]);
>  	return 0;
> +err:
> +	dev_dbg(&intf->dev, "failed=%d\n", ret);
> +	return ret;
>  }