[PATCH 0/5] media: i2c: tvp7002 feature enhancement and cleanup

[PATCH 0/5] media: i2c: tvp7002 feature enhancement and cleanup

[Lad Prabhakar]

From: Lad, Prabhakar <prabhakar.csengg@gmail.com>

This patch series enables tvp7002 decoder driver for asynchronous probing
and adds OF support, with few cleanup patches.

Lad, Prabhakar (5):
  media: i2c: tvp7002: remove duplicate define
  media: i2c: tvp7002: rearrange description of structure members
  media: i2c: tvp7002: rearrange header inclusion alphabetically
  media: i2c: tvp7002: add support for asynchronous probing
  media: i2c: tvp7002: add OF support

 .../devicetree/bindings/media/i2c/tvp7002.txt      |   42 ++++++++++
 drivers/media/i2c/tvp7002.c                        |   82 ++++++++++++++++----
 include/media/tvp7002.h                            |   44 +++++------
 3 files changed, 130 insertions(+), 38 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/tvp7002.txt

-- 
1.7.4.1

[PATCH 1/5] media: i2c: tvp7002: remove duplicate define

[Lad Prabhakar]

From: Lad, Prabhakar <prabhakar.csengg@gmail.com>

this patch removes duplicate #define TVP7002_MODULE_NAME
form the driver file, which was also defined in media/tvp7002.h

Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc: Sakari Ailus <sakari.ailus@iki.fi>
Cc: linux-kernel@vger.kernel.org
Cc: davinci-linux-open-source@linux.davincidsp.com
---
 drivers/media/i2c/tvp7002.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c
index f2d908f..f339e6f 100644
--- a/drivers/media/i2c/tvp7002.c
+++ b/drivers/media/i2c/tvp7002.c
@@ -41,9 +41,6 @@ MODULE_DESCRIPTION("TI TVP7002 Video and Graphics Digitizer driver");
 MODULE_AUTHOR("Santiago Nunez-Corrales <santiago.nunez@ridgerun.com>");
 MODULE_LICENSE("GPL");
 
-/* Module Name */
-#define TVP7002_MODULE_NAME	"tvp7002"
-
 /* I2C retry attempts */
 #define I2C_RETRY_COUNT		(5)
 
-- 
1.7.4.1

[PATCH 2/5] media: i2c: tvp7002: rearrange description of structure members

[Lad Prabhakar]

From: Lad, Prabhakar <prabhakar.csengg@gmail.com>

This patch rearranges the description of field members of
struct tvp7002_config. Also as the all the fields where accepting
a value either 0/1, made the members as bool.

Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc: Sakari Ailus <sakari.ailus@iki.fi>
Cc: linux-kernel@vger.kernel.org
Cc: davinci-linux-open-source@linux.davincidsp.com
---
 include/media/tvp7002.h |   44 ++++++++++++++++++++------------------------
 1 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/include/media/tvp7002.h b/include/media/tvp7002.h
index 7123048..fadb6af 100644
--- a/include/media/tvp7002.h
+++ b/include/media/tvp7002.h
@@ -28,31 +28,27 @@
 
 #define TVP7002_MODULE_NAME "tvp7002"
 
-/* Platform-dependent data
- *
- * clk_polarity:
- * 			0 -> data clocked out on rising edge of DATACLK signal
- * 			1 -> data clocked out on falling edge of DATACLK signal
- * hs_polarity:
- * 			0 -> active low HSYNC output
- * 			1 -> active high HSYNC output
- * sog_polarity:
- * 			0 -> normal operation
- * 			1 -> operation with polarity inverted
- * vs_polarity:
- * 			0 -> active low VSYNC output
- * 			1 -> active high VSYNC output
- * fid_polarity:
- *			0 -> 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).
- *			1 -> operation with polarity inverted.
+/**
+ * struct tvp7002_config - Platform dependent data
+ *@clk_polarity: Clock polarity
+ *		0 - Data clocked out on rising edge of DATACLK signal
+ *		1 - Data clocked out on falling edge of DATACLK signal
+ *@hs_polarity:  HSYNC polarity
+ *		0 - Active low HSYNC output, 1 - Active high HSYNC output
+ *@vs_polarity: VSYNC Polarity
+ *		0 - Active low VSYNC output, 1 - Active high VSYNC output
+ *@fid_polarity: Active-high Field ID polarity.
+ *		0 - 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).
+ *		1 - Operation with polarity inverted.
+ *@sog_polarity: Active high Sync on Green output polarity.
+ *		0 - Normal operation, 1 - Operation with polarity inverted
  */
 struct tvp7002_config {
-	u8 clk_polarity;
-	u8 hs_polarity;
-	u8 vs_polarity;
-	u8 fid_polarity;
-	u8 sog_polarity;
+	bool clk_polarity;
+	bool hs_polarity;
+	bool vs_polarity;
+	bool fid_polarity;
+	bool sog_polarity;
 };
 #endif
