Hiremath, Vaibhav wrote:
[Hiremath, Vaibhav] I am really sorry; actually I was really busy with our
internal release process and still going.
Definitely I will try to find some time tomorrow to test this patch.
No problem.
v2: Make the content of the registers (brightness, \ldots) per
decoder
and not global.
v1: Initial version
[Hiremath, Vaibhav] Just for knowledge, how are you validating these changes?
On which board are you testing/validating?
This is custom design. My HW vendor was using advXXXX chips in the past
but was not very happy with them and switch over to this chip.
drivers/media/video/tvp514x.c | 166 ++++++++++++++++++++++--------
-----------
1 files changed, 90 insertions(+), 76 deletions(-)
diff --git a/drivers/media/video/tvp514x.c
b/drivers/media/video/tvp514x.c
index 8e23aa5..99cfc40 100644
--- a/drivers/media/video/tvp514x.c
+++ b/drivers/media/video/tvp514x.c
@@ -86,45 +86,8 @@ struct tvp514x_std_info {
struct v4l2_standard standard;
};
-/**
- * struct tvp514x_decoded - TVP5146/47 decoder object
- * @v4l2_int_device: Slave handle
- * @pdata: Board specific
- * @client: I2C client data
- * @id: Entry from I2C table
- * @ver: Chip version
- * @state: TVP5146/47 decoder state - detected or not-detected
- * @pix: Current pixel format
- * @num_fmts: Number of formats
- * @fmt_list: Format list
- * @current_std: Current standard
- * @num_stds: Number of standards
- * @std_list: Standards list
- * @route: input and output routing at chip level
- */
-struct tvp514x_decoder {
- struct v4l2_int_device *v4l2_int_device;
- const struct tvp514x_platform_data *pdata;
- struct i2c_client *client;
-
- struct i2c_device_id *id;
-
- int ver;
- enum tvp514x_state state;
-
- struct v4l2_pix_format pix;
- int num_fmts;
- const struct v4l2_fmtdesc *fmt_list;
-
- enum tvp514x_std current_std;
- int num_stds;
- struct tvp514x_std_info *std_list;
-
- struct v4l2_routing route;
-};
-
[Hiremath, Vaibhav] You may want to revisit this change, since there is only one line
addition to the struct "struct tvp514x_reg". Patch will look clean if it only
indicates the changes.
I'm adding also
| + struct tvp514x_reg
| tvp514x_regs[ARRAY_SIZE(tvp514x_reg_list_default)];
which contains vp514x_reg_list_default which is defined below. I prefered
not to do a forward declration of tvp514x_reg_list_default and therefore I
moved tvp514x_decoder just after it.
Would you prefer it with a forward declarion?
/* TVP514x default register values */
-static struct tvp514x_reg tvp514x_reg_list[] = {
+static struct tvp514x_reg tvp514x_reg_list_default[] = {
{TOK_WRITE, REG_INPUT_SEL, 0x05}, /* Composite selected */
{TOK_WRITE, REG_AFE_GAIN_CTRL, 0x0F},
{TOK_WRITE, REG_VIDEO_STD, 0x00}, /* Auto mode */
@@ -186,6 +149,44 @@ static struct tvp514x_reg tvp514x_reg_list[] =
{
{TOK_TERM, 0, 0},
};
+/**
+ * struct tvp514x_decoded - TVP5146/47 decoder object
[Hiremath, Vaibhav] Please correct the spelling mistake
"tvp514x_decoder", I had missed this in my original patch.
okay.
+ * @v4l2_int_device: Slave handle
+ * @pdata: Board specific
+ * @client: I2C client data
+ * @id: Entry from I2C table
+ * @ver: Chip version
+ * @state: TVP5146/47 decoder state - detected or not-detected
+ * @pix: Current pixel format
+ * @num_fmts: Number of formats
+ * @fmt_list: Format list
+ * @current_std: Current standard
+ * @num_stds: Number of standards
+ * @std_list: Standards list
+ * @route: input and output routing at chip level
[Hiremath, Vaibhav] You may want to add the new parameter added to this struct.
good point.
+ */
+struct tvp514x_decoder {
+ struct v4l2_int_device v4l2_int_device;
+ const struct tvp514x_platform_data *pdata;
+ struct i2c_client *client;
+
+ struct i2c_device_id *id;
+
+ int ver;
+ enum tvp514x_state state;
+
+ struct v4l2_pix_format pix;
+ int num_fmts;
+ const struct v4l2_fmtdesc *fmt_list;
+
+ enum tvp514x_std current_std;
+ int num_stds;
+ struct tvp514x_std_info *std_list;
+
+ struct v4l2_routing route;
+ struct tvp514x_reg
tvp514x_regs[ARRAY_SIZE(tvp514x_reg_list_default)];
+};
+
/* 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.
@@ -1392,26 +1390,39 @@ static struct v4l2_int_device
tvp514x_int_device = {
static int
tvp514x_probe(struct i2c_client *client, const struct i2c_device_id
*id)
{
- struct tvp514x_decoder *decoder = &tvp514x_dev;
+ struct tvp514x_decoder *decoder;
int err;
/* Check if the adapter supports the needed features */
if (!i2c_check_functionality(client->adapter,
I2C_FUNC_SMBUS_BYTE_DATA))
return -EIO;
- decoder->pdata = client->dev.platform_data;
- if (!decoder->pdata) {
+ decoder = kzalloc(sizeof(*decoder), GFP_KERNEL);
+ if (!decoder)
+ return -ENOMEM;
+
+ if (!client->dev.platform_data) {
v4l_err(client, "No platform data!!\n");
- return -ENODEV;
+ err = -ENODEV;
+ goto out_free;
}
+
+ *decoder = tvp514x_dev;
+ decoder->v4l2_int_device.priv = decoder;
+ decoder->pdata = client->dev.platform_data;
+
+ BUILD_BUG_ON(sizeof(decoder->tvp514x_regs) !=
+ sizeof(tvp514x_reg_list_default));
[Hiremath, Vaibhav] Do we really need to check this? I think you are defining the
decoder->tvp514x_regs[sizeof(tvp514x_reg_list_default)].
I can get rid of it if you want. It is just a sanity check.
+ memcpy(decoder->tvp514x_regs, tvp514x_reg_list_default,
+ sizeof(tvp514x_reg_list_default));
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
