2013-08-11 06:25:45

by Lad, Prabhakar

[permalink] [raw]
Subject: [PATCH v5] media: i2c: tvp7002: add OF support

From: "Lad, Prabhakar" <[email protected]>

add OF support for the tvp7002 driver.

Signed-off-by: Lad, Prabhakar <[email protected]>
---
This patch depends on https://patchwork.kernel.org/patch/2842680/

Changes for v5:
1: Fixed review comments pointed by Hans.

Changes for v4:
1: Improved descrition of end point properties.

Changes for v3:
1: Fixed review comments pointed by Sylwester.

.../devicetree/bindings/media/i2c/tvp7002.txt | 53 ++++++++++++++++
drivers/media/i2c/tvp7002.c | 67 ++++++++++++++++++--
2 files changed, 113 insertions(+), 7 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/i2c/tvp7002.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/tvp7002.txt b/Documentation/devicetree/bindings/media/i2c/tvp7002.txt
new file mode 100644
index 0000000..5f28b5d
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/tvp7002.txt
@@ -0,0 +1,53 @@
+* Texas Instruments TV7002 video decoder
+
+The TVP7002 device supports digitizing of video and graphics signal in RGB and
+YPbPr color space.
+
+Required Properties :
+- compatible : Must be "ti,tvp7002"
+
+Optional Properties:
+- hsync-active: HSYNC Polarity configuration for the bus. Default value when
+ this property is not specified is <0>.
+
+- vsync-active: VSYNC Polarity configuration for the bus. Default value when
+ this property is not specified is <0>.
+
+- pclk-sample: Clock polarity of the bus. Default value when this property is
+ not specified is <0>.
+
+- sync-on-green-active: Active state of Sync-on-green signal property of the
+ endpoint.
+ 0 = Normal Operation (Active Low, Default)
+ 1 = Inverted operation
+
+- field-even-active: Active-high Field ID output polarity control of the bus.
+ Under normal operation, the field ID output is set to logic 1 for an odd field
+ (field 1) and set to logic 0 for an even field (field 0).
+ 0 = Normal Operation (Active Low, Default)
+ 1 = FID output polarity inverted
+
+For further reading of port node refer Documentation/devicetree/bindings/media/
+video-interfaces.txt.
+
+Example:
+
+ i2c0@1c22000 {
+ ...
+ ...
+ tvp7002@5c {
+ compatible = "ti,tvp7002";
+ reg = <0x5c>;
+
+ port {
+ tvp7002_1: endpoint {
+ hsync-active = <1>;
+ vsync-active = <1>;
+ pclk-sample = <0>;
+ sync-on-green-active = <1>;
+ field-even-active = <0>;
+ };
+ };
+ };
+ ...
+ };
diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c
index f6b1f3f..24a08fa 100644
--- a/drivers/media/i2c/tvp7002.c
+++ b/drivers/media/i2c/tvp7002.c
@@ -35,6 +35,8 @@
#include <media/v4l2-device.h>
#include <media/v4l2-common.h>
#include <media/v4l2-ctrls.h>
+#include <media/v4l2-of.h>
+
#include "tvp7002_reg.h"

MODULE_DESCRIPTION("TI TVP7002 Video and Graphics Digitizer driver");
@@ -943,6 +945,48 @@ static const struct v4l2_subdev_ops tvp7002_ops = {
.pad = &tvp7002_pad_ops,
};