-- 
1.7.4.1

[PATCH 3/5] media: i2c: tvp7002: rearrange header inclusion alphabetically

[Lad Prabhakar]

From: Lad, Prabhakar <prabhakar.csengg@gmail.com>

This patch rearranges the header inclusion alphabetically
and also removes unnecessary includes.

Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc: Sakari Ailus <sakari.ailus@iki.fi>
Cc: linux-kernel@vger.kernel.org
Cc: davinci-linux-open-source@linux.davincidsp.com
---
 drivers/media/i2c/tvp7002.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c
index f339e6f..f4114bf 100644
--- a/drivers/media/i2c/tvp7002.c
+++ b/drivers/media/i2c/tvp7002.c
@@ -24,17 +24,17 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
+
 #include <linux/delay.h>
 #include <linux/i2c.h>
-#include <linux/slab.h>
-#include <linux/videodev2.h>
 #include <linux/module.h>
 #include <linux/v4l2-dv-timings.h>
+
 #include <media/tvp7002.h>
-#include <media/v4l2-device.h>
 #include <media/v4l2-chip-ident.h>
-#include <media/v4l2-common.h>
 #include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+
 #include "tvp7002_reg.h"
 
 MODULE_DESCRIPTION("TI TVP7002 Video and Graphics Digitizer driver");
-- 
1.7.4.1

Re: [PATCH 3/5] media: i2c: tvp7002: rearrange header inclusion alphabetically

[Hans Verkuil]

On Tue 14 May 2013 12:45:32 Lad Prabhakar wrote:
> From: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> 
> This patch rearranges the header inclusion alphabetically
> and also removes unnecessary includes.

As Laurent mentioned in a review for another patch (vpif) you probably
shouldn't remove these headers. videodev2.h is certainly used, as is slab.h
and v4l2-common.h. In the past removing slab.h causes problems on other
architectures where that header isn't automatically included by other
headers.

I would just drop this patch. I've merged the first two patches of
this patch series, the last two I can't merge as long as the async
code isn't in yet.

Regards,

	Hans

> 
> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> Cc: Hans Verkuil <hans.verkuil@cisco.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Cc: Sakari Ailus <sakari.ailus@iki.fi>
> Cc: linux-kernel@vger.kernel.org
> Cc: davinci-linux-open-source@linux.davincidsp.com
> ---
>  drivers/media/i2c/tvp7002.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c
> index f339e6f..f4114bf 100644
> --- a/drivers/media/i2c/tvp7002.c
> +++ b/drivers/media/i2c/tvp7002.c
> @@ -24,17 +24,17 @@
>   * along with this program; if not, write to the Free Software
>   * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
>   */
> +
>  #include <linux/delay.h>
>  #include <linux/i2c.h>
> -#include <linux/slab.h>
> -#include <linux/videodev2.h>
>  #include <linux/module.h>
>  #include <linux/v4l2-dv-timings.h>
> +
>  #include <media/tvp7002.h>
> -#include <media/v4l2-device.h>
>  #include <media/v4l2-chip-ident.h>
> -#include <media/v4l2-common.h>
>  #include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +
>  #include "tvp7002_reg.h"
>  
>  MODULE_DESCRIPTION("TI TVP7002 Video and Graphics Digitizer driver");
> 

