| Message ID | 1413992061-28678-1-git-send-email-jean-michel.hautbois@vodalys.com |
|---|---|
| State | Needs Review / ACK, archived |
| Headers | show |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 1 errors, 0 warnings, 0 lines checked |
| robh/patch-applied | success |
Hi Jean-Michel, Thank you for the patch. On Wednesday 22 October 2014 17:34:21 Jean-Michel Hautbois wrote: > This patch adds support for DT parsing of ADV7604 as well as ADV7611. > It needs to be improved in order to get ports parsing too. Let's improve it then :-) The DT bindings as proposed by this patch are incomplete, that's just asking for trouble. How would you model the adv7604 ports ? > Signed-off-by: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com> > --- > Documentation/devicetree/bindings/media/i2c/adv7604.txt | 1 + > drivers/media/i2c/adv7604.c | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/media/i2c/adv7604.txt > b/Documentation/devicetree/bindings/media/i2c/adv7604.txt index > c27cede..5c8b3e6 100644 > --- a/Documentation/devicetree/bindings/media/i2c/adv7604.txt > +++ b/Documentation/devicetree/bindings/media/i2c/adv7604.txt > @@ -10,6 +10,7 @@ Required Properties: > > - compatible: Must contain one of the following > - "adi,adv7611" for the ADV7611 > + - "adi,adv7604" for the ADV7604 Please switch the two lines to keep them alphabetically sorted. > > - reg: I2C slave address > > diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c > index 47795ff..421035f 100644 > --- a/drivers/media/i2c/adv7604.c > +++ b/drivers/media/i2c/adv7604.c > @@ -2677,6 +2677,7 @@ MODULE_DEVICE_TABLE(i2c, adv7604_i2c_id); > > static struct of_device_id adv7604_of_id[] __maybe_unused = { > { .compatible = "adi,adv7611", .data = &adv7604_chip_info[ADV7611] }, > + { .compatible = "adi,adv7604", .data = &adv7604_chip_info[ADV7604] }, Same comment here. > { } > }; > MODULE_DEVICE_TABLE(of, adv7604_of_id);
Hi Laurent, Thank you for reviewing, 2014-10-23 1:53 GMT+02:00 Laurent Pinchart <laurent.pinchart@ideasonboard.com>: > Hi Jean-Michel, > > Thank you for the patch. > > On Wednesday 22 October 2014 17:34:21 Jean-Michel Hautbois wrote: >> This patch adds support for DT parsing of ADV7604 as well as ADV7611. >> It needs to be improved in order to get ports parsing too. > > Let's improve it then :-) The DT bindings as proposed by this patch are > incomplete, that's just asking for trouble. > > How would you model the adv7604 ports ? I am opened to suggestions :). But it has to remain as simple as possible, ideally allowing for giving names to the ports. As done today, it works, ports are parsed but are all the same... >> Signed-off-by: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com> >> --- >> Documentation/devicetree/bindings/media/i2c/adv7604.txt | 1 + >> drivers/media/i2c/adv7604.c | 1 + >> 2 files changed, 2 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/media/i2c/adv7604.txt >> b/Documentation/devicetree/bindings/media/i2c/adv7604.txt index >> c27cede..5c8b3e6 100644 >> --- a/Documentation/devicetree/bindings/media/i2c/adv7604.txt >> +++ b/Documentation/devicetree/bindings/media/i2c/adv7604.txt >> @@ -10,6 +10,7 @@ Required Properties: >> >> - compatible: Must contain one of the following >> - "adi,adv7611" for the ADV7611 >> + - "adi,adv7604" for the ADV7604 > > Please switch the two lines to keep them alphabetically sorted. >> >> - reg: I2C slave address >> >> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c >> index 47795ff..421035f 100644 >> --- a/drivers/media/i2c/adv7604.c >> +++ b/drivers/media/i2c/adv7604.c >> @@ -2677,6 +2677,7 @@ MODULE_DEVICE_TABLE(i2c, adv7604_i2c_id); >> >> static struct of_device_id adv7604_of_id[] __maybe_unused = { >> { .compatible = "adi,adv7611", .data = &adv7604_chip_info[ADV7611] }, >> + { .compatible = "adi,adv7604", .data = &adv7604_chip_info[ADV7604] }, > > Same comment here. Done on my side, but will wait for your suggestions, in order to add ports parsing ;-). Thanks, JM -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Jean-Michel, On Thursday 23 October 2014 07:51:50 Jean-Michel Hautbois wrote: > 2014-10-23 1:53 GMT+02:00 Laurent Pinchart: > > On Wednesday 22 October 2014 17:34:21 Jean-Michel Hautbois wrote: > >> This patch adds support for DT parsing of ADV7604 as well as ADV7611. > >> It needs to be improved in order to get ports parsing too. > > > > Let's improve it then :-) The DT bindings as proposed by this patch are > > incomplete, that's just asking for trouble. > > > > How would you model the adv7604 ports ? > > I am opened to suggestions :). > But it has to remain as simple as possible, ideally allowing for giving > names to the ports. > As done today, it works, ports are parsed but are all the same... The ADV7611 was easy, it had a single HDMI input only. The ADV7612 is easy as well as it just has two separate HDMI inputs. The ADV7604 is a more complex beast. The HDMI inputs shouldn't be much of an issue as they're independent and multiplexed internally. You can just create one pad per HDMI input. The analog inputs, however, can't be modeled as easily. A naive approach would be to create one pad for each of the 12 analog inputs, but the chip has three separate ADCs and can combine 3 inputs in a single digital video stream. I don't know how we should model support for that. Lars-Peter, Hans, would you have a revolutionary idea to same the world today ? > >> Signed-off-by: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com> [...]
Hi, 2014-10-27 0:30 GMT+01:00 Laurent Pinchart <laurent.pinchart@ideasonboard.com>: > Hi Jean-Michel, > > On Thursday 23 October 2014 07:51:50 Jean-Michel Hautbois wrote: >> 2014-10-23 1:53 GMT+02:00 Laurent Pinchart: >> > On Wednesday 22 October 2014 17:34:21 Jean-Michel Hautbois wrote: >> >> This patch adds support for DT parsing of ADV7604 as well as ADV7611. >> >> It needs to be improved in order to get ports parsing too. >> > >> > Let's improve it then :-) The DT bindings as proposed by this patch are >> > incomplete, that's just asking for trouble. >> > >> > How would you model the adv7604 ports ? >> >> I am opened to suggestions :). >> But it has to remain as simple as possible, ideally allowing for giving >> names to the ports. >> As done today, it works, ports are parsed but are all the same... > > The ADV7611 was easy, it had a single HDMI input only. The ADV7612 is easy as > well as it just has two separate HDMI inputs. > > The ADV7604 is a more complex beast. The HDMI inputs shouldn't be much of an > issue as they're independent and multiplexed internally. You can just create > one pad per HDMI input. > > The analog inputs, however, can't be modeled as easily. A naive approach would > be to create one pad for each of the 12 analog inputs, but the chip has three > separate ADCs and can combine 3 inputs in a single digital video stream. I > don't know how we should model support for that. Lars-Peter, Hans, would you > have a revolutionary idea to same the world today ? I get back to working on this specific part, but I don't know how these analog inputs should be modeled. On page 68 of ADV7604_HW_RevF there is Figure 11 showing typical configurations using AIN_SEL[2:0]. I can see 4 inputs muxed : this would suggest to have 4 pads for analog inputs. Not sure it makes sense though... Thanks, JM -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/media/i2c/adv7604.txt b/Documentation/devicetree/bindings/media/i2c/adv7604.txt index c27cede..5c8b3e6 100644 --- a/Documentation/devicetree/bindings/media/i2c/adv7604.txt +++ b/Documentation/devicetree/bindings/media/i2c/adv7604.txt @@ -10,6 +10,7 @@ Required Properties: - compatible: Must contain one of the following - "adi,adv7611" for the ADV7611 + - "adi,adv7604" for the ADV7604 - reg: I2C slave address diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c index 47795ff..421035f 100644 --- a/drivers/media/i2c/adv7604.c +++ b/drivers/media/i2c/adv7604.c @@ -2677,6 +2677,7 @@ MODULE_DEVICE_TABLE(i2c, adv7604_i2c_id); static struct of_device_id adv7604_of_id[] __maybe_unused = { { .compatible = "adi,adv7611", .data = &adv7604_chip_info[ADV7611] }, + { .compatible = "adi,adv7604", .data = &adv7604_chip_info[ADV7604] }, { } }; MODULE_DEVICE_TABLE(of, adv7604_of_id);
This patch adds support for DT parsing of ADV7604 as well as ADV7611. It needs to be improved in order to get ports parsing too. Signed-off-by: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com> --- Documentation/devicetree/bindings/media/i2c/adv7604.txt | 1 + drivers/media/i2c/adv7604.c | 1 + 2 files changed, 2 insertions(+)