Re: [PATCH v10 1/4] dt-bindings:drm/bridge:anx7625:add vendor define flags

[Rob Herring <robh@kernel.org>]

On Mon, Jul 19, 2021 at 06:10:09PM +0800, Xin Ji wrote:
> Add 'bus-type' and 'data-lanes' define for port0. Define DP tx lane0,
> lane1 swing register array define, and audio enable flag.
> 
> The device which cannot pass DP tx PHY CTS caused by long PCB trace or
> embedded MUX, adjusting ANX7625 PHY parameters can pass the CTS test. The
> adjusting type include Pre-emphasis, Vp-p, Rterm(Resistor Termination)
> and Rsel(Driven Strength). Each lane has maximum 20 registers for
> these settings.
> 
> For the DP tx swing setting, each lane has 10 different combination for
> swing, as Pre0: swing3|swing2|swing1|swing0, Pre1: swing2|swing1|swing0,
> Pre2: swing1|swing0, Pre3: swing0.
> 
> Register definition as:
> [Boost_ctrl]
> These registers control post cursor manual, increase the Boost_Ctrl
> setting can increase Pre-emphasis value separately.
> Lane	Condition	Register address
> Lane0	Swing0_Pre0	0x7a:0x00 bit[3:0]
> Lane0	Swing1_Pre0	0x7a:0x01 bit[3:0]
> Lane0	Swing2_Pre0	0x7a:0x02 bit[3:0]
> Lane0	Swing3_Pre0	0x7a:0x03 bit[3:0]
> Lane0	Swing0_Pre1	0x7a:0x04 bit[3:0]
> Lane0	Swing1_Pre1	0x7a:0x05 bit[3:0]
> Lane0	Swing2_Pre1	0x7a:0x06 bit[3:0]
> Lane0	Swing0_Pre2	0x7a:0x07 bit[3:0]
> Lane0	Swing1_Pre2	0x7a:0x08 bit[3:0]
> Lane0	Swing0_Pre3	0x7a:0x09 bit[3:0]
> Lane1	Swing0_Pre0	0x7a:0x14 bit[3:0]
> Lane1	Swing1_Pre0	0x7a:0x15 bit[3:0]
> Lane1	Swing2_Pre0	0x7a:0x16 bit[3:0]
> Lane1	Swing3_Pre0	0x7a:0x17 bit[3:0]
> Lane1	Swing0_Pre1	0x7a:0x18 bit[3:0]
> Lane1	Swing1_Pre1	0x7a:0x19 bit[3:0]
> Lane1	Swing2_Pre1	0x7a:0x1a bit[3:0]
> Lane1	Swing0_Pre2	0x7a:0x1b bit[3:0]
> Lane1	Swing1_Pre2	0x7a:0x1c bit[3:0]
> Lane1	Swing0_Pre3	0x7a:0x1d bit[3:0]
> 
> [Swing_ctrl]
> These registers control swing manual, increase Swing_Ctrl setting can
> increase Vp-p value separately.
> Lane	Condition	Register address
> Lane0	Swing0_Pre0	0x7a:0x00 bit[6:4]
> Lane0	Swing1_Pre0	0x7a:0x01 bit[6:4]
> Lane0	Swing2_Pre0	0x7a:0x02 bit[6:4]
> Lane0	Swing3_Pre0	0x7a:0x03 bit[6:4]
> Lane0	Swing0_Pre1	0x7a:0x04 bit[6:4]
> Lane0	Swing1_Pre1	0x7a:0x05 bit[6:4]
> Lane0	Swing2_Pre1	0x7a:0x06 bit[6:4]
> Lane0	Swing0_Pre2	0x7a:0x07 bit[6:4]
> Lane0	Swing1_Pre2	0x7a:0x08 bit[6:4]
> Lane0	Swing0_Pre3	0x7a:0x09 bit[6:4]
> Lane1	Swing0_Pre0	0x7a:0x14 bit[6:4]
> Lane1	Swing1_Pre0	0x7a:0x15 bit[6:4]
> Lane1	Swing2_Pre0	0x7a:0x16 bit[6:4]
> Lane1	Swing3_Pre0	0x7a:0x17 bit[6:4]
> Lane1	Swing0_Pre1	0x7a:0x18 bit[6:4]
> Lane1	Swing1_Pre1	0x7a:0x19 bit[6:4]
> Lane1	Swing2_Pre1	0x7a:0x1a bit[6:4]
> Lane1	Swing0_Pre2	0x7a:0x1b bit[6:4]
> Lane1	Swing1_Pre2	0x7a:0x1c bit[6:4]
> Lane1	Swing0_Pre3	0x7a:0x1d bit[6:4]
> 
> [Rsel_ctrl]
> These registers control resistor compensation manual, increase Rsel_ctrl
> can increase the IO driven strength, increase Vp-p simultaneously.
> Lane	Condition	Register address
> Lane0	Swing0_Pre0	0x7a:0x0a bit[4:0]
> Lane0	Swing1_Pre0	0x7a:0x0b bit[4:0]
> Lane0	Swing2_Pre0	0x7a:0x0c bit[4:0]
> Lane0	Swing3_Pre0	0x7a:0x0d bit[4:0]
> Lane0	Swing0_Pre1	0x7a:0x0e bit[4:0]
> Lane0	Swing1_Pre1	0x7a:0x0f bit[4:0]
> Lane0	Swing2_Pre1	0x7a:0x10 bit[4:0]
> Lane0	Swing0_Pre2	0x7a:0x11 bit[4:0]
> Lane0	Swing1_Pre2	0x7a:0x12 bit[4:0]
> Lane0	Swing0_Pre3	0x7a:0x13 bit[4:0]
> Lane1	Swing0_Pre0	0x7a:0x1e bit[4:0]
> Lane1	Swing1_Pre0	0x7a:0x1f bit[4:0]
> Lane1	Swing2_Pre0	0x7a:0x20 bit[4:0]
> Lane1	Swing3_Pre0	0x7a:0x21 bit[4:0]
> Lane1	Swing0_Pre1	0x7a:0x22 bit[4:0]
> Lane1	Swing1_Pre1	0x7a:0x23 bit[4:0]
> Lane1	Swing2_Pre1	0x7a:0x24 bit[4:0]
> Lane1	Swing0_Pre2	0x7a:0x25 bit[4:0]
> Lane1	Swing1_Pre2	0x7a:0x26 bit[4:0]
> Lane1	Swing0_Pre3	0x7a:0x27 bit[4:0]
> 
> [Rterm_ctrl]
> These registers adjust 50ohm impedance of DP tx
> 00:55 ohm
> 01:50 ohm(default)
> 10:45 ohm
> 11:40 ohm
> Lane	Condition	Register address
> Lane0	Swing0_Pre0	0x7a:0x0a bit[6:5]
> Lane0	Swing1_Pre0	0x7a:0x0b bit[6:5]
> Lane0	Swing2_Pre0	0x7a:0x0c bit[6:5]
> Lane0	Swing3_Pre0	0x7a:0x0d bit[6:5]
> Lane0	Swing0_Pre1	0x7a:0x0e bit[6:5]
> Lane0	Swing1_Pre1	0x7a:0x0f bit[6:5]
> Lane0	Swing2_Pre1	0x7a:0x10 bit[6:5]
> Lane0	Swing0_Pre2	0x7a:0x11 bit[6:5]
> Lane0	Swing1_Pre2	0x7a:0x12 bit[6:5]
> Lane0	Swing0_Pre3	0x7a:0x13 bit[6:5]
> lane1	Swing0_Pre0	0x7a:0x1e bit[6:5]
> Lane1	Swing1_Pre0	0x7a:0x1f bit[6:5]
> Lane1	Swing2_Pre0	0x7a:0x20 bit[6:5]
> Lane1	Swing3_Pre0	0x7a:0x21 bit[6:5]
> Lane1	Swing0_Pre1	0x7a:0x22 bit[6:5]
> Lane1	Swing1_Pre1	0x7a:0x23 bit[6:5]
> Lane1	Swing2_Pre1	0x7a:0x24 bit[6:5]
> Lane1	Swing0_Pre2	0x7a:0x25 bit[6:5]
> Lane1	Swing1_Pre2	0x7a:0x26 bit[6:5]
> Lane1	Swing0_Pre3	0x7a:0x27 bit[6:5]

