| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 6 Feb 2013 14:43:59 +0530
From: Prabhakar Lad <prabhakar.csengg@...il.com>
To: Hans Verkuil <hansverk@...co.com>
Cc: LMML <linux-media@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>,
Mauro Carvalho Chehab <mchehab@...hat.com>,
DLOS <davinci-linux-open-source@...ux.davincidsp.com>,
"Lad, Prabhakar" <prabhakar.lad@...com>,
Hans Verkuil <hans.verkuil@...co.com>,
Martin Bugge <marbugge@...co.com>
Subject: Re: [PATCH v2] media: ths7353: add support for ths7353 video amplifier
Hi Hans,
Thanks for the review.
On Wed, Feb 6, 2013 at 2:33 PM, Hans Verkuil <hansverk@...co.com> wrote:
> Hi Prabhakar,
>
> Here is my review. There are a few things that need to be fixed before I
> can Ack it.
>
OK
> On Tue 5 February 2013 20:20:21 Prabhakar Lad wrote:
>> From: Lad, Prabhakar <prabhakar.lad@...com>
>>
>> The patch adds support for THS7353 video amplifier.
>> The the THS7353 amplifier is very much similar to the
>> existing THS7303 video amplifier driver.
>> This patch appropriately makes changes to the existing
>> ths7303 driver and adds support for the THS7353.
>> This patch also adds V4L2_IDENT_THS7353 for the THS7353
>> chip and appropriate changes to Kconfig file for building.
>>
>> Signed-off-by: Lad, Prabhakar <prabhakar.lad@...com>
>> Signed-off-by: Hans Verkuil <hans.verkuil@...co.com>
>> Signed-off-by: Martin Bugge <marbugge@...co.com>
>> Cc: Chaithrika U S <chaithrika@...com>
>> ---
>> Changes for v2:
>> 1: Merged the driver in existing ths7303 driver.
>> 2: Merged the patch which adds the chip indent in same patch.
>>
>> drivers/media/i2c/Kconfig | 4 +-
>> drivers/media/i2c/ths7303.c | 334 +++++++++++++++++++++++++++++++++------
>> include/media/ths7303.h | 35 ++++
>> include/media/v4l2-chip-ident.h | 3 +
>> 4 files changed, 327 insertions(+), 49 deletions(-)
>> create mode 100644 include/media/ths7303.h
>>
>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>> index 1e4b2d0..2e02916 100644
>> --- a/drivers/media/i2c/Kconfig
>> +++ b/drivers/media/i2c/Kconfig
>> @@ -563,9 +563,9 @@ comment "Miscelaneous helper chips"
>>
>> config VIDEO_THS7303
>> tristate "THS7303 Video Amplifier"
>> - depends on I2C
>> + depends on VIDEO_V4L2 && I2C
>> help
>> - Support for TI THS7303 video amplifier
>> + Support for TI THS7303/53 video amplifier
>>
>> To compile this driver as a module, choose M here: the
>> module will be called ths7303.
>> diff --git a/drivers/media/i2c/ths7303.c b/drivers/media/i2c/ths7303.c
>> index e747524..e7f8c59 100644
>> --- a/drivers/media/i2c/ths7303.c
>> +++ b/drivers/media/i2c/ths7303.c
>> @@ -1,7 +1,15 @@
>> /*
>> - * ths7303- THS7303 Video Amplifier driver
>> + * ths7303/53- THS7303/53 Video Amplifier driver
>> *
>> * Copyright (C) 2009 Texas Instruments Incorporated - http://www.ti.com/
>> + * Copyright 2013 Cisco Systems, Inc. and/or its affiliates.
>> + *
>> + * Author: Chaithrika U S <chaithrika@...com>
>> + *
>> + * Contributors:
>> + * Lad, Prabhakar <prabhakar.lad@...com>
>> + * Hans Verkuil <hans.verkuil@...co.com>
>> + * Martin Bugge <marbugge@...co.com>
>> *
>> * This program is free software; you can redistribute it and/or
>> * modify it under the terms of the GNU General Public License as
>> @@ -13,25 +21,27 @@
>> * GNU General Public License for more details.
>> */
>>
>> -#include <linux/kernel.h>
>> -#include <linux/init.h>
>> -#include <linux/ctype.h>
>> -#include <linux/slab.h>
>> #include <linux/i2c.h>
>> -#include <linux/device.h>
>> -#include <linux/delay.h>
>> #include <linux/module.h>
>> -#include <linux/uaccess.h>
>> -#include <linux/videodev2.h>
>> +#include <linux/slab.h>
>>
>> -#include <media/v4l2-device.h>
>> -#include <media/v4l2-subdev.h>
>> +#include <media/ths7303.h>
>> #include <media/v4l2-chip-ident.h>
>> +#include <media/v4l2-device.h>
>>
>> #define THS7303_CHANNEL_1 1
>> #define THS7303_CHANNEL_2 2
>> #define THS7303_CHANNEL_3 3
>>
>> +struct ths7303_state {
>> + struct v4l2_subdev sd;
>> + struct ths7303_platform_data pdata;
>> + struct v4l2_dv_timings dv_timings;
>> + int std_id;
>> + int stream_on;
>> + int driver_data;
>> +};
>> +
>> enum ths7303_filter_mode {
>> THS7303_FILTER_MODE_480I_576I,
>> THS7303_FILTER_MODE_480P_576P,
>> @@ -48,64 +58,97 @@ static int debug;
>> module_param(debug, int, 0644);
>> MODULE_PARM_DESC(debug, "Debug level 0-1");
>>
>> +static inline struct ths7303_state *to_state(struct v4l2_subdev *sd)
>> +{
>> + return container_of(sd, struct ths7303_state, sd);
>> +}
>> +
>> +static int ths7303_read(struct v4l2_subdev *sd, u8 reg)
>> +{
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> +
>> + return i2c_smbus_read_byte_data(client, reg);
>> +}
>> +
>> +static int ths7303_write(struct v4l2_subdev *sd, u8 reg, u8 val)
>> +{
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> + int ret;
>> + int i;
>> +
>> + for (i = 0; i < 3; i++) {
>> + ret = i2c_smbus_write_byte_data(client, reg, val);
>> + if (ret == 0)
>> + return 0;
>> + }
>> + return ret;
>> +}
>> +
>> /* following function is used to set ths7303 */
>> int ths7303_setval(struct v4l2_subdev *sd, enum ths7303_filter_mode mode)
>> {
>> - u8 input_bias_chroma = 3;
>> - u8 input_bias_luma = 3;
>> - int disable = 0;
>> - int err = 0;
>> - u8 val = 0;
>> - u8 temp;
>> -
>> struct i2c_client *client = v4l2_get_subdevdata(sd);
>> + struct ths7303_state *state = to_state(sd);
>> + struct ths7303_platform_data *pdata = &state->pdata;
>> + u8 val, sel = 0;
>> + int err, disable = 0;
>>
>> if (!client)
>> return -EINVAL;
>>
>> + if (!state->stream_on) {
>> + ths7303_write(sd, THS7303_CHANNEL_1,
>> + (ths7303_read(sd, THS7303_CHANNEL_1) & 0xf8) |
>> + 0x00);
>> + ths7303_write(sd, THS7303_CHANNEL_2,
>> + (ths7303_read(sd, THS7303_CHANNEL_2) & 0xf8) |
>> + 0x00);
>> + ths7303_write(sd, THS7303_CHANNEL_3,
>> + (ths7303_read(sd, THS7303_CHANNEL_3) & 0xf8) |
>> + 0x00);
>> + return 0;
>> + }
>> +
>> switch (mode) {
>> case THS7303_FILTER_MODE_1080P:
>> - val = (3 << 6);
>> - val |= (3 << 3);
>> + sel = 0x3; /*1080p and SXGA/UXGA */
>> break;
>> case THS7303_FILTER_MODE_720P_1080I:
>> - val = (2 << 6);
>> - val |= (2 << 3);
>> + sel = 0x2; /*720p, 1080i and SVGA/XGA */
>> break;
>> case THS7303_FILTER_MODE_480P_576P:
>> - val = (1 << 6);
>> - val |= (1 << 3);
>> + sel = 0x1; /* EDTV 480p/576p and VGA */
>> break;
>> case THS7303_FILTER_MODE_480I_576I:
>> + sel = 0x0; /* SDTV, S-Video, 480i/576i */
>> break;
>> - case THS7303_FILTER_MODE_DISABLE:
>> - pr_info("mode disabled\n");
>> - /* disable all channels */
>> - disable = 1;
>> default:
>> /* disable all channels */
>> disable = 1;
>> }
>> - /* Setup channel 2 - Luma - Green */
>> - temp = val;
>> +
>> + val = (sel << 6) | (sel << 3);
>> if (!disable)
>> - val |= input_bias_luma;
>> - err = i2c_smbus_write_byte_data(client, THS7303_CHANNEL_2, val);
>> + val |= (pdata->ch_1 & 0x27);
>> + err = ths7303_write(sd, 0x01, val);
>> if (err)
>> goto out;
>>
>> - /* setup two chroma channels */
>> + val = (sel << 6) | (sel << 3);
>> if (!disable)
>> - temp |= input_bias_chroma;
>> -
>> - err = i2c_smbus_write_byte_data(client, THS7303_CHANNEL_1, temp);
>> + val |= (pdata->ch_2 & 0x27);
>> + err = ths7303_write(sd, 0x02, val);
>> if (err)
>> goto out;
>>
>> - err = i2c_smbus_write_byte_data(client, THS7303_CHANNEL_3, temp);
>> + val = (sel << 6) | (sel << 3);
>> + if (!disable)
>> + val |= (pdata->ch_3 & 0x27);
>> + err = ths7303_write(sd, 0x03, val);
>> if (err)
>> goto out;
>> - return err;
>> +
>> + return 0;
>> out:
>> pr_info("write byte data failed\n");
>> return err;
>> @@ -113,16 +156,47 @@ out:
>>
>> static int ths7303_s_std_output(struct v4l2_subdev *sd, v4l2_std_id norm)
>> {
>> - if (norm & (V4L2_STD_ALL & ~V4L2_STD_SECAM))
>> + struct ths7303_state *state = to_state(sd);
>> +
>> + if (norm & (V4L2_STD_ALL & ~V4L2_STD_SECAM)) {
>> + state->std_id = 1;
>> + return ths7303_setval(sd, THS7303_FILTER_MODE_480I_576I);
>> + }
>> +
>> + return ths7303_setval(sd, THS7303_FILTER_MODE_DISABLE);
>> +}
>> +
>> +static int ths7303_s_stream(struct v4l2_subdev *sd, int enable)
>> +{
>> + struct ths7303_state *state = to_state(sd);
>> + u32 height = state->dv_timings.bt.height;
>> + int interlaced = state->dv_timings.bt.interlaced;
>
> Instead of storing the whole dv_timings struct, just store the relevant
> fields.
>
Ok
>> + int res;
>> +
>> + state->stream_on = enable;
>> +
>> + if (state->std_id)
>> return ths7303_setval(sd, THS7303_FILTER_MODE_480I_576I);
>> +
>> + if (height == 1080 && !interlaced)
>
> You can't use the height as such. It is the pixelclock that determines which
> mode to use. What if I select a 1600x1200 format? That should still work!
> Cisco definitely needs this functionality.
>
> The code I mailed you earlier does this selection based on the pixelclock.
>
Ok I'll fix it.
>> + res = ths7303_setval(sd, THS7303_FILTER_MODE_1080P);
>> + else if ((height == 720 && !interlaced) ||
>> + (height == 1080 && interlaced))
>> + res = ths7303_setval(sd, THS7303_FILTER_MODE_720P_1080I);
>> + else if ((height == 480 || height == 576) && !interlaced)
>> + res = ths7303_setval(sd, THS7303_FILTER_MODE_480P_576P);
>> else
>> - return ths7303_setval(sd, THS7303_FILTER_MODE_DISABLE);
>> + /* disable all channels */
>> + res = ths7303_setval(sd, THS7303_FILTER_MODE_DISABLE);
>> +
>> + return res;
>> }
>>
>> /* for setting filter for HD output */
>> static int ths7303_s_dv_timings(struct v4l2_subdev *sd,
>> struct v4l2_dv_timings *dv_timings)
>> {
>> + struct ths7303_state *state = to_state(sd);
>> u32 height = dv_timings->bt.height;
>> int interlaced = dv_timings->bt.interlaced;
>> int res = 0;
>> @@ -138,6 +212,11 @@ static int ths7303_s_dv_timings(struct v4l2_subdev *sd,
>> /* disable all channels */
>> res = ths7303_setval(sd, THS7303_FILTER_MODE_DISABLE);
>>
>> + if (!res) {
>> + state->dv_timings = *dv_timings;
>> + state->std_id = 0;
>> + }
>> +
>> return res;
>> }
>>
>> @@ -145,17 +224,160 @@ static int ths7303_g_chip_ident(struct v4l2_subdev *sd,
>> struct v4l2_dbg_chip_ident *chip)
>> {
>> struct i2c_client *client = v4l2_get_subdevdata(sd);
>> + u32 indent = V4L2_IDENT_THS7303;
>> + struct ths7303_state *state = to_state(sd);
>>
>> - return v4l2_chip_ident_i2c_client(client, chip, V4L2_IDENT_THS7303, 0);
>> + if (state->driver_data)
>> + indent = V4L2_IDENT_THS7353;
>> +
>> + return v4l2_chip_ident_i2c_client(client, chip, indent, 0);
>> }
>>
>> static const struct v4l2_subdev_video_ops ths7303_video_ops = {
>> + .s_stream = ths7303_s_stream,
>> .s_std_output = ths7303_s_std_output,
>> - .s_dv_timings = ths7303_s_dv_timings,
>> + .s_dv_timings = ths7303_s_dv_timings,
>> +};
>> +
>> +#ifdef CONFIG_VIDEO_ADV_DEBUG
>> +
>> +static int ths7303_g_register(struct v4l2_subdev *sd,
>> + struct v4l2_dbg_register *reg)
>> +{
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> +
>> + if (!v4l2_chip_match_i2c_client(client, ®->match))
>> + return -EINVAL;
>> + if (!capable(CAP_SYS_ADMIN))
>> + return -EPERM;
>> +
>> + reg->size = 1;
>> + reg->val = ths7303_read(sd, reg->reg);
>> + return 0;
>> +}
>> +
>> +static int ths7303_s_register(struct v4l2_subdev *sd,
>> + struct v4l2_dbg_register *reg)
>> +{
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> +
>> + if (!v4l2_chip_match_i2c_client(client, ®->match))
>> + return -EINVAL;
>> + if (!capable(CAP_SYS_ADMIN))
>> + return -EPERM;
>> +
>> + ths7303_write(sd, reg->reg, reg->val);
>> + return 0;
>> +}
>> +#endif
>> +
>> +struct channel_register_bit_decoder {
>> + uint8_t stc_lpf_sel;
>> + uint8_t in_mux_sel;
>> + uint8_t lpf_freq_sel;
>> + uint8_t in_bias_sel_dis_cont;
>> +};
>> +
>> +static const char *stc_lpf_sel_txt[4] = {
>
> Use static const char * const (I think checkpatch complains about that
> these days). Also do this for the static const arrays below.
>
Ok I'll use the same, checkpatch dint catch it for me.
>> + "500-kHz Filter",
>> + "2.5-MHz Filter",
>> + "5-MHz Filter",
>> + "5-MHz Filter",
>> };
>>
>> +static const char *in_mux_sel_txt[2] = {
>> + "Input A Select",
>> + "Input B Select",
>> +};
>> +
>> +static const char *lpf_freq_sel_txt[4] = {
>> + "9-MHz LPF",
>> + "16-MHz LPF",
>> + "35-MHz LPF",
>> + "Bypass LPF",
>> +};
>> +
>> +static const char *in_bias_sel_dis_cont_txt[8] = {
>> + "Disable Channel",
>> + "Mute Function - No Output",
>> + "DC Bias Select",
>> + "DC Bias + 250 mV Offset Select",
>> + "AC Bias Select",
>> + "Sync Tip Clamp with low bias",
>> + "Sync Tip Clamp with mid bias",
>> + "Sync Tip Clamp with high bias",
>> +};
>> +
>> +static void
>> +parse_channel_register(struct channel_register_bit_decoder *ch_reg, u8 val)
>> +{
>> + ch_reg->stc_lpf_sel = (val >> 6) & 0x3;
>> + ch_reg->in_mux_sel = (val >> 5) & 0x1;
>> + ch_reg->lpf_freq_sel = (val >> 3) & 0x3;
>> + ch_reg->in_bias_sel_dis_cont = (val >> 0) & 0x7;
>> +}
>
> I'd merge this function into log_channel_status. That gets rid of the
> channel_register_bit_decoder struct as well.
>
Ok
>> +
>> +static void ths7303_log_channel_status(struct v4l2_subdev *sd, u8 reg)
>> +{
>> + struct channel_register_bit_decoder ch_reg;
>> + u8 val = ths7303_read(sd, reg);
>> +
>> + if ((val & 0x7) == 0) {
>> + v4l2_info(sd, "Channel %d Off\n", reg);
>> + return;
>> + }
>> +
>> + parse_channel_register(&ch_reg, val);
>> +
>> + v4l2_info(sd, "Channel %d On\n", reg);
>> + v4l2_info(sd, " value 0x%x\n", val);
>> + v4l2_info(sd, " %s\n", stc_lpf_sel_txt[ch_reg.stc_lpf_sel]);
>> + v4l2_info(sd, " %s\n", in_mux_sel_txt[ch_reg.in_mux_sel]);
>> + v4l2_info(sd, " %s\n", lpf_freq_sel_txt[ch_reg.lpf_freq_sel]);
>> + v4l2_info(sd, " %s\n",
>> + in_bias_sel_dis_cont_txt[ch_reg.in_bias_sel_dis_cont]);
>> +}
>> +
>> +static int ths7303_log_status(struct v4l2_subdev *sd)
>> +{
>> + struct ths7303_state *state = to_state(sd);
>> +
>> + v4l2_info(sd, "stream %s\n", state->stream_on ? "On" : "Off");
>> +
>> + if (state->dv_timings.type == V4L2_DV_BT_656_1120) {
>> + struct v4l2_bt_timings *bt = bt = &state->dv_timings.bt;
>> + u32 frame_width, frame_height;
>> +
>> + frame_width = bt->width + bt->hfrontporch +
>> + bt->hsync + bt->hbackporch;
>> + frame_height = bt->height + bt->vfrontporch +
>> + bt->vsync + bt->vbackporch;
>> + v4l2_info(sd,
>> + "timings: %dx%d%s%d (%dx%d). Pix freq. = %d Hz. Polarities = 0x%x\n",
>> + bt->width, bt->height, bt->interlaced ? "i" : "p",
>> + (frame_height * frame_width) > 0 ?
>> + (int)bt->pixelclock /
>> + (frame_height * frame_width) : 0,
>> + frame_width, frame_height,
>> + (int)bt->pixelclock, bt->polarities);
>> + } else {
>> + v4l2_info(sd, "no timings set\n");
>> + }
>> +
>> + ths7303_log_channel_status(sd, THS7303_CHANNEL_1);
>> + ths7303_log_channel_status(sd, THS7303_CHANNEL_2);
>> + ths7303_log_channel_status(sd, THS7303_CHANNEL_3);
>> +
>> + return 0;
>> +}
>> +
>> static const struct v4l2_subdev_core_ops ths7303_core_ops = {
>> .g_chip_ident = ths7303_g_chip_ident,
>> + .log_status = ths7303_log_status,
>> +#ifdef CONFIG_VIDEO_ADV_DEBUG
>> + .g_register = ths7303_g_register,
>> + .s_register = ths7303_s_register,
>> +#endif
>> };
>>
>> static const struct v4l2_subdev_ops ths7303_ops = {
>> @@ -166,8 +388,9 @@ static const struct v4l2_subdev_ops ths7303_ops = {
>> static int ths7303_probe(struct i2c_client *client,
>> const struct i2c_device_id *id)
>> {
>> + struct ths7303_platform_data *pdata = client->dev.platform_data;
>> + struct ths7303_state *state;
>> struct v4l2_subdev *sd;
>> - v4l2_std_id std_id = V4L2_STD_NTSC;
>>
>> if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
>> return -ENODEV;
>> @@ -175,13 +398,29 @@ static int ths7303_probe(struct i2c_client *client,
>> v4l_info(client, "chip found @ 0x%x (%s)\n",
>> client->addr << 1, client->adapter->name);
>>
>> - sd = devm_kzalloc(&client->dev, sizeof(struct v4l2_subdev), GFP_KERNEL);
>> - if (sd == NULL)
>> + state = devm_kzalloc(&client->dev, sizeof(struct ths7303_state),
>> + GFP_KERNEL);
>> + if (!state)
>> return -ENOMEM;
>>
>> + if (!pdata) {
>> + v4l_warn(client, "No platform data, using default data!\n");
>> + state->pdata.ch_1 = 0;
>> + state->pdata.ch_1 = 0;
>> + state->pdata.ch_1 = 0;
>
> Copy-and-paste error? I guess this should be ch_1, ch_2 and ch_3 :-)
> Also, since state is zeroed anyway, I'd just drop all assignments.
>
My bad :-) I'll remove it.
>> + state->pdata.init_enable = 0;
>> + } else {
>> + state->pdata = *pdata;
>> + }
>> +
>> + sd = &state->sd;
>> v4l2_i2c_subdev_init(sd, client, &ths7303_ops);
>> + sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>> +
>> + /* store the driver data to differntiate the chip */
>> + state->driver_data = (int)id->driver_data;
>>
>> - return ths7303_s_std_output(sd, std_id);
>> + return ths7303_setval(sd, THS7303_FILTER_MODE_480I_576I);
>> }
>>
>> static int ths7303_remove(struct i2c_client *client)
>> @@ -195,6 +434,7 @@ static int ths7303_remove(struct i2c_client *client)
>>
>> static const struct i2c_device_id ths7303_id[] = {
>> {"ths7303", 0},
>> + {"ths7353", 1},
>> {},
>> };
>>
>> @@ -203,7 +443,7 @@ MODULE_DEVICE_TABLE(i2c, ths7303_id);
>> static struct i2c_driver ths7303_driver = {
>> .driver = {
>> .owner = THIS_MODULE,
>> - .name = "ths7303",
>> + .name = "ths73x3",
I missed here I'll keep it as is and not change it.
>> },
>> .probe = ths7303_probe,
>> .remove = ths7303_remove,
>> diff --git a/include/media/ths7303.h b/include/media/ths7303.h
>> new file mode 100644
>> index 0000000..818cfc3
>> --- /dev/null
>> +++ b/include/media/ths7303.h
>> @@ -0,0 +1,35 @@
>> +/*
>> + * Copyright (C) 2013 Texas Instruments Inc
>> + *
>> + * Copyright 2013 Cisco Systems, Inc. and/or its affiliates.
>> + *
>> + * Contributors:
>> + * Lad, Prabhakar <prabhakar.lad@...com>
>> + * Hans Verkuil <hans.verkuil@...co.com>
>> + * Martin Bugge <marbugge@...co.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation version 2.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>> + */
>> +
>> +#ifndef THS7353_H
>> +#define THS7353_H
>> +
>> +struct ths7303_platform_data {
>> + u8 ch_1;
>> + u8 ch_2;
>> + u8 ch_3;
>> + u8 init_enable;
>> +};
>
> This struct needs to be documented.
>
OK.
>> +
>> +#endif
>> diff --git a/include/media/v4l2-chip-ident.h b/include/media/v4l2-chip-ident.h
>> index 4ee125b..84d1159 100644
>> --- a/include/media/v4l2-chip-ident.h
>> +++ b/include/media/v4l2-chip-ident.h
>> @@ -177,6 +177,9 @@ enum {
>> /* module ths7303: just ident 7303 */
>> V4L2_IDENT_THS7303 = 7303,
>>
>> + /* module ths7353: just ident 7353 */
>> + V4L2_IDENT_THS7353 = 7353,
>> +
>
> This should be moved after ADV7343: this list needs to be sorted numerically.
>
Ok.
Regards,
--Prabhakar
>> /* module adv7343: just ident 7343 */
>> V4L2_IDENT_ADV7343 = 7343,
>>
>>
>
> Regards,
>
> Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists