Re: [PATCH V5 2/5] New control class and features for FM RX

From: Hans Verkuil
Date: Mon May 14 2012 - 15:44:38 EST


Hi Manjunatha,

I wish I could ack this series, but there is one thing that really needs
to change:

On Mon May 14 2012 21:24:26 manjunatha_halli@xxxxxx wrote:
> @@ -1843,13 +1857,22 @@ struct v4l2_frequency {
> __u32 reserved[8];
> };
>
> +
> +#define FM_BAND_TYPE_DEFAULT 0 /* All Bands 65.8 MHz till 108 Mhz
> + or 162.55 MHz if weather band */
> +#define FM_BAND_TYPE_EUROPE_US 1 /* 87.5 Mhz - 108 MHz*/
> +#define FM_BAND_TYPE_JAPAN 2 /* 76 MHz - 90 MHz*/
> +#define FM_BAND_TYPE_RUSSIAN 3 /* 65.8 MHz - 74 MHz*/
> +#define FM_BAND_TYPE_WEATHER 4 /* 162.4 MHz - 162.55 MHz*/

This needs a V4L2_ prefix. And in my opinion the _TYPE part can be dropped.

So it becomes:

#define V4L2_FM_BAND_DEFAULT 0
#define V4L2_FM_BAND_EUROPE_US 1 /* 87.5 Mhz - 108 MHz */
#define V4L2_FM_BAND_JAPAN 2 /* 76 MHz - 90 MHz */
#define V4L2_FM_BAND_RUSSIAN 3 /* 65.8 MHz - 74 MHz */
#define V4L2_FM_BAND_WEATHER 4 /* 162.4 MHz - 162.55 MHz */

BTW, also put a space before '*/'.

If you can make this change quickly (i.e. today) and post the fixed version
(don't forget to update the documentation as well!) then I'll ack it and
there is a change you can get it merged for 3.5.

Regards,

Hans

> +
> struct v4l2_hw_freq_seek {
> __u32 tuner;
> enum v4l2_tuner_type type;
> __u32 seek_upward;
> __u32 wrap_around;
> __u32 spacing;
> - __u32 reserved[7];
> + __u32 band;
> + __u32 reserved[6];
> };
>
> /*
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/