2020-05-13 10:10:15

by Vinod Koul

Subject: [PATCH 2/3] dt-bindings: display: bridge: Add documentation for LT9611

Lontium LT9611 is a DSI to HDMI bridge which supports 2 DSI ports
and I2S port as input and one HDMI port as output

Signed-off-by: Vinod Koul <[email protected]>
---
.../display/bridge/lontium,lt9611.yaml | 178 ++++++++++++++++++
1 file changed, 178 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml

diff --git a/Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml b/Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml
new file mode 100644
index 000000000000..77ee8cc35cd8
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml
@@ -0,0 +1,178 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/lontium,lt9611.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Lontium LT9611 2 Port MIPI to HDMI Bridge
+
+maintainers:
+ - Vinod Koul <[email protected]>
+
+description: |
+ The LT9611 is a bridge device which converts DSI to HDMI
+
+properties:
+ compatible:
+ enum:
+ - lontium,lt9611
+
+ reg:
+ maxItems: 1
+ description: base I2C address of the device.
+
+ "#sound-dai-cells":
+ const: 1
+
+ interrupts:
+ maxItems: 1
+ description: interrupt line for the chip
+
+ reset-gpios:
+ maxItems: 1
+ description: GPIO connected to active high RESET pin.
+
+ vdd-supply:
+ description: Regulator for 1.8V MIPI phy power.
+
+ vcc-supply:
+ description: Regulator for 3.3V IO power.
+
+ ports:
+ type: object
+
+ properties:
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ port@0:
+ type: object
+ additionalProperties: false
+
+ description: |
+ HDMI port for HDMI output
+
+ properties:
+ reg:
+ const: 0
+
+ patternProperties:
+ endpoint:
+ type: object
+ additionalProperties: false
+
+ properties:
+ remote-endpoint: true
+
+ required:
+ - reg
+
+ port@1:
+ type: object
+ additionalProperties: false
+
+ description: |
+ MIPI port-1 for MIPI input
+
+ properties:
+ reg:
+ const: 1
+
+ patternProperties:
+ endpoint:
+ type: object
+ additionalProperties: false
+
+ properties:
+ remote-endpoint: true
+
+ required:
+ - reg
+
+ port@2:
+ type: object
+ additionalProperties: false
+
+ description: |
+ MIPI port-2 for MIPI input
+
+ properties:
+ reg:
+ const: 2
+
+ patternProperties:
+ endpoint:
+ type: object
+ additionalProperties: false
+
+ properties:
+ remote-endpoint: true
+
+ required:
+ - reg
+
+ required:
+ - "#address-cells"
+ - "#size-cells"
+ - port@0
+ - port@1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - vdd-supply
+ - vcc-supply
+ - ports
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c10 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ lt9611_codec: hdmi-bridge@3b {
+ compatible = "lontium,lt9611";
+ reg = <0x3b>;
+
+ reset-gpios = <&tlmm 128 GPIO_ACTIVE_HIGH>;
+ interrupts-extended = <&tlmm 84 IRQ_TYPE_EDGE_FALLING>;
+
+ vdd-supply = <&lt9611_1v8>;
+ vcc-supply = <&lt9611_3v3>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ lt9611_out: endpoint {
+ remote-endpoint = <&hdmi_con>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ lt9611_a: endpoint {
+ remote-endpoint = <&dsi0_out>;
+ };
+ };
+
+ port@2 {
+ reg = <2>;
+ lt9611_b: endpoint {
+ remote-endpoint = <&dsi1_out>;
+ };
+ };
+ };
+ };
+ };
--
2.25.4

2020-05-28 01:39:42

by Rob Herring (Arm)

Subject: Re: [PATCH 2/3] dt-bindings: display: bridge: Add documentation for LT9611

On Wed, 13 May 2020 15:35:32 +0530, Vinod Koul wrote:
> Lontium LT9611 is a DSI to HDMI bridge which supports 2 DSI ports
> and I2S port as input and one HDMI port as output
>
> Signed-off-by: Vinod Koul <[email protected]>
> ---
> .../display/bridge/lontium,lt9611.yaml | 178 ++++++++++++++++++
> 1 file changed, 178 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml
>

Reviewed-by: Rob Herring <[email protected]>

2020-05-28 01:53:04

by Laurent Pinchart

Subject: Re: [PATCH 2/3] dt-bindings: display: bridge: Add documentation for LT9611

Hi Vinod,

Thank you for the patch.

