Re: [PATCH 2/4] dt-bindings: display: ti: Add schema for AM625 OLDI Transmitter

[Rob Herring <robh@kernel.org>]

On Mon, May 13, 2024 at 02:07:44PM +0530, Aradhya Bhatia wrote:
> Hi Laurent,
> 
> Thank you for reviewing the patches!
> 
> On 13-May-24 01:04, Laurent Pinchart wrote:
> > Hi Aradhya,
> > 
> > Thank you for the patch.
> > 
> > On Sun, May 12, 2024 at 01:00:53AM +0530, Aradhya Bhatia wrote:
> >> Add devicetree binding schema for AM625 OLDI Transmitters.
> >>
> >> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
> >> ---
> >>  .../bindings/display/ti/ti,am625-oldi.yaml    | 153 ++++++++++++++++++
> >>  MAINTAINERS                                   |   1 +
> >>  2 files changed, 154 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml b/Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml
> >> new file mode 100644
> >> index 000000000000..0a96e600bc0b
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml
> >> @@ -0,0 +1,153 @@
> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/display/ti/ti,am625-oldi.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Texas Instruments AM625 OLDI Transmitter
> >> +
> >> +maintainers:
> >> +  - Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> >> +  - Aradhya Bhatia <a-bhatia1@ti.com>
> >> +
> >> +description: |
> >> +  The AM625 TI Keystone OpenLDI transmitter (OLDI TX) supports serialized RGB
> >> +  pixel data transmission between host and flat panel display over LVDS (Low
> >> +  Voltage Differential Sampling) interface. The OLDI TX consists of 7-to-1 data
> >> +  serializers, and 4-data and 1-clock LVDS outputs. It supports the LVDS output
> >> +  formats "jeida-18", "jeida-24" and "vesa-18", and can accept 24-bit RGB or
> >> +  padded and un-padded 18-bit RGB bus formats as input.
> >> +
> >> +properties:
> >> +  reg:
> >> +    maxItems: 1
> >> +
> >> +  clocks:
> >> +    maxItems: 1
> >> +    description: serial clock input for the OLDI transmitters
> >> +
> >> +  clock-names:
> >> +    const: s_clk
> >> +
> >> +  ti,companion-oldi:
> >> +    $ref: /schemas/types.yaml#/definitions/phandle
> >> +    description:
> >> +      phandle to companion OLDI transmitter. This property is mandatory for the
> >> +      primarty OLDI TX if the OLDI TXes are expected to work either in dual-lvds
> >> +      mode or in clone mode. This property should point to the secondary OLDI
> >> +      TX.
> >> +
> >> +  ti,secondary-oldi:
> >> +    type: boolean
> >> +    description: Boolean property to mark an OLDI TX as secondary node.
> >> +
> >> +  ti,oldi-io-ctrl:
> >> +    $ref: /schemas/types.yaml#/definitions/phandle
> >> +    description:
> >> +      phandle to syscon device node mapping OLDI IO_CTRL registers found in the
> >> +      control MMR region. This property is needed for OLDI interface to work.
> >> +
> >> +  ports:
> >> +    $ref: /schemas/graph.yaml#/properties/ports
> >> +
> >> +    properties:
> >> +      port@0:
> >> +        $ref: /schemas/graph.yaml#/properties/port
> >> +        description: Parallel RGB input port
> >> +
> >> +      port@1:
> >> +        $ref: /schemas/graph.yaml#/properties/port
> >> +        description: LVDS output port
> >> +
> >> +    required:
> >> +      - port@0
> >> +      - port@1
> >> +
> >> +allOf:
> >> +  - if:
> >> +      properties:
> >> +        ti,secondary-oldi: true
> >> +    then:
> >> +      properties:
> >> +        ti,companion-oldi: false
> >> +        ti,oldi-io-ctrl: false
> >> +        clocks: false
> >> +        clock-names: false
> >> +
> >> +    else:
> >> +      required:
> >> +        - ti,oldi-io-ctrl
> >> +        - clocks
> >> +        - clock-names
> >> +
> >> +required:
> >> +  - reg
> >> +  - ports
> >> +
> >> +additionalProperties: false
> >> +
> >> +examples:
> >> +  - |
> >> +    #include <dt-bindings/soc/ti,sci_pm_domain.h>
> >> +
> >> +    oldi_txes {
> >> +        #address-cells = <1>;
> >> +        #size-cells = <0>;
> >> +        oldi: oldi@0 {
> >> +            reg = <0>;
> >> +            clocks = <&k3_clks 186 0>;
> >> +            clock-names = "s_clk";
> >> +            ti,oldi-io-ctrl = <&dss_oldi_io_ctrl>;
> > 
> > What bus does this device live on ? Couldn't the I/O register space be
> > referenced by the reg property ?.
> > 
> 
> These registers are a part of the system-controller register space
> (ctrl_mmr0). The whole register set is owned by the main_conf[0]
> devicetree node, with sub-nodes pointing to specific regions. That's why
> I cannot reference these registers directly.

Then what does 'reg' represent? Looks like you just made up an index. If 
so, then this should probably be a child of &dss_oldi_io_ctrl instead. 
Or it should just be merged into that node.

Rob