Hi Sylwester, On Thursday 17 April 2014 14:36:32 Laurent Pinchart wrote: > On Thursday 17 April 2014 12:59:22 Sylwester Nawrocki wrote: > > On 11/03/14 00:15, Laurent Pinchart wrote: > > > Parse the device tree node to populate platform data. > > > > > > Cc: devicetree@xxxxxxxxxxxxxxx > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > > > --- > > > > > > .../devicetree/bindings/media/i2c/adv7604.txt | 56 +++++++++++++ > > > drivers/media/i2c/adv7604.c | 92 > > > +++++++++++++---- > > > 2 files changed, 134 insertions(+), 14 deletions(-) > > > create mode 100644 > > > Documentation/devicetree/bindings/media/i2c/adv7604.txt > > > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/adv7604.txt > > > b/Documentation/devicetree/bindings/media/i2c/adv7604.txt new file mode > > > 100644 > > > index 0000000..0845c50 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/media/i2c/adv7604.txt > > > @@ -0,0 +1,56 @@ > > > +* Analog Devices ADV7604/11 video decoder with HDMI receiver > > > + > > > +The ADV7604 and ADV7611 are multiformat video decoders with an > > > integrated > > > HDMI +receiver. The ADV7604 has four multiplexed HDMI inputs and one > > > analog input, +and the ADV7611 has one HDMI input and no analog input. > > > + > > > +Required Properties: > > > + > > > + - compatible: Must contain one of the following > > > + - "adi,adv7604" for the ADV7604 > > > + - "adi,adv7611" for the ADV7611 > > > + > > > + - reg: I2C slave address > > > + > > > + - hpd-gpios: References to the GPIOs that control the HDMI hot-plug > > > + detection pins, one per HDMI input. The active flag indicates the > > > GPIO > > > + level that enables hot-plug detection. > > > + > > > +Optional Properties: > > > + > > > + - reset-gpios: Reference to the GPIO connected to the device's reset > > > pin. + > > > + - adi,default-input: Index of the input to be configured as default. > > > Valid > > > + values are 0..5 for the ADV7604 and 0 for the ADV7611. > > > > I have some doubts about this property. Firstly, it seems it is not needed > > for ADV7611 since it is always 0 for that device ? > > Why can't we hard code in the driver some default input ? > > I've thought about hardcoding a default input in the driver as well, but > Hans wasn't really keen on the idea. Hans, could you please comment on this > ? > > And which inputs it refers to ? HDMI inputs A..D + analog ? If we keep > > this > > property I think exact mapping of numbers to inputs should be included > > in description of this property. > > > > > + - adi,disable-power-down: Boolean property. When set forces the > > > device > > > to > > > + ignore the power-down pin. The property is valid for the ADV7604 > > > only > > > as > > > + the ADV7611 has no power-down pin. > > > > Does it refer to the !PWRDWN pin ? If so I would replace "power-down" with > > PWRDWN, so it is clear what we're talking about when someone looks only > > at the datasheet. > > > > > + - adi,disable-cable-reset: Boolean property. When set disables the > > > HDMI > > > + receiver automatic reset when the HDMI cable is unplugged. > > > > Couldn't this be configured from user space with some default assumed in > > the driver ? > > Good question. I'm not sure what the exact use case for this is. > > Let's be careful not to introduce unneeded properties, I'll drop those two > properties for now, we can implement support for the features later when > needed. > > > > +Example: > > > + > > > + hdmi_receiver@4c { > > > + compatible = "adi,adv7611"; > > > + reg = <0x4c>; > > > + > > > + reset-gpios = <&ioexp 0 GPIO_ACTIVE_LOW>; > > > + hpd-gpios = <&ioexp 2 GPIO_ACTIVE_HIGH>; > > > + > > > + adi,default-input = <0>; > > > + > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + > > > + port@0 { > > > + reg = <0>; > > > + }; > > > + port@1 { > > > + reg = <1>; > > > + hdmi_in: endpoint { > > > + remote-endpoint = <&ccdc_in>; > > > + }; > > > + }; > > > + }; > > > diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c > > > index cce140c..de44213 100644 > > > --- a/drivers/media/i2c/adv7604.c > > > +++ b/drivers/media/i2c/adv7604.c > > [snip] > > > > @@ -2836,21 +2906,15 @@ static int adv7604_remove(struct i2c_client > > > *client)> > > > > > > /* ------------------------------------------------------------------- > > > */ > > > > > > -static struct i2c_device_id adv7604_id[] = { > > > - { "adv7604", ADV7604 }, > > > - { "adv7611", ADV7611 }, > > > - { } > > > -}; > > > -MODULE_DEVICE_TABLE(i2c, adv7604_id); > > > - > > > > > > static struct i2c_driver adv7604_driver = { > > > > > > .driver = { > > > > > > .owner = THIS_MODULE, > > > .name = "adv7604", > > > > > > + .of_match_table = of_match_ptr(adv7604_of_id), > > > > of_match_ptr() isn't necessary here. > > Thanks, will fix in v3. On second thought, as the driver has non-DT users, keeping of_match_ptr() and marking the table as __maybe_unused will optimize the table out if neither CONFIG_OF nor CONFIG_MODULE is set. I'd thus prefer keeping of_match_ptr(). > > > }, > > > .probe = adv7604_probe, > > > .remove = adv7604_remove, > > > > > > - .id_table = adv7604_id, > > > + .id_table = adv7604_i2c_id, > > > > > > }; > > > > > > module_i2c_driver(adv7604_driver); -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html