+static struct tvp7002_config *
+tvp7002_get_pdata(struct i2c_client *client)
+{
+ struct v4l2_of_endpoint bus_cfg;
+ struct tvp7002_config *pdata;
+ struct device_node *endpoint;
+ unsigned int flags;
+
+ if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node)
+ return client->dev.platform_data;
+
+ endpoint = v4l2_of_get_next_endpoint(client->dev.of_node, NULL);
+ if (!endpoint)
+ return NULL;
+
+ pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ goto done;
+
+ v4l2_of_parse_endpoint(endpoint, &bus_cfg);
+ flags = bus_cfg.bus.parallel.flags;
+
+ if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
+ pdata->hs_polarity = 1;
+
+ if (flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
+ pdata->vs_polarity = 1;
+
+ if (flags & V4L2_MBUS_PCLK_SAMPLE_RISING)
+ pdata->clk_polarity = 1;
+
+ if (flags & V4L2_MBUS_FIELD_EVEN_HIGH)
+ pdata->fid_polarity = 1;
+
+ if (flags & V4L2_MBUS_VIDEO_SOG_ACTIVE_HIGH)
+ pdata->sog_polarity = 1;
+
+done:
+ of_node_put(endpoint);
+ return pdata;
+}
+
/*
* tvp7002_probe - Probe a TVP7002 device
* @c: ptr to i2c_client struct
@@ -954,32 +998,32 @@ static const struct v4l2_subdev_ops tvp7002_ops = {
*/
static int tvp7002_probe(struct i2c_client *c, const struct i2c_device_id *id)
{
+ struct tvp7002_config *pdata = tvp7002_get_pdata(c);
struct v4l2_subdev *sd;
struct tvp7002 *device;
struct v4l2_dv_timings timings;
int polarity_a;
int polarity_b;
u8 revision;
-
int error;

+ if (pdata == NULL) {
+ dev_err(&c->dev, "No platform data\n");
+ return -EINVAL;
+ }
+
/* Check if the adapter supports the needed features */
if (!i2c_check_functionality(c->adapter,
I2C_FUNC_SMBUS_READ_BYTE | I2C_FUNC_SMBUS_WRITE_BYTE_DATA))
return -EIO;

- if (!c->dev.platform_data) {
- v4l_err(c, "No platform data!!\n");
- return -ENODEV;
- }
-
device = devm_kzalloc(&c->dev, sizeof(struct tvp7002), GFP_KERNEL);

if (!device)
return -ENOMEM;

sd = &device->sd;
- device->pdata = c->dev.platform_data;
+ device->pdata = pdata;
device->current_timings = tvp7002_timings;

/* Tell v4l2 the device is ready */
@@ -1084,9 +1128,18 @@ static const struct i2c_device_id tvp7002_id[] = {
};
MODULE_DEVICE_TABLE(i2c, tvp7002_id);

+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id tvp7002_of_match[] = {
+ { .compatible = "ti,tvp7002", },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, tvp7002_of_match);
+#endif
+
/* I2C driver data */
static struct i2c_driver tvp7002_driver = {
.driver = {
+ .of_match_table = of_match_ptr(tvp7002_of_match),
.owner = THIS_MODULE,
.name = TVP7002_MODULE_NAME,
},
--
1.7.9.5


2013-08-13 01:01:00

by Kumar Gala

[permalink] [raw]
Subject: Re: [PATCH v5] media: i2c: tvp7002: add OF support


On Aug 11, 2013, at 1:25 AM, Lad, Prabhakar wrote:

> From: "Lad, Prabhakar" <[email protected]>
>
> add OF support for the tvp7002 driver.
>
> Signed-off-by: Lad, Prabhakar <[email protected]>
> ---
> This patch depends on https://patchwork.kernel.org/patch/2842680/
>
> Changes for v5:
> 1: Fixed review comments pointed by Hans.
>
> Changes for v4:
> 1: Improved descrition of end point properties.
>
> Changes for v3:
> 1: Fixed review comments pointed by Sylwester.
>
> .../devicetree/bindings/media/i2c/tvp7002.txt | 53 ++++++++++++++++
> drivers/media/i2c/tvp7002.c | 67 ++++++++++++++++++--
> 2 files changed, 113 insertions(+), 7 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/tvp7002.txt
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/tvp7002.txt b/Documentation/devicetree/bindings/media/i2c/tvp7002.txt
> new file mode 100644
> index 0000000..5f28b5d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/tvp7002.txt
> @@ -0,0 +1,53 @@
> +* Texas Instruments TV7002 video decoder
> +
> +The TVP7002 device supports digitizing of video and graphics signal in RGB and
> +YPbPr color space.
> +
> +Required Properties :
> +- compatible : Must be "ti,tvp7002"
> +
> +Optional Properties:


> +- hsync-active: HSYNC Polarity configuration for the bus. Default value when
> + this property is not specified is <0>.
> +
> +- vsync-active: VSYNC Polarity configuration for the bus. Default value when
> + this property is not specified is <0>.
> +
> +- pclk-sample: Clock polarity of the bus. Default value when this property is
> + not specified is <0>.
> +
> +- sync-on-green-active: Active state of Sync-on-green signal property of the
> + endpoint.
> + 0 = Normal Operation (Active Low, Default)
> + 1 = Inverted operation

These seems better than what you have in video-interfaces.txt

> +
> +- field-even-active: Active-high Field ID output polarity control of the bus.
> + Under normal operation, the field ID output is set to logic 1 for an odd field
> + (field 1) and set to logic 0 for an even field (field 0).
> + 0 = Normal Operation (Active Low, Default)
> + 1 = FID output polarity inverted
> +

Why the duplication if this is covered in video-interfaces.txt?

> +For further reading of port node refer Documentation/devicetree/bindings/media/
> +video-interfaces.txt.
> +
> +Example:
> +
> + i2c0@1c22000 {
> + ...
> + ...
> + tvp7002@5c {
> + compatible = "ti,tvp7002";
> + reg = <0x5c>;
> +
> + port {
> + tvp7002_1: endpoint {
> + hsync-active = <1>;
> + vsync-active = <1>;
> + pclk-sample = <0>;
> + sync-on-green-active = <1>;
> + field-even-active = <0>;
> + };
> + };
> + };
> + ...
> + };
>

[ snip ]

- k

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

2013-08-13 02:46:54

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH v5] media: i2c: tvp7002: add OF support

On Tue, Aug 13, 2013 at 6:30 AM, Kumar Gala <[email protected]> wrote:
>
> On Aug 11, 2013, at 1:25 AM, Lad, Prabhakar wrote:
>
>> From: "Lad, Prabhakar" <[email protected]>
>>
>> add OF support for the tvp7002 driver.
>>
>> Signed-off-by: Lad, Prabhakar <[email protected]>
>> ---
>> This patch depends on https://patchwork.kernel.org/patch/2842680/
>>
>> Changes for v5:
>> 1: Fixed review comments pointed by Hans.
>>
>> Changes for v4:
>> 1: Improved descrition of end point properties.
>>
>> Changes for v3:
>> 1: Fixed review comments pointed by Sylwester.
>>
>> .../devicetree/bindings/media/i2c/tvp7002.txt | 53 ++++++++++++++++
>> drivers/media/i2c/tvp7002.c | 67 ++++++++++++++++++--
>> 2 files changed, 113 insertions(+), 7 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/media/i2c/tvp7002.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/tvp7002.txt b/Documentation/devicetree/bindings/media/i2c/tvp7002.txt
>> new file mode 100644
>> index 0000000..5f28b5d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/tvp7002.txt
>> @@ -0,0 +1,53 @@
>> +* Texas Instruments TV7002 video decoder
>> +
>> +The TVP7002 device supports digitizing of video and graphics signal in RGB and
>> +YPbPr color space.
>> +
>> +Required Properties :
>> +- compatible : Must be "ti,tvp7002"
>> +
>> +Optional Properties:
>
>
>> +- hsync-active: HSYNC Polarity configuration for the bus. Default value when
>> + this property is not specified is <0>.
>> +
>> +- vsync-active: VSYNC Polarity configuration for the bus. Default value when
>> + this property is not specified is <0>.
>> +
>> +- pclk-sample: Clock polarity of the bus. Default value when this property is
>> + not specified is <0>.
>> +
>> +- sync-on-green-active: Active state of Sync-on-green signal property of the
>> + endpoint.
>> + 0 = Normal Operation (Active Low, Default)
>> + 1 = Inverted operation
>
> These seems better than what you have in video-interfaces.txt
>
Well it sounds the same, I would keep it as is, let me know if you still
want me to change.

>> +
>> +- field-even-active: Active-high Field ID output polarity control of the bus.
>> + Under normal operation, the field ID output is set to logic 1 for an odd field
>> + (field 1) and set to logic 0 for an even field (field 0).
>> + 0 = Normal Operation (Active Low, Default)
>> + 1 = FID output polarity inverted
>> +
>
> Why the duplication if this is covered in video-interfaces.txt?
>
The explanation in video-interfaces.txt is more kind of generic and
the explanation
here is specific to this device.

Regards,
--Prabhakar Lad

2013-08-23 17:25:29

by Sylwester Nawrocki

[permalink] [raw]
Subject: Re: [PATCH v5] media: i2c: tvp7002: add OF support

On 08/13/2013 03:00 AM, Kumar Gala wrote:
> On Aug 11, 2013, at 1:25 AM, Lad, Prabhakar wrote:
>
>> From: "Lad, Prabhakar" <[email protected]>
>>
>> add OF support for the tvp7002 driver.
>>
>> Signed-off-by: Lad, Prabhakar <[email protected]>
>> ---
[...]
>> .../devicetree/bindings/media/i2c/tvp7002.txt | 53 ++++++++++++++++
>> drivers/media/i2c/tvp7002.c | 67 ++++++++++++++++++--
>> 2 files changed, 113 insertions(+), 7 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/media/i2c/tvp7002.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/tvp7002.txt b/Documentation/devicetree/bindings/media/i2c/tvp7002.txt
>> new file mode 100644
>> index 0000000..5f28b5d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/tvp7002.txt
>> @@ -0,0 +1,53 @@
>> +* Texas Instruments TV7002 video decoder
>> +
>> +The TVP7002 device supports digitizing of video and graphics signal in RGB and
>> +YPbPr color space.
>> +
>> +Required Properties :
>> +- compatible : Must be "ti,tvp7002"
>> +
>> +Optional Properties:
>
>
>> +- hsync-active: HSYNC Polarity configuration for the bus. Default value when
>> + this property is not specified is <0>.
>> +
>> +- vsync-active: VSYNC Polarity configuration for the bus. Default value when
>> + this property is not specified is <0>.
>> +
>> +- pclk-sample: Clock polarity of the bus. Default value when this property is
>> + not specified is <0>.
>> +
>> +- sync-on-green-active: Active state of Sync-on-green signal property of the
>> + endpoint.
>> + 0 = Normal Operation (Active Low, Default)
>> + 1 = Inverted operation
>
> These seems better than what you have in video-interfaces.txt

We probably should specify default values in in the common binding description.
Then duplication could be avoided. Not sure if it's not too late for this, all
drivers would need to have same default values.

What's normal and what's inverted depends on a particular device.

>> +- field-even-active: Active-high Field ID output polarity control of the bus.
>> + Under normal operation, the field ID output is set to logic 1 for an odd field
>> + (field 1) and set to logic 0 for an even field (field 0).
>> + 0 = Normal Operation (Active Low, Default)
>> + 1 = FID output polarity inverted
>> +
>
> Why the duplication if this is covered in video-interfaces.txt?

Yes, it would be better to avoid redefining these properties in each specific
device's binding. Presumably, for easier matching of DT properties with the
hardware's description, we could only say in device specific document which
value of a property corresponds to "normal" and which to "inverted" operation ?

>> +For further reading of port node refer Documentation/devicetree/bindings/media/
>> +video-interfaces.txt.

--
Sylwester Nawrocki
Samsung R&D Institute Poland

2013-08-25 15:33:13

by naim.dahnoun

[permalink] [raw]
Subject: Re: [PATCH v5] media: i2c: tvp7002: add OF support



Sent from my iPhone

On 23 Aug 2013, at 18:25, Sylwester Nawrocki <[email protected]> wrote:

> On 08/13/2013 03:00 AM, Kumar Gala wrote:
>> On Aug 11, 2013, at 1:25 AM, Lad, Prabhakar wrote:
>>
>>> From: "Lad, Prabhakar" <[email protected]>
>>>
>>> add OF support for the tvp7002 driver.
>>>
>>> Signed-off-by: Lad, Prabhakar <[email protected]>
>>> ---
> [...]
>>> .../devicetree/bindings/media/i2c/tvp7002.txt | 53 ++++++++++++++++
>>> drivers/media/i2c/tvp7002.c | 67 ++++++++++++++++++--
>>> 2 files changed, 113 insertions(+), 7 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/media/i2c/tvp7002.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/media/i2c/tvp7002.txt b/Documentation/devicetree/bindings/media/i2c/tvp7002.txt
>>> new file mode 100644
>>> index 0000000..5f28b5d
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/media/i2c/tvp7002.txt
>>> @@ -0,0 +1,53 @@
>>> +* Texas Instruments TV7002 video decoder
>>> +
>>> +The TVP7002 device supports digitizing of video and graphics signal in RGB and
>>> +YPbPr color space.
>>> +
>>> +Required Properties :
>>> +- compatible : Must be "ti,tvp7002"
>>> +
>>> +Optional Properties:
>>
>>
>>> +- hsync-active: HSYNC Polarity configuration for the bus. Default value when
>>> + this property is not specified is <0>.
>>> +
>>> +- vsync-active: VSYNC Polarity configuration for the bus. Default value when
>>> + this property is not specified is <0>.
>>> +
>>> +- pclk-sample: Clock polarity of the bus. Default value when this property is
>>> + not specified is <0>.
>>> +
>>> +- sync-on-green-active: Active state of Sync-on-green signal property of the
>>> + endpoint.
>>> + 0 = Normal Operation (Active Low, Default)
>>> + 1 = Inverted operation
>>
>> These seems better than what you have in video-interfaces.txt
>
> We probably should specify default values in in the common binding description.
> Then duplication could be avoided. Not sure if it's not too late for this, all
> drivers would need to have same default values.
>
> What's normal and what's inverted depends on a particular device.
>
>>> +- field-even-active: Active-high Field ID output polarity control of the bus.
>>> + Under normal operation, the field ID output is set to logic 1 for an odd field
>>> + (field 1) and set to logic 0 for an even field (field 0).
>>> + 0 = Normal Operation (Active Low, Default)
>>> + 1 = FID output polarity inverted
>>> +
>>
>> Why the duplication if this is covered in video-interfaces.txt?
>
> Yes, it would be better to avoid redefining these properties in each specific
> device's binding. Presumably, for easier matching of DT properties with the
> hardware's description, we could only say in device specific document which
> value of a property corresponds to "normal" and which to "inverted" operation ?
>
>>> +For further reading of port node refer Documentation/devicetree/bindings/media/
>>> +video-interfaces.txt.
>
> --
> Sylwester Nawrocki
> Samsung R&D Institute Poland
> _______________________________________________
> Davinci-linux-open-source mailing list
> [email protected]
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source