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

[Hans Verkuil <hverkuil@xs4all.nl>]

Hi Antti,

I've got one comment:

On 06/06/2015 02:03 PM, 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 | 855 ++++++++++++++++++++++++++++----------
>  1 file changed, 640 insertions(+), 215 deletions(-)
> 
> diff --git a/drivers/media/usb/hackrf/hackrf.c b/drivers/media/usb/hackrf/hackrf.c
> index 5bd291b..6ad6937 100644
> --- a/drivers/media/usb/hackrf/hackrf.c
> +++ b/drivers/media/usb/hackrf/hackrf.c
> +/*
> + * TODO: That blocks whole transmitter device open when receiver is opened and
> + * the other way around, even only streaming is not allowed. Better solution
> + * needed...

Exactly. Why not use a similar approach as for video:

Return EBUSY when the applications tries to call:

S_FREQUENCY, S_MODULATOR, S_TUNER or REQBUFS/CREATE_BUFS and the other
vb2 queue is marked 'busy'. The check for REQBUFS/CREATE_BUFS can be done
in hackrf_queue_setup.

You should always be able to open a device node in V4L2.

Regards,

	Hans

> + */
> +static int hackrf_v4l2_open(struct file *file)
> +{
> +	struct hackrf_dev *dev = video_drvdata(file);
> +	struct video_device *vdev = video_devdata(file);
> +	int ret;
> +
> +	dev_dbg(dev->dev, "\n");
> +
> +	if (mutex_lock_interruptible(&dev->v4l2_open_release_mutex))
> +		return -ERESTARTSYS;
> +
> +	if (vdev->vfl_dir == VFL_DIR_RX) {
> +		if (test_bit(TX_V4L2_DEV_OPEN, &dev->flags)) {
> +			ret = -EBUSY;
> +			goto err_mutex_unlock;
> +		}
> +	} else {
> +		if (test_bit(RX_V4L2_DEV_OPEN, &dev->flags)) {
> +			ret = -EBUSY;
> +			goto err_mutex_unlock;
> +		}
> +	}
> +
> +	ret = v4l2_fh_open(file);
> +	if (ret)
> +		goto err_mutex_unlock;
> +
> +	dev->users++;
> +
> +	if (vdev->vfl_dir == VFL_DIR_RX)
> +		set_bit(RX_V4L2_DEV_OPEN, &dev->flags);
> +	else
> +		set_bit(TX_V4L2_DEV_OPEN, &dev->flags);
> +
> +	mutex_unlock(&dev->v4l2_open_release_mutex);
> +
> +	return 0;
> +err_mutex_unlock:
> +	mutex_unlock(&dev->v4l2_open_release_mutex);
> +	return ret;
> +}