Re: [PATCH 3/5] media: i2c: tvp7002: rearrange header inclusion alphabetically

[Prabhakar Lad]

Hi Hans,

On Thu, May 23, 2013 at 3:05 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On Tue 14 May 2013 12:45:32 Lad Prabhakar wrote:
>> From: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>>
>> This patch rearranges the header inclusion alphabetically
>> and also removes unnecessary includes.
>
> As Laurent mentioned in a review for another patch (vpif) you probably
> shouldn't remove these headers. videodev2.h is certainly used, as is slab.h
> and v4l2-common.h. In the past removing slab.h causes problems on other
> architectures where that header isn't automatically included by other
> headers.
>
OK

> I would just drop this patch. I've merged the first two patches of
> this patch series, the last two I can't merge as long as the async
> code isn't in yet.
>
Thanks. yes the last two depend on v4l-async patches.

Regards,
--Prabhakar Lad

[PATCH 4/5] media: i2c: tvp7002: add support for asynchronous probing

[Lad Prabhakar]

From: Lad, Prabhakar <prabhakar.csengg@gmail.com>

Both synchronous and asynchronous tvp7002 subdevice probing is supported by
this patch.

Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc: Sakari Ailus <sakari.ailus@iki.fi>
Cc: linux-kernel@vger.kernel.org
Cc: davinci-linux-open-source@linux.davincidsp.com
---
 drivers/media/i2c/tvp7002.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c
index f4114bf..d5113d1 100644
--- a/drivers/media/i2c/tvp7002.c
+++ b/drivers/media/i2c/tvp7002.c
@@ -31,6 +31,7 @@
 #include <linux/v4l2-dv-timings.h>
 
 #include <media/tvp7002.h>
+#include <media/v4l2-async.h>
 #include <media/v4l2-chip-ident.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
@@ -1073,6 +1074,11 @@ static int tvp7002_probe(struct i2c_client *c, const struct i2c_device_id *id)
 	}
 	v4l2_ctrl_handler_setup(&device->hdl);
 
+	device->sd.dev = &c->dev;
+	error = v4l2_async_register_subdev(&device->sd);
+	if (error)
+		goto error;
+
 	return 0;
 
 error:
@@ -1097,6 +1103,7 @@ static int tvp7002_remove(struct i2c_client *c)
 
 	v4l2_dbg(1, debug, sd, "Removing tvp7002 adapter"
 				"on address 0x%x\n", c->addr);
+	v4l2_async_unregister_subdev(&device->sd);
 #if defined(CONFIG_MEDIA_CONTROLLER)
 	media_entity_cleanup(&device->sd.entity);
 #endif
-- 
1.7.4.1

[PATCH 5/5] media: i2c: tvp7002: add OF support

[Lad Prabhakar]

From: Lad, Prabhakar <prabhakar.csengg@gmail.com>

add OF support for the tvp7002 driver.

Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc: Sakari Ailus <sakari.ailus@iki.fi>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Rob Landley <rob@landley.net>
Cc: devicetree-discuss@lists.ozlabs.org
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: davinci-linux-open-source@linux.davincidsp.com
---
 .../devicetree/bindings/media/i2c/tvp7002.txt      |   42 +++++++++++++
 drivers/media/i2c/tvp7002.c                        |   64 +++++++++++++++++--
 2 files changed, 99 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..1ebd8b1
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/tvp7002.txt
@@ -0,0 +1,42 @@
+* 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"
+
+- hsync-active: HSYNC Polarity configuration for endpoint.
+
+- vsync-active: VSYNC Polarity configuration for endpoint.
+
+- pclk-sample: Clock polarity of the endpoint.
+
+- ti,tvp7002-fid-polarity: Active-high Field ID polarity of the endpoint.
+
+- ti,tvp7002-sog-polarity: Sync on Green output polarity of the endpoint.
+
+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>;
+					ti,tvp7002-fid-polarity;
+					ti,tvp7002-sog-polarity;
+				};
+			};
+		};
+		...
+	};
diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c
index d5113d1..942e0d8 100644
--- a/drivers/media/i2c/tvp7002.c
+++ b/drivers/media/i2c/tvp7002.c
@@ -35,6 +35,7 @@
 #include <media/v4l2-chip-ident.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
