| Message ID | 1451910332-23385-8-git-send-email-javier@osg.samsung.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Mon, Jan 04, 2016 at 09:25:29AM -0300, Javier Martinez Canillas wrote: > Add a Device Tree binding document for the TVP5150 video decoder. > > Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> > --- > > .../devicetree/bindings/media/i2c/tvp5150.txt | 35 ++++++++++++++++++++++ > 1 file changed, 35 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/i2c/tvp5150.txt > > diff --git a/Documentation/devicetree/bindings/media/i2c/tvp5150.txt b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt > new file mode 100644 > index 000000000000..bf0b3f3128ce > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt > @@ -0,0 +1,35 @@ > +* Texas Instruments TVP5150 and TVP5151 video decoders > + > +The TVP5150 and TVP5151 are video decoders that convert baseband NTSC and PAL > +(and also SECAM in the TVP5151 case) video signals to either 8-bit 4:2:2 YUV > +with discrete syncs or 8-bit ITU-R BT.656 with embedded syncs output formats. > + > +Required Properties: > +- compatible: value must be "ti,tvp5150" What about the 5151? The driver never needs to know if SECAM is supported or not? > +- reg: I2C slave address > + > +Optional Properties: > +- powerdown-gpios: phandle for the GPIO connected to the PDN pin, if any. > +- reset-gpios: phandle for the GPIO connected to the RESETB pin, if any. > + > +The device node must contain one 'port' child node for its digital output > +video port, in accordance with the video interface bindings defined in > +Documentation/devicetree/bindings/media/video-interfaces.txt. > + > +Example: > + > +&i2c2 { > + ... > + tvp5150@5c { > + compatible = "ti,tvp5150"; Too much indentation here. > + reg = <0x5c>; > + powerdown-gpios = <&gpio4 30 GPIO_ACTIVE_LOW>; > + reset-gpios = <&gpio6 7 GPIO_ACTIVE_LOW>; > + > + port { > + tvp5150_1: endpoint { > + remote-endpoint = <&ccdc_ep>; > + }; > + }; > + }; > +}; > -- > 2.4.3 > -- 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
Hello Rob, Thanks a lot for your feedback. On 01/04/2016 11:07 AM, Rob Herring wrote: > On Mon, Jan 04, 2016 at 09:25:29AM -0300, Javier Martinez Canillas wrote: >> Add a Device Tree binding document for the TVP5150 video decoder. >> >> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> >> --- >> >> .../devicetree/bindings/media/i2c/tvp5150.txt | 35 ++++++++++++++++++++++ >> 1 file changed, 35 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/media/i2c/tvp5150.txt >> >> diff --git a/Documentation/devicetree/bindings/media/i2c/tvp5150.txt b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt >> new file mode 100644 >> index 000000000000..bf0b3f3128ce >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt >> @@ -0,0 +1,35 @@ >> +* Texas Instruments TVP5150 and TVP5151 video decoders >> + >> +The TVP5150 and TVP5151 are video decoders that convert baseband NTSC and PAL >> +(and also SECAM in the TVP5151 case) video signals to either 8-bit 4:2:2 YUV >> +with discrete syncs or 8-bit ITU-R BT.656 with embedded syncs output formats. >> + >> +Required Properties: >> +- compatible: value must be "ti,tvp5150" > > What about the 5151? The driver never needs to know if SECAM is > supported or not? > The device ID can be detected at runtime by reading the TVP5150_MSB_DEV_ID and TVP5150_LSB_DEV_ID registers in both tvp5150 and tvp5151 (see patch #2 in this series). So I thought there was no need to have another compatible string for "ti,tvp5151". >> +- reg: I2C slave address >> + >> +Optional Properties: >> +- powerdown-gpios: phandle for the GPIO connected to the PDN pin, if any. >> +- reset-gpios: phandle for the GPIO connected to the RESETB pin, if any. >> + >> +The device node must contain one 'port' child node for its digital output >> +video port, in accordance with the video interface bindings defined in >> +Documentation/devicetree/bindings/media/video-interfaces.txt. >> + >> +Example: >> + >> +&i2c2 { >> + ... >> + tvp5150@5c { >> + compatible = "ti,tvp5150"; > > Too much indentation here. > Right, I copied the example from a DTS that had another level of indentation but it seems that I forgot to fix all lines, sorry about that. I will in v2. Best regards,
Hi Javier, Thank you for the patch. On Monday 04 January 2016 09:25:29 Javier Martinez Canillas wrote: > Add a Device Tree binding document for the TVP5150 video decoder. > > Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> > --- > > .../devicetree/bindings/media/i2c/tvp5150.txt | 35 +++++++++++++++++++ > 1 file changed, 35 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/i2c/tvp5150.txt > > diff --git a/Documentation/devicetree/bindings/media/i2c/tvp5150.txt > b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt new file mode > 100644 > index 000000000000..bf0b3f3128ce > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt > @@ -0,0 +1,35 @@ > +* Texas Instruments TVP5150 and TVP5151 video decoders > + > +The TVP5150 and TVP5151 are video decoders that convert baseband NTSC and > PAL > +(and also SECAM in the TVP5151 case) video signals to either 8-bit 4:2:2 > YUV > +with discrete syncs or 8-bit ITU-R BT.656 with embedded syncs output > formats. > + > +Required Properties: > +- compatible: value must be "ti,tvp5150" > +- reg: I2C slave address > + > +Optional Properties: > +- powerdown-gpios: phandle for the GPIO connected to the PDN pin, if any. The signal is called PDN in the datasheet, so it might make sense to call this pdn-gpios. I have no strong opinion on this, I'll let you decide what you think is best. Apart from that (and the indentation issue pointed out by Rob), Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > +- reset-gpios: phandle for the GPIO connected to the RESETB pin, if any. > + > +The device node must contain one 'port' child node for its digital output > +video port, in accordance with the video interface bindings defined in > +Documentation/devicetree/bindings/media/video-interfaces.txt. > + > +Example: > + > +&i2c2 { > + ... > + tvp5150@5c { > + compatible = "ti,tvp5150"; > + reg = <0x5c>; > + powerdown-gpios = <&gpio4 30 GPIO_ACTIVE_LOW>; > + reset-gpios = <&gpio6 7 GPIO_ACTIVE_LOW>; > + > + port { > + tvp5150_1: endpoint { > + remote-endpoint = <&ccdc_ep>; > + }; > + }; > + }; > +};
Hello Laurent, On 01/06/2016 07:39 AM, Laurent Pinchart wrote: > Hi Javier, > > Thank you for the patch. > Thanks a lot for your feedback. [snip] >> + >> +Optional Properties: >> +- powerdown-gpios: phandle for the GPIO connected to the PDN pin, if any. > > The signal is called PDN in the datasheet, so it might make sense to call this > pdn-gpios. I have no strong opinion on this, I'll let you decide what you > think is best. > Yes, I wondered if the convention was to use a descriptive name or the one used in the datasheet but Documentation/devicetree/bindings/gpio/gpio.txt says nothing about it. I'll change it to pdn-gpios since it could be easier to match with the doc. Best regards,
On Wednesday 06 January 2016 07:46:35 Javier Martinez Canillas wrote: > On 01/06/2016 07:39 AM, Laurent Pinchart wrote: > > Hi Javier, > > > > Thank you for the patch. > > Thanks a lot for your feedback. > > [snip] > > >> + > >> +Optional Properties: > >> +- powerdown-gpios: phandle for the GPIO connected to the PDN pin, if > >> any. > > > > The signal is called PDN in the datasheet, so it might make sense to call > > this pdn-gpios. I have no strong opinion on this, I'll let you decide > > what you think is best. > > Yes, I wondered if the convention was to use a descriptive name or the one > used in the datasheet but Documentation/devicetree/bindings/gpio/gpio.txt > says nothing about it. The device tree maintainers might want to comment on that :-) > I'll change it to pdn-gpios since it could be easier to match with the doc.
diff --git a/Documentation/devicetree/bindings/media/i2c/tvp5150.txt b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt new file mode 100644 index 000000000000..bf0b3f3128ce --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/tvp5150.txt @@ -0,0 +1,35 @@ +* Texas Instruments TVP5150 and TVP5151 video decoders + +The TVP5150 and TVP5151 are video decoders that convert baseband NTSC and PAL +(and also SECAM in the TVP5151 case) video signals to either 8-bit 4:2:2 YUV +with discrete syncs or 8-bit ITU-R BT.656 with embedded syncs output formats. + +Required Properties: +- compatible: value must be "ti,tvp5150" +- reg: I2C slave address + +Optional Properties: +- powerdown-gpios: phandle for the GPIO connected to the PDN pin, if any. +- reset-gpios: phandle for the GPIO connected to the RESETB pin, if any. + +The device node must contain one 'port' child node for its digital output +video port, in accordance with the video interface bindings defined in +Documentation/devicetree/bindings/media/video-interfaces.txt. + +Example: + +&i2c2 { + ... + tvp5150@5c { + compatible = "ti,tvp5150"; + reg = <0x5c>; + powerdown-gpios = <&gpio4 30 GPIO_ACTIVE_LOW>; + reset-gpios = <&gpio6 7 GPIO_ACTIVE_LOW>; + + port { + tvp5150_1: endpoint { + remote-endpoint = <&ccdc_ep>; + }; + }; + }; +};
Add a Device Tree binding document for the TVP5150 video decoder. Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> --- .../devicetree/bindings/media/i2c/tvp5150.txt | 35 ++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/tvp5150.txt