On Wed, May 13, 2020 at 03:35:32PM +0530, Vinod Koul wrote:
> Lontium LT9611 is a DSI to HDMI bridge which supports 2 DSI ports
> and I2S port as input and one HDMI port as output
>
> Signed-off-by: Vinod Koul <[email protected]>
> ---
> .../display/bridge/lontium,lt9611.yaml | 178 ++++++++++++++++++
> 1 file changed, 178 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml b/Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml
> new file mode 100644
> index 000000000000..77ee8cc35cd8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml
> @@ -0,0 +1,178 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/lontium,lt9611.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Lontium LT9611 2 Port MIPI to HDMI Bridge
> +
> +maintainers:
> + - Vinod Koul <[email protected]>
> +
> +description: |
> + The LT9611 is a bridge device which converts DSI to HDMI
> +
> +properties:
> + compatible:
> + enum:
> + - lontium,lt9611
> +
> + reg:
> + maxItems: 1
> + description: base I2C address of the device.
> +
> + "#sound-dai-cells":
> + const: 1
> +
> + interrupts:
> + maxItems: 1
> + description: interrupt line for the chip

I think you could drop the descriptions for the reg and interrupt
properties, they don't add much.

> +
> + reset-gpios:
> + maxItems: 1
> + description: GPIO connected to active high RESET pin.
> +
> + vdd-supply:
> + description: Regulator for 1.8V MIPI phy power.
> +
> + vcc-supply:
> + description: Regulator for 3.3V IO power.
> +
> + ports:
> + type: object
> +
> + properties:
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + port@0:
> + type: object
> + additionalProperties: false
> +
> + description: |
> + HDMI port for HDMI output

The usual practice is to have the input ports first, followed by the
output ports. Is there a reason not to follow that rule ?

> +
> + properties:
> + reg:
> + const: 0
> +
> + patternProperties:
> + endpoint:

If you want to use patternProperties, this should be

"^endpoint@[0-9]+$":

(including the quotes). Same below.

> + type: object
> + additionalProperties: false
> +
> + properties:
> + remote-endpoint: true

How about

remote-endpoint:
$ref: /schemas/types.yaml#/definitions/phandle

and the same below ?

You also need a reg property if multiple endpoints are present.

> +
> + required:
> + - reg
> +
> + port@1:
> + type: object
> + additionalProperties: false
> +
> + description: |
> + MIPI port-1 for MIPI input
> +
> + properties:
> + reg:
> + const: 1
> +
> + patternProperties:
> + endpoint:
> + type: object
> + additionalProperties: false
> +
> + properties:
> + remote-endpoint: true
> +
> + required:
> + - reg
> +
> + port@2:
> + type: object
> + additionalProperties: false
> +
> + description: |
> + MIPI port-2 for MIPI input

A description of how the two MIPI inputs differ would be useful. In
particular, are both mandatory, or is it valid to connect only one of
the two ? If using a single input is supported, can it be either, or
does it have to be the first one ? When using both inputs, what should
be connected to them ?

> +
> + properties:
> + reg:
> + const: 2
> +
> + patternProperties:
> + endpoint:
> + type: object
> + additionalProperties: false
> +
> + properties:
> + remote-endpoint: true
> +
> + required:
> + - reg
> +
> + required:
> + - "#address-cells"
> + - "#size-cells"
> + - port@0
> + - port@1
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - vdd-supply
> + - vcc-supply
> + - ports
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + i2c10 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + lt9611_codec: hdmi-bridge@3b {

Please drop unused labels.

> + compatible = "lontium,lt9611";
> + reg = <0x3b>;
> +
> + reset-gpios = <&tlmm 128 GPIO_ACTIVE_HIGH>;
> + interrupts-extended = <&tlmm 84 IRQ_TYPE_EDGE_FALLING>;
> +
> + vdd-supply = <&lt9611_1v8>;
> + vcc-supply = <&lt9611_3v3>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + lt9611_out: endpoint {
> + remote-endpoint = <&hdmi_con>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + lt9611_a: endpoint {
> + remote-endpoint = <&dsi0_out>;
> + };
> + };
> +
> + port@2 {
> + reg = <2>;
> + lt9611_b: endpoint {
> + remote-endpoint = <&dsi1_out>;
> + };
> + };
> + };
> + };
> + };

It's customary to end YAML schema files with ... on a separate line.

--
Regards,

Laurent Pinchart

2020-06-04 07:32:56

by Laurent Pinchart

Subject: Re: [PATCH 2/3] dt-bindings: display: bridge: Add documentation for LT9611

Hi Vinod,

On Thu, Jun 04, 2020 at 12:48:59PM +0530, Vinod Koul wrote:
> Hi Laurent,
>
> Sorry for late reply, I was out last week.

No worries.