All this information should be in the properties description. But the 
above form is not all that clear in defining how to fill in the 
properties. You need to describe what is in each word of the properties 
and assume the reader has the datasheet. 
> 
> Signed-off-by: Xin Ji <xji@analogixsemi.com>
> ---
>  .../display/bridge/analogix,anx7625.yaml      | 55 ++++++++++++++++++-
>  1 file changed, 54 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> index ab48ab2f4240..77b160d7c269 100644
> --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> @@ -43,6 +43,24 @@ properties:
>    vdd33-supply:
>      description: Regulator that provides the supply 3.3V power.
>  
> +  analogix,lane0-swing:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 20
> +    description:
> +      an array of swing register setting for DP tx lane0 PHY.
> +
> +  analogix,lane1-swing:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 20
> +    description:
> +      an array of swing register setting for DP tx lane1 PHY.
> +
> +  analogix,audio-enable:
> +    type: boolean
> +    description: let the driver enable audio HDMI codec function or not.
> +
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
>  
> @@ -50,13 +68,43 @@ properties:
>        port@0:
>          $ref: /schemas/graph.yaml#/properties/port

This needs to be #/$defs/port-base instead since you are adding child 
properties.

Also needs 'unevaluatedProperties: false'.

>          description:
> -          Video port for MIPI DSI input.
> +          MIPI DSI/DPI input.
> +
> +        properties:
> +          endpoint:
> +            $ref: /schemas/media/video-interfaces.yaml#
> +            type: object
> +            additionalProperties: false
> +
> +            properties:
> +              remote-endpoint: true
> +              bus-type: true