+#include <media/v4l2-of.h>
 
 #include "tvp7002_reg.h"
 
@@ -976,6 +977,46 @@ 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)
+		return NULL;
+
+	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;
+
+	pdata->fid_polarity = of_property_read_bool(endpoint,
+						    "ti,tvp7002-fid-polarity");
+
+	pdata->sog_polarity = of_property_read_bool(endpoint,
+						    "ti,tvp7002-sog-polarity");
+
+	return pdata;
+}
+
 /*
  * tvp7002_probe - Probe a TVP7002 device
  * @c: ptr to i2c_client struct
@@ -987,32 +1028,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 */
@@ -1119,9 +1160,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.4.1

Re: [PATCH 5/5] media: i2c: tvp7002: add OF support

[Laurent Pinchart]

Hi Prabhakar,

Thank you for the patch.

On Tuesday 14 May 2013 16:15:34 Lad Prabhakar wrote:
> From: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> 
> add OF support for the tvp7002 driver.
> 
> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> Cc: Hans Verkuil <hans.verkuil@cisco.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Cc: Sakari Ailus <sakari.ailus@iki.fi>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Rob Landley <rob@landley.net>
> Cc: devicetree-discuss@lists.ozlabs.org
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: davinci-linux-open-source@linux.davincidsp.com
> ---
>  .../devicetree/bindings/media/i2c/tvp7002.txt      |   42 +++++++++++++
>  drivers/media/i2c/tvp7002.c                        |   64 +++++++++++++++--
>  2 files changed, 99 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..1ebd8b1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/tvp7002.txt
> @@ -0,0 +1,42 @@
> +* 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"
> +
> +- hsync-active: HSYNC Polarity configuration for endpoint.
> +
> +- vsync-active: VSYNC Polarity configuration for endpoint.
> +
> +- pclk-sample: Clock polarity of the endpoint.
> +
> +- ti,tvp7002-fid-polarity: Active-high Field ID polarity of the endpoint.
> +
> +- ti,tvp7002-sog-polarity: Sync on Green output polarity of the endpoint.

Would it make sense to define field-active and sog-active properties in the 
V4L2 bindings instead of having per-chip properties ?

> +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>;
> +					ti,tvp7002-fid-polarity;
> +					ti,tvp7002-sog-polarity;
> +				};
> +			};
> +		};
> +		...
> +	};
> diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c
> index d5113d1..942e0d8 100644
> --- a/drivers/media/i2c/tvp7002.c
> +++ b/drivers/media/i2c/tvp7002.c
> @@ -35,6 +35,7 @@
>  #include <media/v4l2-chip-ident.h>
>  #include <media/v4l2-ctrls.h>
>  #include <media/v4l2-device.h>
> +#include <media/v4l2-of.h>
> 
>  #include "tvp7002_reg.h"
> 
> @@ -976,6 +977,46 @@ 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)
> +		return NULL;
> +
> +	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;
> +
> +	pdata->fid_polarity = of_property_read_bool(endpoint,
> +						    "ti,tvp7002-fid-polarity");
> +
> +	pdata->sog_polarity = of_property_read_bool(endpoint,
> +						    "ti,tvp7002-sog-polarity");
> +
> +	return pdata;
> +}
> +
>  /*
>   * tvp7002_probe - Probe a TVP7002 device
>   * @c: ptr to i2c_client struct
> @@ -987,32 +1028,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 */
> @@ -1119,9 +1160,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,
>  	},
-- 
Regards,

Laurent Pinchart

Re: [PATCH 5/5] media: i2c: tvp7002: add OF support

[Prabhakar Lad]

Hi Laurent,

Thanks for the review.

