| Message ID | 1368528334-13595-6-git-send-email-prabhakar.csengg@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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, > },
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
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).
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, },