Re: [LINUX PATCH 1/2] dt-bindings: display: xlnx: Add DSI 2.0 Tx subsystem documentation

Hi GVRao,

Thank you for the patch.

On Thu, May 12, 2022 at 07:23:12PM +0530, Venkateshwar Rao Gannavarapu wrote:

This patch adds dt binding for Xilinx DSI TX subsystem.

The Xilinx MIPI DSI (Display serial interface) Transmitter Subsystem implements the Mobile Industry Processor Interface (MIPI) based display interface. It supports the interface with the programmable logic (FPGA).

Signed-off-by: Venkateshwar Rao Gannavarapu venkateshwar.rao.gannavarapu@xilinx.com

.../bindings/display/xlnx/xlnx,dsi-tx.yaml | 105 +++++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml

diff --git a/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml b/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml new file mode 100644 index 0000000..8e23cf5 --- /dev/null +++ b/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml @@ -0,0 +1,105 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/xlnx/xlnx,dsi-tx.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml#

+title: Xilinx DSI Transmitter subsystem

+maintainers:

• • Venkateshwar Rao Gannavarapu venkateshwar.rao.gannavarapu@xilinx.com

+description: |

• The Xilinx DSI Transmitter Subsystem implements the Mobile Industry
• Processor Interface based display interface. It supports the interface
• with the programmable logic (FPGA).
• For more details refer to PG238 Xilinx MIPI DSI-V2.0 Tx Subsystem.

+properties:

• compatible:
• const: xlnx,dsi-tx-v2.0
• reg:
• maxItems: 1
• clocks:
• description: List of clock specifiers

You can drop the description, clocks is always a list of clock specifiers.

• items:

•  - description: AXI Lite CPU clock
  

•  - description: D-phy clock
  

s/D-phy/D-PHY/

• clock-names:
• items:

•  - const: s_axis_aclk
  

•  - const: dphy_clk_200M
  

• ports:
• $ref: /schemas/graph.yaml#/properties/ports
• properties:

•  port@0:
  

•    $ref: /schemas/graph.yaml#/$defs/port-base
  

•    description:
  

•      Input port node to receive pixel data from the
  

•      display controller. Exactly one endpoint must be
  

•      specified.
  

•    properties:
  

•      endpoint:
  

•        $ref: /schemas/graph.yaml#/properties/endpoint
  

•        description: sub-node describing the input from CRTC
  

"CRTC" is a DRM term, and DT bindings should document the hardware, not the driver. I'd drop the endpoint description as I don't think it brings much, and use /schemas/graph.yaml#/properties/port instead of port-base.

•  port@1:
  

•    $ref: /schemas/graph.yaml#/properties/port
  

•    description:
  

•      DSI output port node to the panel or the next bridge
  

•      in the chain
  

Same ere about "bridge". Maybe just

description: Output port node to DSI device.

+required:

• • compatible
• • reg
• • clocks
• • clock-names
• • ports

+additionalProperties: false

+examples:

• • |
• dsi_tx@80020000 {

•    compatible = "xlnx,dsi-tx-v2.0";
  

•    reg = <0x80020000 0x20000>;
  

•    clock-names = "s_axi_aclk", "dphy_clk_200M";
  

Wrong clock name.

•    clocks = <&misc_clk_0>, <&misc_clk_1>;
  

You need #address-cells = <1> and #size-cells = <0> here to specify an address for the panel.

This should have been caught by the schema validation. Please see Documentation/devicetree/bindings/writing-schema.rst for instructions on how to validate bindings.

•    panel@0 {
  

This will also fail to validate. You need to reference dsi-controller.yaml. You can check the other bindings for DSI controller for examples.

•            compatible = "auo,b101uan01";
  

•            reg = <0>;
  

•            port {
  

•                    panel_in: endpoint {
  

•                            remote-endpoint = <&mipi_dsi_out>;
  

•                    };
  

•            };
  

•    };
  

•    ports {
  

•            #address-cells = <1>;
  

•            #size-cells = <0>;
  

•            port@0 {
  

•                    #size-cells = <0>;
  

•                    #address-cells = <1>;
  

•                    reg = <0>;
  

•                    mipi_dsi_in: endpoint@0 {
  

•                            reg = <0>;
  

With a single endpoint you can drop the reg as well as the @0, and the size and address cells in the parent.

•                            remote-endpoint = <&pl_disp_crtc>;
  

•                    };
  

•            };
  

•            port@1 {
  

•                    reg = <1>;
  

•                    mipi_dsi_out: endpoint {
  

•                            remote-endpoint = <&panel_in>;
  

•                    };
  

•            };
  

•    };
  

• };