> On 28-05-20, 04:48, Laurent Pinchart wrote:
> > > +
> > > + interrupts:
> > > + maxItems: 1
> > > + description: interrupt line for the chip
> >
> > I think you could drop the descriptions for the reg and interrupt
> > properties, they don't add much.
>
> Sure, will do
>
> > > + reset-gpios:
> > > + maxItems: 1
> > > + description: GPIO connected to active high RESET pin.
> > > +
> > > + vdd-supply:
> > > + description: Regulator for 1.8V MIPI phy power.
> > > +
> > > + vcc-supply:
> > > + description: Regulator for 3.3V IO power.
> > > +
> > > + ports:
> > > + type: object
> > > +
> > > + properties:
> > > + "#address-cells":
> > > + const: 1
> > > +
> > > + "#size-cells":
> > > + const: 0
> > > +
> > > + port@0:
> > > + type: object
> > > + additionalProperties: false
> > > +
> > > + description: |
> > > + HDMI port for HDMI output
> >
> > The usual practice is to have the input ports first, followed by the
> > output ports. Is there a reason not to follow that rule ?
>
> I was not aware of this rule, is it documented somewhere?
> Nevertheless will update..

I don't think it's documented, no. It's just a common practice.

> > > +
> > > + properties:
> > > + reg:
> > > + const: 0
> > > +
> > > + patternProperties:
> > > + endpoint:
> >
> > If you want to use patternProperties, this should be
> >
> > "^endpoint@[0-9]+$":
> >
> > (including the quotes). Same below.
>
> Ok
>
> > > + type: object
> > > + additionalProperties: false
> > > +
> > > + properties:
> > > + remote-endpoint: true
> >
> > How about
> >
> > remote-endpoint:
> > $ref: /schemas/types.yaml#/definitions/phandle
> >
> > and the same below ?
>
> Ok
>
> > You also need a reg property if multiple endpoints are present.
>
> Will update
>
> > > +
> > > + required:
> > > + - reg
> > > +
> > > + port@1:
> > > + type: object
> > > + additionalProperties: false
> > > +
> > > + description: |
> > > + MIPI port-1 for MIPI input
> > > +
> > > + properties:
> > > + reg:
> > > + const: 1
> > > +
> > > + patternProperties:
> > > + endpoint:
> > > + type: object
> > > + additionalProperties: false
> > > +
> > > + properties:
> > > + remote-endpoint: true
> > > +
> > > + required:
> > > + - reg
> > > +
> > > + port@2:
> > > + type: object
> > > + additionalProperties: false
> > > +
> > > + description: |
> > > + MIPI port-2 for MIPI input
> >
> > A description of how the two MIPI inputs differ would be useful. In
> > particular, are both mandatory, or is it valid to connect only one of
> > the two ? If using a single input is supported, can it be either, or
> > does it have to be the first one ? When using both inputs, what should
> > be connected to them ?
>
> Sure I will add details. port-1 is mandatory and port-2 optional. port-2
> is used in combination with port-1 to drive displays for higher
> resolution like 4k
>
> > > +
> > > + properties:
> > > + reg:
> > > + const: 2
> > > +
> > > + patternProperties:
> > > + endpoint:
> > > + type: object
> > > + additionalProperties: false
> > > +
> > > + properties:
> > > + remote-endpoint: true
> > > +
> > > + required:
> > > + - reg
> > > +
> > > + required:
> > > + - "#address-cells"
> > > + - "#size-cells"
> > > + - port@0
> > > + - port@1
> > > +
> > > +required:
> > > + - compatible
> > > + - reg
> > > + - interrupts
> > > + - vdd-supply
> > > + - vcc-supply
> > > + - ports
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > + - |
> > > + #include <dt-bindings/gpio/gpio.h>
> > > + #include <dt-bindings/interrupt-controller/irq.h>
> > > +
> > > + i2c10 {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + lt9611_codec: hdmi-bridge@3b {
> >
> > Please drop unused labels.
>
> ok
>
> > > + compatible = "lontium,lt9611";
> > > + reg = <0x3b>;
> > > +
> > > + reset-gpios = <&tlmm 128 GPIO_ACTIVE_HIGH>;
> > > + interrupts-extended = <&tlmm 84 IRQ_TYPE_EDGE_FALLING>;
> > > +
> > > + vdd-supply = <&lt9611_1v8>;
> > > + vcc-supply = <&lt9611_3v3>;
> > > +
> > > + ports {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + port@0 {
> > > + reg = <0>;
> > > + lt9611_out: endpoint {
> > > + remote-endpoint = <&hdmi_con>;
> > > + };
> > > + };
> > > +
> > > + port@1 {
> > > + reg = <1>;
> > > + lt9611_a: endpoint {
> > > + remote-endpoint = <&dsi0_out>;
> > > + };
> > > + };
> > > +
> > > + port@2 {
> > > + reg = <2>;
> > > + lt9611_b: endpoint {
> > > + remote-endpoint = <&dsi1_out>;
> > > + };
> > > + };
> > > + };
> > > + };
> > > + };
> >
> > It's customary to end YAML schema files with ... on a separate line.
>
> Will update
>
> Thanks for the review

