Hi Antti,
Two comments, see below:
On 09/01/2015 11:59 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.
>
> Signed-off-by: Antti Palosaari <crope@xxxxxx>
> ---
> drivers/media/usb/hackrf/hackrf.c | 923 ++++++++++++++++++++++++++------------
> 1 file changed, 648 insertions(+), 275 deletions(-)
>
> diff --git a/drivers/media/usb/hackrf/hackrf.c b/drivers/media/usb/hackrf/hackrf.c
> -static unsigned int hackrf_convert_stream(struct hackrf_dev *dev,
> - void *dst, void *src, unsigned int src_len)
> +void hackrf_copy_stream(struct hackrf_dev *dev, void *dst,
Is there any reason 'static' was removed here? It's not used externally as
far as I can tell.
> + void *src, unsigned int src_len)
> {
> memcpy(dst, src, src_len);
>
<snip>
> +static int hackrf_s_modulator(struct file *file, void *fh,
> + const struct v4l2_modulator *a)
> +{
> + struct hackrf_dev *dev = video_drvdata(file);
> + int ret;
> +
> + dev_dbg(dev->dev, "index=%d\n", a->index);
> +
> + if (a->index == 0)
> + ret = 0;
> + else if (a->index == 1)
> + ret = 0;
> + else
> + ret = -EINVAL;
> +
> + return ret;
> +}
Why implement this at all? It's not doing anything. I'd just drop s_modulator
support.
If there is a reason why you do need it, then simplify it to:
return a->index > 1 ? -EINVAL : 0;
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