On Tue, May 14, 2013 at 9:04 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Prabhakar,
>
> Thank you for the patch.
>
> On Tuesday 14 May 2013 16:15:34 Lad Prabhakar wrote:
>> From: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>>
>> add OF support for the tvp7002 driver.
>>
>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>> Cc: Hans Verkuil <hans.verkuil@cisco.com>
>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
>> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
>> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> Cc: Sakari Ailus <sakari.ailus@iki.fi>
>> Cc: Grant Likely <grant.likely@secretlab.ca>
>> Cc: Rob Herring <rob.herring@calxeda.com>
>> Cc: Rob Landley <rob@landley.net>
>> Cc: devicetree-discuss@lists.ozlabs.org
>> Cc: linux-doc@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: davinci-linux-open-source@linux.davincidsp.com
>> ---
>>  .../devicetree/bindings/media/i2c/tvp7002.txt      |   42 +++++++++++++
>>  drivers/media/i2c/tvp7002.c                        |   64 +++++++++++++++--
>>  2 files changed, 99 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..1ebd8b1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/tvp7002.txt
>> @@ -0,0 +1,42 @@
>> +* 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"
>> +
>> +- hsync-active: HSYNC Polarity configuration for endpoint.
>> +
>> +- vsync-active: VSYNC Polarity configuration for endpoint.
>> +
>> +- pclk-sample: Clock polarity of the endpoint.
>> +
>> +- ti,tvp7002-fid-polarity: Active-high Field ID polarity of the endpoint.
>> +
>> +- ti,tvp7002-sog-polarity: Sync on Green output polarity of the endpoint.
>
> Would it make sense to define field-active and sog-active properties in the
> V4L2 bindings instead of having per-chip properties ?
>
yes you are right these properties need to be in the device node
rather than the
port node. I'll send alone this patch of the series as v2 fixing the above.

Regards,
--Prabhakar Lad

Re: [PATCH 5/5] media: i2c: tvp7002: add OF support

[Laurent Pinchart]

Hi Prabhakar,

On Wednesday 15 May 2013 10:48:05 Prabhakar Lad wrote:
> On Tue, May 14, 2013 at 9:04 PM, Laurent Pinchart wrote:
> > On Tuesday 14 May 2013 16:15:34 Lad Prabhakar wrote:
> >> From: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> >> 
> >> add OF support for the tvp7002 driver.
> >> 
> >> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> >> Cc: Hans Verkuil <hans.verkuil@cisco.com>
> >> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
> >> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> >> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
> >> Cc: Sakari Ailus <sakari.ailus@iki.fi>
> >> Cc: Grant Likely <grant.likely@secretlab.ca>
> >> Cc: Rob Herring <rob.herring@calxeda.com>
> >> Cc: Rob Landley <rob@landley.net>
> >> Cc: devicetree-discuss@lists.ozlabs.org
> >> Cc: linux-doc@vger.kernel.org
> >> Cc: linux-kernel@vger.kernel.org
> >> Cc: davinci-linux-open-source@linux.davincidsp.com
> >> ---
> >> 
> >>  .../devicetree/bindings/media/i2c/tvp7002.txt      |   42 +++++++++++++
> >>  drivers/media/i2c/tvp7002.c                        |   64 ++++++++++++--
> >>  2 files changed, 99 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..1ebd8b1
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/media/i2c/tvp7002.txt
> >> @@ -0,0 +1,42 @@
> >> +* 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"
> >> +
> >> +- hsync-active: HSYNC Polarity configuration for endpoint.
> >> +
> >> +- vsync-active: VSYNC Polarity configuration for endpoint.
> >> +
> >> +- pclk-sample: Clock polarity of the endpoint.
> >> +
> >> +- ti,tvp7002-fid-polarity: Active-high Field ID polarity of the
> >> endpoint.
> >> +
> >> +- ti,tvp7002-sog-polarity: Sync on Green output polarity of the
> >> endpoint.
> > 
> > Would it make sense to define field-active and sog-active properties in
> > the V4L2 bindings instead of having per-chip properties ?
> 
> yes you are right these properties need to be in the device node rather than
> the port node. I'll send alone this patch of the series as v2 fixing the
> above.

That wasn't my point. What I'm wondering is whether ti,tvp7002-fid-polarity 
shouldn't be named field-active and specified in 
Documentation/devicetree/bindings/media/video-interfaces.txt (same for 
ti,tvp7002-sog-polarity).

-- 
Regards,

Laurent Pinchart