--
Regards,

Laurent Pinchart

2020-06-04 07:53:15

by Vinod Koul

Subject: Re: [PATCH 2/3] dt-bindings: display: bridge: Add documentation for LT9611

Hi Laurent,

Sorry for late reply, I was out last week.

On 28-05-20, 04:48, Laurent Pinchart wrote:
> > +
> > + interrupts:
> > + maxItems: 1
> > + description: interrupt line for the chip
>
> I think you could drop the descriptions for the reg and interrupt
> properties, they don't add much.

Sure, will do

> > + reset-gpios:
> > + maxItems: 1
> > + description: GPIO connected to active high RESET pin.
> > +
> > + vdd-supply:
> > + description: Regulator for 1.8V MIPI phy power.
> > +
> > + vcc-supply:
> > + description: Regulator for 3.3V IO power.
> > +
> > + ports:
> > + type: object
> > +
> > + properties:
> > + "#address-cells":
> > + const: 1
> > +
> > + "#size-cells":
> > + const: 0
> > +
> > + port@0:
> > + type: object
> > + additionalProperties: false
> > +
> > + description: |
> > + HDMI port for HDMI output
>
> The usual practice is to have the input ports first, followed by the
> output ports. Is there a reason not to follow that rule ?

I was not aware of this rule, is it documented somewhere?
Nevertheless will update..

> > +
> > + properties:
> > + reg:
> > + const: 0
> > +
> > + patternProperties:
> > + endpoint:
>
> If you want to use patternProperties, this should be
>
> "^endpoint@[0-9]+$":
>
> (including the quotes). Same below.

Ok

>
> > + type: object
> > + additionalProperties: false
> > +
> > + properties:
> > + remote-endpoint: true
>
> How about
>
> remote-endpoint:
> $ref: /schemas/types.yaml#/definitions/phandle
>
> and the same below ?

Ok

>
> You also need a reg property if multiple endpoints are present.

Will update
>
> > +
> > + required:
> > + - reg
> > +
> > + port@1:
> > + type: object
> > + additionalProperties: false
> > +
> > + description: |
> > + MIPI port-1 for MIPI input
> > +
> > + properties:
> > + reg:
> > + const: 1
> > +
> > + patternProperties:
> > + endpoint:
> > + type: object
> > + additionalProperties: false
> > +
> > + properties:
> > + remote-endpoint: true
> > +
> > + required:
> > + - reg
> > +
> > + port@2:
> > + type: object
> > + additionalProperties: false
> > +
> > + description: |
> > + MIPI port-2 for MIPI input
>
> A description of how the two MIPI inputs differ would be useful. In
> particular, are both mandatory, or is it valid to connect only one of
> the two ? If using a single input is supported, can it be either, or
> does it have to be the first one ? When using both inputs, what should
> be connected to them ?

Sure I will add details. port-1 is mandatory and port-2 optional. port-2
is used in combination with port-1 to drive displays for higher
resolution like 4k

> > +
> > + properties:
> > + reg:
> > + const: 2
> > +
> > + patternProperties:
> > + endpoint:
> > + type: object
> > + additionalProperties: false
> > +
> > + properties:
> > + remote-endpoint: true
> > +
> > + required:
> > + - reg
> > +
> > + required:
> > + - "#address-cells"
> > + - "#size-cells"
> > + - port@0
> > + - port@1
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - vdd-supply
> > + - vcc-supply
> > + - ports
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/gpio/gpio.h>
> > + #include <dt-bindings/interrupt-controller/irq.h>
> > +
> > + i2c10 {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + lt9611_codec: hdmi-bridge@3b {
>
> Please drop unused labels.

ok

>
> > + compatible = "lontium,lt9611";
> > + reg = <0x3b>;
> > +
> > + reset-gpios = <&tlmm 128 GPIO_ACTIVE_HIGH>;
> > + interrupts-extended = <&tlmm 84 IRQ_TYPE_EDGE_FALLING>;
> > +
> > + vdd-supply = <&lt9611_1v8>;
> > + vcc-supply = <&lt9611_3v3>;
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port@0 {
> > + reg = <0>;
> > + lt9611_out: endpoint {
> > + remote-endpoint = <&hdmi_con>;
> > + };
> > + };
> > +
> > + port@1 {
> > + reg = <1>;
> > + lt9611_a: endpoint {
> > + remote-endpoint = <&dsi0_out>;
> > + };
> > + };
> > +
> > + port@2 {
> > + reg = <2>;
> > + lt9611_b: endpoint {
> > + remote-endpoint = <&dsi1_out>;
> > + };
> > + };
> > + };
> > + };
> > + };
>
> It's customary to end YAML schema files with ... on a separate line.

Will update

Thanks for the review
--
~Vinod