Re: [PATCH RFC v2] media: tvp514x: add OF support
Hi Prabhakar,
On 01/29/2013 02:07 PM, Prabhakar Lad wrote:
[...]
diff --git a/Documentation/devicetree/bindings/media/i2c/tvp514x.txt
b/Documentation/devicetree/bindings/media/i2c/tvp514x.txt
new file mode 100644
index 000..55d3ffd
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/tvp514x.txt
@@ -0,0 +1,38 @@
+* Texas Instruments TVP514x video decoder
+
+The TVP5146/TVP5146m2/TVP5147/TVP5147m1 device is high quality, single-chip
+digital video decoder that digitizes and decodes all popular baseband analog
+video formats into digital video component. The tvp514x decoder supports
analog-
+to-digital (A/D) conversion of component RGB and YPbPr signals as well as A/D
+conversion and decoding of NTSC, PAL and SECAM composite and S-video into
+component YCbCr.
+
+Required Properties :
+- compatible: Must be "ti,tvp514x-decoder"
There are no significant differences among TVP514* devices as listed above,
you would like to handle above ?
I'm just wondering if you don't need ,for instance, two separate compatible
properties, e.g. "ti,tvp5146-decoder" and "ti,tvp5147-decoder" ?
+- hsync-active: HSYNC Polarity configuration for current interface.
+- vsync-active: VSYNC Polarity configuration for current interface.
+- data-active: Clock polarity of the current interface.
I guess you mean "pclk-sample: Clock polarity..." ?
+
+Example:
+
+i2c0@1c22000 {
+ ...
+ ...
+
+ tvp514x@5c {
+ compatible = "ti,tvp514x-decoder";
+ reg =<0x5c>;
+
+ port {
+ tvp514x_1: endpoint {
+ /* Active high (Defaults to 0) */
+ hsync-active =<1>;
+ /* Active high (Defaults to 0) */
+ hsync-active =<1>;
Should it be vsync-active ?
+ /* Active low (Defaults to 0) */
+ data-active =<0>;
and this pclk-sample ?
+ };
+ };
+ };
+ ...
+};
diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
index a4f0a70..24ce759 100644
--- a/drivers/media/i2c/tvp514x.c
+++ b/drivers/media/i2c/tvp514x.c
@@ -12,6 +12,7 @@
* Hardik Shah
* Manjunath Hadli
* Karicheri Muralidharan
+ * Prabhakar Lad
*
* This package is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
@@ -33,7 +34,9 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -930,6 +933,58 @@ static struct tvp514x_decoder tvp514x_dev = {
};
+#if defined(CONFIG_OF)
+static const struct of_device_id tvp514x_of_match[] = {
+ {.compatible = "ti,tvp514x-decoder", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, tvp514x_of_match);
+
+static struct tvp514x_platform_data
+ *tvp514x_get_pdata(struct i2c_client *client)
+{
+ if (!client->dev.platform_data&& client->dev.of_node) {
+ struct tvp514x_platform_data *pdata;
+ struct v4l2_of_endpoint bus_cfg;
+ struct device_node *endpoint;
+
+ pdata = devm_kzalloc(&client->dev,
+ sizeof(struct tvp514x_platform_data),
+ GFP_KERNEL);
+ client->dev.platform_data = pdata;
Do you really need to set client->dev.platform_data this way ?
What about passing struct tvp514x_decoder pointer to this function
and initializing struct tvp514x_decoder::pdata instead ?
+ if (!pdata)
+ return NULL;
+
+ endpoint = of_get_child_by_name(client->dev.of_node, "port");
+ if (endpoint)
+ endpoint = of_get_child_by_name(endpoint, "endpoint");
I think you could replace these two calls above with
endpoint = v4l2_of_get_next_endpoint(client->dev.of_node);
Now I see I should have modified this function to ensure it works also when
'port' nodes are grouped in a 'ports' node.
+ if (!endpoint) {
+ v4l2_info(client, "Using default data!!\n");
+ } else {
+ v4l2_of_parse_parallel_bus(endpoint,&bus_cfg);
+
+ if (bus_cfg.mbus_flags& V4L2_MBUS_HSYNC_ACTIVE_HIGH)
+ pdata->hs_polarity = 1;
+ if (bus_cfg.mbus_flags& V4L2_MBUS_VSYNC_ACTIVE_HIGH)
+ pdata->vs_polarity = 1;
+ if (bus_cfg.mbus_flags& V4L2_MBUS_PCLK_SAMPLE_RISING)
+ pdata->clk_polarity = 1;
+ }
+ }
+
+ return client->dev.platform_data;
+}
+#else
+#define tvp514x_of_match NULL
+
+static struct tvp514x_platform_data
+ *tvp514x_get_pdata(struct i2c_client *client)
+{
+ return client->dev.platform_data;
+}
+#endif
+
/**
* tvp514x_
Re: [PATCH RFC v2] media: tvp514x: add OF support
Hi Sylwester,
Thanks for the review.
On Sat, Feb 2, 2013 at 4:07 AM, Sylwester Nawrocki
wrote:
> Hi Prabhakar,
>
> On 01/29/2013 02:07 PM, Prabhakar Lad wrote:
> [...]
>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/tvp514x.txt
>> b/Documentation/devicetree/bindings/media/i2c/tvp514x.txt
>> new file mode 100644
>> index 000..55d3ffd
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/tvp514x.txt
>> @@ -0,0 +1,38 @@
>> +* Texas Instruments TVP514x video decoder
>> +
>> +The TVP5146/TVP5146m2/TVP5147/TVP5147m1 device is high quality,
>> single-chip
>> +digital video decoder that digitizes and decodes all popular baseband
>> analog
>> +video formats into digital video component. The tvp514x decoder supports
>> analog-
>> +to-digital (A/D) conversion of component RGB and YPbPr signals as well as
>> A/D
>> +conversion and decoding of NTSC, PAL and SECAM composite and S-video into
>> +component YCbCr.
>> +
>> +Required Properties :
>> +- compatible: Must be "ti,tvp514x-decoder"
>
>
> There are no significant differences among TVP514* devices as listed above,
> you would like to handle above ?
>
> I'm just wondering if you don't need ,for instance, two separate compatible
> properties, e.g. "ti,tvp5146-decoder" and "ti,tvp5147-decoder" ?
>
There are few differences in init/power sequence tough, I would still
like to have
single compatible property "ti,tvp514x-decoder", If you feel we need separate
property I will change it please let me know on this.
>
>> +- hsync-active: HSYNC Polarity configuration for current interface.
>> +- vsync-active: VSYNC Polarity configuration for current interface.
>> +- data-active: Clock polarity of the current interface.
>
>
> I guess you mean "pclk-sample: Clock polarity..." ?
>
>
yeah this needs to be 'pclk-sample' property, thanks for pointing it.
>> +
>> +Example:
>> +
>> +i2c0@1c22000 {
>> + ...
>> + ...
>> +
>> + tvp514x@5c {
>> + compatible = "ti,tvp514x-decoder";
>> + reg =<0x5c>;
>> +
>> + port {
>> + tvp514x_1: endpoint {
>> + /* Active high (Defaults to 0) */
>> + hsync-active =<1>;
>> + /* Active high (Defaults to 0) */
>> + hsync-active =<1>;
>
>
> Should it be vsync-active ?
>
Yes, I'll change it.
>
>> + /* Active low (Defaults to 0) */
>> + data-active =<0>;
>
>
> and this pclk-sample ?
>
Yes.
>> + };
>> + };
>> + };
>> + ...
>> +};
>> diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
>> index a4f0a70..24ce759 100644
>> --- a/drivers/media/i2c/tvp514x.c
>> +++ b/drivers/media/i2c/tvp514x.c
>> @@ -12,6 +12,7 @@
>>* Hardik Shah
>>* Manjunath Hadli
>>* Karicheri Muralidharan
>> + * Prabhakar Lad
>>*
>>* This package is free software; you can redistribute it and/or modify
>>* it under the terms of the GNU General Public License version 2 as
>> @@ -33,7 +34,9 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> +#include
>> #include
>> #include
>> #include
>> @@ -930,6 +933,58 @@ static struct tvp514x_decoder tvp514x_dev = {
>>
>> };
>>
>> +#if defined(CONFIG_OF)
>> +static const struct of_device_id tvp514x_of_match[] = {
>> + {.compatible = "ti,tvp514x-decoder", },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, tvp514x_of_match);
>> +
>> +static struct tvp514x_platform_data
>> + *tvp514x_get_pdata(struct i2c_client *client)
>> +{
>> + if (!client->dev.platform_data&& client->dev.of_node) {
>>
>> + struct tvp514x_platform_data *pdata;
>> + struct v4l2_of_endpoint bus_cfg;
>> + struct device_node *endpoint;
>> +
>> + pdata = devm_kzalloc(&client->dev,
>> + sizeof(struct tvp514x_platform_data),
>> + GFP_KERNEL);
>> + client->dev.platform_data = pdata;
>
>
> Do you really need to set client->dev.platform_data this way ?
> What about passing struct tvp514x_decoder pointer to this function
> and initializing struct tvp514x_decoder::pdata instead ?
>
>
Yes that can work too, I'll do the same.
>> + if (!pdata)
>> + return NULL;
>> +
>> + endpoint = of_get_child_by_name(client->dev.of_node,
>> "port");
>> + if (endpoint)
>> + endpoint = of_get_child_by_name(endpoint,
>> "endpoint");
>
>
> I think you could replace these two calls above with
>
> endpoint = v4l2_of_get_next_endpoint(client->dev.of_node);
>
Ok
> Now I see I should have modified this function to ensure it works also when
> 'port' nodes are grouped in a 'ports' node.
>
So V5 series of V4l OF parser doesn't have this fix ?
Regards,
--Prabhakar
Re: [PATCH RFC v2] media: tvp514x: add OF support
On 2/3/2013 3:43 PM, Prabhakar Lad wrote: > Hi Sylwester, > > Thanks for the review. > > On Sat, Feb 2, 2013 at 4:07 AM, Sylwester Nawrocki > wrote: >> Hi Prabhakar, >> >> On 01/29/2013 02:07 PM, Prabhakar Lad wrote: >> [...] >> >>> diff --git a/Documentation/devicetree/bindings/media/i2c/tvp514x.txt >>> b/Documentation/devicetree/bindings/media/i2c/tvp514x.txt >>> new file mode 100644 >>> index 000..55d3ffd >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/media/i2c/tvp514x.txt >>> @@ -0,0 +1,38 @@ >>> +* Texas Instruments TVP514x video decoder >>> + >>> +The TVP5146/TVP5146m2/TVP5147/TVP5147m1 device is high quality, >>> single-chip >>> +digital video decoder that digitizes and decodes all popular baseband >>> analog >>> +video formats into digital video component. The tvp514x decoder supports >>> analog- >>> +to-digital (A/D) conversion of component RGB and YPbPr signals as well as >>> A/D >>> +conversion and decoding of NTSC, PAL and SECAM composite and S-video into >>> +component YCbCr. >>> + >>> +Required Properties : >>> +- compatible: Must be "ti,tvp514x-decoder" >> >> >> There are no significant differences among TVP514* devices as listed above, >> you would like to handle above ? >> >> I'm just wondering if you don't need ,for instance, two separate compatible >> properties, e.g. "ti,tvp5146-decoder" and "ti,tvp5147-decoder" ? >> > There are few differences in init/power sequence tough, I would still > like to have > single compatible property "ti,tvp514x-decoder", If you feel we need separate > property I will change it please let me know on this. Compatible properties should not use generic part numbers. See one past discussion here: http://en.usenet.digipedia.org/thread/18472/20788/ Thanks, Sekhar ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH RFC v2] media: tvp514x: add OF support
Hi Prabhakar, On Sunday 03 February 2013 15:43:49 Prabhakar Lad wrote: > On Sat, Feb 2, 2013 at 4:07 AM, Sylwester Nawrocki wrote: > > On 01/29/2013 02:07 PM, Prabhakar Lad wrote: > > [...] > > > >> diff --git a/Documentation/devicetree/bindings/media/i2c/tvp514x.txt > >> b/Documentation/devicetree/bindings/media/i2c/tvp514x.txt > >> new file mode 100644 > >> index 000..55d3ffd > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/media/i2c/tvp514x.txt > >> @@ -0,0 +1,38 @@ > >> +* Texas Instruments TVP514x video decoder > >> + > >> +The TVP5146/TVP5146m2/TVP5147/TVP5147m1 device is high quality, > >> +single-chip digital video decoder that digitizes and decodes all popular > >> +baseband analog video formats into digital video component. The tvp514x > >> +decoder supports analog-to-digital (A/D) conversion of component RGB and > >> +YPbPr signals as well as A/D conversion and decoding of NTSC, PAL and > >> +SECAM composite and S-video into component YCbCr. > >> + > >> +Required Properties : > >> +- compatible: Must be "ti,tvp514x-decoder" > > > > There are no significant differences among TVP514* devices as listed > > above, you would like to handle above ? > > > > I'm just wondering if you don't need, for instance, two separate > > compatible properties, e.g. "ti,tvp5146-decoder" and "ti,tvp5147-decoder" > > ? > > There are few differences in init/power sequence tough, I would still > like to have single compatible property "ti,tvp514x-decoder", If you feel we > need separate property I will change it please let me know on this. If there's any difference between the chips that need to be handled in the driver, using two compatible properties is a good practice. Your driver will then be able to easily differentiate between the two chips, and there's no real drawback in doing so. -- Regards, Laurent Pinchart ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH RFC v2] media: tvp514x: add OF support
Hi Prabhakar,
On 02/03/2013 11:13 AM, Prabhakar Lad wrote:
[...]
+Required Properties :
+- compatible: Must be "ti,tvp514x-decoder"
There are no significant differences among TVP514* devices as listed above,
you would like to handle above ?
Sorry for the mangled sentence, I intended to write "in the driver" instead
of the last "above".
I'm just wondering if you don't need ,for instance, two separate compatible
properties, e.g. "ti,tvp5146-decoder" and "ti,tvp5147-decoder" ?
There are few differences in init/power sequence tough, I would still
like to have
single compatible property "ti,tvp514x-decoder", If you feel we need separate
property I will change it please let me know on this.
As Sekhar already mentioned, wildcards in the compatible property should
not be used. You could just use exact part names in the compatible
properties and list them all in the tvp514x_of_match[] array. Even though
this driver doesn't care about the differences between various tvp514?
chips, there might be others that do.
[...]
+#if defined(CONFIG_OF)
+static const struct of_device_id tvp514x_of_match[] = {
+ {.compatible = "ti,tvp514x-decoder", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, tvp514x_of_match);
+
+static struct tvp514x_platform_data
+ *tvp514x_get_pdata(struct i2c_client *client)
+{
+ if (!client->dev.platform_data&& client->dev.of_node) {
+ struct tvp514x_platform_data *pdata;
+ struct v4l2_of_endpoint bus_cfg;
+ struct device_node *endpoint;
+
+ pdata = devm_kzalloc(&client->dev,
+ sizeof(struct tvp514x_platform_data),
+ GFP_KERNEL);
+ client->dev.platform_data = pdata;
Do you really need to set client->dev.platform_data this way ?
What about passing struct tvp514x_decoder pointer to this function
and initializing struct tvp514x_decoder::pdata instead ?
Yes that can work too, I'll do the same.
Ok, thanks.
+ if (!pdata)
+ return NULL;
+
+ endpoint = of_get_child_by_name(client->dev.of_node,
"port");
+ if (endpoint)
+ endpoint = of_get_child_by_name(endpoint,
"endpoint");
I think you could replace these two calls above with
endpoint = v4l2_of_get_next_endpoint(client->dev.of_node);
Ok
Now I see I should have modified this function to ensure it works also when
'port' nodes are grouped in a 'ports' node.
So V5 series of V4l OF parser doesn't have this fix ?
No, it doesn't. I think we need something along the lines of:
diff --git a/drivers/media/v4l2-core/v4l2-of.c
b/drivers/media/v4l2-core/v4l2-of.c
index e1f570b..8a286f1 100644
--- a/drivers/media/v4l2-core/v4l2-of.c
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -185,10 +185,15 @@ struct device_node
*v4l2_of_get_next_endpoint(const struct device_node *parent,
* This is the first call, we have to find a port within
* this node.
*/
- for_each_child_of_node(parent, port) {
+ while (port = of_get_next_child(parent, port)) {
if (!of_node_cmp(port->name, "port"))
break;
- }
+ if (!of_node_cmp(port->name, "ports")) {
+ parent = port;
+ of_node_put(port);
+ port = NULL:
+ }
+ };
if (port) {
/* Found a port, get an endpoint. */
endpoint = of_get_next_child(port, NULL);
However this shouldn't affect you, as you don't use the 'ports' node...
I will likely post v6 including this fix tomorrow.
--
Regards,
Sylwester
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH RFC v2] media: tvp514x: add OF support
Sekhar, On Sun, Feb 3, 2013 at 6:40 PM, Sekhar Nori wrote: > On 2/3/2013 3:43 PM, Prabhakar Lad wrote: >> Hi Sylwester, >> >> Thanks for the review. >> >> On Sat, Feb 2, 2013 at 4:07 AM, Sylwester Nawrocki >> wrote: >>> Hi Prabhakar, >>> >>> On 01/29/2013 02:07 PM, Prabhakar Lad wrote: >>> [...] >>> diff --git a/Documentation/devicetree/bindings/media/i2c/tvp514x.txt b/Documentation/devicetree/bindings/media/i2c/tvp514x.txt new file mode 100644 index 000..55d3ffd --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/tvp514x.txt @@ -0,0 +1,38 @@ +* Texas Instruments TVP514x video decoder + +The TVP5146/TVP5146m2/TVP5147/TVP5147m1 device is high quality, single-chip +digital video decoder that digitizes and decodes all popular baseband analog +video formats into digital video component. The tvp514x decoder supports analog- +to-digital (A/D) conversion of component RGB and YPbPr signals as well as A/D +conversion and decoding of NTSC, PAL and SECAM composite and S-video into +component YCbCr. + +Required Properties : +- compatible: Must be "ti,tvp514x-decoder" >>> >>> >>> There are no significant differences among TVP514* devices as listed above, >>> you would like to handle above ? >>> >>> I'm just wondering if you don't need ,for instance, two separate compatible >>> properties, e.g. "ti,tvp5146-decoder" and "ti,tvp5147-decoder" ? >>> >> There are few differences in init/power sequence tough, I would still >> like to have >> single compatible property "ti,tvp514x-decoder", If you feel we need separate >> property I will change it please let me know on this. > > Compatible properties should not use generic part numbers. See one past > discussion here: http://en.usenet.digipedia.org/thread/18472/20788/ > Thanks for the link, I'll have separate compatible properties. Regards, --Prabhakar > Thanks, > Sekhar ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH RFC v2] media: tvp514x: add OF support
Hi Laurent, On Sun, Feb 3, 2013 at 6:44 PM, Laurent Pinchart wrote: > Hi Prabhakar, > > On Sunday 03 February 2013 15:43:49 Prabhakar Lad wrote: >> On Sat, Feb 2, 2013 at 4:07 AM, Sylwester Nawrocki wrote: >> > On 01/29/2013 02:07 PM, Prabhakar Lad wrote: >> > [...] >> > >> >> diff --git a/Documentation/devicetree/bindings/media/i2c/tvp514x.txt >> >> b/Documentation/devicetree/bindings/media/i2c/tvp514x.txt >> >> new file mode 100644 >> >> index 000..55d3ffd >> >> --- /dev/null >> >> +++ b/Documentation/devicetree/bindings/media/i2c/tvp514x.txt >> >> @@ -0,0 +1,38 @@ >> >> +* Texas Instruments TVP514x video decoder >> >> + >> >> +The TVP5146/TVP5146m2/TVP5147/TVP5147m1 device is high quality, >> >> +single-chip digital video decoder that digitizes and decodes all popular >> >> +baseband analog video formats into digital video component. The tvp514x >> >> +decoder supports analog-to-digital (A/D) conversion of component RGB and >> >> +YPbPr signals as well as A/D conversion and decoding of NTSC, PAL and >> >> +SECAM composite and S-video into component YCbCr. >> >> + >> >> +Required Properties : >> >> +- compatible: Must be "ti,tvp514x-decoder" >> > >> > There are no significant differences among TVP514* devices as listed >> > above, you would like to handle above ? >> > >> > I'm just wondering if you don't need, for instance, two separate >> > compatible properties, e.g. "ti,tvp5146-decoder" and "ti,tvp5147-decoder" >> > ? >> >> There are few differences in init/power sequence tough, I would still >> like to have single compatible property "ti,tvp514x-decoder", If you feel we >> need separate property I will change it please let me know on this. > > If there's any difference between the chips that need to be handled in the > driver, using two compatible properties is a good practice. Your driver will > then be able to easily differentiate between the two chips, and there's no > real drawback in doing so. > Ok I'll will have separate compatible properties. Regards, --Prabhakar > -- > Regards, > > Laurent Pinchart > ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH RFC v2] media: tvp514x: add OF support
Hi Sylwester,
On Sun, Feb 3, 2013 at 6:57 PM, Sylwester Nawrocki
wrote:
> Hi Prabhakar,
>
> On 02/03/2013 11:13 AM, Prabhakar Lad wrote:
> [...]
>
+Required Properties :
+- compatible: Must be "ti,tvp514x-decoder"
>>>
>>>
>>>
>>> There are no significant differences among TVP514* devices as listed
>>> above,
>>> you would like to handle above ?
>
>
> Sorry for the mangled sentence, I intended to write "in the driver" instead
> of the last "above".
>
>
Not a problem I got what you intended :)
>>> I'm just wondering if you don't need ,for instance, two separate
>>> compatible
>>> properties, e.g. "ti,tvp5146-decoder" and "ti,tvp5147-decoder" ?
>>>
>> There are few differences in init/power sequence tough, I would still
>> like to have
>> single compatible property "ti,tvp514x-decoder", If you feel we need
>> separate
>> property I will change it please let me know on this.
>
>
> As Sekhar already mentioned, wildcards in the compatible property should
> not be used. You could just use exact part names in the compatible
> properties and list them all in the tvp514x_of_match[] array. Even though
> this driver doesn't care about the differences between various tvp514?
> chips, there might be others that do.
>
> [...]
>
Ok, I'll have separate compatible properties.
+#if defined(CONFIG_OF)
+static const struct of_device_id tvp514x_of_match[] = {
+ {.compatible = "ti,tvp514x-decoder", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, tvp514x_of_match);
+
+static struct tvp514x_platform_data
+ *tvp514x_get_pdata(struct i2c_client *client)
+{
+ if (!client->dev.platform_data&& client->dev.of_node) {
+ struct tvp514x_platform_data *pdata;
+ struct v4l2_of_endpoint bus_cfg;
+ struct device_node *endpoint;
+
+ pdata = devm_kzalloc(&client->dev,
+ sizeof(struct tvp514x_platform_data),
+ GFP_KERNEL);
+ client->dev.platform_data = pdata;
>>>
>>>
>>>
>>> Do you really need to set client->dev.platform_data this way ?
>>> What about passing struct tvp514x_decoder pointer to this function
>>> and initializing struct tvp514x_decoder::pdata instead ?
>>>
>>>
>> Yes that can work too, I'll do the same.
>
>
> Ok, thanks.
>
>
+ if (!pdata)
+ return NULL;
+
+ endpoint = of_get_child_by_name(client->dev.of_node,
"port");
+ if (endpoint)
+ endpoint = of_get_child_by_name(endpoint,
"endpoint");
>>>
>>>
>>>
>>> I think you could replace these two calls above with
>>>
>>> endpoint =
>>> v4l2_of_get_next_endpoint(client->dev.of_node);
>>>
>> Ok
>>
>>> Now I see I should have modified this function to ensure it works also
>>> when
>>> 'port' nodes are grouped in a 'ports' node.
>>>
>> So V5 series of V4l OF parser doesn't have this fix ?
>
>
> No, it doesn't. I think we need something along the lines of:
>
> diff --git a/drivers/media/v4l2-core/v4l2-of.c
> b/drivers/media/v4l2-core/v4l2-of.c
> index e1f570b..8a286f1 100644
> --- a/drivers/media/v4l2-core/v4l2-of.c
> +++ b/drivers/media/v4l2-core/v4l2-of.c
> @@ -185,10 +185,15 @@ struct device_node *v4l2_of_get_next_endpoint(const
> struct device_node *parent,
> * This is the first call, we have to find a port within
> * this node.
> */
> - for_each_child_of_node(parent, port) {
> + while (port = of_get_next_child(parent, port)) {
> if (!of_node_cmp(port->name, "port"))
> break;
> - }
> + if (!of_node_cmp(port->name, "ports")) {
> + parent = port;
> + of_node_put(port);
> + port = NULL:
> + }
> + };
> if (port) {
> /* Found a port, get an endpoint. */
> endpoint = of_get_next_child(port, NULL);
>
> However this shouldn't affect you, as you don't use the 'ports' node...
> I will likely post v6 including this fix tomorrow.
>
yes I don't need it, Just asked so that I can test my driver on latest
version :)
Regards,
--Prabhakar
> --
>
> Regards,
> Sylwester
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
