Hi Sam, thanks for your comments.
On Mon, Apr 27, 2020 at 08:49:09PM +0200, Sam Ravnborg wrote:
Hi Xin Ji
On Mon, Apr 27, 2020 at 02:17:46PM +0800, 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.
Thanks for providing this binding. When you re-submit please also send to devicetree@vger.kernel.org.
OK
Running the binding through dt_binding_check gives me:
/home/sam/drm/linux.git/Documentation/devicetree/bindings/display/bridge/anx7625.yaml: ignoring, error in schema: warning: no schema found in file: /home/sam/drm/linux.git/Documentation/devicetree/bindings/display/bridge/anx7625.yaml make[2]: *** [/home/sam/drm/linux.git/Documentation/devicetree/bindings/Makefile:42: Documentation/devicetree/bindings/processed-schema.yaml] Error 255 make[2]: *** Waiting for unfinished jobs.... /home/sam/drm/linux.git/Documentation/devicetree/bindings/display/bridge/anx7625.yaml: Additional properties are not allowed ('example' was unexpected) /home/sam/drm/linux.git/Documentation/devicetree/bindings/display/bridge/anx7625.yaml: Additional properties are not allowed ('example' was unexpected)
See writing-schemas.rst for instruction installing tools etc.
OK
You can add support to your board with binding.
Example: anx7625_bridge: encoder@58 { compatible = "analogix,anx7625"; reg = <0x58>; status = "okay"; panel-flags = <1>; enable-gpios = <&pio 45 GPIO_ACTIVE_HIGH>; reset-gpios = <&pio 73 GPIO_ACTIVE_HIGH>; #address-cells = <1>; #size-cells = <0>;
port@0 { reg = <0>; anx_1_in: endpoint { remote-endpoint = <&mipi_dsi>; }; }; port@2 { reg = <2>; anx_1_out: endpoint { remote-endpoint = <&panel_in>; }; };};
Signed-off-by: Xin Ji xji@analogixsemi.com
.../bindings/display/bridge/anx7625.yaml | 91 ++++++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/anx7625.yaml
diff --git a/Documentation/devicetree/bindings/display/bridge/anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/anx7625.yaml new file mode 100644 index 0000000..1149ebb --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/anx7625.yaml
Name the file "analogix,anx7625.yaml". (We should rename anx6345.yaml, so others do not omit the company name)
OK
@@ -0,0 +1,91 @@ +# 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 xji@analogixsemi.com
+description: |
- The ANX7625 is an ultra-low power 4K Mobile HD Transmitter
- designed for portable devices.
+properties:
- "#address-cells": true
- "#size-cells": true
- compatible:
- items:
- const: analogix,anx7625- reg:
- maxItems: 1
- panel-flags:
- description: indicate the panel is internal or external.
- maxItems: 1
This property hint at something needs to be modelled in a different way. I do not recall other bindings need similar info.
I'll change it to vendor,int-property
- interrupts:
- maxItems: 1
A description would be nice.
OK
- enable-gpios:
- description: used for power on chip control, POWER_EN pin D2.
- maxItems: 1
- reset-gpios:
- description: used for reset chip control, RESET_N pin B7.
- maxItems: 1
- 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.Maybe explain how it differs from port@0 and why it is optional.
It seems this port is not need any more, I'll remove this port.
- port@2:
- type: object
- description:
A port node pointing to panel port node.Unless there is a good reason not to then please use a ports node, like you see in almost (all?) other bridge bindings.
I'll use ports node.
+required:
- "#address-cells"
- "#size-cells"
- compatible
- reg
- port@0
- port@2
additionalProperties: false??
OK
+example:
It must be named "examples:" - this is what dt_binding_check complains about.
OK
- |
- anx7625_bridge: encoder@58 {
compatible = "analogix,anx7625";reg = <0x58>;status = "okay";No status in examples.
OK
panel-flags = <1>;enable-gpios = <&pio 45 GPIO_ACTIVE_HIGH>;You need to include a header file to pull in GPIO_ACTIVE_HIGH - see what other bindings do.
reset-gpios = <&pio 73 GPIO_ACTIVE_HIGH>;#address-cells = <1>;#size-cells = <0>;port@0 {reg = <0>;Be consistent with indent. My personal preference is 4 spaces.
OK
anx_1_in: endpoint {remote-endpoint = <&mipi_dsi>;};};port@2 {reg = <2>;anx_1_out: endpoint {remote-endpoint = <&panel_in>;};};- };
Lot's of small comments. But if this is your first binding then this is expected. Looks forward for v2.
Sam