Re: [PATCH v1 1/2] dt-bindings: display: bridge: Document Solomon SSD2825

[Svyatoslav Ryhel <clamor95@xxxxxxxxx>]

чт, 13 лют. 2025 р. о 22:34 Conor Dooley <conor@xxxxxxxxxx> пише:
>
> On Thu, Feb 13, 2025 at 03:56:04PM +0200, Svyatoslav Ryhel wrote:
> > Add bindings for Solomon SSD2825 MIPI master bridge chip that connects an
> > application processor with traditional parallel LCD interface and an LCD
> > driver with MIPI slave interface. The SSD2825 supports both parallel RGB
> > interface and serial SPI interface.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@xxxxxxxxx>
> > ---
> >  .../display/bridge/solomon,ssd2825.yaml       | 140 ++++++++++++++++++
> >  1 file changed, 140 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/bridge/solomon,ssd2825.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/solomon,ssd2825.yaml b/Documentation/devicetree/bindings/display/bridge/solomon,ssd2825.yaml
> > new file mode 100644
> > index 000000000000..cd7ff971495c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bridge/solomon,ssd2825.yaml
> > @@ -0,0 +1,140 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/bridge/solomon,ssd2825.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Solomon SSD2825 RGB to MIPI-DSI bridge
> > +
> > +maintainers:
> > +  - Svyatoslav Ryhel <clamor95@xxxxxxxxx>
> > +
> > +allOf:
> > +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    const: solomon,ssd2825
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  reset-gpios: true
> > +
> > +  dvdd-supply:
> > +    description: Regulator for 1.2V digital power supply.
> > +
> > +  avdd-supply:
> > +    description: Regulator for 1.2V analog power supply.
> > +
> > +  vddio-supply:
> > +    description: Regulator for 1.8V IO power supply.
> > +
> > +  spi-max-frequency:
> > +    maximum: 1000000
> > +
> > +  spi-cpha: true
> > +  spi-cpol: true
>
> Should these be required? Supplies should really be required too, since
> the device probably cannot function without them?
>

No, since spi-* define mode in which device works. If both are present
it is mode 3, if none it is mode 0.

About supplies, device cannot work without power supply obviously, but
often exact supplies are not known and I would like to not enforce
someone to add random regulators just because they are mandatory.

> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    const: tx_clk
>
> Drop the _clk, since this cannot be anything else! clock-names isn't
> really useful when you have just one, so I'd be inclined to say remove
> it entirely...
>

TX_CLK is the name which datasheet refers to hence I have included
clock name solely to have clear link between datasheet clock
references and clock used here.

> > +  solomon,hs-zero-delay-ns:
> > +    description:
> > +      HS zero delay period
> > +    default: 133
> > +
> > +  solomon,hs-prep-delay-ns:
> > +    description:
> > +      HS prep delay period
> > +    default: 40
>
> Do these two have limits? Use maximum/minimum to set them if so.
> Cheers,
> Conor.
>

Datasheet does not specify limits actually, only defaults. I will try
to calculate boundaries.

> > +
> > +  ports:
> > +    $ref: /schemas/graph.yaml#/properties/ports
> > +
> > +    properties:
> > +      port@0:
> > +        $ref: /schemas/graph.yaml#/$defs/port-base
> > +        unevaluatedProperties: false
> > +        description:
> > +          Video port for RGB input
> > +
> > +        properties:
> > +          endpoint:
> > +            $ref: /schemas/graph.yaml#/$defs/endpoint-base
> > +            unevaluatedProperties: false
> > +
> > +            properties:
> > +              bus-width:
> > +                enum: [ 16, 18, 24 ]
> > +
> > +      port@1:
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +        description:
> > +          Video port for DSI output (panel or connector)
> > +
> > +    required:
> > +      - port@0
> > +      - port@1
> > +
> > +required:
> > +  - compatible
> > +  - ports
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +
> > +    spi {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        dsi@2 {
> > +            compatible = "solomon,ssd2825";
> > +            reg = <2>;
> > +
> > +            spi-max-frequency = <1000000>;
> > +
> > +            spi-cpha;
> > +            spi-cpol;
> > +
> > +            reset-gpios = <&gpio 114 GPIO_ACTIVE_LOW>;
> > +
> > +            dvdd-supply = <&vdd_1v2>;
> > +            avdd-supply = <&vdd_1v2>;
> > +            vddio-supply = <&vdd_1v8_io>;
> > +
> > +            solomon,hs-zero-delay-ns = <300>;
> > +            solomon,hs-prep-delay-ns = <65>;
> > +
> > +            clocks = <&ssd2825_tx_clk>;
> > +            clock-names = "tx_clk";
> > +
> > +            ports {
> > +                #address-cells = <1>;
> > +                #size-cells = <0>;
> > +
> > +                port@0 {
> > +                    reg = <0>;
> > +
> > +                    bridge_input: endpoint {
> > +                        remote-endpoint = <&dpi_output>;
> > +                        bus-width = <24>;
> > +                    };
> > +                };
> > +
> > +                port@1 {
> > +                    reg = <1>;
> > +
> > +                    bridge_output: endpoint {
> > +                        remote-endpoint = <&panel_input>;
> > +                    };
> > +                };
> > +            };
> > +        };
> > +    };
> > --
> > 2.43.0
> >