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.verk...@cisco.com>
> > 
> > This ioctl is defined as IOW, so pass the argument as const.
> > 
> > Signed-off-by: Hans Verkuil <hans.verk...@cisco.com>
> > ---
> 
> [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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to