From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xin Ji Subject: Re: [PATCH v2 1/2] dt-bindings: drm/bridge: anx7625: MIPI to DP transmitter binding Date: Thu, 10 Oct 2019 03:38:40 +0000 Message-ID: <20191010033822.GB10005@xin-VirtualBox> References: <82d9e3d505699da8f32069844b3cfe7c9fbfd0f1.1570588741.git.xji@analogixsemi.com> <20191009121003.GM22998@pendragon.ideasonboard.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <20191009121003.GM22998@pendragon.ideasonboard.com> Content-Language: en-US Content-ID: <277EF7BA861E9D4F924E8A7EF332BAE0@namprd04.prod.outlook.com> Sender: linux-kernel-owner@vger.kernel.org To: Laurent Pinchart Cc: "devel@driverdev.osuosl.org" , Dan Carpenter , Andrzej Hajda , Neil Armstrong , Jonas Karlman , Jernej Skrabec , David Airlie , Daniel Vetter , "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , Sheng Pan List-Id: dri-devel@lists.freedesktop.org Hi Laurent Pinchart, thanks for your comments. On Wed, Oct 09, 2019 at 03:10:03PM +0300, Laurent Pinchart wrote: > Hi Xin Ji, >=20 > Thank you for the patch. >=20 > On Wed, Oct 09, 2019 at 09:27:07AM +0000, Xin Ji wrote: > > The ANX7625 is an ultra-low power 4K Mobile HD Transmitter designed > > for portable device. It converts MIPI to DisplayPort 1.3 4K. > >=20 > > You can add support to your board with binding. > >=20 > > Example: > > anx_bridge: anx7625@58 { > > compatible =3D "analogix,anx7625"; > > reg =3D <0x58>; > > enable-gpios =3D <&gpio0 45 GPIO_ACTIVE_LOW>; > > reset-gpios =3D <&gpio0 73 GPIO_ACTIVE_LOW>; > > status =3D "okay"; > > port@0 { > > reg =3D <0>; > > anx7625_1_in: endpoint { > > remote-endpoint =3D <&mipi_dsi_bridge_1>; > > }; > > }; > > }; > >=20 > > Signed-off-by: Xin Ji > > --- > > .../bindings/display/bridge/anx7625.yaml | 79 ++++++++++++++= ++++++++ > > 1 file changed, 79 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/display/bridge/an= x7625.yaml > >=20 > > diff --git a/Documentation/devicetree/bindings/display/bridge/anx7625.y= aml b/Documentation/devicetree/bindings/display/bridge/anx7625.yaml > > new file mode 100644 > > index 0000000..0ef6271 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/bridge/anx7625.yaml > > @@ -0,0 +1,79 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +# Copyright 2019 Analogix Semiconductor, Inc. > > +%YAML 1.2 > > +--- > > +$id: "http://devicetree.org/schemas/display/bridge/anx7625.yaml#" > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > > + > > +title: Analogix ANX7625 SlimPort (4K Mobile HD Transmitter) > > + > > +maintainers: > > + - Xin Ji > > + > > +description: | > > + The ANX7625 is an ultra-low power 4K Mobile HD Transmitter > > + designed for portable devices. > > + > > +properties: > > + compatible: > > + items: > > + - const: analogix,anx7625 > > + > > + reg: > > + maxItems: 1 > > + > > + hpd-gpios: > > + description: used for HPD interrupt > > + maxItems: 1 >=20 > You explained in your reply to v1 review that this describes the > interrupt generated by the ANX7625. It should be replaced by an > interrupts property. OK, I'll change it to interrupts property. >=20 > > + > > + enable-gpios: > > + description: used for power on chip control > > + maxItems: 1 > > + > > + reset-gpios: > > + description: used for reset chip control > > + maxItems: 1 >=20 > Could you please mention the exact name of the corresponding pins on the > chip for enable and reset ? OK, I'll mention the exact name of the corresponding pins on the chip. >=20 > > + > > + port@0: > > + type: object > > + description: > > + A port node pointing to MIPI DSI host port node. > > + > > + port@1: > > + type: object > > + description: > > + A port node pointing to MIPI DPI host port node. > > + > > + port@2: > > + type: object > > + description: > > + A port node pointing to external connector port node. > > + > > + port@3: > > + type: object > > + description: > > + A port node pointing to internal panel port node. > > + > > + port@4: > > + type: object > > + description: > > + A port node pointing to normal eDP port node. >=20 > I don't think three output ports is correct. Ports 3 and 4 are really > the same. I'm even unsure about port 2 and 3, someone with better > knowledge of USB-C and DisplayPort would be in a better position to > comment. I used it to distinguish different output, port@2 used for USB Type-C output, I need it to register external connector port notifier interface, and I think it is OK to conbine port@3 and port@4. >=20 > > + >=20 > You're missing the #address-cells and #size-cells properties required > for the ports. As the device is an I2C device we're lucky that the > parent will specify compatible address and size cells numbers, but I'm > not sure we should rely on that luck. OK, I'll add it. >=20 > Rob, how does yaml schema handle this ? >=20 > > +required: > > + - compatible > > + - reg > > + - port@0 | port@1 > > + > > +example: > > + - | > > + anx_bridge: anx7625@58 { >=20 > The node name should describe the device's function. How about > encoder@58 ? OK, I'll change it. >=20 > > + compatible =3D "analogix,anx7625"; > > + reg =3D <0x58>; > > + status =3D "okay"; > > + port@0 { > > + reg =3D <0>; > > + anx7625_1_in: endpoint { > > + remote-endpoint =3D <&mipi_dsi_bridge_1>; > > + }; > > + }; > > + }; >=20 > --=20 > Regards, >=20 > Laurent Pinchart