Hi Vaibhav,
On Fri, Nov 21, 2008 at 8:52 PM, <hvaibhav@xxxxxx> wrote:
> From: Vaibhav Hiremath <hvaibhav@xxxxxx>
>
> Added new V4L2 slave driver for TVP514x.
>
> The Driver interface has been tested on OMAP3EVM board
> with TI daughter card (TVP5146). Soon the patch for Daughter card will
> be posted on community.
You may want to add some of the TVP5146 video decoder capabilities in
commit text. Useful for someone who just sees this chip for first
time.
>
> Signed-off-by: Brijesh Jadav <brijesh.j@xxxxxx>
> Hardik Shah <hardik.shah@xxxxxx>
> Manjunath Hadli <mrh@xxxxxx>
> R Sivaraj <sivaraj@xxxxxx>
> Vaibhav Hiremath <hvaibhav@xxxxxx>
> Karicheri Muralidharan <m-karicheri2@xxxxxx>
I suggested to change this in another email.
> +
> +#include <linux/i2c.h>
> +#include <linux/delay.h>
> +#include <linux/videodev2.h>
> +#include <media/v4l2-int-device.h>
Convention is is to put empty line between #include <linux/xxx.h> and
first #include <nonlinux/xxx.h> which is #include
<media/v4l2-int-device.h>
> +#include <media/tvp514x.h>
> +
> +
> +/* List of image formats supported by TVP5146/47 decoder
> + * Currently we are using 8 bit mode only, but can be
> + * extended to 10/20 bit mode.
> + */
> +static const struct v4l2_fmtdesc tvp514x_fmt_list[] = {
> + {
> + .index = 0,
> + .type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
> + .flags = 0,
> + .description = "8-bit UYVY 4:2:2 Format",
> + .pixelformat = V4L2_PIX_FMT_UYVY,
> + }
Good to add "," after the last element.
> +static struct tvp514x_std_info tvp514x_std_list[] = {
> + {
> + .width = NTSC_NUM_ACTIVE_PIXELS,
> + .height = NTSC_NUM_ACTIVE_LINES,
> + .video_std = VIDEO_STD_NTSC_MJ_BIT,
> + .standard = {
> + .index = 0,
> + .id = V4L2_STD_NTSC,
> + .name = "NTSC",
> + .frameperiod = {1001, 30000},
> + .framelines = 525}
"{" after 525 looks weird.
> + },
> + {
You can put "{" with "}" to save one line . Like this
"}, {"
You may want to make similar changes at other places in the patch.
> +static enum tvp514x_std tvp514x_get_current_std(struct tvp514x_decoder
> + *decoder)
> +{
> + u8 std, std_status;
> +
> + if (tvp514x_read_reg(decoder->client, REG_VIDEO_STD, &std))
> + return STD_INVALID;
> +
> + if ((std & VIDEO_STD_MASK) == VIDEO_STD_AUTO_SWITCH_BIT) {
> + /* use the standard status register */
> + if (tvp514x_read_reg(decoder->client, REG_VIDEO_STD_STATUS,
> + &std_status))
> + return STD_INVALID;
> + } else
> + std_status = std; /* use the standard register itself */
> +
> + switch (std_status & VIDEO_STD_MASK) {
> + case VIDEO_STD_NTSC_MJ_BIT:
> + return STD_NTSC_MJ;
> + break;
No need of " break" here.
> +
> + case VIDEO_STD_PAL_BDGHIN_BIT:
> + return STD_PAL_BDGHIN;
> + break;
Ditto.
> +
> + default:
> + return STD_INVALID;
> + break;
Tritto?
> + }
> +
> + return STD_INVALID;
> +}
> +
> +
> +static int
> +tvp514x_probe(struct i2c_client *client, const struct i2c_device_id *id)
> +{
Mark this as __init please.
--
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
