| Message ID | 1596454753-13612-2-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | media: i2c: ov772x: Enable BT656 mode and test pattern support | expand |
Hi Prabhakar, On Mon, Aug 03, 2020 at 12:39:11PM +0100, Lad Prabhakar wrote: > Document endpoint properties required for parallel interface > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > --- > .../devicetree/bindings/media/i2c/ov772x.txt | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt b/Documentation/devicetree/bindings/media/i2c/ov772x.txt > index 0b3ede5b8e6a..1f4153484717 100644 > --- a/Documentation/devicetree/bindings/media/i2c/ov772x.txt > +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.txt > @@ -21,6 +21,22 @@ subnode for its digital output video port, in accordance with the video > interface bindings defined in Documentation/devicetree/bindings/media/ > video-interfaces.txt. > > +Endpoint node required properties for parallel connection are: > +- remote-endpoint: a phandle to the bus receiver's endpoint node. we allow endpoints without a remote end connected usually. They can be filled in later, in example, with an overlay. > +- bus-width: shall be set to <8> for 8 bits parallel bus > + or <10> for 10 bits parallel bus > +- data-shift: shall be set to <2> for 8 bits parallel bus > + (lines 9:2 are used) or <0> for 10 bits parallel bus defining what is required or optional might be hard. I don't see the driver enforcing their presence and I assume they have safe default. Maybe make them optional and specify what the defaul value is ? > +- hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH respectively. > + (Not required for bus-type equal 6) > +- vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH respectively. > + (Not required for bus-type equal 6) If they're not required, they're optional, aren't they ? :) > +- pclk-sample: sample data on rising (1) or falling (0) edge of the pixel clock > + signal. (Not required for bus-type equal 6) Why the pclk polarity is does not apply to BT.656 ? > +- bus-type: data bus type. Possible values are: > + 5 - Parallel > + 6 - Bt.656 Are we making this required, or do we expect this to be deduced depending on which other properties have been specified ? Sakari it seems you would like this to become a properties that has to be specified most of the times, right ? (I tend to agree with that FWIW), but does it impact retro-compatibility ? > + > Example: > > &i2c0 { > -- > 2.17.1 >
Hi Jacopo, Thank you for the review. On Thu, Aug 6, 2020 at 3:32 PM Jacopo Mondi <jacopo@jmondi.org> wrote: > > Hi Prabhakar, > > On Mon, Aug 03, 2020 at 12:39:11PM +0100, Lad Prabhakar wrote: > > Document endpoint properties required for parallel interface > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > --- > > .../devicetree/bindings/media/i2c/ov772x.txt | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt b/Documentation/devicetree/bindings/media/i2c/ov772x.txt > > index 0b3ede5b8e6a..1f4153484717 100644 > > --- a/Documentation/devicetree/bindings/media/i2c/ov772x.txt > > +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.txt > > @@ -21,6 +21,22 @@ subnode for its digital output video port, in accordance with the video > > interface bindings defined in Documentation/devicetree/bindings/media/ > > video-interfaces.txt. > > > > +Endpoint node required properties for parallel connection are: > > +- remote-endpoint: a phandle to the bus receiver's endpoint node. > > we allow endpoints without a remote end connected usually. They can be > filled in later, in example, with an overlay. > Agreed. > > +- bus-width: shall be set to <8> for 8 bits parallel bus > > + or <10> for 10 bits parallel bus > > +- data-shift: shall be set to <2> for 8 bits parallel bus > > + (lines 9:2 are used) or <0> for 10 bits parallel bus > > defining what is required or optional might be hard. I don't see the > driver enforcing their presence and I assume they have safe default. > Maybe make them optional and specify what the defaul value is ? > Will do. > > > +- hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH respectively. > > + (Not required for bus-type equal 6) > > +- vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH respectively. > > + (Not required for bus-type equal 6) > > If they're not required, they're optional, aren't they ? :) > Agreed. > > +- pclk-sample: sample data on rising (1) or falling (0) edge of the pixel clock > > + signal. (Not required for bus-type equal 6) > > Why the pclk polarity is does not apply to BT.656 ? > No it should apply. > > +- bus-type: data bus type. Possible values are: > > + 5 - Parallel > > + 6 - Bt.656 > > Are we making this required, or do we expect this to be deduced > depending on which other properties have been specified ? Sakari it > seems you would like this to become a properties that has to be > specified most of the times, right ? (I tend to agree with that FWIW), > but does it impact retro-compatibility ? > Agreed can be deduced from other properties. But shall wait for Sakari to comment. Cheers, Prabhakar > > + > > Example: > > > > &i2c0 { > > -- > > 2.17.1 > >
Hi Jacopo, On Mon, Aug 3, 2020 at 12:39 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > Document endpoint properties required for parallel interface > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > --- > .../devicetree/bindings/media/i2c/ov772x.txt | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > I see you already have a patch for YAML conversion for OV772x binding [1], if you plan to post a v2 would you be OK to pick these changes as part of your conversion changes ? [1] https://www.spinics.net/lists/linux-media/msg173201.html Cheers, Prabhakar > diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt b/Documentation/devicetree/bindings/media/i2c/ov772x.txt > index 0b3ede5b8e6a..1f4153484717 100644 > --- a/Documentation/devicetree/bindings/media/i2c/ov772x.txt > +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.txt > @@ -21,6 +21,22 @@ subnode for its digital output video port, in accordance with the video > interface bindings defined in Documentation/devicetree/bindings/media/ > video-interfaces.txt. > > +Endpoint node required properties for parallel connection are: > +- remote-endpoint: a phandle to the bus receiver's endpoint node. > +- bus-width: shall be set to <8> for 8 bits parallel bus > + or <10> for 10 bits parallel bus > +- data-shift: shall be set to <2> for 8 bits parallel bus > + (lines 9:2 are used) or <0> for 10 bits parallel bus > +- hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH respectively. > + (Not required for bus-type equal 6) > +- vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH respectively. > + (Not required for bus-type equal 6) > +- pclk-sample: sample data on rising (1) or falling (0) edge of the pixel clock > + signal. (Not required for bus-type equal 6) > +- bus-type: data bus type. Possible values are: > + 5 - Parallel > + 6 - Bt.656 > + > Example: > > &i2c0 { > -- > 2.17.1 >
Hello Prabhakar, On Sat, Aug 15, 2020 at 11:34:33AM +0100, Lad, Prabhakar wrote: > Hi Jacopo, > > On Mon, Aug 3, 2020 at 12:39 PM Lad Prabhakar > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > > > Document endpoint properties required for parallel interface > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > --- > > .../devicetree/bindings/media/i2c/ov772x.txt | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > I see you already have a patch for YAML conversion for OV772x binding > [1], if you plan to post a v2 would you be OK to pick these changes as > part of your conversion changes ? Sure thing, I'll add the following properties to the series! Thanks j > > [1] https://www.spinics.net/lists/linux-media/msg173201.html > > Cheers, > Prabhakar > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt b/Documentation/devicetree/bindings/media/i2c/ov772x.txt > > index 0b3ede5b8e6a..1f4153484717 100644 > > --- a/Documentation/devicetree/bindings/media/i2c/ov772x.txt > > +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.txt > > @@ -21,6 +21,22 @@ subnode for its digital output video port, in accordance with the video > > interface bindings defined in Documentation/devicetree/bindings/media/ > > video-interfaces.txt. > > > > +Endpoint node required properties for parallel connection are: > > +- remote-endpoint: a phandle to the bus receiver's endpoint node. > > +- bus-width: shall be set to <8> for 8 bits parallel bus > > + or <10> for 10 bits parallel bus > > +- data-shift: shall be set to <2> for 8 bits parallel bus > > + (lines 9:2 are used) or <0> for 10 bits parallel bus > > +- hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH respectively. > > + (Not required for bus-type equal 6) > > +- vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH respectively. > > + (Not required for bus-type equal 6) > > +- pclk-sample: sample data on rising (1) or falling (0) edge of the pixel clock > > + signal. (Not required for bus-type equal 6) > > +- bus-type: data bus type. Possible values are: > > + 5 - Parallel > > + 6 - Bt.656 > > + > > Example: > > > > &i2c0 { > > -- > > 2.17.1 > >
On Mon, Aug 17, 2020 at 9:07 AM Jacopo Mondi <jacopo@jmondi.org> wrote: > > Hello Prabhakar, > > On Sat, Aug 15, 2020 at 11:34:33AM +0100, Lad, Prabhakar wrote: > > Hi Jacopo, > > > > On Mon, Aug 3, 2020 at 12:39 PM Lad Prabhakar > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > > > > > Document endpoint properties required for parallel interface > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > --- > > > .../devicetree/bindings/media/i2c/ov772x.txt | 16 ++++++++++++++++ > > > 1 file changed, 16 insertions(+) > > > > > I see you already have a patch for YAML conversion for OV772x binding > > [1], if you plan to post a v2 would you be OK to pick these changes as > > part of your conversion changes ? > > Sure thing, I'll add the following properties to the series! > Thank you Jacopo. I'll get on with the v3 version of the series. Cheers, Prabhakar > Thanks > j > > > > > [1] https://www.spinics.net/lists/linux-media/msg173201.html > > > > Cheers, > > Prabhakar > > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt b/Documentation/devicetree/bindings/media/i2c/ov772x.txt > > > index 0b3ede5b8e6a..1f4153484717 100644 > > > --- a/Documentation/devicetree/bindings/media/i2c/ov772x.txt > > > +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.txt > > > @@ -21,6 +21,22 @@ subnode for its digital output video port, in accordance with the video > > > interface bindings defined in Documentation/devicetree/bindings/media/ > > > video-interfaces.txt. > > > > > > +Endpoint node required properties for parallel connection are: > > > +- remote-endpoint: a phandle to the bus receiver's endpoint node. > > > +- bus-width: shall be set to <8> for 8 bits parallel bus > > > + or <10> for 10 bits parallel bus > > > +- data-shift: shall be set to <2> for 8 bits parallel bus > > > + (lines 9:2 are used) or <0> for 10 bits parallel bus > > > +- hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH respectively. > > > + (Not required for bus-type equal 6) > > > +- vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH respectively. > > > + (Not required for bus-type equal 6) > > > +- pclk-sample: sample data on rising (1) or falling (0) edge of the pixel clock > > > + signal. (Not required for bus-type equal 6) > > > +- bus-type: data bus type. Possible values are: > > > + 5 - Parallel > > > + 6 - Bt.656 > > > + > > > Example: > > > > > > &i2c0 { > > > -- > > > 2.17.1 > > >
diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt b/Documentation/devicetree/bindings/media/i2c/ov772x.txt index 0b3ede5b8e6a..1f4153484717 100644 --- a/Documentation/devicetree/bindings/media/i2c/ov772x.txt +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.txt @@ -21,6 +21,22 @@ subnode for its digital output video port, in accordance with the video interface bindings defined in Documentation/devicetree/bindings/media/ video-interfaces.txt. +Endpoint node required properties for parallel connection are: +- remote-endpoint: a phandle to the bus receiver's endpoint node. +- bus-width: shall be set to <8> for 8 bits parallel bus + or <10> for 10 bits parallel bus +- data-shift: shall be set to <2> for 8 bits parallel bus + (lines 9:2 are used) or <0> for 10 bits parallel bus +- hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH respectively. + (Not required for bus-type equal 6) +- vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH respectively. + (Not required for bus-type equal 6) +- pclk-sample: sample data on rising (1) or falling (0) edge of the pixel clock + signal. (Not required for bus-type equal 6) +- bus-type: data bus type. Possible values are: + 5 - Parallel + 6 - Bt.656 + Example: &i2c0 {
Document endpoint properties required for parallel interface Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> --- .../devicetree/bindings/media/i2c/ov772x.txt | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)