On Tue March 19 2013 00:17:32 Laurent Pinchart wrote:
> Hi Hans,
>
> Thanks for the patch.
>
> On Monday 18 March 2013 15:12:00 Hans Verkuil wrote:
> > From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> >
> > This ioctl is defined as IOW, so pass the argument as const.
> >
> > Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> > ---
>
> [snip]
>
> > diff --git a/drivers/media/radio/radio-keene.c
> > b/drivers/media/radio/radio-keene.c index 296941a..a598852 100644
> > --- a/drivers/media/radio/radio-keene.c
> > +++ b/drivers/media/radio/radio-keene.c
> > @@ -82,9 +82,12 @@ static inline struct keene_device *to_keene_dev(struct
> > v4l2_device *v4l2_dev) /* Set frequency (if non-0), PA, mute and turn
> > on/off the FM transmitter. */ static int keene_cmd_main(struct keene_device
> > *radio, unsigned freq, bool play) {
> > - unsigned short freq_send = freq ? (freq - 76 * 16000) / 800 : 0;
> > + unsigned short freq_send;
> > int ret;
> >
> > + if (freq)
> > + freq = clamp(freq, FREQ_MIN * FREQ_MUL, FREQ_MAX * FREQ_MUL);
> > + freq_send = freq ? (freq - 76 * 16000) / 800 : 0;
> > radio->buffer[0] = 0x00;
> > radio->buffer[1] = 0x50;
> > radio->buffer[2] = (freq_send >> 8) & 0xff;
> > @@ -215,15 +218,15 @@ static int vidioc_s_modulator(struct file *file, void
> > *priv, }
> >
> > static int vidioc_s_frequency(struct file *file, void *priv,
> > - struct v4l2_frequency *f)
> > + const struct v4l2_frequency *f)
> > {
> > struct keene_device *radio = video_drvdata(file);
> >
> > if (f->tuner != 0 || f->type != V4L2_TUNER_RADIO)
> > return -EINVAL;
> > - f->frequency = clamp(f->frequency,
> > - FREQ_MIN * FREQ_MUL, FREQ_MAX * FREQ_MUL);
> > - return keene_cmd_main(radio, f->frequency, true);
> > + /* Take care: keene_cmd_main handles a frequency of 0 as a
> > + * special case, so make sure we never give that from here. */
> > + return keene_cmd_main(radio, f->frequency ? f->frequency : 1, true);
>
> Can't you keep the clamp() here ? That looks easier.
Grrr. Sometimes you get so bogged down in details that you forget the big
picture.
I've updated my patch for radio-keene.c to this:
diff --git a/drivers/media/radio/radio-keene.c b/drivers/media/radio/radio-keene.c
index 296941a..4c9ae76 100644
--- a/drivers/media/radio/radio-keene.c
+++ b/drivers/media/radio/radio-keene.c
@@ -215,15 +215,15 @@ static int vidioc_s_modulator(struct file *file, void *priv,
}
static int vidioc_s_frequency(struct file *file, void *priv,
- struct v4l2_frequency *f)
+ const struct v4l2_frequency *f)
{
struct keene_device *radio = video_drvdata(file);
+ unsigned freq = f->frequency;
if (f->tuner != 0 || f->type != V4L2_TUNER_RADIO)
return -EINVAL;
- f->frequency = clamp(f->frequency,
- FREQ_MIN * FREQ_MUL, FREQ_MAX * FREQ_MUL);
- return keene_cmd_main(radio, f->frequency, true);
+ freq = clamp(freq, FREQ_MIN * FREQ_MUL, FREQ_MAX * FREQ_MUL);
+ return keene_cmd_main(radio, freq, true);
}
static int vidioc_g_frequency(struct file *file, void *priv,
Much cleaner. Thanks!
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