All the possible bus-type values are supported by this h/w?

> +              data-lanes: true
> +
> +            required:
> +              - remote-endpoint

Drop this.

> +
> +        required:
> +          - endpoint

Drop this.

> +
>  
>        port@1:
>          $ref: /schemas/graph.yaml#/properties/port
>          description:
>            Video port for panel or connector.
>  
> +        properties:
> +          endpoint:
> +            $ref: /schemas/media/video-interfaces.yaml#

No additional properties, you don't need this reference or the rest of 
this addition.

> +            type: object
> +            additionalProperties: false
> +
> +            properties:
> +              remote-endpoint: true
> +
> +            required:
> +              - remote-endpoint
> +
>      required:
>        - port@0
>        - port@1
> @@ -87,6 +135,9 @@ examples:
>              vdd10-supply = <&pp1000_mipibrdg>;
>              vdd18-supply = <&pp1800_mipibrdg>;
>              vdd33-supply = <&pp3300_mipibrdg>;
> +            analogix,audio-enable;
> +            analogix,lane0-swing = <0x14 0x54 0x64 0x74 0x29 0x7b 0x77 0x5b>;
> +            analogix,lane1-swing = <0x14 0x54 0x64 0x74 0x29 0x7b 0x77 0x5b>;

If the values are only 8-bit, then make the type uint8-array.

>  
>              ports {
>                  #address-cells = <1>;
> @@ -96,6 +147,8 @@ examples:
>                      reg = <0>;
>                      anx7625_in: endpoint {
>                          remote-endpoint = <&mipi_dsi>;
> +                        bus-type = <5>;
> +                        data-lanes = <0 1 2 3>;
>                      };
>                  };
>  
> -- 
> 2.25.1
> 
>