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

On Tue, Jul 13, 2021 at 04:10:10PM -0600, Rob Herring wrote:

On Wed, Jul 07, 2021 at 03:30:51PM +0800, Xin Ji wrote:

quoted

On Thu, Jun 24, 2021 at 01:57:22PM +0200, Robert Foss wrote:

quoted

Hey Xin,

I would like to merge this series now, but this patch needs a review
first. Maybe Laurent/Rob Herring are good candidates.


Rob.

Hi Rob, I get Laurent/Rob comments before, and explained why we needs
these DT properties, so far, I didn't get any response.

Do I have to go dig that up? If it was more than a week ago, assume I 
don't remember. This is 1 of 100 bindings a week.

Justify why this is needed in your commit message.

Hi Rob, I'll give more detail description in commit message.

quoted

Hi Rob Herring and Laurent, for the DT property lane0/1-swing, Google
engineer has strong demond for them, they don't want to move DP swing
adjusting to kernel, thus may cause change the driver code in each
project, so config them in DT is a best option.

Where's the ack from a Google engineer?

They didn't give the review ack, but we discussed it offline. Nicolas
Boichat known this.

Thanks,
Xin

quoted

Thanks,
Xin

quoted

On Tue, 22 Jun 2021 at 14:31, Xin Ji [off-list ref] wrote:

quoted

Add 'bus-type' and 'data-lanes' define for port0. Define DP tx lane0,
lane1 swing register array define, and audio enable flag.

Signed-off-by: Xin Ji <redacted>
---
 .../display/bridge/analogix,anx7625.yaml      | 57 ++++++++++++++++++-
 1 file changed, 56 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..9e604d19a3d5 100644
--- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml

@@ -43,6 +43,26 @@ 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, please don't
+      add this property, or contact vendor.

Why do we have the property if we're not supposed to add it.

quoted

quoted

quoted

+
+  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, please don't
+      add this property, or contact vendor.
+
+  analogix,audio-enable:
+    type: boolean
+    description: let the driver enable audio HDMI codec function or not.

Wouldn't we have a 'port' node if audio is to be enabled?

quoted

quoted

quoted

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

@@ -50,13 +70,43 @@ properties:
       port@0:
         $ref: /schemas/graph.yaml#/properties/port
         description:
-          Video port for MIPI DSI input.
+          MIPI DSI/DPI input.
+
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            type: object
+            additionalProperties: false

Use 'unevaluatedProperties: false' instead...

quoted

quoted

quoted

+
+            properties:
+              remote-endpoint: true

...And drop this.

quoted

quoted

quoted

+              bus-type: true

This device supports all the possible bus types? What's the default as 
it is not required?

quoted

quoted

quoted

+              data-lanes: true

And up to 8 lanes? 

quoted

quoted

quoted

+
+            required:
+              - remote-endpoint
+
+        required:
+          - endpoint

You can drop both 'required'.

quoted

quoted

quoted

+

       port@1:
         $ref: /schemas/graph.yaml#/properties/port
         description:
           Video port for panel or connector.

+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#

Doesn't look like anything from video-interfaces.yaml is used. This 
whole chunk is not needed.

quoted

quoted

quoted

+            type: object
+            additionalProperties: false
+
+            properties:
+              remote-endpoint: true
+
+            required:
+              - remote-endpoint
+
     required:
       - port@0
       - port@1

@@ -87,6 +137,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>;

             ports {
                 #address-cells = <1>;

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

--

2.25.1