Message ID | 1596454753-13612-2-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
---|---|
State | Changes Requested, archived |
Delegated to: | Sakari Ailus |
Headers |
Received: from vger.kernel.org ([23.128.96.18]) by www.linuxtv.org with esmtp (Exim 4.92) (envelope-from <linux-media-owner@vger.kernel.org>) id 1k2Yk7-006OUr-Ht; Mon, 03 Aug 2020 11:35:00 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726536AbgHCLkP (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Mon, 3 Aug 2020 07:40:15 -0400 Received: from relmlor1.renesas.com ([210.160.252.171]:25127 "EHLO relmlie5.idc.renesas.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726398AbgHCLj5 (ORCPT <rfc822;linux-media@vger.kernel.org>); Mon, 3 Aug 2020 07:39:57 -0400 X-IronPort-AV: E=Sophos;i="5.75,429,1589209200"; d="scan'208";a="53742345" Received: from unknown (HELO relmlir6.idc.renesas.com) ([10.200.68.152]) by relmlie5.idc.renesas.com with ESMTP; 03 Aug 2020 20:39:34 +0900 Received: from localhost.localdomain (unknown [10.226.36.204]) by relmlir6.idc.renesas.com (Postfix) with ESMTP id 9C0B742389C3; Mon, 3 Aug 2020 20:39:31 +0900 (JST) From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> To: Jacopo Mondi <jacopo@jmondi.org>, Mauro Carvalho Chehab <mchehab@kernel.org>, Rob Herring <robh+dt@kernel.org>, Hans Verkuil <hverkuil-cisco@xs4all.nl>, Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>, Sakari Ailus <sakari.ailus@linux.intel.com>, linux-media@vger.kernel.org Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Biju Das <biju.das.jz@bp.renesas.com>, Prabhakar <prabhakar.csengg@gmail.com>, linux-renesas-soc@vger.kernel.org, Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Subject: [PATCH v2 1/3] dt-bindings: media: ov772x: Document endpoint properties Date: Mon, 3 Aug 2020 12:39:11 +0100 Message-Id: <1596454753-13612-2-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1596454753-13612-1-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com> References: <1596454753-13612-1-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com> Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-LSpam-Score: -2.4 (--) X-LSpam-Report: No, score=-2.4 required=5.0 tests=BAYES_00=-1.9,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1 autolearn=ham autolearn_force=no |
Series |
media: i2c: ov772x: Enable BT656 mode and test pattern support
|
|
Commit Message
Prabhakar Mahadev Lad
Aug. 3, 2020, 11:39 a.m. UTC
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(+)
Comments
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 {