All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] dt-bindings: usb: typec: anx7688: start a binding document
@ 2024-02-09 19:51 Pavel Machek
  2024-02-11 13:56 ` Krzysztof Kozlowski
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Pavel Machek @ 2024-02-09 19:51 UTC (permalink / raw)
  To: phone-devel, kernel list, fiona.klute, martijn, samuel,
	heikki.krogerus, gregkh, linux-usb, robh+dt,
	krzysztof.kozlowski+dt, devicetree, megi

[-- Attachment #1: Type: text/plain, Size: 22975 bytes --]

Add binding for anx7688 usb type-c bridge. I don't have a datasheet,
but I did best I could.

Signed-off-by: Pavel Machek <pavel@ucw.cz>

diff --git a/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
new file mode 100644
index 000000000000..b9d60586937f
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
@@ -0,0 +1,140 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/analogix,anx7688.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analogix ANX7688 Type-C controller
+
+maintainers:
+  - Pavel Machek <pavel@ucw.cz>
+
+properties:
+  compatible:
+    enum:
+      - analogix,anx7688
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+  enable-gpios:
+    maxItems: 1
+  cabledet-gpios:
+    maxItems: 1
+
+  avdd10-supply:
+    description:
+      1.0V power supply
+  dvdd10-supply:
+    description:
+      1.0V power supply
+  avdd18-supply:
+    description:
+      1.8V power supply
+  dvdd18-supply:
+    description:
+      1.8V power supply
+  avdd33-supply:
+    description:
+      3.3V power supply
+  i2c-supply:
+    description:
+      Power supply
+  vconn-supply:
+    description:
+      Power supply
+  hdmi_vt-supply:
+    description:
+      Power supply
+
+  vbus-supply:
+    description:
+      Power supply
+  vbus_in-supply:
+    description:
+      Power supply
+
+  connector:
+    type: object
+    $ref: ../connector/usb-connector.yaml
+    unevaluatedProperties: false
+
+    description:
+      Properties for usb c connector.
+
+    properties:
+      compatible:
+        const: usb-c-connector
+
+      power-role: true
+
+      data-role: true
+
+      try-power-role: true
+
+    required:
+      - compatible
+
+required:
+  - compatible
+  - reg
+  - connector
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        typec@2c {
+            compatible = "analogix,anx7688";
+            reg = <0x2c>;
+            interrupts = <8 IRQ_TYPE_EDGE_FALLING>;
+            interrupt-parent = <&gpio0>;
+
+            enable-gpios = <&pio 3 10 GPIO_ACTIVE_LOW>; /* PD10 */
+            reset-gpios = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
+            cabledet-gpios = <&r_pio 0 8 GPIO_ACTIVE_HIGH>; /* PL8 */
+
+            avdd10-supply = <&reg_anx1v0>;
+            dvdd10-supply = <&reg_anx1v0>;
+            avdd18-supply = <&reg_ldo_io1>;
+            dvdd18-supply = <&reg_ldo_io1>;
+            avdd33-supply = <&reg_dcdc1>;
+            i2c-supply = <&reg_ldo_io0>;
+            vconn-supply = <&reg_vconn5v0>;
+            hdmi_vt-supply = <&reg_dldo1>;
+
+            vbus-supply = <&reg_usb_5v>;
+            vbus_in-supply = <&usb_power_supply>;
+
+            typec_con: connector {
+                compatible = "usb-c-connector";
+                power-role = "dual";
+                data-role = "dual";
+                try-power-role = "source";
+
+                ports {
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+                    port@0 {
+                        reg = <0>;
+                        typec_con_ep: endpoint {
+                            remote-endpoint = <&usbotg_hs_ep>;
+                        };
+                    };
+                };
+            };
+        };
+    };
+...
diff --git a/Documentation/devicetree/bindings/usb/generic-xhci.yaml b/Documentation/devicetree/bindings/usb/generic-xhci.yaml
index 594ebb3ee432..6ceafa4af292 100644
--- a/Documentation/devicetree/bindings/usb/generic-xhci.yaml
+++ b/Documentation/devicetree/bindings/usb/generic-xhci.yaml
@@ -9,9 +9,6 @@ title: USB xHCI Controller
 maintainers:
   - Mathias Nyman <mathias.nyman@intel.com>
 
-allOf:
-  - $ref: usb-xhci.yaml#
-
 properties:
   compatible:
     oneOf:
@@ -25,6 +22,11 @@ properties:
               - marvell,armada-380-xhci
               - marvell,armada-8k-xhci
           - const: generic-xhci
+      - description: Broadcom SoCs with power domains
+        items:
+          - enum:
+              - brcm,bcm2711-xhci
+          - const: brcm,xhci-brcm-v2
       - description: Broadcom STB SoCs with xHCI
         enum:
           - brcm,xhci-brcm-v2
@@ -49,6 +51,9 @@ properties:
       - const: core
       - const: reg
 
+  power-domains:
+    maxItems: 1
+
 unevaluatedProperties: false
 
 required:
@@ -56,6 +61,20 @@ required:
   - reg
   - interrupts
 
+allOf:
+  - $ref: usb-xhci.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: brcm,bcm2711-xhci
+    then:
+      required:
+        - power-domains
+    else:
+      properties:
+        power-domains: false
+
 examples:
   - |
     usb@f0931000 {
diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
index ee08b9c3721f..37cf5249e526 100644
--- a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
+++ b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
@@ -29,6 +29,11 @@ properties:
     description:
       the regulator that provides 3.3V core power to the hub.
 
+  peer-hub:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      phandle to the peer hub on the controller.
+
 required:
   - compatible
   - reg
diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
index e9644e333d78..924fd3d748a8 100644
--- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
+++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
@@ -124,6 +124,17 @@ properties:
       defined in the xHCI spec on MTK's controller.
     default: 5000
 
+  rx-fifo-depth:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      It is a quirk used to work around Gen1 isoc-in endpoint transfer issue
+      that still send out unexpected ACK after device finishes the burst
+      transfer with a short packet and cause an exception, specially on a 4K
+      camera device, it happens on controller before about IPM v1.6.0;
+      the side-effect is that it may cause performance drop about 10%,
+      including bulk transfer, prefer to use 3k here. The size is in bytes.
+    enum: [1024, 2048, 3072, 4096]
+
   # the following properties are only used for case 1
   wakeup-source:
     description: enable USB remote wakeup, see power/wakeup-source.txt
diff --git a/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml b/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml
index 28eb25ecba74..eaedb4cc6b6c 100644
--- a/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml
+++ b/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/usb/nxp,ptn5110.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: NXP PTN5110 Typec Port Cotroller
+title: NXP PTN5110 Type-C Port Controller
 
 maintainers:
   - Li Jun <jun.li@nxp.com>
diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
index 915c8205623b..63d150b216c5 100644
--- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
@@ -46,6 +46,8 @@ properties:
           - qcom,sm8350-dwc3
           - qcom,sm8450-dwc3
           - qcom,sm8550-dwc3
+          - qcom,sm8650-dwc3
+          - qcom,x1e80100-dwc3
       - const: qcom,dwc3
 
   reg:
@@ -97,12 +99,29 @@ properties:
       - const: apps-usb
 
   interrupts:
-    minItems: 1
-    maxItems: 4
+    description: |
+      Different types of interrupts are used based on HS PHY used on target:
+        - pwr_event: Used for wakeup based on other power events.
+        - hs_phY_irq: Apart from DP/DM/QUSB2 PHY interrupts, there is
+                       hs_phy_irq which is not triggered by default and its
+                       functionality is mutually exclusive to that of
+                       {dp/dm}_hs_phy_irq and qusb2_phy_irq.
+        - qusb2_phy: SoCs with QUSB2 PHY do not have separate DP/DM IRQs and
+                      expose only a single IRQ whose behavior can be modified
+                      by the QUSB2PHY_INTR_CTRL register. The required DPSE/
+                      DMSE configuration is done in QUSB2PHY_INTR_CTRL register
+                      of PHY address space.
+        - {dp/dm}_hs_phy_irq: These IRQ's directly reflect changes on the DP/
+                               DM pads of the SoC. These are used for wakeup
+                               only on SoCs with non-QUSB2 targets with
+                               exception of SDM670/SDM845/SM6350.
+        - ss_phy_irq: Used for remote wakeup in Super Speed mode of operation.
+    minItems: 2
+    maxItems: 5
 
   interrupt-names:
-    minItems: 1
-    maxItems: 4
+    minItems: 2
+    maxItems: 5
 
   qcom,select-utmi-as-pipe-clk:
     description:
@@ -263,6 +282,7 @@ allOf:
           contains:
             enum:
               - qcom,sc8280xp-dwc3
+              - qcom,x1e80100-dwc3
     then:
       properties:
         clocks:
@@ -288,8 +308,8 @@ allOf:
     then:
       properties:
         clocks:
-          minItems: 5
-          maxItems: 6
+          minItems: 4
+          maxItems: 5
         clock-names:
           oneOf:
             - items:
@@ -298,13 +318,11 @@ allOf:
                 - const: iface
                 - const: sleep
                 - const: mock_utmi
-                - const: bus
             - items:
                 - const: cfg_noc
                 - const: core
                 - const: sleep
                 - const: mock_utmi
-                - const: bus
 
   - if:
       properties:
@@ -318,6 +336,7 @@ allOf:
               - qcom,sm8250-dwc3
               - qcom,sm8450-dwc3
               - qcom,sm8550-dwc3
+              - qcom,sm8650-dwc3
     then:
       properties:
         clocks:
@@ -357,131 +376,96 @@ allOf:
         compatible:
           contains:
             enum:
-              - qcom,ipq4019-dwc3
+              - qcom,ipq5018-dwc3
               - qcom,ipq6018-dwc3
-              - qcom,ipq8064-dwc3
               - qcom,ipq8074-dwc3
-              - qcom,msm8994-dwc3
+              - qcom,msm8953-dwc3
+              - qcom,msm8998-dwc3
+    then:
+      properties:
+        interrupts:
+          minItems: 2
+          maxItems: 3
+        interrupt-names:
+          items:
+            - const: pwr_event
+            - const: qusb2_phy
+            - const: ss_phy_irq
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,msm8996-dwc3
               - qcom,qcs404-dwc3
+              - qcom,sdm660-dwc3
+              - qcom,sm6115-dwc3
+              - qcom,sm6125-dwc3
+    then:
+      properties:
+        interrupts:
+          minItems: 3
+          maxItems: 4
+        interrupt-names:
+          items:
+            - const: pwr_event
+            - const: qusb2_phy
+            - const: hs_phy_irq
+            - const: ss_phy_irq
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,ipq5332-dwc3
+              - qcom,x1e80100-dwc3
+    then:
+      properties:
+        interrupts:
+          maxItems: 4
+        interrupt-names:
+          items:
+            - const: pwr_event
+            - const: dp_hs_phy_irq
+            - const: dm_hs_phy_irq
+            - const: ss_phy_irq
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,ipq4019-dwc3
+              - qcom,ipq8064-dwc3
+              - qcom,msm8994-dwc3
+              - qcom,sa8775p-dwc3
               - qcom,sc7180-dwc3
+              - qcom,sc7280-dwc3
+              - qcom,sc8280xp-dwc3
               - qcom,sdm670-dwc3
               - qcom,sdm845-dwc3
               - qcom,sdx55-dwc3
               - qcom,sdx65-dwc3
               - qcom,sdx75-dwc3
               - qcom,sm4250-dwc3
-              - qcom,sm6125-dwc3
               - qcom,sm6350-dwc3
               - qcom,sm8150-dwc3
               - qcom,sm8250-dwc3
               - qcom,sm8350-dwc3
               - qcom,sm8450-dwc3
               - qcom,sm8550-dwc3
+              - qcom,sm8650-dwc3
     then:
       properties:
         interrupts:
-          items:
-            - description: The interrupt that is asserted
-                when a wakeup event is received on USB2 bus.
-            - description: The interrupt that is asserted
-                when a wakeup event is received on USB3 bus.
-            - description: Wakeup event on DM line.
-            - description: Wakeup event on DP line.
-        interrupt-names:
-          items:
-            - const: hs_phy_irq
-            - const: ss_phy_irq
-            - const: dm_hs_phy_irq
-            - const: dp_hs_phy_irq
-
-  - if:
-      properties:
-        compatible:
-          contains:
-            enum:
-              - qcom,msm8953-dwc3
-              - qcom,msm8996-dwc3
-              - qcom,msm8998-dwc3
-              - qcom,sm6115-dwc3
-    then:
-      properties:
-        interrupts:
-          maxItems: 2
-        interrupt-names:
-          items:
-            - const: hs_phy_irq
-            - const: ss_phy_irq
-
-  - if:
-      properties:
-        compatible:
-          contains:
-            enum:
-              - qcom,ipq5018-dwc3
-              - qcom,ipq5332-dwc3
-              - qcom,sdm660-dwc3
-    then:
-      properties:
-        interrupts:
-          minItems: 1
-          maxItems: 2
-        interrupt-names:
-          minItems: 1
-          items:
-            - const: hs_phy_irq
-            - const: ss_phy_irq
-
-  - if:
-      properties:
-        compatible:
-          contains:
-            enum:
-              - qcom,sc7280-dwc3
-    then:
-      properties:
-        interrupts:
-          minItems: 3
-          maxItems: 4
-        interrupt-names:
-          minItems: 3
-          items:
-            - const: hs_phy_irq
-            - const: dp_hs_phy_irq
-            - const: dm_hs_phy_irq
-            - const: ss_phy_irq
-
-  - if:
-      properties:
-        compatible:
-          contains:
-            enum:
-              - qcom,sc8280xp-dwc3
-    then:
-      properties:
-        interrupts:
-          maxItems: 4
+          minItems: 4
+          maxItems: 5
         interrupt-names:
           items:
             - const: pwr_event
-            - const: dp_hs_phy_irq
-            - const: dm_hs_phy_irq
-            - const: ss_phy_irq
-
-  - if:
-      properties:
-        compatible:
-          contains:
-            enum:
-              - qcom,sa8775p-dwc3
-    then:
-      properties:
-        interrupts:
-          minItems: 3
-          maxItems: 4
-        interrupt-names:
-          minItems: 3
-          items:
-            - const: pwr_event
+            - const: hs_phy_irq
             - const: dp_hs_phy_irq
             - const: dm_hs_phy_irq
             - const: ss_phy_irq
@@ -519,12 +503,13 @@ examples:
                           <&gcc GCC_USB30_PRIM_MASTER_CLK>;
             assigned-clock-rates = <19200000>, <150000000>;
 
-            interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>,
-                         <GIC_SPI 486 IRQ_TYPE_LEVEL_HIGH>,
+            interrupts = <GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 489 IRQ_TYPE_EDGE_BOTH>,
                          <GIC_SPI 488 IRQ_TYPE_EDGE_BOTH>,
-                         <GIC_SPI 489 IRQ_TYPE_EDGE_BOTH>;
-            interrupt-names = "hs_phy_irq", "ss_phy_irq",
-                          "dm_hs_phy_irq", "dp_hs_phy_irq";
+                         <GIC_SPI 486 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "pwr_event", "hs_phy_irq",
+                          "dp_hs_phy_irq", "dm_hs_phy_irq", "ss_phy_irq";
 
             power-domains = <&gcc USB30_PRIM_GDSC>;
 
diff --git a/Documentation/devicetree/bindings/usb/qcom,wcd939x-usbss.yaml b/Documentation/devicetree/bindings/usb/qcom,wcd939x-usbss.yaml
new file mode 100644
index 000000000000..7ddfd3313a18
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/qcom,wcd939x-usbss.yaml
@@ -0,0 +1,102 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/qcom,wcd939x-usbss.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm WCD9380/WCD9385 USB SubSystem Altmode/Analog Audio Switch
+
+maintainers:
+  - Neil Armstrong <neil.armstrong@linaro.org>
+
+description:
+  Qualcomm WCD9390/WCD9395 is a standalone Hi-Fi audio codec IC with a
+  functionally separate USB SubSystem for Altmode/Analog Audio Switch
+  accessible over an I2C interface.
+  The Audio Headphone and Microphone data path between the Codec and the
+  USB-C Mux subsystems are external to the IC, thus requiring DT port-endpoint
+  graph description to handle USB-C altmode & orientation switching for Audio
+  Accessory Mode.
+
+properties:
+  compatible:
+    oneOf:
+      - const: qcom,wcd9390-usbss
+      - items:
+          - const: qcom,wcd9395-usbss
+          - const: qcom,wcd9390-usbss
+
+  reg:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+  vdd-supply:
+    description: USBSS VDD power supply
+
+  mode-switch:
+    description: Flag the port as possible handle of altmode switching
+    type: boolean
+
+  orientation-switch:
+    description: Flag the port as possible handler of orientation switching
+    type: boolean
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/properties/port
+        description:
+          A port node to link the WCD939x USB SubSystem to a TypeC controller for the
+          purpose of handling altmode muxing and orientation switching.
+
+      port@1:
+        $ref: /schemas/graph.yaml#/properties/port
+        description:
+          A port node to link the WCD939x USB SubSystem to the Codec SubSystem for the
+          purpose of handling USB-C Audio Accessory Mode muxing and orientation switching.
+
+required:
+  - compatible
+  - reg
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        typec-mux@42 {
+            compatible = "qcom,wcd9390-usbss";
+            reg = <0x42>;
+
+            vdd-supply = <&vreg_bob>;
+
+            mode-switch;
+            orientation-switch;
+
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+                    wcd9390_usbss_sbu: endpoint {
+                        remote-endpoint = <&typec_sbu>;
+                    };
+                };
+                port@1 {
+                    reg = <1>;
+                    wcd9390_usbss_codec: endpoint {
+                        remote-endpoint = <&wcd9390_codec_usbss>;
+                    };
+                };
+            };
+        };
+    };
+...
diff --git a/Documentation/devicetree/bindings/usb/renesas,usbhs.yaml b/Documentation/devicetree/bindings/usb/renesas,usbhs.yaml
index bad55dfb2fa0..40ada78f2328 100644
--- a/Documentation/devicetree/bindings/usb/renesas,usbhs.yaml
+++ b/Documentation/devicetree/bindings/usb/renesas,usbhs.yaml
@@ -19,7 +19,7 @@ properties:
       - items:
           - enum:
               - renesas,usbhs-r7s9210   # RZ/A2
-              - renesas,usbhs-r9a07g043 # RZ/G2UL
+              - renesas,usbhs-r9a07g043 # RZ/G2UL and RZ/Five
               - renesas,usbhs-r9a07g044 # RZ/G2{L,LC}
               - renesas,usbhs-r9a07g054 # RZ/V2L
           - const: renesas,rza2-usbhs
diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index ee5af4b381b1..203a1eb66691 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -432,6 +432,10 @@ properties:
     items:
       enum: [1, 4, 8, 16, 32, 64, 128, 256]
 
+  num-hc-interrupters:
+    maximum: 8
+    default: 1
+
   port:
     $ref: /schemas/graph.yaml#/properties/port
     description:
diff --git a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
index 323d664ae06a..1745e28b3110 100644
--- a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
+++ b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
@@ -38,6 +38,10 @@ properties:
       - const: main
       - const: patch-address
 
+  reset-gpios:
+    description: GPIO used for the HRESET pin.
+    maxItems: 1
+
   wakeup-source: true
 
   interrupts:
@@ -90,6 +94,7 @@ additionalProperties: false
 
 examples:
   - |
+    #include <dt-bindings/gpio/gpio.h>
     #include <dt-bindings/interrupt-controller/irq.h>
     i2c {
         #address-cells = <1>;
@@ -106,6 +111,7 @@ examples:
 
             pinctrl-names = "default";
             pinctrl-0 = <&typec_pins>;
+            reset-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;
 
             typec_con: connector {
                 compatible = "usb-c-connector";
diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.yaml b/Documentation/devicetree/bindings/usb/usb-xhci.yaml
index 180a261c3e8f..4238ae896ef6 100644
--- a/Documentation/devicetree/bindings/usb/usb-xhci.yaml
+++ b/Documentation/devicetree/bindings/usb/usb-xhci.yaml
@@ -29,6 +29,12 @@ properties:
     description: Interrupt moderation interval
     default: 5000
 
+  num-hc-interrupters:
+    description: Maximum number of interrupters to allocate
+    $ref: /schemas/types.yaml#/definitions/uint16
+    minimum: 1
+    maximum: 1024
+
 additionalProperties: true
 
 examples:

-- 
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] dt-bindings: usb: typec: anx7688: start a binding document
  2024-02-09 19:51 [PATCH] dt-bindings: usb: typec: anx7688: start a binding document Pavel Machek
@ 2024-02-11 13:56 ` Krzysztof Kozlowski
  2024-02-12 11:02   ` Pavel Machek
  2024-02-21 22:45   ` Pavel Machek
  2024-02-23 21:28 ` [PATCHv2 1/2] " Pavel Machek
  2024-02-23 21:28 ` [PATCHv2 2/2] usb: typec: anx7688: Add driver for ANX7688 USB-C HDMI bridge Pavel Machek
  2 siblings, 2 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2024-02-11 13:56 UTC (permalink / raw)
  To: Pavel Machek, phone-devel, kernel list, fiona.klute, martijn,
	samuel, heikki.krogerus, gregkh, linux-usb, robh+dt,
	krzysztof.kozlowski+dt, devicetree, megi

On 09/02/2024 20:51, Pavel Machek wrote:
> Add binding for anx7688 usb type-c bridge. I don't have a datasheet,
> but I did best I could.
> 
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 

You miss proper diffstat which makes reviewing difficult.

Actually entire patch is corrupted and impossible to apply.

Anyway, where is any user of this? Nothing in commit msg explains this.


> diff --git a/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
> new file mode 100644
> index 000000000000..b9d60586937f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
> @@ -0,0 +1,140 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/analogix,anx7688.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analogix ANX7688 Type-C controller
> +
> +maintainers:
> +  - Pavel Machek <pavel@ucw.cz>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - analogix,anx7688
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  reset-gpios:
> +    maxItems: 1

blank line

> +  enable-gpios:
> +    maxItems: 1
> +  cabledet-gpios:
> +    maxItems: 1
> +
> +  avdd10-supply:
> +    description:
> +      1.0V power supply

Keep description in one line and add a blank line. This code is not that
readable.

> +  dvdd10-supply:
> +    description:
> +      1.0V power supply
> +  avdd18-supply:
> +    description:
> +      1.8V power supply
> +  dvdd18-supply:
> +    description:
> +      1.8V power supply
> +  avdd33-supply:
> +    description:
> +      3.3V power supply
> +  i2c-supply:
> +    description:
> +      Power supply

Drop all useless descriptions (so : true)

> +  vconn-supply:
> +    description:
> +      Power supply
> +  hdmi_vt-supply:
> +    description:
> +      Power supply
> +
> +  vbus-supply:
> +    description:
> +      Power supply
> +  vbus_in-supply:

No underscores in property names.

> +    description:
> +      Power supply
> +
> +  connector:
> +    type: object
> +    $ref: ../connector/usb-connector.yaml

Full path, so /schemas/connector/......

> +    unevaluatedProperties: false

Drop

> +
> +    description:
> +      Properties for usb c connector.

> +
> +    properties:
> +      compatible:
> +        const: usb-c-connector
> +
> +      power-role: true
> +
> +      data-role: true
> +
> +      try-power-role: true

I don't understand why do you have here properties. Do you see any
binding like this?

> +
> +    required:
> +      - compatible

Drop, why is it needed?

> +
> +required:
> +  - compatible
> +  - reg
> +  - connector
> +
> +additionalProperti

I don't know what's further but this patch is not a patch... Please read
submitting-patches, organize your patchset correctly into manageable
logical chunks and send them as proper patchset, not one junk.

b4 helps here a lot...

> 

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] dt-bindings: usb: typec: anx7688: start a binding document
  2024-02-11 13:56 ` Krzysztof Kozlowski
@ 2024-02-12 11:02   ` Pavel Machek
  2024-02-12 11:16     ` Krzysztof Kozlowski
  2024-02-21 22:45   ` Pavel Machek
  1 sibling, 1 reply; 14+ messages in thread
From: Pavel Machek @ 2024-02-12 11:02 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: phone-devel, kernel list, fiona.klute, martijn, samuel,
	heikki.krogerus, gregkh, linux-usb, robh+dt,
	krzysztof.kozlowski+dt, devicetree, megi

[-- Attachment #1: Type: text/plain, Size: 3694 bytes --]

Hi!
> > Add binding for anx7688 usb type-c bridge. I don't have a datasheet,
> > but I did best I could.
> > 
> > Signed-off-by: Pavel Machek <pavel@ucw.cz>
> > 
> 
> You miss proper diffstat which makes reviewing difficult.

> Actually entire patch is corrupted and impossible to apply.

Sorry about that.

> Anyway, where is any user of this? Nothing in commit msg explains
>  this.

User being is worked on:

https://lore.kernel.org/lkml/2024020126-emote-unsubtly-3394@gregkh/T/

Thanks for comments. I'll go through them and try to improve things.

Best regards,
								Pavel

> 
> > diff --git a/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
> > new file mode 100644
> > index 000000000000..b9d60586937f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
> > @@ -0,0 +1,140 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/usb/analogix,anx7688.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analogix ANX7688 Type-C controller
> > +
> > +maintainers:
> > +  - Pavel Machek <pavel@ucw.cz>
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - analogix,anx7688
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  reset-gpios:
> > +    maxItems: 1
> 
> blank line
> 
> > +  enable-gpios:
> > +    maxItems: 1
> > +  cabledet-gpios:
> > +    maxItems: 1
> > +
> > +  avdd10-supply:
> > +    description:
> > +      1.0V power supply
> 
> Keep description in one line and add a blank line. This code is not that
> readable.
> 
> > +  dvdd10-supply:
> > +    description:
> > +      1.0V power supply
> > +  avdd18-supply:
> > +    description:
> > +      1.8V power supply
> > +  dvdd18-supply:
> > +    description:
> > +      1.8V power supply
> > +  avdd33-supply:
> > +    description:
> > +      3.3V power supply
> > +  i2c-supply:
> > +    description:
> > +      Power supply
> 
> Drop all useless descriptions (so : true)
> 
> > +  vconn-supply:
> > +    description:
> > +      Power supply
> > +  hdmi_vt-supply:
> > +    description:
> > +      Power supply
> > +
> > +  vbus-supply:
> > +    description:
> > +      Power supply
> > +  vbus_in-supply:
> 
> No underscores in property names.
> 
> > +    description:
> > +      Power supply
> > +
> > +  connector:
> > +    type: object
> > +    $ref: ../connector/usb-connector.yaml
> 
> Full path, so /schemas/connector/......
> 
> > +    unevaluatedProperties: false
> 
> Drop
> 
> > +
> > +    description:
> > +      Properties for usb c connector.
> 
> > +
> > +    properties:
> > +      compatible:
> > +        const: usb-c-connector
> > +
> > +      power-role: true
> > +
> > +      data-role: true
> > +
> > +      try-power-role: true
> 
> I don't understand why do you have here properties. Do you see any
> binding like this?
> 
> > +
> > +    required:
> > +      - compatible
> 
> Drop, why is it needed?
> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - connector
> > +
> > +additionalProperti
> 
> I don't know what's further but this patch is not a patch... Please read
> submitting-patches, organize your patchset correctly into manageable
> logical chunks and send them as proper patchset, not one junk.
> 
> b4 helps here a lot...
> 
> > 
> 
> Best regards,
> Krzysztof

-- 
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] dt-bindings: usb: typec: anx7688: start a binding document
  2024-02-12 11:02   ` Pavel Machek
@ 2024-02-12 11:16     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2024-02-12 11:16 UTC (permalink / raw)
  To: Pavel Machek
  Cc: phone-devel, kernel list, fiona.klute, martijn, samuel,
	heikki.krogerus, gregkh, linux-usb, robh+dt,
	krzysztof.kozlowski+dt, devicetree, megi

On 12/02/2024 12:02, Pavel Machek wrote:
> Hi!
>>> Add binding for anx7688 usb type-c bridge. I don't have a datasheet,
>>> but I did best I could.
>>>
>>> Signed-off-by: Pavel Machek <pavel@ucw.cz>
>>>
>>
>> You miss proper diffstat which makes reviewing difficult.
> 
>> Actually entire patch is corrupted and impossible to apply.
> 
> Sorry about that.
> 
>> Anyway, where is any user of this? Nothing in commit msg explains
>>  this.
> 
> User being is worked on:
> 
> https://lore.kernel.org/lkml/2024020126-emote-unsubtly-3394@gregkh/T/
> 
> Thanks for comments. I'll go through them and try to improve things.

OK, please send bindings patch in the same patchset as the driver.
Preferably first bindings patch, then driver patch(es). The bindings
document ABI exposed by driver, so we expect to see and apply both of
them together (apply through subsystem, so USB in this case).

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] dt-bindings: usb: typec: anx7688: start a binding document
  2024-02-11 13:56 ` Krzysztof Kozlowski
  2024-02-12 11:02   ` Pavel Machek
@ 2024-02-21 22:45   ` Pavel Machek
  2024-02-22  8:22     ` Krzysztof Kozlowski
  1 sibling, 1 reply; 14+ messages in thread
From: Pavel Machek @ 2024-02-21 22:45 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: phone-devel, kernel list, fiona.klute, martijn, samuel,
	heikki.krogerus, gregkh, linux-usb, robh+dt,
	krzysztof.kozlowski+dt, devicetree, megi

[-- Attachment #1: Type: text/plain, Size: 1540 bytes --]

Hi!

> > +  reset-gpios:
> > +    maxItems: 1
> 
> blank line
> 
> > +  avdd10-supply:
> > +    description:
> > +      1.0V power supply
> 
> Keep description in one line and add a blank line. This code is not that
> readable.
> 
> > +  i2c-supply:
> > +    description:
> > +      Power supply
> 
> Drop all useless descriptions (so : true)

Ok, fixed, I hope I got it right.

> > +  vbus-supply:
> > +    description:
> > +      Power supply
> > +  vbus_in-supply:
> 
> No underscores in property names.

Ok.

> > +  connector:
> > +    type: object
> > +    $ref: ../connector/usb-connector.yaml
> 
> Full path, so /schemas/connector/......
> 
> > +    unevaluatedProperties: false
> 
> Drop

> > +
> > +    description:
> > +      Properties for usb c connector.
> 
> > +
> > +    properties:
> > +      compatible:
> > +        const: usb-c-connector
> > +
> > +      power-role: true
> > +
> > +      data-role: true
> > +
> > +      try-power-role: true
> 
> I don't understand why do you have here properties. Do you see any
> binding like this?

Well, it looks like I copied these mistakes from analogix,anx7411.yaml .

> > +
> > +    required:
> > +      - compatible
> 
> Drop, why is it needed?

Again, copy from analogix,anx7411.yaml .

Should I try to fix up analogix,anx7411.yaml , and submit that, first,
or is it easier if you do that?

Thanks and best regards,
								Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] dt-bindings: usb: typec: anx7688: start a binding document
  2024-02-21 22:45   ` Pavel Machek
@ 2024-02-22  8:22     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2024-02-22  8:22 UTC (permalink / raw)
  To: Pavel Machek
  Cc: phone-devel, kernel list, fiona.klute, martijn, samuel,
	heikki.krogerus, gregkh, linux-usb, robh+dt,
	krzysztof.kozlowski+dt, devicetree, megi

On 21/02/2024 23:45, Pavel Machek wrote:
>>> +    properties:
>>> +      compatible:
>>> +        const: usb-c-connector
>>> +
>>> +      power-role: true
>>> +
>>> +      data-role: true
>>> +
>>> +      try-power-role: true
>>
>> I don't understand why do you have here properties. Do you see any
>> binding like this?
> 
> Well, it looks like I copied these mistakes from analogix,anx7411.yaml .
> 
>>> +
>>> +    required:
>>> +      - compatible
>>
>> Drop, why is it needed?
> 
> Again, copy from analogix,anx7411.yaml .
> 
> Should I try to fix up analogix,anx7411.yaml , and submit that, first,
> or is it easier if you do that?

I'll fix it up, thanks.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCHv2 1/2] dt-bindings: usb: typec: anx7688: start a binding document
  2024-02-09 19:51 [PATCH] dt-bindings: usb: typec: anx7688: start a binding document Pavel Machek
  2024-02-11 13:56 ` Krzysztof Kozlowski
@ 2024-02-23 21:28 ` Pavel Machek
  2024-02-23 22:59   ` Rob Herring
  2024-03-11 21:22   ` Pavel Machek
  2024-02-23 21:28 ` [PATCHv2 2/2] usb: typec: anx7688: Add driver for ANX7688 USB-C HDMI bridge Pavel Machek
  2 siblings, 2 replies; 14+ messages in thread
From: Pavel Machek @ 2024-02-23 21:28 UTC (permalink / raw)
  To: phone-devel, kernel list, fiona.klute, martijn, samuel,
	heikki.krogerus, gregkh, linux-usb, robh+dt,
	krzysztof.kozlowski+dt, devicetree, megi

[-- Attachment #1: Type: text/plain, Size: 3970 bytes --]

Add binding for anx7688 usb type-c bridge. I don't have a datasheet,
but I did best I could.

Signed-off-by: Pavel Machek <pavel@ucw.cz>

---

v2: implement review feedback

diff --git a/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
new file mode 100644
index 000000000000..9e887eafb5fc
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
@@ -0,0 +1,127 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/analogix,anx7688.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+# Pin names can be deduced from
+# https://files.pine64.org/doc/PinePhone/PinePhone%20v1.2b%20Released%20Schematic.pdf
+
+title: Analogix ANX7688 Type-C controller
+
+maintainers:
+  - Pavel Machek <pavel@ucw.cz>
+
+properties:
+  compatible:
+    enum:
+      - analogix,anx7688
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+    description: GPIO controlling RESET_N (B7) pin.
+
+  enable-gpios:
+    maxItems: 1
+    description: GPIO controlling POWER_EN (D2) pin.
+
+  cabledet-gpios:
+    maxItems: 1
+    description: GPIO controlling CABLE_DET (C3) pin.
+
+  avdd10-supply:
+    description: 1.0V power supply going to AVDD10 (A4, ...) pins
+
+  dvdd10-supply:
+    description: 1.0V power supply going to DVDD10 (D6, ...) pins
+
+  avdd18-supply:
+    description: 1.8V power supply going to AVDD18 (E3, ...) pins
+
+  dvdd18-supply:
+    description: 1.8V power supply going to DVDD18 (G4, ...) pins
+
+  avdd33-supply:
+    description: 3.3V power supply going to AVDD33 (C4, ...) pins
+
+  i2c-supply: true
+  vconn-supply: true
+  hdmi-vt-supply: true
+  vbus-supply: true
+  vbus-in-supply: true
+
+  connector:
+    type: object
+    $ref: /schemas/connector/usb-connector.yaml
+
+    description:
+      Properties for usb c connector.
+
+    properties:
+      compatible:
+        const: usb-c-connector
+
+required:
+  - compatible
+  - reg
+  - connector
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        typec@2c {
+            compatible = "analogix,anx7688";
+            reg = <0x2c>;
+            interrupts = <8 IRQ_TYPE_EDGE_FALLING>;
+            interrupt-parent = <&gpio0>;
+
+            enable-gpios = <&pio 3 10 GPIO_ACTIVE_LOW>; /* PD10 */
+            reset-gpios = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
+            cabledet-gpios = <&r_pio 0 8 GPIO_ACTIVE_HIGH>; /* PL8 */
+
+            avdd10-supply = <&reg_anx1v0>;
+            dvdd10-supply = <&reg_anx1v0>;
+            avdd18-supply = <&reg_ldo_io1>;
+            dvdd18-supply = <&reg_ldo_io1>;
+            avdd33-supply = <&reg_dcdc1>;
+            i2c-supply = <&reg_ldo_io0>;
+            vconn-supply = <&reg_vconn5v0>;
+            hdmi_vt-supply = <&reg_dldo1>;
+
+            vbus-supply = <&reg_usb_5v>;
+            vbus-in-supply = <&usb_power_supply>;
+
+            typec_con: connector {
+                compatible = "usb-c-connector";
+                power-role = "dual";
+                data-role = "dual";
+                try-power-role = "source";
+
+                ports {
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+                    port@0 {
+                        reg = <0>;
+                        typec_con_ep: endpoint {
+                            remote-endpoint = <&usbotg_hs_ep>;
+                        };
+                    };
+                };
+            };
+        };
+    };
+...

-- 
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCHv2 2/2] usb: typec: anx7688: Add driver for ANX7688 USB-C HDMI bridge
  2024-02-09 19:51 [PATCH] dt-bindings: usb: typec: anx7688: start a binding document Pavel Machek
  2024-02-11 13:56 ` Krzysztof Kozlowski
  2024-02-23 21:28 ` [PATCHv2 1/2] " Pavel Machek
@ 2024-02-23 21:28 ` Pavel Machek
  2024-03-11 21:22   ` Pavel Machek
  2024-03-12 14:12   ` Heikki Krogerus
  2 siblings, 2 replies; 14+ messages in thread
From: Pavel Machek @ 2024-02-23 21:28 UTC (permalink / raw)
  To: phone-devel, kernel list, fiona.klute, martijn, samuel,
	heikki.krogerus, gregkh, linux-usb, megi

[-- Attachment #1: Type: text/plain, Size: 58335 bytes --]

From: Ondrej Jirman <megi@xff.cz>

This is driver for ANX7688 USB-C HDMI, with flashing and debugging
features removed. ANX7688 is rather criticial piece on PinePhone,
there's no display and no battery charging without it.

There's likely more work to be done here, but having basic support
in mainline is needed to be able to work on the other stuff
(networking, cameras, power management).

Signed-off-by: Ondrej Jirman <megi@xff.cz>
Co-developed-by: Martijn Braam <martijn@brixit.nl>
Co-developed-by: Samuel Holland <samuel@sholland.org>
Signed-off-by: Pavel Machek <pavel@ucw.cz>

---

v2: Fix checkpatch stuff. Some cleanups, adapt to dts format in 1/2.

diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 2f80c2792dbd..c9043ae61546 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -64,6 +64,17 @@ config TYPEC_ANX7411
 	  If you choose to build this driver as a dynamically linked module, the
 	  module will be called anx7411.ko.
 
+config TYPEC_ANX7688
+	tristate "Analogix ANX7688 Type-C DRP Port controller and mux driver"
+	depends on I2C
+	depends on USB_ROLE_SWITCH
+	help
+	  Say Y or M here if your system has Analogix ANX7688 Type-C Bridge
+	  controller driver.
+
+	  If you choose to build this driver as a dynamically linked module, the
+	  module will be called anx7688.ko.
+
 config TYPEC_RT1719
 	tristate "Richtek RT1719 Sink Only Type-C controller driver"
 	depends on USB_ROLE_SWITCH || !USB_ROLE_SWITCH
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 7a368fea61bc..3f8ff94ad294 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_TYPEC_TCPM)	+= tcpm/
 obj-$(CONFIG_TYPEC_UCSI)	+= ucsi/
 obj-$(CONFIG_TYPEC_TPS6598X)	+= tipd/
 obj-$(CONFIG_TYPEC_ANX7411)	+= anx7411.o
+obj-$(CONFIG_TYPEC_ANX7688)	+= anx7688.o
 obj-$(CONFIG_TYPEC_HD3SS3220)	+= hd3ss3220.o
 obj-$(CONFIG_TYPEC_STUSB160X) 	+= stusb160x.o
 obj-$(CONFIG_TYPEC_RT1719)	+= rt1719.o
diff --git a/drivers/usb/typec/anx7688.c b/drivers/usb/typec/anx7688.c
new file mode 100644
index 000000000000..14d033bbc0c7
--- /dev/null
+++ b/drivers/usb/typec/anx7688.c
@@ -0,0 +1,1927 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ANX7688 USB-C HDMI bridge/PD driver
+ *
+ * Warning, this driver is somewhat PinePhone specific.
+ *
+ * How this works:
+ * - this driver allows to program firmware into ANX7688 EEPROM, and
+ *   initialize it
+ * - it then communicates with the firmware running on the OCM (on-chip
+ *   microcontroller)
+ * - it detects whether there is cable plugged in or not and powers
+ *   up or down the ANX7688 based on that
+ * - when the cable is connected the firmware on the OCM will handle
+ *   the detection of the nature of the device on the other end
+ *   of the USB-C cable
+ * - this driver then communicates with the USB phy to let it swap
+ *   data roles accordingly
+ * - it also enables VBUS and VCONN regulators as appropriate
+ * - USB phy driver (Allwinner) needs to know whether to switch to
+ *   device or host mode, or whether to turn off
+ * - when the firmware detects SRC.1.5A or SRC.3.0A via CC pins
+ *   or something else via PD, it notifies this driver via software
+ *   interrupt and this driver will determine how to update the TypeC
+ *   port status and what input current limit is appropriate
+ * - input current limit determination happens 500ms after cable
+ *   insertion or hard reset (delay is necessary to determine whether
+ *   the remote end is PD capable or not)
+ * - this driver tells to the PMIC driver that the input current limit
+ *   needs to be changed
+ * - this driver also monitors PMIC status and re-sets the input current
+ *   limit if it changes for some reason (due to PMIC internal decision
+ *   making) (this is disabled for now)
+ *
+ * ANX7688 FW behavior as observed:
+ *
+ * - DO NOT SET MORE THAN 1 SINK CAPABILITY! Firmware will ignore what
+ *   you set and send hardcoded PDO_BATT 5-21V 30W message!
+ *
+ * Product brief:
+ * https://www.analogix.com/en/system/files/AA-002281-PB-6-ANX7688_Product_Brief_0.pdf
+ * Development notes:
+ * https://xnux.eu/devices/feature/anx7688.html
+ */
+
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/extcon-provider.h>
+#include <linux/firmware.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/irqreturn.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_irq.h>
+#include <linux/power_supply.h>
+#include <linux/regulator/consumer.h>
+#include <linux/usb/pd.h>
+#include <linux/usb/role.h>
+#include <linux/usb/typec.h>
+
+/* firmware regs */
+
+#define ANX7688_REG_VBUS_OFF_DELAY_TIME 0x22
+#define ANX7688_REG_FEATURE_CTRL        0x27
+#define ANX7688_REG_EEPROM_LOAD_STATUS1 0x11
+#define ANX7688_REG_EEPROM_LOAD_STATUS0 0x12
+#define ANX7688_REG_FW_VERSION1         0x15
+#define ANX7688_REG_FW_VERSION0         0x16
+
+#define ANX7688_EEPROM_FW_LOADED	0x01
+
+#define ANX7688_REG_STATUS_INT_MASK     0x17
+#define ANX7688_REG_STATUS_INT          0x28
+#define ANX7688_IRQS_RECEIVED_MSG       BIT(0)
+#define ANX7688_IRQS_RECEIVED_ACK       BIT(1)
+#define ANX7688_IRQS_VCONN_CHANGE       BIT(2)
+#define ANX7688_IRQS_VBUS_CHANGE        BIT(3)
+#define ANX7688_IRQS_CC_STATUS_CHANGE   BIT(4)
+#define ANX7688_IRQS_DATA_ROLE_CHANGE   BIT(5)
+
+#define ANX7688_REG_STATUS              0x29
+#define ANX7688_VCONN_STATUS            BIT(2) /* 0 = off  1 = on */
+#define ANX7688_VBUS_STATUS             BIT(3) /* 0 = off  1 = on */
+#define ANX7688_DATA_ROLE_STATUS        BIT(5) /* 0 = device 1 = host */
+
+#define ANX7688_REG_CC_STATUS           0x2a
+#define ANX7688_REG_TRY_UFP_TIMER       0x23
+#define ANX7688_REG_TIME_CTRL           0x24
+
+#define ANX7688_REG_MAX_VOLTAGE         0x1b
+#define ANX7688_REG_MAX_POWER           0x1c
+#define ANX7688_REG_MIN_POWER           0x1d
+#define ANX7688_REG_MAX_VOLTAGE_STATUS  0x1e
+#define ANX7688_REG_MAX_POWER_STATUS    0x1f
+
+#define ANX7688_SOFT_INT_MASK           0x7f
+
+/* tcpc regs */
+
+#define ANX7688_TCPC_REG_VENDOR_ID0     0x00
+#define ANX7688_TCPC_REG_VENDOR_ID1     0x01
+#define ANX7688_TCPC_REG_ALERT0         0x10
+#define ANX7688_TCPC_REG_ALERT1         0x11
+#define ANX7688_TCPC_REG_ALERT_MASK0    0x12
+#define ANX7688_TCPC_REG_ALERT_MASK1    0x13
+#define ANX7688_TCPC_REG_INTERFACE_SEND 0x30
+#define ANX7688_TCPC_REG_INTERFACE_RECV 0x51
+
+/* hw regs */
+
+#define ANX7688_REG_IRQ_EXT_SOURCE0     0x3e
+#define ANX7688_REG_IRQ_EXT_SOURCE1     0x4e
+#define ANX7688_REG_IRQ_EXT_SOURCE2     0x4f
+#define ANX7688_REG_IRQ_EXT_MASK0       0x3b
+#define ANX7688_REG_IRQ_EXT_MASK1       0x3c
+#define ANX7688_REG_IRQ_EXT_MASK2       0x3d
+#define ANX7688_REG_IRQ_SOURCE0         0x54
+#define ANX7688_REG_IRQ_SOURCE1         0x55
+#define ANX7688_REG_IRQ_SOURCE2         0x56
+#define ANX7688_REG_IRQ_MASK0           0x57
+#define ANX7688_REG_IRQ_MASK1           0x58
+#define ANX7688_REG_IRQ_MASK2           0x59
+
+#define ANX7688_IRQ2_SOFT_INT           BIT(2)
+
+#define ANX7688_REG_USBC_RESET_CTRL		0x05
+#define ANX7688_USBC_RESET_CTRL_OCM_RESET	BIT(4)
+
+/* ocm messages */
+
+#define ANX7688_OCM_MSG_PWR_SRC_CAP     0x00
+#define ANX7688_OCM_MSG_PWR_SNK_CAP     0x01
+#define ANX7688_OCM_MSG_DP_SNK_IDENTITY 0x02
+#define ANX7688_OCM_MSG_SVID            0x03
+#define ANX7688_OCM_MSG_GET_DP_SNK_CAP  0x04
+#define ANX7688_OCM_MSG_ACCEPT          0x05
+#define ANX7688_OCM_MSG_REJECT          0x06
+#define ANX7688_OCM_MSG_PSWAP_REQ       0x10
+#define ANX7688_OCM_MSG_DSWAP_REQ       0x11
+#define ANX7688_OCM_MSG_GOTO_MIN_REQ    0x12
+#define ANX7688_OCM_MSG_VCONN_SWAP_REQ  0x13
+#define ANX7688_OCM_MSG_VDM             0x14
+#define ANX7688_OCM_MSG_DP_SNK_CFG      0x15
+#define ANX7688_OCM_MSG_PWR_OBJ_REQ     0x16
+#define ANX7688_OCM_MSG_PD_STATUS_REQ   0x17
+#define ANX7688_OCM_MSG_DP_ALT_ENTER    0x19
+#define ANX7688_OCM_MSG_DP_ALT_EXIT     0x1a
+#define ANX7688_OCM_MSG_GET_SNK_CAP     0x1b
+#define ANX7688_OCM_MSG_RESPONSE_TO_REQ 0xf0
+#define ANX7688_OCM_MSG_SOFT_RST        0xf1
+#define ANX7688_OCM_MSG_HARD_RST        0xf2
+#define ANX7688_OCM_MSG_RESTART         0xf3
+
+static const char * const anx7688_supply_names[] = {
+	"avdd33",
+	"avdd18",
+	"dvdd18",
+	"avdd10",
+	"dvdd10",
+	"i2c",
+	"hdmi-vt",
+
+	"vconn", // power for VCONN1/VCONN2 switches
+	"vbus", // vbus power
+};
+
+#define ANX7688_NUM_SUPPLIES ARRAY_SIZE(anx7688_supply_names)
+#define ANX7688_NUM_ALWAYS_ON_SUPPLIES (ANX7688_NUM_SUPPLIES - 1)
+
+#define ANX7688_I2C_INDEX (ANX7688_NUM_SUPPLIES - 4)
+#define ANX7688_VCONN_INDEX (ANX7688_NUM_SUPPLIES - 2)
+#define ANX7688_VBUS_INDEX (ANX7688_NUM_SUPPLIES - 1)
+
+enum {
+	ANX7688_F_POWERED,
+	ANX7688_F_CONNECTED,
+	ANX7688_F_FW_FAILED,
+	ANX7688_F_PWRSUPPLY_CHANGE,
+	ANX7688_F_CURRENT_UPDATE,
+};
+
+struct anx7688 {
+	struct device *dev;
+	struct i2c_client *client;
+	struct i2c_client *client_tcpc;
+	struct regulator_bulk_data supplies[ANX7688_NUM_SUPPLIES];
+	struct power_supply *vbus_in_supply;
+	struct notifier_block vbus_in_nb;
+	int input_current_limit; // mA
+	struct gpio_desc *gpio_enable;
+	struct gpio_desc *gpio_reset;
+	struct gpio_desc *gpio_cabledet;
+
+	uint32_t src_caps[8];
+	unsigned int n_src_caps;
+
+	uint32_t snk_caps[8];
+	unsigned int n_snk_caps;
+
+	unsigned long flags[1];
+
+	struct delayed_work work;
+	struct timer_list work_timer;
+
+	struct mutex lock;
+	bool vbus_on, vconn_on;
+	bool pd_capable;
+	int pd_current_limit; // mA
+	ktime_t current_update_deadline;
+
+	struct typec_port *port;
+	struct typec_partner *partner;
+	struct usb_pd_identity partner_identity;
+	struct usb_role_switch *role_sw;
+	int pwr_role, data_role;
+
+	struct dentry *debug_root;
+
+	/* for debug */
+	int last_status;
+	int last_cc_status;
+	int last_dp_state;
+	int last_bc_result;
+
+	/* for HDMI HPD */
+	struct extcon_dev *extcon;
+	int last_extcon_state;
+};
+
+static const unsigned int anx7688_extcon_cable[] = {
+	EXTCON_DISP_HDMI,
+	EXTCON_NONE,
+};
+
+static int anx7688_reg_read(struct anx7688 *anx7688, u8 reg_addr)
+{
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(anx7688->client, reg_addr);
+	if (ret < 0)
+		dev_err(anx7688->dev, "i2c read failed at 0x%x (%d)\n",
+			reg_addr, ret);
+
+	return ret;
+}
+
+static int anx7688_reg_write(struct anx7688 *anx7688, u8 reg_addr, u8 value)
+{
+	int ret;
+
+	ret = i2c_smbus_write_byte_data(anx7688->client, reg_addr, value);
+	if (ret < 0)
+		dev_err(anx7688->dev, "i2c write failed at 0x%x (%d)\n",
+			reg_addr, ret);
+
+	return ret;
+}
+
+static int anx7688_reg_update_bits(struct anx7688 *anx7688, u8 reg_addr,
+				   u8 mask, u8 value)
+{
+	int ret;
+
+	ret = anx7688_reg_read(anx7688, reg_addr);
+	if (ret < 0)
+		return ret;
+
+	ret &= ~mask;
+	ret |= value;
+
+	return anx7688_reg_write(anx7688, reg_addr, ret);
+}
+
+static int anx7688_tcpc_reg_read(struct anx7688 *anx7688, u8 reg_addr)
+{
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(anx7688->client_tcpc, reg_addr);
+	if (ret < 0)
+		dev_err(anx7688->dev, "tcpc i2c read failed at 0x%x (%d)\n",
+			reg_addr, ret);
+
+	return ret;
+}
+
+static int anx7688_tcpc_reg_write(struct anx7688 *anx7688, u8 reg_addr, u8 value)
+{
+	int ret;
+
+	ret = i2c_smbus_write_byte_data(anx7688->client_tcpc, reg_addr, value);
+	if (ret < 0)
+		dev_err(anx7688->dev, "tcpc i2c write failed at 0x%x (%d)\n",
+			reg_addr, ret);
+
+	return ret;
+}
+
+static void anx7688_power_enable(struct anx7688 *anx7688)
+{
+	gpiod_set_value(anx7688->gpio_reset, 1);
+	gpiod_set_value(anx7688->gpio_enable, 1);
+
+	/* wait for power to stabilize and release reset */
+	msleep(10);
+	gpiod_set_value(anx7688->gpio_reset, 0);
+	udelay(2);
+
+	dev_dbg(anx7688->dev, "power enabled\n");
+
+	set_bit(ANX7688_F_POWERED, anx7688->flags);
+}
+
+static void anx7688_power_disable(struct anx7688 *anx7688)
+{
+	gpiod_set_value(anx7688->gpio_reset, 1);
+	msleep(5);
+	gpiod_set_value(anx7688->gpio_enable, 0);
+
+	dev_dbg(anx7688->dev, "power disabled\n");
+
+	clear_bit(ANX7688_F_POWERED, anx7688->flags);
+}
+
+static int anx7688_send_ocm_message(struct anx7688 *anx7688, int cmd,
+				    const u8 *data, int data_len)
+{
+	int ret = 0, i;
+	u8 pkt[32];
+
+	if (data_len > sizeof(pkt) - 3) {
+		dev_dbg(anx7688->dev,
+			"invalid ocm message length cmd=0x%02x len=%d\n",
+			cmd, data_len);
+		return -EINVAL;
+	}
+
+	// prepare pd packet
+	pkt[0] = data_len + 1;
+	pkt[1] = cmd;
+	if (data_len > 0)
+		memcpy(pkt + 2, data, data_len);
+	pkt[2 + data_len] = 0;
+	for (i = 0; i < data_len + 2; i++)
+		pkt[data_len + 2] -= pkt[i];
+
+	dev_dbg(anx7688->dev, "send pd packet cmd=0x%02x %*ph\n",
+		cmd, data_len + 3, pkt);
+
+	ret = anx7688_tcpc_reg_read(anx7688, ANX7688_TCPC_REG_INTERFACE_SEND);
+	if (ret) {
+		dev_err(anx7688->dev,
+			"failed to send pd packet (tx buffer full)\n");
+		return -EBUSY;
+	}
+
+	ret = i2c_smbus_write_i2c_block_data(anx7688->client_tcpc,
+					     ANX7688_TCPC_REG_INTERFACE_SEND,
+					     data_len + 3, pkt);
+	if (ret < 0)
+		dev_err(anx7688->dev,
+			"failed to send pd packet (err=%d)\n", ret);
+
+	// wait until the message is processed (30ms max)
+	for (i = 0; i < 300; i++) {
+		ret = anx7688_tcpc_reg_read(anx7688, ANX7688_TCPC_REG_INTERFACE_SEND);
+		if (ret <= 0)
+			return ret;
+
+		udelay(100);
+	}
+
+	dev_err(anx7688->dev, "timeout waiting for the message queue flush\n");
+	return -ETIMEDOUT;
+}
+
+static int anx7688_connect(struct anx7688 *anx7688)
+{
+	struct typec_partner_desc desc = {};
+	int ret, i;
+	u8 fw[2];
+	const u8 dp_snk_identity[16] = {
+		0x00, 0x00, 0x00, 0xec,	/* id header */
+		0x00, 0x00, 0x00, 0x00,	/* cert stat */
+		0x00, 0x00, 0x00, 0x00,	/* product type */
+		0x39, 0x00, 0x00, 0x51	/* alt mode adapter */
+	};
+	const u8 svid[4] = {
+		0x00, 0x00, 0x01, 0xff,
+	};
+	u32 caps[8];
+
+	dev_dbg(anx7688->dev, "cable inserted\n");
+
+	anx7688->last_status = -1;
+	anx7688->last_cc_status = -1;
+	anx7688->last_dp_state = -1;
+
+	msleep(10);
+	anx7688_power_enable(anx7688);
+
+	ret = regulator_enable(anx7688->supplies[ANX7688_VCONN_INDEX].consumer);
+	if (ret) {
+		dev_err(anx7688->dev, "failed to enable vconn\n");
+		goto err_poweroff;
+	}
+	anx7688->vconn_on = true;
+
+	/* wait till the firmware is loaded (typically ~30ms) */
+	for (i = 0; i < 100; i++) {
+		ret = anx7688_reg_read(anx7688, ANX7688_REG_EEPROM_LOAD_STATUS0);
+
+		if (ret >= 0 && (ret & ANX7688_EEPROM_FW_LOADED) == ANX7688_EEPROM_FW_LOADED) {
+			dev_dbg(anx7688->dev, "eeprom0 = 0x%02x\n", ret);
+			dev_info(anx7688->dev, "fw loaded after %d ms\n", i * 10);
+			goto fw_loaded;
+		}
+
+		msleep(5);
+	}
+
+	set_bit(ANX7688_F_FW_FAILED, anx7688->flags);
+	dev_err(anx7688->dev, "boot firmware load failed (you may need to flash FW to anx7688 first)\n");
+	ret = -ETIMEDOUT;
+	goto err_vconoff;
+
+fw_loaded:
+	ret = i2c_smbus_read_i2c_block_data(anx7688->client,
+					    ANX7688_REG_FW_VERSION1, 2, fw);
+	if (ret < 0) {
+		dev_err(anx7688->dev, "failed to read firmware version\n");
+		goto err_vconoff;
+	}
+
+	dev_info(anx7688->dev, "OCM firmware loaded (version 0x%04x)\n",
+		 fw[1] | fw[0] << 8);
+
+	/* Unmask interrupts */
+	ret = anx7688_reg_write(anx7688, ANX7688_REG_STATUS_INT, 0);
+	if (ret)
+		goto err_vconoff;
+
+	ret = anx7688_reg_write(anx7688, ANX7688_REG_STATUS_INT_MASK, ~ANX7688_SOFT_INT_MASK);
+	if (ret)
+		goto err_vconoff;
+
+	ret = anx7688_reg_write(anx7688, ANX7688_REG_IRQ_EXT_SOURCE2, 0xff);
+	if (ret)
+		goto err_vconoff;
+
+	ret = anx7688_reg_write(anx7688, ANX7688_REG_IRQ_EXT_MASK2, (u8)~ANX7688_IRQ2_SOFT_INT);
+	if (ret)
+		goto err_vconoff;
+
+	/* time to turn off vbus after cc disconnect (unit is 4 ms) */
+	ret = anx7688_reg_write(anx7688, ANX7688_REG_VBUS_OFF_DELAY_TIME, 100 / 4);
+	if (ret)
+		goto err_vconoff;
+
+	/* 300ms (unit is 2 ms) */
+	ret = anx7688_reg_write(anx7688, ANX7688_REG_TRY_UFP_TIMER, 300 / 2);
+	if (ret)
+		goto err_vconoff;
+
+	/* maximum voltage in 100 mV units */
+	ret = anx7688_reg_write(anx7688, ANX7688_REG_MAX_VOLTAGE, 50); /* 5 V */
+	if (ret)
+		goto err_vconoff;
+
+	/* min/max power in 500 mW units */
+	ret = anx7688_reg_write(anx7688, ANX7688_REG_MAX_POWER, 15 * 2); /* 15 W */
+	if (ret)
+		goto err_vconoff;
+
+	ret = anx7688_reg_write(anx7688, ANX7688_REG_MIN_POWER, 1);  /* 0.5 W */
+	if (ret)
+		goto err_vconoff;
+
+	/* auto_pd, try.src, try.sink, goto safe 5V */
+	ret = anx7688_reg_write(anx7688, ANX7688_REG_FEATURE_CTRL, 0x1e & ~BIT(2)); // disable try_src
+	if (ret)
+		goto err_vconoff;
+
+	for (i = 0; i < anx7688->n_src_caps; i++)
+		caps[i] = cpu_to_le32(anx7688->src_caps[i]);
+	ret = anx7688_send_ocm_message(anx7688, ANX7688_OCM_MSG_PWR_SRC_CAP,
+				       (u8 *)&caps, 4 * anx7688->n_src_caps);
+	if (ret)
+		goto err_vconoff;
+
+	for (i = 0; i < anx7688->n_snk_caps; i++)
+		caps[i] = cpu_to_le32(anx7688->snk_caps[i]);
+	ret = anx7688_send_ocm_message(anx7688, ANX7688_OCM_MSG_PWR_SNK_CAP,
+				       (u8 *)&caps, 4 * anx7688->n_snk_caps);
+	if (ret)
+		goto err_vconoff;
+
+	/* Send DP SNK identity */
+	ret = anx7688_send_ocm_message(anx7688, ANX7688_OCM_MSG_DP_SNK_IDENTITY,
+				       dp_snk_identity, sizeof(dp_snk_identity));
+	if (ret)
+		goto err_vconoff;
+
+	ret = anx7688_send_ocm_message(anx7688, ANX7688_OCM_MSG_SVID,
+				       svid, sizeof(svid));
+	if (ret)
+		goto err_vconoff;
+
+	dev_dbg(anx7688->dev, "OCM configuration completed\n");
+
+	desc.accessory = TYPEC_ACCESSORY_NONE;
+
+	typec_unregister_partner(anx7688->partner);
+
+	anx7688->partner = typec_register_partner(anx7688->port, &desc);
+	if (IS_ERR(anx7688->partner)) {
+		ret = PTR_ERR(anx7688->partner);
+		goto err_vconoff;
+	}
+
+	// after this deadline passes we'll check if device is pd_capable and
+	// set up the current limit accordingly
+	anx7688->current_update_deadline = ktime_add_ms(ktime_get(), 3000);
+
+	set_bit(ANX7688_F_CONNECTED, anx7688->flags);
+	return 0;
+
+err_vconoff:
+	regulator_disable(anx7688->supplies[ANX7688_VCONN_INDEX].consumer);
+	anx7688->vconn_on = false;
+err_poweroff:
+	anx7688_power_disable(anx7688);
+	dev_err(anx7688->dev, "OCM configuration failed\n");
+	return ret;
+}
+
+static void anx7688_set_hdmi_hpd(struct anx7688 *anx7688, int state)
+{
+	if (!anx7688->extcon)
+		return;
+
+	if (anx7688->last_extcon_state != state) {
+		extcon_set_state_sync(anx7688->extcon, EXTCON_DISP_HDMI, state);
+		anx7688->last_extcon_state = state;
+	}
+}
+
+static void anx7688_disconnect(struct anx7688 *anx7688)
+{
+	union power_supply_propval val = {0,};
+	struct device *dev = anx7688->dev;
+	int ret;
+
+	dev_dbg(dev, "cable removed\n");
+
+	anx7688->current_update_deadline = 0;
+
+	anx7688_set_hdmi_hpd(anx7688, 0);
+
+	if (anx7688->vconn_on) {
+		regulator_disable(anx7688->supplies[ANX7688_VCONN_INDEX].consumer);
+		anx7688->vconn_on = false;
+	}
+
+	if (anx7688->vbus_on) {
+		regulator_disable(anx7688->supplies[ANX7688_VBUS_INDEX].consumer);
+		anx7688->vbus_on = false;
+	}
+
+	anx7688_power_disable(anx7688);
+
+	anx7688->pd_capable = false;
+
+	typec_unregister_partner(anx7688->partner);
+	anx7688->partner = NULL;
+
+	anx7688->pwr_role = TYPEC_SINK;
+	anx7688->data_role = TYPEC_DEVICE;
+	typec_set_pwr_role(anx7688->port, anx7688->pwr_role);
+	typec_set_data_role(anx7688->port, anx7688->data_role);
+	typec_set_pwr_opmode(anx7688->port, TYPEC_PWR_MODE_USB);
+	typec_set_vconn_role(anx7688->port, TYPEC_SINK);
+
+	usb_role_switch_set_role(anx7688->role_sw, USB_ROLE_NONE);
+
+	val.intval = 500 * 1000;
+	dev_dbg(dev, "setting vbus_in current limit to %d mA\n", val.intval);
+	ret = power_supply_set_property(anx7688->vbus_in_supply,
+					POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
+					&val);
+	if (ret)
+		dev_err(dev, "failed to set vbus_in current to %d mA\n",
+			val.intval / 1000);
+
+	val.intval = 0;
+	dev_dbg(dev, "disabling vbus_in power path\n");
+	ret = power_supply_set_property(anx7688->vbus_in_supply,
+					POWER_SUPPLY_PROP_ONLINE,
+					&val);
+	if (ret)
+		dev_err(dev, "failed to offline vbus_in\n");
+
+	dev_dbg(dev, "enabling USB BC 1.2 detection\n");
+
+	clear_bit(ANX7688_F_CONNECTED, anx7688->flags);
+}
+
+static void anx7688_handle_cable_change(struct anx7688 *anx7688)
+{
+	int cabledet;
+	bool connected;
+
+	connected = test_bit(ANX7688_F_CONNECTED, anx7688->flags);
+	cabledet = gpiod_get_value(anx7688->gpio_cabledet);
+
+	if (cabledet && !connected)
+		anx7688_connect(anx7688);
+	else if (!cabledet && connected)
+		anx7688_disconnect(anx7688);
+}
+
+static irqreturn_t anx7688_irq_plug_handler(int irq, void *data)
+{
+	struct anx7688 *anx7688 = data;
+
+	dev_dbg(anx7688->dev, "plug irq (cd=%d)\n",
+		gpiod_get_value(anx7688->gpio_cabledet));
+
+	/*
+	 * After each cabledet change the scheduled work timer is reset
+	 * to fire in ~10ms. So the work is done only after the cabledet
+	 * is stable for ~10ms.
+	 */
+	schedule_delayed_work(&anx7688->work, msecs_to_jiffies(10));
+
+	return IRQ_HANDLED;
+}
+
+enum {
+	CMD_SUCCESS,
+	CMD_REJECT,
+	CMD_FAIL,
+	CMD_BUSY,
+};
+
+static const char * const anx7688_cmd_statuses[] = {
+	"SUCCESS",
+	"REJECT",
+	"FAIL",
+	"BUSY",
+};
+
+static int anx7688_handle_pd_message_response(struct anx7688 *anx7688,
+					      u8 to_cmd, u8 resp)
+{
+	const char *status = resp <= CMD_BUSY ? anx7688_cmd_statuses[resp] : "UNKNOWN";
+
+	switch (to_cmd) {
+	case ANX7688_OCM_MSG_PSWAP_REQ:
+		dev_info(anx7688->dev, "received response to PSWAP_REQ (%s)\n", status);
+		break;
+
+	case ANX7688_OCM_MSG_DSWAP_REQ:
+		dev_info(anx7688->dev, "received response to DSWAP_REQ (%s)\n", status);
+		break;
+
+	case ANX7688_OCM_MSG_VCONN_SWAP_REQ:
+		dev_info(anx7688->dev, "received response to VCONN_SWAP_REQ (%s)\n", status);
+		break;
+
+	case ANX7688_OCM_MSG_PWR_OBJ_REQ:
+		dev_info(anx7688->dev, "received response to PWR_OBJ_REQ (%s)\n", status);
+		break;
+
+	case ANX7688_OCM_MSG_VDM:
+		dev_info(anx7688->dev, "received response to VDM (%s)\n", status);
+		break;
+
+	case ANX7688_OCM_MSG_GOTO_MIN_REQ:
+		dev_info(anx7688->dev, "received response to GOTO_MIN_REQ (%s)\n", status);
+		break;
+
+	case ANX7688_OCM_MSG_GET_SNK_CAP:
+		dev_info(anx7688->dev, "received response to GET_SNK_CAP (%s)\n", status);
+		break;
+
+	default:
+		dev_info(anx7688->dev, "received response to unknown request (%s)\n", status);
+		break;
+	}
+
+	return 0;
+}
+
+static int anx7688_handle_pd_message(struct anx7688 *anx7688,
+				     u8 cmd, u8 *msg, unsigned int len)
+{
+	struct device *dev = anx7688->dev;
+	union power_supply_propval psy_val = {0,};
+	uint32_t *pdos = (uint32_t *)msg;
+	int ret, i, rdo_max_v, rdo_max_p;
+	uint32_t pdo, rdo;
+
+	switch (cmd) {
+	case ANX7688_OCM_MSG_PWR_SRC_CAP:
+		dev_info(anx7688->dev, "received SRC_CAP\n");
+
+		if (len % 4 != 0) {
+			dev_warn(anx7688->dev, "received invalid sized PDO array\n");
+			break;
+		}
+
+		/* the partner is PD capable */
+		anx7688->pd_capable = true;
+
+		for (i = 0; i < len / 4; i++) {
+			pdo = le32_to_cpu(pdos[i]);
+
+			if (pdo_type(pdo) == PDO_TYPE_FIXED) {
+				unsigned int voltage = pdo_fixed_voltage(pdo);
+				unsigned int max_curr = pdo_max_current(pdo);
+
+				dev_info(anx7688->dev, "SRC_CAP PDO_FIXED (%umV %umA)\n", voltage, max_curr);
+			} else if (pdo_type(pdo) == PDO_TYPE_BATT) {
+				unsigned int min_volt = pdo_min_voltage(pdo);
+				unsigned int max_volt = pdo_max_voltage(pdo);
+				unsigned int max_pow = pdo_max_power(pdo);
+
+				dev_info(anx7688->dev, "SRC_CAP PDO_BATT (%umV-%umV %umW)\n", min_volt, max_volt, max_pow);
+			} else if (pdo_type(pdo) == PDO_TYPE_VAR) {
+				unsigned int min_volt = pdo_min_voltage(pdo);
+				unsigned int max_volt = pdo_max_voltage(pdo);
+				unsigned int max_curr = pdo_max_current(pdo);
+
+				dev_info(anx7688->dev, "SRC_CAP PDO_VAR (%umV-%umV %umA)\n", min_volt, max_volt, max_curr);
+			} else {
+				dev_info(anx7688->dev, "SRC_CAP PDO_APDO (0x%08X)\n", pdo);
+			}
+		}
+
+		/* when auto_pd mode is enabled, the FW has already set
+		 * RDO_MAX_VOLTAGE and RDO_MAX_POWER for the RDO it sent to the
+		 * partner based on the received SOURCE_CAPs. This does not
+		 * mean, the request was acked, but we can't do better here than
+		 * calculate the current_limit to set later and hope for the best.
+		 */
+		rdo_max_v = anx7688_reg_read(anx7688, ANX7688_REG_MAX_VOLTAGE_STATUS);
+		if (rdo_max_v < 0)
+			return rdo_max_v;
+		if (rdo_max_v == 0)
+			return -EINVAL;
+
+		rdo_max_p = anx7688_reg_read(anx7688, ANX7688_REG_MAX_POWER_STATUS);
+		if (rdo_max_p < 0)
+			return rdo_max_p;
+
+		anx7688->pd_current_limit = rdo_max_p * 5000 / rdo_max_v;
+
+		dev_dbg(anx7688->dev, "RDO max voltage = %dmV, max power = %dmW, PD current limit = %dmA\n",
+			rdo_max_v * 100, rdo_max_p * 500, anx7688->pd_current_limit);
+
+		// update current limit sooner, now that we have PD negotiation result
+		anx7688->current_update_deadline = ktime_add_ms(ktime_get(), 500);
+		break;
+
+	case ANX7688_OCM_MSG_PWR_SNK_CAP:
+		dev_info(anx7688->dev, "received SNK_CAP\n");
+
+		if (len % 4 != 0) {
+			dev_warn(anx7688->dev, "received invalid sized PDO array\n");
+			break;
+		}
+
+		for (i = 0; i < len / 4; i++) {
+			pdo = le32_to_cpu(pdos[i]);
+
+			if (pdo_type(pdo) == PDO_TYPE_FIXED) {
+				unsigned int voltage = pdo_fixed_voltage(pdo);
+				unsigned int max_curr = pdo_max_current(pdo);
+
+				dev_info(anx7688->dev, "SNK_CAP PDO_FIXED (%umV %umA)\n", voltage, max_curr);
+			} else if (pdo_type(pdo) == PDO_TYPE_BATT) {
+				unsigned int min_volt = pdo_min_voltage(pdo);
+				unsigned int max_volt = pdo_max_voltage(pdo);
+				unsigned int max_pow = pdo_max_power(pdo);
+
+				dev_info(anx7688->dev, "SNK_CAP PDO_BATT (%umV-%umV %umW)\n", min_volt, max_volt, max_pow);
+			} else if (pdo_type(pdo) == PDO_TYPE_VAR) {
+				unsigned int min_volt = pdo_min_voltage(pdo);
+				unsigned int max_volt = pdo_max_voltage(pdo);
+				unsigned int max_curr = pdo_max_current(pdo);
+
+				dev_info(anx7688->dev, "SNK_CAP PDO_VAR (%umV-%umV %umA)\n", min_volt, max_volt, max_curr);
+			} else {
+				dev_info(anx7688->dev, "SNK_CAP PDO_APDO (0x%08X)\n", pdo);
+			}
+		}
+
+		break;
+
+	case ANX7688_OCM_MSG_PWR_OBJ_REQ:
+		dev_info(anx7688->dev, "received PWR_OBJ_REQ\n");
+
+		anx7688->pd_capable = true;
+
+		if (len != 4) {
+			dev_warn(anx7688->dev, "received invalid sized RDO\n");
+			break;
+		}
+
+		rdo = le32_to_cpu(pdos[0]);
+
+		if (rdo_index(rdo) >= 1 && rdo_index(rdo) <= anx7688->n_src_caps) {
+			unsigned int rdo_op_curr = rdo_op_current(rdo);
+			unsigned int rdo_max_curr = rdo_max_current(rdo);
+			unsigned int rdo_idx = rdo_index(rdo) - 1;
+			unsigned int pdo_volt, pdo_max_curr;
+
+			pdo = anx7688->src_caps[rdo_idx];
+			pdo_volt = pdo_fixed_voltage(pdo);
+			pdo_max_curr = pdo_max_current(pdo);
+
+			dev_info(anx7688->dev, "RDO (idx=%d op=%umA max=%umA)\n",
+				 rdo_idx, rdo_op_curr, rdo_max_curr);
+
+			dev_info(anx7688->dev, "PDO_FIXED (%umV %umA)\n",
+				 pdo_volt, pdo_max_curr);
+
+			// We could check the req and respond with accept/reject
+			// but we're using auto_pd feature, so the FW will do
+			// this for us
+		} else {
+			dev_info(anx7688->dev, "PWR_OBJ RDO index out of range (RDO = 0x%08X)\n", rdo);
+		}
+
+		break;
+
+	case ANX7688_OCM_MSG_ACCEPT:
+		dev_info(anx7688->dev, "received ACCEPT\n");
+		break;
+
+	case ANX7688_OCM_MSG_REJECT:
+		dev_info(anx7688->dev, "received REJECT\n");
+		break;
+
+	case ANX7688_OCM_MSG_RESPONSE_TO_REQ:
+		if (len < 2) {
+			dev_warn(anx7688->dev, "received short RESPONSE_TO_REQ\n");
+			break;
+		}
+
+		anx7688_handle_pd_message_response(anx7688, msg[0], msg[1]);
+		break;
+
+	case ANX7688_OCM_MSG_SOFT_RST:
+		dev_info(anx7688->dev, "received SOFT_RST\n");
+		break;
+
+	case ANX7688_OCM_MSG_HARD_RST:
+		if (anx7688->pd_capable) {
+			dev_info(anx7688->dev, "received HARD_RST\n");
+
+			// stop drawing power from VBUS
+			psy_val.intval = 0;
+			dev_dbg(dev, "disabling vbus_in power path\n");
+			ret = power_supply_set_property(anx7688->vbus_in_supply,
+							POWER_SUPPLY_PROP_ONLINE,
+							&psy_val);
+			if (ret)
+				dev_err(anx7688->dev, "failed to offline vbus_in\n");
+
+			// wait till the dust settles
+			anx7688->current_update_deadline = ktime_add_ms(ktime_get(), 3000);
+		} else {
+			dev_dbg(anx7688->dev, "received HARD_RST, idiot firmware is bored\n");
+		}
+
+		break;
+
+	case ANX7688_OCM_MSG_RESTART:
+		dev_info(anx7688->dev, "received RESTART\n");
+		break;
+
+	case ANX7688_OCM_MSG_PSWAP_REQ:
+		dev_info(anx7688->dev, "received PSWAP_REQ\n");
+		break;
+
+	case ANX7688_OCM_MSG_DSWAP_REQ:
+		dev_info(anx7688->dev, "received DSWAP_REQ\n");
+		break;
+
+	case ANX7688_OCM_MSG_VCONN_SWAP_REQ:
+		dev_info(anx7688->dev, "received VCONN_SWAP_REQ\n");
+		break;
+
+	case ANX7688_OCM_MSG_DP_ALT_ENTER:
+		dev_info(anx7688->dev, "received DP_ALT_ENTER\n");
+		break;
+
+	case ANX7688_OCM_MSG_DP_ALT_EXIT:
+		dev_info(anx7688->dev, "received DP_ALT_EXIT\n");
+		break;
+
+	case ANX7688_OCM_MSG_DP_SNK_IDENTITY:
+		dev_info(anx7688->dev, "received DP_SNK_IDENTITY\n");
+		break;
+
+	case ANX7688_OCM_MSG_SVID:
+		dev_info(anx7688->dev, "received SVID\n");
+		break;
+
+	case ANX7688_OCM_MSG_VDM:
+		dev_info(anx7688->dev, "received VDM\n");
+		break;
+
+	case ANX7688_OCM_MSG_GOTO_MIN_REQ:
+		dev_info(anx7688->dev, "received GOTO_MIN_REQ\n");
+		break;
+
+	case ANX7688_OCM_MSG_PD_STATUS_REQ:
+		dev_info(anx7688->dev, "received PD_STATUS_REQ\n");
+		break;
+
+	case ANX7688_OCM_MSG_GET_DP_SNK_CAP:
+		dev_info(anx7688->dev, "received GET_DP_SNK_CAP\n");
+		break;
+
+	case ANX7688_OCM_MSG_DP_SNK_CFG:
+		dev_info(anx7688->dev, "received DP_SNK_CFG\n");
+		break;
+
+	default:
+		dev_info(anx7688->dev, "received unknown message 0x%02x\n", cmd);
+		break;
+	}
+
+	return 0;
+}
+
+static int anx7688_receive_msg(struct anx7688 *anx7688)
+{
+	u8 pkt[32], checksum = 0;
+	int i, ret;
+
+	ret = i2c_smbus_read_i2c_block_data(anx7688->client_tcpc,
+					    ANX7688_TCPC_REG_INTERFACE_RECV,
+					    32, pkt);
+	if (ret < 0) {
+		dev_err(anx7688->dev, "failed to read pd msg\n");
+		return ret;
+	}
+
+	ret = anx7688_tcpc_reg_write(anx7688, ANX7688_TCPC_REG_INTERFACE_RECV, 0);
+	if (ret)
+		dev_warn(anx7688->dev, "failed to clear recv FIFO\n");
+
+	if (pkt[0] == 0 || pkt[0] > sizeof(pkt) - 2) {
+		dev_err(anx7688->dev, "received invalid pd message: %*ph\n",
+			(int)sizeof(pkt), pkt);
+		return -EINVAL;
+	}
+
+	dev_dbg(anx7688->dev, "recv ocm message cmd=0x%02x %*ph\n",
+		pkt[1], pkt[0] + 2, pkt);
+
+	for (i = 0; i < pkt[0] + 2; i++)
+		checksum += pkt[i];
+
+	if (checksum != 0) {
+		dev_err(anx7688->dev, "bad checksum on received message\n");
+		return -EINVAL;
+	}
+
+	anx7688_handle_pd_message(anx7688, pkt[1], pkt + 2, pkt[0] - 1);
+	return 0;
+}
+
+static const char *anx7688_cc_status_string(unsigned int v)
+{
+	switch (v) {
+	case 0: return "SRC.Open";
+	case 1: return "SRC.Rd";
+	case 2: return "SRC.Ra";
+	case 4: return "SNK.Default";
+	case 8: return "SNK.Power1.5";
+	case 12: return "SNK.Power3.0";
+	default: return "UNK";
+	}
+}
+
+static int anx7688_update_status(struct anx7688 *anx7688)
+{
+	struct device *dev = anx7688->dev;
+	bool vbus_on, vconn_on, dr_dfp;
+	int status, cc_status, dp_state, dp_substate, ret;
+
+	status = anx7688_reg_read(anx7688, ANX7688_REG_STATUS);
+	if (status < 0)
+		return status;
+
+	cc_status = anx7688_reg_read(anx7688, ANX7688_REG_CC_STATUS);
+	if (cc_status < 0)
+		return cc_status;
+
+	dp_state = anx7688_tcpc_reg_read(anx7688, 0x87);
+	if (dp_state < 0)
+		return dp_state;
+
+	dp_substate = anx7688_tcpc_reg_read(anx7688, 0x88);
+	if (dp_substate < 0)
+		return dp_substate;
+
+	anx7688_set_hdmi_hpd(anx7688, dp_state >= 3);
+
+	dp_state = (dp_state << 8) | dp_substate;
+
+	if (anx7688->last_status == -1 || anx7688->last_status != status) {
+		anx7688->last_status = status;
+		dev_dbg(dev, "status changed to 0x%02x\n", status);
+	}
+
+	if (anx7688->last_cc_status == -1 || anx7688->last_cc_status != cc_status) {
+		anx7688->last_cc_status = cc_status;
+		dev_dbg(dev, "cc_status changed to CC1 = %s CC2 = %s\n",
+			anx7688_cc_status_string(cc_status & 0xf),
+			anx7688_cc_status_string((cc_status >> 4) & 0xf));
+	}
+
+	if (anx7688->last_dp_state == -1 || anx7688->last_dp_state != dp_state) {
+		anx7688->last_dp_state = dp_state;
+		dev_dbg(dev, "DP state changed to 0x%04x\n", dp_state);
+	}
+
+	vbus_on = !!(status & ANX7688_VBUS_STATUS);
+	vconn_on = !!(status & ANX7688_VCONN_STATUS);
+	dr_dfp = !!(status & ANX7688_DATA_ROLE_STATUS);
+
+	if (anx7688->vbus_on != vbus_on) {
+		dev_dbg(anx7688->dev, "POWER role change to %s\n",
+			vbus_on ? "SOURCE" : "SINK");
+
+		if (vbus_on) {
+			ret = regulator_enable(anx7688->supplies[ANX7688_VBUS_INDEX].consumer);
+			if (ret) {
+				dev_err(anx7688->dev, "failed to enable vbus\n");
+				return ret;
+			}
+		} else {
+			ret = regulator_disable(anx7688->supplies[ANX7688_VBUS_INDEX].consumer);
+			if (ret) {
+				dev_err(anx7688->dev, "failed to disable vbus\n");
+				return ret;
+			}
+		}
+
+		anx7688->pwr_role = vbus_on ? TYPEC_SOURCE : TYPEC_SINK;
+		typec_set_pwr_role(anx7688->port, anx7688->pwr_role);
+		anx7688->vbus_on = vbus_on;
+	}
+
+	if (anx7688->vconn_on != vconn_on) {
+		dev_dbg(anx7688->dev, "VCONN role change to %s\n",
+			vconn_on ? "SOURCE" : "SINK");
+
+		if (vconn_on) {
+			ret = regulator_enable(anx7688->supplies[ANX7688_VCONN_INDEX].consumer);
+			if (ret) {
+				dev_err(anx7688->dev, "failed to enable vconn\n");
+				return ret;
+			}
+		} else {
+			ret = regulator_disable(anx7688->supplies[ANX7688_VCONN_INDEX].consumer);
+			if (ret) {
+				dev_err(anx7688->dev, "failed to disable vconn\n");
+				return ret;
+			}
+		}
+
+		typec_set_vconn_role(anx7688->port, vconn_on ? TYPEC_SOURCE : TYPEC_SINK);
+		anx7688->vconn_on = vconn_on;
+	}
+
+	anx7688->data_role = dr_dfp ? TYPEC_HOST : TYPEC_DEVICE;
+	typec_set_data_role(anx7688->port, anx7688->data_role);
+
+	if (usb_role_switch_get_role(anx7688->role_sw) !=
+	    (dr_dfp ? USB_ROLE_HOST : USB_ROLE_DEVICE)) {
+		dev_dbg(anx7688->dev, "DATA role change requested to %s\n",
+			dr_dfp ? "DFP" : "UFP");
+
+		ret = usb_role_switch_set_role(anx7688->role_sw,
+					       dr_dfp ? USB_ROLE_HOST : USB_ROLE_DEVICE);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static irqreturn_t anx7688_irq_status_handler(int irq, void *data)
+{
+	struct anx7688 *anx7688 = data;
+	struct device *dev = anx7688->dev;
+	int tcpc_status, ext2_status, soft_status;
+
+	mutex_lock(&anx7688->lock);
+
+	if (!test_bit(ANX7688_F_CONNECTED, anx7688->flags)) {
+		dev_dbg(dev, "spurious status irq\n");
+		/*
+		 * anx chip should be disabled and power off, nothing
+		 * more to do
+		 */
+		goto out_unlock;
+	}
+
+	// clear tcpc interrupt
+	tcpc_status = anx7688_tcpc_reg_read(anx7688, ANX7688_TCPC_REG_ALERT0);
+	if (tcpc_status > 0)
+		anx7688_tcpc_reg_write(anx7688, ANX7688_TCPC_REG_ALERT0, tcpc_status);
+
+	ext2_status = anx7688_reg_read(anx7688, ANX7688_REG_IRQ_EXT_SOURCE2);
+	if (ext2_status & ANX7688_IRQ2_SOFT_INT) {
+		soft_status = anx7688_reg_read(anx7688, ANX7688_REG_STATUS_INT);
+		anx7688_reg_write(anx7688, ANX7688_REG_STATUS_INT, 0);
+
+		if (soft_status > 0) {
+			soft_status &= ANX7688_SOFT_INT_MASK;
+
+			if (soft_status & ANX7688_IRQS_RECEIVED_MSG)
+				anx7688_receive_msg(anx7688);
+
+			if (soft_status & (ANX7688_IRQS_CC_STATUS_CHANGE |
+					   ANX7688_IRQS_VBUS_CHANGE |
+					   ANX7688_IRQS_VCONN_CHANGE |
+					   ANX7688_IRQS_DATA_ROLE_CHANGE)) {
+				anx7688_update_status(anx7688);
+			}
+		}
+
+		anx7688_reg_write(anx7688, ANX7688_REG_IRQ_EXT_SOURCE2, ANX7688_IRQ2_SOFT_INT);
+	}
+
+out_unlock:
+	mutex_unlock(&anx7688->lock);
+
+	return IRQ_HANDLED;
+}
+
+static int anx7688_dr_set(struct typec_port *port, enum typec_data_role role)
+{
+	struct anx7688 *anx7688 = typec_get_drvdata(port);
+	int ret = 0;
+
+	dev_info(anx7688->dev, "data role set %d\n", role);
+
+	if (anx7688->data_role != role) {
+		mutex_lock(&anx7688->lock);
+		ret = anx7688_send_ocm_message(anx7688, ANX7688_OCM_MSG_DSWAP_REQ, 0, 0);
+		mutex_unlock(&anx7688->lock);
+	}
+
+	return ret;
+}
+
+static int anx7688_pr_set(struct typec_port *port, enum typec_role role)
+{
+	struct anx7688 *anx7688 = typec_get_drvdata(port);
+	int ret = 0;
+
+	dev_info(anx7688->dev, "power role set %d\n", role);
+
+	if (anx7688->pwr_role != role) {
+		mutex_lock(&anx7688->lock);
+		ret = anx7688_send_ocm_message(anx7688, ANX7688_OCM_MSG_PSWAP_REQ, 0, 0);
+		mutex_unlock(&anx7688->lock);
+	}
+
+	return ret;
+}
+
+/*
+ * Calls to the EEPROM functions need to be taken under the anx7688 lock.
+ */
+static int anx7688_eeprom_set_address(struct anx7688 *anx7688, u16 addr)
+{
+	int ret;
+
+	ret = anx7688_reg_write(anx7688, 0xe0, (addr >> 8) & 0xff);
+	if (ret < 0)
+		return ret;
+
+	return anx7688_reg_write(anx7688, 0xe1, addr & 0xff);
+}
+
+static int anx7688_eeprom_wait_done(struct anx7688 *anx7688)
+{
+	ktime_t timeout;
+	int ret;
+
+	// wait for read to be done
+	timeout = ktime_add_us(ktime_get(), 50000);
+	while (true) {
+		ret = anx7688_reg_read(anx7688, 0xe2);
+		if (ret < 0)
+			return ret;
+
+		if (ret & BIT(3))
+			return 0;
+
+		if (ktime_after(ktime_get(), timeout)) {
+			dev_err(anx7688->dev, "timeout waiting for eeprom\n");
+			return -ETIMEDOUT;
+		}
+	}
+}
+
+/* wait for internal FSM of EEPROM to be in a state ready for
+ * programming/reading
+ */
+static int anx7688_eeprom_wait_ready(struct anx7688 *anx7688)
+{
+	ktime_t timeout;
+	int ret;
+
+	// wait until eeprom is ready
+	timeout = ktime_add_us(ktime_get(), 1000000);
+	while (true) {
+		ret = anx7688_reg_read(anx7688, 0x7f);
+		if (ret < 0)
+			return ret;
+
+		if ((ret & 0x0f) == 7)
+			return 0;
+
+		if (ktime_after(ktime_get(), timeout)) {
+			dev_err(anx7688->dev, "timeout waiting for eeprom to initialize\n");
+			return -ETIMEDOUT;
+		}
+
+		msleep(5);
+	}
+}
+
+static int anx7688_eeprom_read(struct anx7688 *anx7688, unsigned int addr, u8 buf[16])
+{
+	int ret;
+
+	ret = anx7688_eeprom_set_address(anx7688, addr);
+	if (ret)
+		return ret;
+
+	// initiate read
+	ret = anx7688_reg_write(anx7688, 0xe2, 0x06);
+	if (ret < 0)
+		return ret;
+
+	ret = anx7688_eeprom_wait_done(anx7688);
+	if (ret)
+		return ret;
+
+	ret = i2c_smbus_read_i2c_block_data(anx7688->client, 0xd0, 16, buf);
+	if (ret < 0) {
+		dev_err(anx7688->dev,
+			"failed to read eeprom data (err=%d)\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct typec_operations anx7688_typec_ops = {
+	.dr_set = anx7688_dr_set,
+	.pr_set = anx7688_pr_set,
+};
+
+/*
+ * This function has to work when the ANX7688 is active, and when
+ * it is powered down. It power cycles the chip and asserts the OCM
+ * reset, to prevent OCM FW interfering with EEPROM reading.
+ *
+ * After reading EEPROM, the reconnection is scheduled.
+ */
+static int anx7688_firmware_show(struct seq_file *s, void *data)
+{
+	struct anx7688 *anx7688 = s->private;
+	unsigned int addr;
+	u8 buf[16];
+	int ret;
+
+	mutex_lock(&anx7688->lock);
+
+	if (test_bit(ANX7688_F_CONNECTED, anx7688->flags))
+		anx7688_disconnect(anx7688);
+
+	msleep(20);
+
+	anx7688_power_enable(anx7688);
+
+	ret = anx7688_reg_update_bits(anx7688, ANX7688_REG_USBC_RESET_CTRL,
+				      ANX7688_USBC_RESET_CTRL_OCM_RESET,
+				      ANX7688_USBC_RESET_CTRL_OCM_RESET);
+	if (ret < 0)
+		goto out_powerdown;
+
+	ret = anx7688_eeprom_wait_ready(anx7688);
+	if (ret)
+		goto out_powerdown;
+
+	msleep(10);
+
+	for (addr = 0x10; addr < 0x10000; addr += 16) {
+		// set address
+		ret = anx7688_eeprom_read(anx7688, addr, buf);
+		if (ret < 0)
+			goto out_powerdown;
+
+		seq_write(s, buf, sizeof(buf));
+	}
+
+out_powerdown:
+	anx7688_power_disable(anx7688);
+	schedule_delayed_work(&anx7688->work, 0);
+	mutex_unlock(&anx7688->lock);
+
+	return ret;
+}
+DEFINE_SHOW_ATTRIBUTE(anx7688_firmware);
+
+static int anx7688_regs_show(struct seq_file *s, void *data)
+{
+	struct anx7688 *anx7688 = s->private;
+	u8 buf[16];
+	unsigned int i, addr;
+	int ret = -ENODEV;
+
+	mutex_lock(&anx7688->lock);
+
+	if (!test_bit(ANX7688_F_POWERED, anx7688->flags))
+		goto out_unlock;
+
+	for (addr = 0; addr < 256; addr += 16) {
+		ret = i2c_smbus_read_i2c_block_data(anx7688->client, addr,
+						    sizeof(buf), buf);
+		if (ret < 0) {
+			dev_err(anx7688->dev,
+				"failed to read registers (err=%d)\n", ret);
+			goto out_unlock;
+		}
+
+		for (i = 0; i < 16; i++)
+			seq_printf(s, "50%02x: %02x\n", addr + i, buf[i]);
+	}
+
+	for (addr = 0; addr < 256; addr += 16) {
+		ret = i2c_smbus_read_i2c_block_data(anx7688->client_tcpc, addr,
+						    sizeof(buf), buf);
+		if (ret < 0) {
+			dev_err(anx7688->dev,
+				"failed to read registers (err=%d)\n", ret);
+			goto out_unlock;
+		}
+
+		for (i = 0; i < 16; i++)
+			seq_printf(s, "58%02x: %02x\n", addr + i, buf[i]);
+	}
+
+out_unlock:
+	mutex_unlock(&anx7688->lock);
+
+	return ret;
+}
+DEFINE_SHOW_ATTRIBUTE(anx7688_regs);
+
+static int anx7688_status_show(struct seq_file *s, void *data)
+{
+	struct anx7688 *anx7688 = s->private;
+
+	mutex_lock(&anx7688->lock);
+
+	seq_puts(s, "not much\n");
+
+	mutex_unlock(&anx7688->lock);
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(anx7688_status);
+
+/*
+ * This is just a 1s watchdog checking the state if cabledet pin.
+ */
+static void anx7688_cabledet_timer_fn(struct timer_list *t)
+{
+	struct anx7688 *anx7688 = from_timer(anx7688, t, work_timer);
+
+	schedule_delayed_work(&anx7688->work, 0);
+	mod_timer(t, jiffies + msecs_to_jiffies(1000));
+}
+
+static void anx7688_handle_vbus_in_notify(struct anx7688 *anx7688)
+{
+	union power_supply_propval psy_val = {0,};
+	struct device *dev = anx7688->dev;
+	int ret;
+
+	ret = power_supply_get_property(anx7688->vbus_in_supply,
+					POWER_SUPPLY_PROP_USB_TYPE,
+					&psy_val);
+	if (ret) {
+		dev_err(dev, "failed to get USB BC1.2 result\n");
+		return;
+	}
+
+	if (anx7688->last_bc_result == psy_val.intval)
+		return;
+
+	anx7688->last_bc_result = psy_val.intval;
+
+	switch (psy_val.intval) {
+	case POWER_SUPPLY_USB_TYPE_DCP:
+	case POWER_SUPPLY_USB_TYPE_CDP:
+		dev_dbg(dev, "BC 1.2 result: DCP or CDP\n");
+		break;
+	case POWER_SUPPLY_USB_TYPE_SDP:
+	default:
+		dev_dbg(dev, "BC 1.2 result: SDP\n");
+		break;
+	}
+}
+
+static int anx7688_cc_status(unsigned int v)
+{
+	switch (v) {
+	case 0: return -1;
+	case 1: return -1;
+	case 2: return -1;
+	case 4: return TYPEC_PWR_MODE_USB;
+	case 8: return TYPEC_PWR_MODE_1_5A;
+	case 12: return TYPEC_PWR_MODE_3_0A;
+	default: return -1;
+	}
+}
+
+static const char *anx7688_get_power_mode_name(enum typec_pwr_opmode mode)
+{
+	switch (mode) {
+	case TYPEC_PWR_MODE_USB: return "USB";
+	case TYPEC_PWR_MODE_1_5A: return "1.5A";
+	case TYPEC_PWR_MODE_3_0A: return "3.0A";
+	case TYPEC_PWR_MODE_PD: return "PD";
+	default: return "Unknown";
+	}
+}
+
+/*
+ * This is called after 500ms after connection when the PD contract should have
+ * been negotiated. We should inspect CC pins or PD status here and decide what
+ * input current limit to set.
+ */
+static void anx7688_handle_current_update(struct anx7688 *anx7688)
+{
+	unsigned int cc_status = anx7688->last_cc_status;
+	union power_supply_propval val = {0,};
+	struct device *dev = anx7688->dev;
+	int pwr_mode, ret, current_limit = 0;
+
+	if (anx7688->pd_capable) {
+		pwr_mode = TYPEC_PWR_MODE_PD;
+	} else if (cc_status < 0) {
+		pwr_mode = TYPEC_PWR_MODE_USB;
+	} else {
+		pwr_mode = anx7688_cc_status(cc_status & 0xf);
+		if (pwr_mode < 0)
+			pwr_mode = anx7688_cc_status((cc_status >> 4) & 0xf);
+		if (pwr_mode < 0)
+			pwr_mode = TYPEC_PWR_MODE_USB;
+	}
+
+	if (pwr_mode == TYPEC_PWR_MODE_1_5A)
+		current_limit = 1500;
+	else if (pwr_mode == TYPEC_PWR_MODE_3_0A)
+		current_limit = 3000;
+	else if (pwr_mode == TYPEC_PWR_MODE_PD)
+		current_limit = anx7688->pd_current_limit;
+
+	anx7688->input_current_limit = current_limit;
+
+	dev_info(anx7688->dev, "updating power mode to %s, current limit %dmA (0 => BC1.2)\n",
+		 anx7688_get_power_mode_name(pwr_mode), current_limit);
+
+	if (current_limit) {
+		/*
+		 * Disable BC1.2 detection, because we'll be setting
+		 * a current limit determined by USB-PD
+		 */
+		dev_dbg(dev, "disabling USB BC 1.2 detection\n");
+
+		val.intval = current_limit * 1000;
+		dev_dbg(dev, "setting vbus_in current limit to %d mA\n", current_limit);
+		ret = power_supply_set_property(anx7688->vbus_in_supply,
+						POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
+						&val);
+		if (ret)
+			dev_err(dev, "failed to set vbus_in current to %d mA\n",
+				current_limit);
+	}
+		/*
+		 * else use the result of BC1.2 detection performed by PMIC.
+		 */
+
+	/* Turn on VBUS power path inside PMIC. */
+	val.intval = 1;
+	dev_dbg(dev, "enabling vbus_in power path\n");
+	ret = power_supply_set_property(anx7688->vbus_in_supply,
+					POWER_SUPPLY_PROP_ONLINE,
+					&val);
+	if (ret)
+		dev_err(anx7688->dev, "failed to enable vbus_in\n");
+
+	typec_set_pwr_opmode(anx7688->port, pwr_mode);
+}
+
+static int anx7688_vbus_in_notify(struct notifier_block *nb,
+				  unsigned long val, void *v)
+{
+	struct anx7688 *anx7688 = container_of(nb, struct anx7688, vbus_in_nb);
+	struct power_supply *psy = v;
+
+	/* atomic context */
+	if (val == PSY_EVENT_PROP_CHANGED && psy == anx7688->vbus_in_supply) {
+		set_bit(ANX7688_F_PWRSUPPLY_CHANGE, anx7688->flags);
+		schedule_delayed_work(&anx7688->work, 0);
+	}
+
+	return NOTIFY_OK;
+}
+
+static void anx7688_work(struct work_struct *work)
+{
+	struct anx7688 *anx7688 = container_of(work, struct anx7688, work.work);
+
+	if (test_bit(ANX7688_F_FW_FAILED, anx7688->flags))
+		return;
+
+	mutex_lock(&anx7688->lock);
+
+	if (test_and_clear_bit(ANX7688_F_PWRSUPPLY_CHANGE, anx7688->flags))
+		anx7688_handle_vbus_in_notify(anx7688);
+
+	anx7688_handle_cable_change(anx7688);
+
+	if (test_bit(ANX7688_F_CONNECTED, anx7688->flags)) {
+		/*
+		 * We check status periodically outside of interrupt, just to
+		 * be sure we didn't miss any status interrupts
+		 */
+		anx7688_update_status(anx7688);
+
+		if (anx7688->current_update_deadline &&
+			ktime_after(ktime_get(), anx7688->current_update_deadline)) {
+			anx7688->current_update_deadline = 0;
+			anx7688_handle_current_update(anx7688);
+		}
+	}
+
+	mutex_unlock(&anx7688->lock);
+}
+
+static int anx7688_parse_connector(struct device *dev, struct anx7688 *anx7688, struct typec_capability *cap)
+{
+	struct fwnode_handle *fwnode;
+	const char *buf;
+	int ret;
+
+	fwnode = device_get_named_child_node(dev, "connector");
+	if (!fwnode)
+		return -EINVAL;
+
+	ret = fwnode_property_read_string(fwnode, "power-role", &buf);
+	if (ret) {
+		dev_err(dev, "power-role not found: %d\n", ret);
+		return ret;
+	}
+
+	ret = typec_find_port_power_role(buf);
+	if (ret < 0)
+		return ret;
+	cap->type = ret;
+
+	ret = fwnode_property_read_string(fwnode, "data-role", &buf);
+	if (ret) {
+		dev_err(dev, "data-role not found: %d\n", ret);
+		return ret;
+	}
+
+	ret = typec_find_port_data_role(buf);
+	if (ret < 0)
+		return ret;
+	cap->data = ret;
+
+	ret = fwnode_property_read_string(fwnode, "try-power-role", &buf);
+	if (ret) {
+		dev_err(dev, "try-power-role not found: %d\n", ret);
+		return ret;
+	}
+
+	ret = typec_find_power_role(buf);
+	if (ret < 0)
+		return ret;
+	cap->prefer_role = ret;
+
+	/* Get source pdos */
+	ret = fwnode_property_count_u32(fwnode, "source-pdos");
+	if (ret <= 0)
+		return -EINVAL;
+
+	anx7688->n_src_caps = min_t(u8, ret, ARRAY_SIZE(anx7688->src_caps));
+	ret = fwnode_property_read_u32_array(fwnode, "source-pdos",
+					     anx7688->src_caps,
+					     anx7688->n_src_caps);
+	if (ret < 0) {
+		dev_err(dev, "source cap validate failed: %d\n", ret);
+		return -EINVAL;
+	}
+
+	ret = fwnode_property_count_u32(fwnode, "sink-pdos");
+	if (ret <= 0)
+		return -EINVAL;
+
+	anx7688->n_snk_caps = min_t(u8, ret, ARRAY_SIZE(anx7688->snk_caps));
+	ret = fwnode_property_read_u32_array(fwnode, "sink-pdos",
+					     anx7688->snk_caps,
+					     anx7688->n_snk_caps);
+	if (ret < 0) {
+		dev_err(dev, "sink cap validate failed: %d\n", ret);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int anx7688_i2c_probe(struct i2c_client *client)
+{
+	struct anx7688 *anx7688;
+	struct device *dev = &client->dev;
+	struct typec_capability typec_cap = { };
+	int i, vid_h, vid_l;
+	int irq_cabledet;
+	int ret = 0;
+
+	anx7688 = devm_kzalloc(dev, sizeof(*anx7688), GFP_KERNEL);
+	if (!anx7688)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, anx7688);
+	anx7688->client = client;
+	anx7688->dev = &client->dev;
+	mutex_init(&anx7688->lock);
+	INIT_DELAYED_WORK(&anx7688->work, anx7688_work);
+	anx7688->last_extcon_state = -1;
+
+	ret = anx7688_parse_connector(dev, anx7688, &typec_cap);
+	if (ret < 0) {
+		dev_err(dev, "failed to parse connector from DT\n");
+		return ret;
+	}
+
+	for (i = 0; i < ANX7688_NUM_SUPPLIES; i++)
+		anx7688->supplies[i].supply = anx7688_supply_names[i];
+	ret = devm_regulator_bulk_get(dev, ANX7688_NUM_SUPPLIES,
+				      anx7688->supplies);
+	if (ret)
+		return ret;
+
+	anx7688->vbus_in_supply =
+		devm_power_supply_get_by_phandle(dev, "vbus-in-supply");
+	if (IS_ERR(anx7688->vbus_in_supply)) {
+		dev_err(dev, "Couldn't get the VBUS power supply\n");
+		return PTR_ERR(anx7688->vbus_in_supply);
+	}
+
+	if (!anx7688->vbus_in_supply)
+		return -EPROBE_DEFER;
+
+	anx7688->gpio_enable = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW);
+	if (IS_ERR(anx7688->gpio_enable)) {
+		dev_err(dev, "Could not get enable gpio\n");
+		return PTR_ERR(anx7688->gpio_enable);
+	}
+
+	anx7688->gpio_reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(anx7688->gpio_reset)) {
+		dev_err(dev, "Could not get reset gpio\n");
+		return PTR_ERR(anx7688->gpio_reset);
+	}
+
+	anx7688->gpio_cabledet = devm_gpiod_get(dev, "cabledet", GPIOD_IN);
+	if (IS_ERR(anx7688->gpio_cabledet)) {
+		dev_err(dev, "Could not get cabledet gpio\n");
+		return PTR_ERR(anx7688->gpio_cabledet);
+	}
+
+	irq_cabledet = gpiod_to_irq(anx7688->gpio_cabledet);
+	if (irq_cabledet < 0) {
+		dev_err(dev, "Could not get cabledet irq\n");
+		return irq_cabledet;
+	}
+
+	// Initialize extcon device
+	anx7688->extcon = devm_extcon_dev_allocate(dev, anx7688_extcon_cable);
+	if (IS_ERR(anx7688->extcon))
+		return -ENOMEM;
+
+	ret = devm_extcon_dev_register(dev, anx7688->extcon);
+	if (ret) {
+		dev_err(dev, "failed to register extcon device\n");
+		return ret;
+	}
+
+	// Register the TCPC i2c interface as second interface (0x58)
+	anx7688->client_tcpc = i2c_new_dummy_device(client->adapter, 0x2c);
+	if (IS_ERR(anx7688->client_tcpc)) {
+		dev_err(dev, "Could not register tcpc i2c client\n");
+		return PTR_ERR(anx7688->client_tcpc);
+	}
+	i2c_set_clientdata(anx7688->client_tcpc, anx7688);
+
+	// powerup and probe the ANX chip
+
+	ret = regulator_bulk_enable(ANX7688_NUM_ALWAYS_ON_SUPPLIES,
+				    anx7688->supplies);
+	if (ret) {
+		dev_err(dev, "Could not enable regulators\n");
+		goto err_dummy_dev;
+	}
+
+	msleep(10);
+
+	anx7688_power_enable(anx7688);
+
+	vid_l = anx7688_tcpc_reg_read(anx7688, ANX7688_TCPC_REG_VENDOR_ID0);
+	vid_h = anx7688_tcpc_reg_read(anx7688, ANX7688_TCPC_REG_VENDOR_ID1);
+	if (vid_l < 0 || vid_h < 0) {
+		ret = vid_l < 0 ? vid_l : vid_h;
+		anx7688_power_disable(anx7688);
+		goto err_disable_reg;
+	}
+
+	dev_info(dev, "Vendor id 0x%04x\n", vid_l | vid_h << 8);
+
+	anx7688_power_disable(anx7688);
+
+	anx7688->role_sw = usb_role_switch_get(dev);
+	if (IS_ERR(anx7688->role_sw)) {
+		dev_err(dev, "Could not get role switch\n");
+		ret = PTR_ERR(anx7688->role_sw);
+		goto err_disable_reg;
+	}
+
+	// setup a typec port device
+	typec_cap.revision = USB_TYPEC_REV_1_2;
+	typec_cap.pd_revision = 0x200;
+	ret = -EINVAL;
+	if (typec_cap.type != TYPEC_PORT_DRP)
+		goto err_disable_reg;
+	if (typec_cap.data != TYPEC_PORT_DRD)
+		goto err_disable_reg;
+	typec_cap.driver_data = anx7688;
+	typec_cap.ops = &anx7688_typec_ops;
+
+	anx7688->port = typec_register_port(dev, &typec_cap);
+	if (IS_ERR(anx7688->port)) {
+		dev_err(dev, "Could not register type-c port\n");
+		ret = PTR_ERR(anx7688->port);
+		goto err_role_sw;
+	}
+
+	anx7688->pwr_role = TYPEC_SINK;
+	anx7688->data_role = TYPEC_DEVICE;
+	typec_set_pwr_role(anx7688->port, anx7688->pwr_role);
+	typec_set_data_role(anx7688->port, anx7688->data_role);
+	typec_set_pwr_opmode(anx7688->port, TYPEC_PWR_MODE_USB);
+	typec_set_vconn_role(anx7688->port, TYPEC_SINK);
+
+	// make sure BC1.2 detection in PMIC is enabled
+	anx7688->last_bc_result = -1;
+
+	dev_dbg(dev, "enabling USB BC 1.2 detection\n");
+
+	ret = devm_request_irq(dev, irq_cabledet, anx7688_irq_plug_handler,
+			       IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+			       "anx7688-cabledet", anx7688);
+	if (ret < 0) {
+		dev_err(dev, "Could not request cabledet irq (%d)\n", ret);
+		goto err_cport;
+	}
+
+	ret = devm_request_threaded_irq(dev, client->irq,
+					NULL, anx7688_irq_status_handler,
+					IRQF_ONESHOT, NULL, anx7688);
+	if (ret < 0) {
+		dev_err(dev, "Could not request irq (%d)\n", ret);
+		goto err_cport;
+	}
+
+	anx7688->vbus_in_nb.notifier_call = anx7688_vbus_in_notify;
+	anx7688->vbus_in_nb.priority = 0;
+	ret = power_supply_reg_notifier(&anx7688->vbus_in_nb);
+	if (ret)
+		goto err_cport;
+
+	anx7688->debug_root = debugfs_create_dir("anx7688", NULL);
+	debugfs_create_file("firmware", 0444, anx7688->debug_root, anx7688,
+			    &anx7688_firmware_fops);
+	debugfs_create_file("regs", 0444, anx7688->debug_root, anx7688,
+			    &anx7688_regs_fops);
+	debugfs_create_file("status", 0444, anx7688->debug_root, anx7688,
+			    &anx7688_status_fops);
+
+	schedule_delayed_work(&anx7688->work, msecs_to_jiffies(10));
+
+	timer_setup(&anx7688->work_timer, anx7688_cabledet_timer_fn, 0);
+	mod_timer(&anx7688->work_timer, jiffies + msecs_to_jiffies(1000));
+	return 0;
+
+err_cport:
+	typec_unregister_port(anx7688->port);
+err_role_sw:
+	usb_role_switch_put(anx7688->role_sw);
+err_disable_reg:
+	regulator_bulk_disable(ANX7688_NUM_ALWAYS_ON_SUPPLIES, anx7688->supplies);
+err_dummy_dev:
+	i2c_unregister_device(anx7688->client_tcpc);
+	return ret;
+}
+
+static void anx7688_i2c_remove(struct i2c_client *client)
+{
+	struct anx7688 *anx7688 = i2c_get_clientdata(client);
+
+	mutex_lock(&anx7688->lock);
+
+	power_supply_unreg_notifier(&anx7688->vbus_in_nb);
+
+	del_timer_sync(&anx7688->work_timer);
+
+	cancel_delayed_work_sync(&anx7688->work);
+
+	if (test_bit(ANX7688_F_CONNECTED, anx7688->flags))
+		anx7688_disconnect(anx7688);
+
+	typec_unregister_partner(anx7688->partner);
+	typec_unregister_port(anx7688->port);
+	usb_role_switch_put(anx7688->role_sw);
+
+	regulator_bulk_disable(ANX7688_NUM_ALWAYS_ON_SUPPLIES, anx7688->supplies);
+	i2c_unregister_device(anx7688->client_tcpc);
+
+	debugfs_remove(anx7688->debug_root);
+
+	mutex_unlock(&anx7688->lock);
+}
+
+static int __maybe_unused anx7688_suspend(struct device *dev)
+{
+	struct anx7688 *anx7688 = i2c_get_clientdata(to_i2c_client(dev));
+
+	del_timer_sync(&anx7688->work_timer);
+	cancel_delayed_work_sync(&anx7688->work);
+
+	regulator_disable(anx7688->supplies[ANX7688_I2C_INDEX].consumer);
+
+	return 0;
+}
+
+static int __maybe_unused anx7688_resume(struct device *dev)
+{
+	struct anx7688 *anx7688 = i2c_get_clientdata(to_i2c_client(dev));
+	int ret;
+
+	ret = regulator_enable(anx7688->supplies[ANX7688_I2C_INDEX].consumer);
+	if (ret)
+		dev_warn(anx7688->dev,
+			 "failed to enable I2C regulator (%d)\n", ret);
+
+	// check status right after resume, since it could have changed during
+	// sleep
+	schedule_delayed_work(&anx7688->work, msecs_to_jiffies(50));
+	mod_timer(&anx7688->work_timer, jiffies + msecs_to_jiffies(1000));
+
+	return 0;
+}
+
+static const struct dev_pm_ops anx7688_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(anx7688_suspend, anx7688_resume)
+};
+
+static const struct i2c_device_id anx7688_ids[] = {
+	{ "anx7688", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, anx7688_ids);
+
+static const struct of_device_id anx7688_of_match_table[] = {
+	{ .compatible = "analogix,anx7688" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, anx7688_of_match_table);
+
+static struct i2c_driver anx7688_driver = {
+	.driver = {
+		.name = "anx7688",
+		.of_match_table = anx7688_of_match_table,
+		.pm = &anx7688_pm_ops,
+	},
+	.probe = anx7688_i2c_probe,
+	.remove = anx7688_i2c_remove,
+	.id_table = anx7688_ids,
+};
+
+module_i2c_driver(anx7688_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Martijn Braam <martijn@brixit.nl>");
+MODULE_AUTHOR("Ondrej Jirman <megi@xff.cz>");
+MODULE_DESCRIPTION("Analogix ANX7688 USB-C DisplayPort bridge");


-- 
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCHv2 1/2] dt-bindings: usb: typec: anx7688: start a binding document
  2024-02-23 21:28 ` [PATCHv2 1/2] " Pavel Machek
@ 2024-02-23 22:59   ` Rob Herring
  2024-03-11 21:22   ` Pavel Machek
  1 sibling, 0 replies; 14+ messages in thread
From: Rob Herring @ 2024-02-23 22:59 UTC (permalink / raw)
  To: Pavel Machek
  Cc: martijn, phone-devel, megi, devicetree, heikki.krogerus,
	krzysztof.kozlowski+dt, gregkh, kernel list, linux-usb,
	fiona.klute, robh+dt, samuel


On Fri, 23 Feb 2024 22:28:42 +0100, Pavel Machek wrote:
> Add binding for anx7688 usb type-c bridge. I don't have a datasheet,
> but I did best I could.
> 
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> ---
> 
> v2: implement review feedback
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/analogix,anx7688.example.dtb: typec@2c: 'hdmi_vt-supply' does not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/usb/analogix,anx7688.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/ZdkOCqPKqa/u9Ftb@duo.ucw.cz

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCHv2 1/2] dt-bindings: usb: typec: anx7688: start a binding document
  2024-02-23 21:28 ` [PATCHv2 1/2] " Pavel Machek
  2024-02-23 22:59   ` Rob Herring
@ 2024-03-11 21:22   ` Pavel Machek
  2024-03-12 10:51     ` Krzysztof Kozlowski
  1 sibling, 1 reply; 14+ messages in thread
From: Pavel Machek @ 2024-03-11 21:22 UTC (permalink / raw)
  To: phone-devel, kernel list, fiona.klute, martijn, samuel,
	heikki.krogerus, gregkh, linux-usb, robh+dt,
	krzysztof.kozlowski+dt, devicetree, megi

[-- Attachment #1: Type: text/plain, Size: 4387 bytes --]

Hi!

> Add binding for anx7688 usb type-c bridge. I don't have a datasheet,
> but I did best I could.
> 
> Signed-off-by: Pavel Machek <pavel@ucw.cz>

Any more comments here? Automatic system told me I need to replace one
character...

Best regards,
								Pavel

> ---
> 
> v2: implement review feedback
> 
> diff --git a/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
> new file mode 100644
> index 000000000000..9e887eafb5fc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
> @@ -0,0 +1,127 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/analogix,anx7688.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +# Pin names can be deduced from
> +# https://files.pine64.org/doc/PinePhone/PinePhone%20v1.2b%20Released%20Schematic.pdf
> +
> +title: Analogix ANX7688 Type-C controller
> +
> +maintainers:
> +  - Pavel Machek <pavel@ucw.cz>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - analogix,anx7688
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  reset-gpios:
> +    maxItems: 1
> +    description: GPIO controlling RESET_N (B7) pin.
> +
> +  enable-gpios:
> +    maxItems: 1
> +    description: GPIO controlling POWER_EN (D2) pin.
> +
> +  cabledet-gpios:
> +    maxItems: 1
> +    description: GPIO controlling CABLE_DET (C3) pin.
> +
> +  avdd10-supply:
> +    description: 1.0V power supply going to AVDD10 (A4, ...) pins
> +
> +  dvdd10-supply:
> +    description: 1.0V power supply going to DVDD10 (D6, ...) pins
> +
> +  avdd18-supply:
> +    description: 1.8V power supply going to AVDD18 (E3, ...) pins
> +
> +  dvdd18-supply:
> +    description: 1.8V power supply going to DVDD18 (G4, ...) pins
> +
> +  avdd33-supply:
> +    description: 3.3V power supply going to AVDD33 (C4, ...) pins
> +
> +  i2c-supply: true
> +  vconn-supply: true
> +  hdmi-vt-supply: true
> +  vbus-supply: true
> +  vbus-in-supply: true
> +
> +  connector:
> +    type: object
> +    $ref: /schemas/connector/usb-connector.yaml
> +
> +    description:
> +      Properties for usb c connector.
> +
> +    properties:
> +      compatible:
> +        const: usb-c-connector
> +
> +required:
> +  - compatible
> +  - reg
> +  - connector
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        typec@2c {
> +            compatible = "analogix,anx7688";
> +            reg = <0x2c>;
> +            interrupts = <8 IRQ_TYPE_EDGE_FALLING>;
> +            interrupt-parent = <&gpio0>;
> +
> +            enable-gpios = <&pio 3 10 GPIO_ACTIVE_LOW>; /* PD10 */
> +            reset-gpios = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
> +            cabledet-gpios = <&r_pio 0 8 GPIO_ACTIVE_HIGH>; /* PL8 */
> +
> +            avdd10-supply = <&reg_anx1v0>;
> +            dvdd10-supply = <&reg_anx1v0>;
> +            avdd18-supply = <&reg_ldo_io1>;
> +            dvdd18-supply = <&reg_ldo_io1>;
> +            avdd33-supply = <&reg_dcdc1>;
> +            i2c-supply = <&reg_ldo_io0>;
> +            vconn-supply = <&reg_vconn5v0>;
> +            hdmi_vt-supply = <&reg_dldo1>;
> +
> +            vbus-supply = <&reg_usb_5v>;
> +            vbus-in-supply = <&usb_power_supply>;
> +
> +            typec_con: connector {
> +                compatible = "usb-c-connector";
> +                power-role = "dual";
> +                data-role = "dual";
> +                try-power-role = "source";
> +
> +                ports {
> +                    #address-cells = <1>;
> +                    #size-cells = <0>;
> +                    port@0 {
> +                        reg = <0>;
> +                        typec_con_ep: endpoint {
> +                            remote-endpoint = <&usbotg_hs_ep>;
> +                        };
> +                    };
> +                };
> +            };
> +        };
> +    };
> +...
> 



-- 
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCHv2 2/2] usb: typec: anx7688: Add driver for ANX7688 USB-C HDMI bridge
  2024-02-23 21:28 ` [PATCHv2 2/2] usb: typec: anx7688: Add driver for ANX7688 USB-C HDMI bridge Pavel Machek
@ 2024-03-11 21:22   ` Pavel Machek
  2024-03-12 14:12   ` Heikki Krogerus
  1 sibling, 0 replies; 14+ messages in thread
From: Pavel Machek @ 2024-03-11 21:22 UTC (permalink / raw)
  To: phone-devel, kernel list, fiona.klute, martijn, samuel,
	heikki.krogerus, gregkh, linux-usb, megi

[-- Attachment #1: Type: text/plain, Size: 759 bytes --]

Hi!

> From: Ondrej Jirman <megi@xff.cz>
> 
> This is driver for ANX7688 USB-C HDMI, with flashing and debugging
> features removed. ANX7688 is rather criticial piece on PinePhone,
> there's no display and no battery charging without it.
> 
> There's likely more work to be done here, but having basic support
> in mainline is needed to be able to work on the other stuff
> (networking, cameras, power management).
> 
> Signed-off-by: Ondrej Jirman <megi@xff.cz>
> Co-developed-by: Martijn Braam <martijn@brixit.nl>
> Co-developed-by: Samuel Holland <samuel@sholland.org>
> Signed-off-by: Pavel Machek <pavel@ucw.cz>

Any comments here?

Best regards,
								Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCHv2 1/2] dt-bindings: usb: typec: anx7688: start a binding document
  2024-03-11 21:22   ` Pavel Machek
@ 2024-03-12 10:51     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2024-03-12 10:51 UTC (permalink / raw)
  To: Pavel Machek, phone-devel, kernel list, fiona.klute, martijn,
	samuel, heikki.krogerus, gregkh, linux-usb, robh+dt,
	krzysztof.kozlowski+dt, devicetree, megi

On 11/03/2024 22:22, Pavel Machek wrote:
> Hi!
> 
>> Add binding for anx7688 usb type-c bridge. I don't have a datasheet,
>> but I did best I could.
>>
>> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> Any more comments here? Automatic system told me I need to replace one
> character...

Well, I often do not review patches which have build failure so were not
compiled. In the terms of bindings `dt_binding_check` is like test
compilation of C, so just like no one reviews onbuildable C code, I
don't usually look at "unbuildable" bindings.

Plus original v2 patch went into some other email thread, because of
"References" field, so a bit disappeared from my inbox.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCHv2 2/2] usb: typec: anx7688: Add driver for ANX7688 USB-C HDMI bridge
  2024-02-23 21:28 ` [PATCHv2 2/2] usb: typec: anx7688: Add driver for ANX7688 USB-C HDMI bridge Pavel Machek
  2024-03-11 21:22   ` Pavel Machek
@ 2024-03-12 14:12   ` Heikki Krogerus
  2024-03-21 22:10     ` Pavel Machek
  1 sibling, 1 reply; 14+ messages in thread
From: Heikki Krogerus @ 2024-03-12 14:12 UTC (permalink / raw)
  To: Pavel Machek
  Cc: phone-devel, kernel list, fiona.klute, martijn, samuel, gregkh,
	linux-usb, megi

Hi Pavel,

I'm sorry to keep you waiting.

On Fri, Feb 23, 2024 at 10:28:49PM +0100, Pavel Machek wrote:
> From: Ondrej Jirman <megi@xff.cz>
> 
> This is driver for ANX7688 USB-C HDMI, with flashing and debugging
> features removed. ANX7688 is rather criticial piece on PinePhone,
> there's no display and no battery charging without it.
> 
> There's likely more work to be done here, but having basic support
> in mainline is needed to be able to work on the other stuff
> (networking, cameras, power management).
> 
> Signed-off-by: Ondrej Jirman <megi@xff.cz>
> Co-developed-by: Martijn Braam <martijn@brixit.nl>
> Co-developed-by: Samuel Holland <samuel@sholland.org>
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> ---
> 
> v2: Fix checkpatch stuff. Some cleanups, adapt to dts format in 1/2.
> 
> diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> index 2f80c2792dbd..c9043ae61546 100644
> --- a/drivers/usb/typec/Kconfig
> +++ b/drivers/usb/typec/Kconfig
> @@ -64,6 +64,17 @@ config TYPEC_ANX7411
>  	  If you choose to build this driver as a dynamically linked module, the
>  	  module will be called anx7411.ko.
>  
> +config TYPEC_ANX7688
> +	tristate "Analogix ANX7688 Type-C DRP Port controller and mux driver"
> +	depends on I2C
> +	depends on USB_ROLE_SWITCH
> +	help
> +	  Say Y or M here if your system has Analogix ANX7688 Type-C Bridge
> +	  controller driver.
> +
> +	  If you choose to build this driver as a dynamically linked module, the
> +	  module will be called anx7688.ko.
> +
>  config TYPEC_RT1719
>  	tristate "Richtek RT1719 Sink Only Type-C controller driver"
>  	depends on USB_ROLE_SWITCH || !USB_ROLE_SWITCH
> diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
> index 7a368fea61bc..3f8ff94ad294 100644
> --- a/drivers/usb/typec/Makefile
> +++ b/drivers/usb/typec/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_TYPEC_TCPM)	+= tcpm/
>  obj-$(CONFIG_TYPEC_UCSI)	+= ucsi/
>  obj-$(CONFIG_TYPEC_TPS6598X)	+= tipd/
>  obj-$(CONFIG_TYPEC_ANX7411)	+= anx7411.o
> +obj-$(CONFIG_TYPEC_ANX7688)	+= anx7688.o
>  obj-$(CONFIG_TYPEC_HD3SS3220)	+= hd3ss3220.o
>  obj-$(CONFIG_TYPEC_STUSB160X) 	+= stusb160x.o
>  obj-$(CONFIG_TYPEC_RT1719)	+= rt1719.o
> diff --git a/drivers/usb/typec/anx7688.c b/drivers/usb/typec/anx7688.c
> new file mode 100644
> index 000000000000..14d033bbc0c7
> --- /dev/null
> +++ b/drivers/usb/typec/anx7688.c
> @@ -0,0 +1,1927 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ANX7688 USB-C HDMI bridge/PD driver
> + *
> + * Warning, this driver is somewhat PinePhone specific.
> + *
> + * How this works:
> + * - this driver allows to program firmware into ANX7688 EEPROM, and
> + *   initialize it
> + * - it then communicates with the firmware running on the OCM (on-chip
> + *   microcontroller)
> + * - it detects whether there is cable plugged in or not and powers
> + *   up or down the ANX7688 based on that
> + * - when the cable is connected the firmware on the OCM will handle
> + *   the detection of the nature of the device on the other end
> + *   of the USB-C cable
> + * - this driver then communicates with the USB phy to let it swap
> + *   data roles accordingly
> + * - it also enables VBUS and VCONN regulators as appropriate
> + * - USB phy driver (Allwinner) needs to know whether to switch to
> + *   device or host mode, or whether to turn off
> + * - when the firmware detects SRC.1.5A or SRC.3.0A via CC pins
> + *   or something else via PD, it notifies this driver via software
> + *   interrupt and this driver will determine how to update the TypeC
> + *   port status and what input current limit is appropriate
> + * - input current limit determination happens 500ms after cable
> + *   insertion or hard reset (delay is necessary to determine whether
> + *   the remote end is PD capable or not)
> + * - this driver tells to the PMIC driver that the input current limit
> + *   needs to be changed
> + * - this driver also monitors PMIC status and re-sets the input current
> + *   limit if it changes for some reason (due to PMIC internal decision
> + *   making) (this is disabled for now)
> + *
> + * ANX7688 FW behavior as observed:
> + *
> + * - DO NOT SET MORE THAN 1 SINK CAPABILITY! Firmware will ignore what
> + *   you set and send hardcoded PDO_BATT 5-21V 30W message!
> + *
> + * Product brief:
> + * https://www.analogix.com/en/system/files/AA-002281-PB-6-ANX7688_Product_Brief_0.pdf
> + * Development notes:
> + * https://xnux.eu/devices/feature/anx7688.html
> + */
> +
> +#include <linux/debugfs.h>
> +#include <linux/delay.h>
> +#include <linux/extcon-provider.h>
> +#include <linux/firmware.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/irqreturn.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_irq.h>
> +#include <linux/power_supply.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/usb/pd.h>
> +#include <linux/usb/role.h>
> +#include <linux/usb/typec.h>
> +
> +/* firmware regs */
> +
> +#define ANX7688_REG_VBUS_OFF_DELAY_TIME 0x22
> +#define ANX7688_REG_FEATURE_CTRL        0x27
> +#define ANX7688_REG_EEPROM_LOAD_STATUS1 0x11
> +#define ANX7688_REG_EEPROM_LOAD_STATUS0 0x12
> +#define ANX7688_REG_FW_VERSION1         0x15
> +#define ANX7688_REG_FW_VERSION0         0x16
> +
> +#define ANX7688_EEPROM_FW_LOADED	0x01
> +
> +#define ANX7688_REG_STATUS_INT_MASK     0x17
> +#define ANX7688_REG_STATUS_INT          0x28
> +#define ANX7688_IRQS_RECEIVED_MSG       BIT(0)
> +#define ANX7688_IRQS_RECEIVED_ACK       BIT(1)
> +#define ANX7688_IRQS_VCONN_CHANGE       BIT(2)
> +#define ANX7688_IRQS_VBUS_CHANGE        BIT(3)
> +#define ANX7688_IRQS_CC_STATUS_CHANGE   BIT(4)
> +#define ANX7688_IRQS_DATA_ROLE_CHANGE   BIT(5)
> +
> +#define ANX7688_REG_STATUS              0x29
> +#define ANX7688_VCONN_STATUS            BIT(2) /* 0 = off  1 = on */
> +#define ANX7688_VBUS_STATUS             BIT(3) /* 0 = off  1 = on */
> +#define ANX7688_DATA_ROLE_STATUS        BIT(5) /* 0 = device 1 = host */
> +
> +#define ANX7688_REG_CC_STATUS           0x2a
> +#define ANX7688_REG_TRY_UFP_TIMER       0x23
> +#define ANX7688_REG_TIME_CTRL           0x24
> +
> +#define ANX7688_REG_MAX_VOLTAGE         0x1b
> +#define ANX7688_REG_MAX_POWER           0x1c
> +#define ANX7688_REG_MIN_POWER           0x1d
> +#define ANX7688_REG_MAX_VOLTAGE_STATUS  0x1e
> +#define ANX7688_REG_MAX_POWER_STATUS    0x1f
> +
> +#define ANX7688_SOFT_INT_MASK           0x7f
> +
> +/* tcpc regs */
> +
> +#define ANX7688_TCPC_REG_VENDOR_ID0     0x00
> +#define ANX7688_TCPC_REG_VENDOR_ID1     0x01
> +#define ANX7688_TCPC_REG_ALERT0         0x10
> +#define ANX7688_TCPC_REG_ALERT1         0x11
> +#define ANX7688_TCPC_REG_ALERT_MASK0    0x12
> +#define ANX7688_TCPC_REG_ALERT_MASK1    0x13
> +#define ANX7688_TCPC_REG_INTERFACE_SEND 0x30
> +#define ANX7688_TCPC_REG_INTERFACE_RECV 0x51
> +
> +/* hw regs */
> +
> +#define ANX7688_REG_IRQ_EXT_SOURCE0     0x3e
> +#define ANX7688_REG_IRQ_EXT_SOURCE1     0x4e
> +#define ANX7688_REG_IRQ_EXT_SOURCE2     0x4f
> +#define ANX7688_REG_IRQ_EXT_MASK0       0x3b
> +#define ANX7688_REG_IRQ_EXT_MASK1       0x3c
> +#define ANX7688_REG_IRQ_EXT_MASK2       0x3d
> +#define ANX7688_REG_IRQ_SOURCE0         0x54
> +#define ANX7688_REG_IRQ_SOURCE1         0x55
> +#define ANX7688_REG_IRQ_SOURCE2         0x56
> +#define ANX7688_REG_IRQ_MASK0           0x57
> +#define ANX7688_REG_IRQ_MASK1           0x58
> +#define ANX7688_REG_IRQ_MASK2           0x59
> +
> +#define ANX7688_IRQ2_SOFT_INT           BIT(2)
> +
> +#define ANX7688_REG_USBC_RESET_CTRL		0x05
> +#define ANX7688_USBC_RESET_CTRL_OCM_RESET	BIT(4)
> +
> +/* ocm messages */
> +
> +#define ANX7688_OCM_MSG_PWR_SRC_CAP     0x00
> +#define ANX7688_OCM_MSG_PWR_SNK_CAP     0x01
> +#define ANX7688_OCM_MSG_DP_SNK_IDENTITY 0x02
> +#define ANX7688_OCM_MSG_SVID            0x03
> +#define ANX7688_OCM_MSG_GET_DP_SNK_CAP  0x04
> +#define ANX7688_OCM_MSG_ACCEPT          0x05
> +#define ANX7688_OCM_MSG_REJECT          0x06
> +#define ANX7688_OCM_MSG_PSWAP_REQ       0x10
> +#define ANX7688_OCM_MSG_DSWAP_REQ       0x11
> +#define ANX7688_OCM_MSG_GOTO_MIN_REQ    0x12
> +#define ANX7688_OCM_MSG_VCONN_SWAP_REQ  0x13
> +#define ANX7688_OCM_MSG_VDM             0x14
> +#define ANX7688_OCM_MSG_DP_SNK_CFG      0x15
> +#define ANX7688_OCM_MSG_PWR_OBJ_REQ     0x16
> +#define ANX7688_OCM_MSG_PD_STATUS_REQ   0x17
> +#define ANX7688_OCM_MSG_DP_ALT_ENTER    0x19
> +#define ANX7688_OCM_MSG_DP_ALT_EXIT     0x1a
> +#define ANX7688_OCM_MSG_GET_SNK_CAP     0x1b
> +#define ANX7688_OCM_MSG_RESPONSE_TO_REQ 0xf0
> +#define ANX7688_OCM_MSG_SOFT_RST        0xf1
> +#define ANX7688_OCM_MSG_HARD_RST        0xf2
> +#define ANX7688_OCM_MSG_RESTART         0xf3
> +
> +static const char * const anx7688_supply_names[] = {
> +	"avdd33",
> +	"avdd18",
> +	"dvdd18",
> +	"avdd10",
> +	"dvdd10",
> +	"i2c",
> +	"hdmi-vt",
> +
> +	"vconn", // power for VCONN1/VCONN2 switches
> +	"vbus", // vbus power
> +};
> +
> +#define ANX7688_NUM_SUPPLIES ARRAY_SIZE(anx7688_supply_names)
> +#define ANX7688_NUM_ALWAYS_ON_SUPPLIES (ANX7688_NUM_SUPPLIES - 1)
> +
> +#define ANX7688_I2C_INDEX (ANX7688_NUM_SUPPLIES - 4)
> +#define ANX7688_VCONN_INDEX (ANX7688_NUM_SUPPLIES - 2)
> +#define ANX7688_VBUS_INDEX (ANX7688_NUM_SUPPLIES - 1)
> +
> +enum {
> +	ANX7688_F_POWERED,
> +	ANX7688_F_CONNECTED,
> +	ANX7688_F_FW_FAILED,
> +	ANX7688_F_PWRSUPPLY_CHANGE,
> +	ANX7688_F_CURRENT_UPDATE,
> +};
> +
> +struct anx7688 {
> +	struct device *dev;
> +	struct i2c_client *client;
> +	struct i2c_client *client_tcpc;
> +	struct regulator_bulk_data supplies[ANX7688_NUM_SUPPLIES];
> +	struct power_supply *vbus_in_supply;
> +	struct notifier_block vbus_in_nb;
> +	int input_current_limit; // mA
> +	struct gpio_desc *gpio_enable;
> +	struct gpio_desc *gpio_reset;
> +	struct gpio_desc *gpio_cabledet;
> +
> +	uint32_t src_caps[8];

Use u32 instead of uint32_t.

> +	unsigned int n_src_caps;
> +
> +	uint32_t snk_caps[8];

Ditto.

> +	unsigned int n_snk_caps;
> +
> +	unsigned long flags[1];
> +
> +	struct delayed_work work;
> +	struct timer_list work_timer;
> +
> +	struct mutex lock;
> +	bool vbus_on, vconn_on;
> +	bool pd_capable;
> +	int pd_current_limit; // mA
> +	ktime_t current_update_deadline;
> +
> +	struct typec_port *port;
> +	struct typec_partner *partner;
> +	struct usb_pd_identity partner_identity;
> +	struct usb_role_switch *role_sw;
> +	int pwr_role, data_role;
> +
> +	struct dentry *debug_root;
> +
> +	/* for debug */
> +	int last_status;
> +	int last_cc_status;
> +	int last_dp_state;
> +	int last_bc_result;
> +
> +	/* for HDMI HPD */
> +	struct extcon_dev *extcon;
> +	int last_extcon_state;
> +};
> +
> +static const unsigned int anx7688_extcon_cable[] = {
> +	EXTCON_DISP_HDMI,
> +	EXTCON_NONE,
> +};
> +
> +static int anx7688_reg_read(struct anx7688 *anx7688, u8 reg_addr)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_read_byte_data(anx7688->client, reg_addr);
> +	if (ret < 0)
> +		dev_err(anx7688->dev, "i2c read failed at 0x%x (%d)\n",
> +			reg_addr, ret);

dev_dbg instead.

> +
> +	return ret;
> +}
> +
> +static int anx7688_reg_write(struct anx7688 *anx7688, u8 reg_addr, u8 value)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_write_byte_data(anx7688->client, reg_addr, value);
> +	if (ret < 0)
> +		dev_err(anx7688->dev, "i2c write failed at 0x%x (%d)\n",
> +			reg_addr, ret);

Ditto.

> +
> +	return ret;
> +}
> +
> +static int anx7688_reg_update_bits(struct anx7688 *anx7688, u8 reg_addr,
> +				   u8 mask, u8 value)
> +{
> +	int ret;
> +
> +	ret = anx7688_reg_read(anx7688, reg_addr);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret &= ~mask;
> +	ret |= value;
> +
> +	return anx7688_reg_write(anx7688, reg_addr, ret);
> +}
> +
> +static int anx7688_tcpc_reg_read(struct anx7688 *anx7688, u8 reg_addr)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_read_byte_data(anx7688->client_tcpc, reg_addr);
> +	if (ret < 0)
> +		dev_err(anx7688->dev, "tcpc i2c read failed at 0x%x (%d)\n",
> +			reg_addr, ret);

Ditto.

> +
> +	return ret;
> +}
> +
> +static int anx7688_tcpc_reg_write(struct anx7688 *anx7688, u8 reg_addr, u8 value)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_write_byte_data(anx7688->client_tcpc, reg_addr, value);
> +	if (ret < 0)
> +		dev_err(anx7688->dev, "tcpc i2c write failed at 0x%x (%d)\n",
> +			reg_addr, ret);

Ditto.

> +
> +	return ret;
> +}
> +
> +static void anx7688_power_enable(struct anx7688 *anx7688)
> +{
> +	gpiod_set_value(anx7688->gpio_reset, 1);
> +	gpiod_set_value(anx7688->gpio_enable, 1);
> +
> +	/* wait for power to stabilize and release reset */
> +	msleep(10);

So is it okay that the sleep may take up to 20ms?

> +	gpiod_set_value(anx7688->gpio_reset, 0);
> +	udelay(2);

Use usleep_range() instead.

> +	dev_dbg(anx7688->dev, "power enabled\n");
> +
> +	set_bit(ANX7688_F_POWERED, anx7688->flags);
> +}
> +
> +static void anx7688_power_disable(struct anx7688 *anx7688)
> +{
> +	gpiod_set_value(anx7688->gpio_reset, 1);
> +	msleep(5);

The same question here, is it a problem if the sleep ends up taking
20ms?

> +	gpiod_set_value(anx7688->gpio_enable, 0);
> +
> +	dev_dbg(anx7688->dev, "power disabled\n");
> +
> +	clear_bit(ANX7688_F_POWERED, anx7688->flags);
> +}
> +
> +static int anx7688_send_ocm_message(struct anx7688 *anx7688, int cmd,
> +				    const u8 *data, int data_len)
> +{
> +	int ret = 0, i;
> +	u8 pkt[32];
> +
> +	if (data_len > sizeof(pkt) - 3) {
> +		dev_dbg(anx7688->dev,
> +			"invalid ocm message length cmd=0x%02x len=%d\n",
> +			cmd, data_len);
> +		return -EINVAL;
> +	}
> +
> +	// prepare pd packet
> +	pkt[0] = data_len + 1;
> +	pkt[1] = cmd;
> +	if (data_len > 0)
> +		memcpy(pkt + 2, data, data_len);
> +	pkt[2 + data_len] = 0;
> +	for (i = 0; i < data_len + 2; i++)
> +		pkt[data_len + 2] -= pkt[i];
> +
> +	dev_dbg(anx7688->dev, "send pd packet cmd=0x%02x %*ph\n",
> +		cmd, data_len + 3, pkt);
> +
> +	ret = anx7688_tcpc_reg_read(anx7688, ANX7688_TCPC_REG_INTERFACE_SEND);
> +	if (ret) {
> +		dev_err(anx7688->dev,
> +			"failed to send pd packet (tx buffer full)\n");

One line should be enought for that one.

> +		return -EBUSY;
> +	}
> +
> +	ret = i2c_smbus_write_i2c_block_data(anx7688->client_tcpc,
> +					     ANX7688_TCPC_REG_INTERFACE_SEND,
> +					     data_len + 3, pkt);
> +	if (ret < 0)
> +		dev_err(anx7688->dev,
> +			"failed to send pd packet (err=%d)\n", ret);

Ditto.

> +	// wait until the message is processed (30ms max)
> +	for (i = 0; i < 300; i++) {
> +		ret = anx7688_tcpc_reg_read(anx7688, ANX7688_TCPC_REG_INTERFACE_SEND);
> +		if (ret <= 0)
> +			return ret;
> +
> +		udelay(100);
> +	}
> +
> +	dev_err(anx7688->dev, "timeout waiting for the message queue flush\n");

Maybe dev_dbg for this too.

> +	return -ETIMEDOUT;
> +}
> +
> +static int anx7688_connect(struct anx7688 *anx7688)
> +{
> +	struct typec_partner_desc desc = {};
> +	int ret, i;
> +	u8 fw[2];
> +	const u8 dp_snk_identity[16] = {
> +		0x00, 0x00, 0x00, 0xec,	/* id header */
> +		0x00, 0x00, 0x00, 0x00,	/* cert stat */
> +		0x00, 0x00, 0x00, 0x00,	/* product type */
> +		0x39, 0x00, 0x00, 0x51	/* alt mode adapter */
> +	};
> +	const u8 svid[4] = {
> +		0x00, 0x00, 0x01, 0xff,
> +	};
> +	u32 caps[8];
> +
> +	dev_dbg(anx7688->dev, "cable inserted\n");
> +
> +	anx7688->last_status = -1;
> +	anx7688->last_cc_status = -1;
> +	anx7688->last_dp_state = -1;
> +
> +	msleep(10);
> +	anx7688_power_enable(anx7688);
> +
> +	ret = regulator_enable(anx7688->supplies[ANX7688_VCONN_INDEX].consumer);
> +	if (ret) {
> +		dev_err(anx7688->dev, "failed to enable vconn\n");
> +		goto err_poweroff;
> +	}
> +	anx7688->vconn_on = true;
> +
> +	/* wait till the firmware is loaded (typically ~30ms) */
> +	for (i = 0; i < 100; i++) {
> +		ret = anx7688_reg_read(anx7688, ANX7688_REG_EEPROM_LOAD_STATUS0);
> +
> +		if (ret >= 0 && (ret & ANX7688_EEPROM_FW_LOADED) == ANX7688_EEPROM_FW_LOADED) {
> +			dev_dbg(anx7688->dev, "eeprom0 = 0x%02x\n", ret);
> +			dev_info(anx7688->dev, "fw loaded after %d ms\n", i * 10);

Debugging information. Use dev_dbg.

> +			goto fw_loaded;
> +		}
> +
> +		msleep(5);
> +	}
> +
> +	set_bit(ANX7688_F_FW_FAILED, anx7688->flags);
> +	dev_err(anx7688->dev, "boot firmware load failed (you may need to flash FW to anx7688 first)\n");
> +	ret = -ETIMEDOUT;
> +	goto err_vconoff;
> +
> +fw_loaded:

This label looks a bit messy to me. You could also move that firmware
loading wait to its own function.

> +	ret = i2c_smbus_read_i2c_block_data(anx7688->client,
> +					    ANX7688_REG_FW_VERSION1, 2, fw);
> +	if (ret < 0) {
> +		dev_err(anx7688->dev, "failed to read firmware version\n");
> +		goto err_vconoff;
> +	}
> +
> +	dev_info(anx7688->dev, "OCM firmware loaded (version 0x%04x)\n",
> +		 fw[1] | fw[0] << 8);

deb_dbg.

> +	/* Unmask interrupts */
> +	ret = anx7688_reg_write(anx7688, ANX7688_REG_STATUS_INT, 0);
> +	if (ret)
> +		goto err_vconoff;
> +
> +	ret = anx7688_reg_write(anx7688, ANX7688_REG_STATUS_INT_MASK, ~ANX7688_SOFT_INT_MASK);
> +	if (ret)
> +		goto err_vconoff;
> +
> +	ret = anx7688_reg_write(anx7688, ANX7688_REG_IRQ_EXT_SOURCE2, 0xff);
> +	if (ret)
> +		goto err_vconoff;
> +
> +	ret = anx7688_reg_write(anx7688, ANX7688_REG_IRQ_EXT_MASK2, (u8)~ANX7688_IRQ2_SOFT_INT);
> +	if (ret)
> +		goto err_vconoff;
> +
> +	/* time to turn off vbus after cc disconnect (unit is 4 ms) */
> +	ret = anx7688_reg_write(anx7688, ANX7688_REG_VBUS_OFF_DELAY_TIME, 100 / 4);
> +	if (ret)
> +		goto err_vconoff;
> +
> +	/* 300ms (unit is 2 ms) */
> +	ret = anx7688_reg_write(anx7688, ANX7688_REG_TRY_UFP_TIMER, 300 / 2);
> +	if (ret)
> +		goto err_vconoff;
> +
> +	/* maximum voltage in 100 mV units */
> +	ret = anx7688_reg_write(anx7688, ANX7688_REG_MAX_VOLTAGE, 50); /* 5 V */
> +	if (ret)
> +		goto err_vconoff;
> +
> +	/* min/max power in 500 mW units */
> +	ret = anx7688_reg_write(anx7688, ANX7688_REG_MAX_POWER, 15 * 2); /* 15 W */
> +	if (ret)
> +		goto err_vconoff;
> +
> +	ret = anx7688_reg_write(anx7688, ANX7688_REG_MIN_POWER, 1);  /* 0.5 W */
> +	if (ret)
> +		goto err_vconoff;
> +
> +	/* auto_pd, try.src, try.sink, goto safe 5V */
> +	ret = anx7688_reg_write(anx7688, ANX7688_REG_FEATURE_CTRL, 0x1e & ~BIT(2)); // disable try_src
> +	if (ret)
> +		goto err_vconoff;
> +
> +	for (i = 0; i < anx7688->n_src_caps; i++)
> +		caps[i] = cpu_to_le32(anx7688->src_caps[i]);
> +	ret = anx7688_send_ocm_message(anx7688, ANX7688_OCM_MSG_PWR_SRC_CAP,
> +				       (u8 *)&caps, 4 * anx7688->n_src_caps);
> +	if (ret)
> +		goto err_vconoff;
> +
> +	for (i = 0; i < anx7688->n_snk_caps; i++)
> +		caps[i] = cpu_to_le32(anx7688->snk_caps[i]);
> +	ret = anx7688_send_ocm_message(anx7688, ANX7688_OCM_MSG_PWR_SNK_CAP,
> +				       (u8 *)&caps, 4 * anx7688->n_snk_caps);
> +	if (ret)
> +		goto err_vconoff;
> +
> +	/* Send DP SNK identity */
> +	ret = anx7688_send_ocm_message(anx7688, ANX7688_OCM_MSG_DP_SNK_IDENTITY,
> +				       dp_snk_identity, sizeof(dp_snk_identity));
> +	if (ret)
> +		goto err_vconoff;
> +
> +	ret = anx7688_send_ocm_message(anx7688, ANX7688_OCM_MSG_SVID,
> +				       svid, sizeof(svid));
> +	if (ret)
> +		goto err_vconoff;
> +
> +	dev_dbg(anx7688->dev, "OCM configuration completed\n");
> +
> +	desc.accessory = TYPEC_ACCESSORY_NONE;
> +
> +	typec_unregister_partner(anx7688->partner);
> +
> +	anx7688->partner = typec_register_partner(anx7688->port, &desc);
> +	if (IS_ERR(anx7688->partner)) {
> +		ret = PTR_ERR(anx7688->partner);
> +		goto err_vconoff;
> +	}
> +
> +	// after this deadline passes we'll check if device is pd_capable and
> +	// set up the current limit accordingly
> +	anx7688->current_update_deadline = ktime_add_ms(ktime_get(), 3000);
> +
> +	set_bit(ANX7688_F_CONNECTED, anx7688->flags);
> +	return 0;
> +
> +err_vconoff:
> +	regulator_disable(anx7688->supplies[ANX7688_VCONN_INDEX].consumer);
> +	anx7688->vconn_on = false;
> +err_poweroff:
> +	anx7688_power_disable(anx7688);
> +	dev_err(anx7688->dev, "OCM configuration failed\n");
> +	return ret;
> +}
> +
> +static void anx7688_set_hdmi_hpd(struct anx7688 *anx7688, int state)
> +{
> +	if (!anx7688->extcon)
> +		return;
> +
> +	if (anx7688->last_extcon_state != state) {
> +		extcon_set_state_sync(anx7688->extcon, EXTCON_DISP_HDMI, state);
> +		anx7688->last_extcon_state = state;
> +	}
> +}
> +
> +static void anx7688_disconnect(struct anx7688 *anx7688)
> +{
> +	union power_supply_propval val = {0,};
> +	struct device *dev = anx7688->dev;
> +	int ret;
> +
> +	dev_dbg(dev, "cable removed\n");
> +
> +	anx7688->current_update_deadline = 0;
> +
> +	anx7688_set_hdmi_hpd(anx7688, 0);
> +
> +	if (anx7688->vconn_on) {
> +		regulator_disable(anx7688->supplies[ANX7688_VCONN_INDEX].consumer);
> +		anx7688->vconn_on = false;
> +	}
> +
> +	if (anx7688->vbus_on) {
> +		regulator_disable(anx7688->supplies[ANX7688_VBUS_INDEX].consumer);
> +		anx7688->vbus_on = false;
> +	}
> +
> +	anx7688_power_disable(anx7688);
> +
> +	anx7688->pd_capable = false;
> +
> +	typec_unregister_partner(anx7688->partner);
> +	anx7688->partner = NULL;
> +
> +	anx7688->pwr_role = TYPEC_SINK;
> +	anx7688->data_role = TYPEC_DEVICE;
> +	typec_set_pwr_role(anx7688->port, anx7688->pwr_role);
> +	typec_set_data_role(anx7688->port, anx7688->data_role);
> +	typec_set_pwr_opmode(anx7688->port, TYPEC_PWR_MODE_USB);
> +	typec_set_vconn_role(anx7688->port, TYPEC_SINK);
> +
> +	usb_role_switch_set_role(anx7688->role_sw, USB_ROLE_NONE);
> +
> +	val.intval = 500 * 1000;
> +	dev_dbg(dev, "setting vbus_in current limit to %d mA\n", val.intval);
> +	ret = power_supply_set_property(anx7688->vbus_in_supply,
> +					POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
> +					&val);
> +	if (ret)
> +		dev_err(dev, "failed to set vbus_in current to %d mA\n",
> +			val.intval / 1000);
> +
> +	val.intval = 0;
> +	dev_dbg(dev, "disabling vbus_in power path\n");
> +	ret = power_supply_set_property(anx7688->vbus_in_supply,
> +					POWER_SUPPLY_PROP_ONLINE,
> +					&val);
> +	if (ret)
> +		dev_err(dev, "failed to offline vbus_in\n");
> +
> +	dev_dbg(dev, "enabling USB BC 1.2 detection\n");
> +
> +	clear_bit(ANX7688_F_CONNECTED, anx7688->flags);
> +}
> +
> +static void anx7688_handle_cable_change(struct anx7688 *anx7688)
> +{
> +	int cabledet;
> +	bool connected;
> +
> +	connected = test_bit(ANX7688_F_CONNECTED, anx7688->flags);
> +	cabledet = gpiod_get_value(anx7688->gpio_cabledet);
> +
> +	if (cabledet && !connected)
> +		anx7688_connect(anx7688);
> +	else if (!cabledet && connected)
> +		anx7688_disconnect(anx7688);
> +}
> +
> +static irqreturn_t anx7688_irq_plug_handler(int irq, void *data)
> +{
> +	struct anx7688 *anx7688 = data;
> +
> +	dev_dbg(anx7688->dev, "plug irq (cd=%d)\n",
> +		gpiod_get_value(anx7688->gpio_cabledet));
> +
> +	/*
> +	 * After each cabledet change the scheduled work timer is reset
> +	 * to fire in ~10ms. So the work is done only after the cabledet
> +	 * is stable for ~10ms.
> +	 */
> +	schedule_delayed_work(&anx7688->work, msecs_to_jiffies(10));
> +
> +	return IRQ_HANDLED;
> +}
> +
> +enum {
> +	CMD_SUCCESS,
> +	CMD_REJECT,
> +	CMD_FAIL,
> +	CMD_BUSY,
> +};
> +
> +static const char * const anx7688_cmd_statuses[] = {
> +	"SUCCESS",
> +	"REJECT",
> +	"FAIL",
> +	"BUSY",
> +};
> +
> +static int anx7688_handle_pd_message_response(struct anx7688 *anx7688,
> +					      u8 to_cmd, u8 resp)
> +{
> +	const char *status = resp <= CMD_BUSY ? anx7688_cmd_statuses[resp] : "UNKNOWN";
> +
> +	switch (to_cmd) {
> +	case ANX7688_OCM_MSG_PSWAP_REQ:
> +		dev_info(anx7688->dev, "received response to PSWAP_REQ (%s)\n", status);
> +		break;
> +
> +	case ANX7688_OCM_MSG_DSWAP_REQ:
> +		dev_info(anx7688->dev, "received response to DSWAP_REQ (%s)\n", status);
> +		break;
> +
> +	case ANX7688_OCM_MSG_VCONN_SWAP_REQ:
> +		dev_info(anx7688->dev, "received response to VCONN_SWAP_REQ (%s)\n", status);
> +		break;
> +
> +	case ANX7688_OCM_MSG_PWR_OBJ_REQ:
> +		dev_info(anx7688->dev, "received response to PWR_OBJ_REQ (%s)\n", status);
> +		break;
> +
> +	case ANX7688_OCM_MSG_VDM:
> +		dev_info(anx7688->dev, "received response to VDM (%s)\n", status);
> +		break;
> +
> +	case ANX7688_OCM_MSG_GOTO_MIN_REQ:
> +		dev_info(anx7688->dev, "received response to GOTO_MIN_REQ (%s)\n", status);
> +		break;
> +
> +	case ANX7688_OCM_MSG_GET_SNK_CAP:
> +		dev_info(anx7688->dev, "received response to GET_SNK_CAP (%s)\n", status);
> +		break;
> +
> +	default:
> +		dev_info(anx7688->dev, "received response to unknown request (%s)\n", status);
> +		break;
> +	}
> +
> +	return 0;
> +}

Noise. Drop this whole function. If you need this kind of information,
then please consider trace points, or just use some debugfs trick like
what we have in drivers/usb/typec/tcpm/tcpm.c and few other drivers.

> +static int anx7688_handle_pd_message(struct anx7688 *anx7688,
> +				     u8 cmd, u8 *msg, unsigned int len)
> +{
> +	struct device *dev = anx7688->dev;
> +	union power_supply_propval psy_val = {0,};
> +	uint32_t *pdos = (uint32_t *)msg;

u32

> +	int ret, i, rdo_max_v, rdo_max_p;
> +	uint32_t pdo, rdo;

u32

> +	switch (cmd) {
> +	case ANX7688_OCM_MSG_PWR_SRC_CAP:
> +		dev_info(anx7688->dev, "received SRC_CAP\n");

Noise.

> +
> +		if (len % 4 != 0) {
> +			dev_warn(anx7688->dev, "received invalid sized PDO array\n");
> +			break;
> +		}
> +
> +		/* the partner is PD capable */
> +		anx7688->pd_capable = true;
> +
> +		for (i = 0; i < len / 4; i++) {
> +			pdo = le32_to_cpu(pdos[i]);
> +
> +			if (pdo_type(pdo) == PDO_TYPE_FIXED) {
> +				unsigned int voltage = pdo_fixed_voltage(pdo);
> +				unsigned int max_curr = pdo_max_current(pdo);
> +
> +				dev_info(anx7688->dev, "SRC_CAP PDO_FIXED (%umV %umA)\n", voltage, max_curr);

Noise.

> +			} else if (pdo_type(pdo) == PDO_TYPE_BATT) {
> +				unsigned int min_volt = pdo_min_voltage(pdo);
> +				unsigned int max_volt = pdo_max_voltage(pdo);
> +				unsigned int max_pow = pdo_max_power(pdo);
> +
> +				dev_info(anx7688->dev, "SRC_CAP PDO_BATT (%umV-%umV %umW)\n", min_volt, max_volt, max_pow);

Noise. That line also really should be split in two.

I'm stopping my review here. This driver is too noisy. All dev_info
calls need to be dropped. If the driver is working correctly then it
needs to quiet.

Most of those prints are useful for debugging only, so I think similar
debugfs log like the one tcpm.c uses could be a good idea for them
since you already use debugfs in this driver in any case.

thanks,

> +			} else if (pdo_type(pdo) == PDO_TYPE_VAR) {
> +				unsigned int min_volt = pdo_min_voltage(pdo);
> +				unsigned int max_volt = pdo_max_voltage(pdo);
> +				unsigned int max_curr = pdo_max_current(pdo);
> +
> +				dev_info(anx7688->dev, "SRC_CAP PDO_VAR (%umV-%umV %umA)\n", min_volt, max_volt, max_curr);
> +			} else {
> +				dev_info(anx7688->dev, "SRC_CAP PDO_APDO (0x%08X)\n", pdo);
> +			}
> +		}
> +
> +		/* when auto_pd mode is enabled, the FW has already set
> +		 * RDO_MAX_VOLTAGE and RDO_MAX_POWER for the RDO it sent to the
> +		 * partner based on the received SOURCE_CAPs. This does not
> +		 * mean, the request was acked, but we can't do better here than
> +		 * calculate the current_limit to set later and hope for the best.
> +		 */
> +		rdo_max_v = anx7688_reg_read(anx7688, ANX7688_REG_MAX_VOLTAGE_STATUS);
> +		if (rdo_max_v < 0)
> +			return rdo_max_v;
> +		if (rdo_max_v == 0)
> +			return -EINVAL;
> +
> +		rdo_max_p = anx7688_reg_read(anx7688, ANX7688_REG_MAX_POWER_STATUS);
> +		if (rdo_max_p < 0)
> +			return rdo_max_p;
> +
> +		anx7688->pd_current_limit = rdo_max_p * 5000 / rdo_max_v;
> +
> +		dev_dbg(anx7688->dev, "RDO max voltage = %dmV, max power = %dmW, PD current limit = %dmA\n",
> +			rdo_max_v * 100, rdo_max_p * 500, anx7688->pd_current_limit);
> +
> +		// update current limit sooner, now that we have PD negotiation result
> +		anx7688->current_update_deadline = ktime_add_ms(ktime_get(), 500);
> +		break;
> +
> +	case ANX7688_OCM_MSG_PWR_SNK_CAP:
> +		dev_info(anx7688->dev, "received SNK_CAP\n");
> +
> +		if (len % 4 != 0) {
> +			dev_warn(anx7688->dev, "received invalid sized PDO array\n");
> +			break;
> +		}
> +
> +		for (i = 0; i < len / 4; i++) {
> +			pdo = le32_to_cpu(pdos[i]);
> +
> +			if (pdo_type(pdo) == PDO_TYPE_FIXED) {
> +				unsigned int voltage = pdo_fixed_voltage(pdo);
> +				unsigned int max_curr = pdo_max_current(pdo);
> +
> +				dev_info(anx7688->dev, "SNK_CAP PDO_FIXED (%umV %umA)\n", voltage, max_curr);
> +			} else if (pdo_type(pdo) == PDO_TYPE_BATT) {
> +				unsigned int min_volt = pdo_min_voltage(pdo);
> +				unsigned int max_volt = pdo_max_voltage(pdo);
> +				unsigned int max_pow = pdo_max_power(pdo);
> +
> +				dev_info(anx7688->dev, "SNK_CAP PDO_BATT (%umV-%umV %umW)\n", min_volt, max_volt, max_pow);
> +			} else if (pdo_type(pdo) == PDO_TYPE_VAR) {
> +				unsigned int min_volt = pdo_min_voltage(pdo);
> +				unsigned int max_volt = pdo_max_voltage(pdo);
> +				unsigned int max_curr = pdo_max_current(pdo);
> +
> +				dev_info(anx7688->dev, "SNK_CAP PDO_VAR (%umV-%umV %umA)\n", min_volt, max_volt, max_curr);
> +			} else {
> +				dev_info(anx7688->dev, "SNK_CAP PDO_APDO (0x%08X)\n", pdo);
> +			}
> +		}
> +
> +		break;
> +
> +	case ANX7688_OCM_MSG_PWR_OBJ_REQ:
> +		dev_info(anx7688->dev, "received PWR_OBJ_REQ\n");
> +
> +		anx7688->pd_capable = true;
> +
> +		if (len != 4) {
> +			dev_warn(anx7688->dev, "received invalid sized RDO\n");
> +			break;
> +		}
> +
> +		rdo = le32_to_cpu(pdos[0]);
> +
> +		if (rdo_index(rdo) >= 1 && rdo_index(rdo) <= anx7688->n_src_caps) {
> +			unsigned int rdo_op_curr = rdo_op_current(rdo);
> +			unsigned int rdo_max_curr = rdo_max_current(rdo);
> +			unsigned int rdo_idx = rdo_index(rdo) - 1;
> +			unsigned int pdo_volt, pdo_max_curr;
> +
> +			pdo = anx7688->src_caps[rdo_idx];
> +			pdo_volt = pdo_fixed_voltage(pdo);
> +			pdo_max_curr = pdo_max_current(pdo);
> +
> +			dev_info(anx7688->dev, "RDO (idx=%d op=%umA max=%umA)\n",
> +				 rdo_idx, rdo_op_curr, rdo_max_curr);
> +
> +			dev_info(anx7688->dev, "PDO_FIXED (%umV %umA)\n",
> +				 pdo_volt, pdo_max_curr);
> +
> +			// We could check the req and respond with accept/reject
> +			// but we're using auto_pd feature, so the FW will do
> +			// this for us
> +		} else {
> +			dev_info(anx7688->dev, "PWR_OBJ RDO index out of range (RDO = 0x%08X)\n", rdo);
> +		}
> +
> +		break;
> +
> +	case ANX7688_OCM_MSG_ACCEPT:
> +		dev_info(anx7688->dev, "received ACCEPT\n");
> +		break;
> +
> +	case ANX7688_OCM_MSG_REJECT:
> +		dev_info(anx7688->dev, "received REJECT\n");
> +		break;
> +
> +	case ANX7688_OCM_MSG_RESPONSE_TO_REQ:
> +		if (len < 2) {
> +			dev_warn(anx7688->dev, "received short RESPONSE_TO_REQ\n");
> +			break;
> +		}
> +
> +		anx7688_handle_pd_message_response(anx7688, msg[0], msg[1]);
> +		break;
> +
> +	case ANX7688_OCM_MSG_SOFT_RST:
> +		dev_info(anx7688->dev, "received SOFT_RST\n");
> +		break;
> +
> +	case ANX7688_OCM_MSG_HARD_RST:
> +		if (anx7688->pd_capable) {
> +			dev_info(anx7688->dev, "received HARD_RST\n");
> +
> +			// stop drawing power from VBUS
> +			psy_val.intval = 0;
> +			dev_dbg(dev, "disabling vbus_in power path\n");
> +			ret = power_supply_set_property(anx7688->vbus_in_supply,
> +							POWER_SUPPLY_PROP_ONLINE,
> +							&psy_val);
> +			if (ret)
> +				dev_err(anx7688->dev, "failed to offline vbus_in\n");
> +
> +			// wait till the dust settles
> +			anx7688->current_update_deadline = ktime_add_ms(ktime_get(), 3000);
> +		} else {
> +			dev_dbg(anx7688->dev, "received HARD_RST, idiot firmware is bored\n");
> +		}
> +
> +		break;
> +
> +	case ANX7688_OCM_MSG_RESTART:
> +		dev_info(anx7688->dev, "received RESTART\n");
> +		break;
> +
> +	case ANX7688_OCM_MSG_PSWAP_REQ:
> +		dev_info(anx7688->dev, "received PSWAP_REQ\n");
> +		break;
> +
> +	case ANX7688_OCM_MSG_DSWAP_REQ:
> +		dev_info(anx7688->dev, "received DSWAP_REQ\n");
> +		break;
> +
> +	case ANX7688_OCM_MSG_VCONN_SWAP_REQ:
> +		dev_info(anx7688->dev, "received VCONN_SWAP_REQ\n");
> +		break;
> +
> +	case ANX7688_OCM_MSG_DP_ALT_ENTER:
> +		dev_info(anx7688->dev, "received DP_ALT_ENTER\n");
> +		break;
> +
> +	case ANX7688_OCM_MSG_DP_ALT_EXIT:
> +		dev_info(anx7688->dev, "received DP_ALT_EXIT\n");
> +		break;
> +
> +	case ANX7688_OCM_MSG_DP_SNK_IDENTITY:
> +		dev_info(anx7688->dev, "received DP_SNK_IDENTITY\n");
> +		break;
> +
> +	case ANX7688_OCM_MSG_SVID:
> +		dev_info(anx7688->dev, "received SVID\n");
> +		break;
> +
> +	case ANX7688_OCM_MSG_VDM:
> +		dev_info(anx7688->dev, "received VDM\n");
> +		break;
> +
> +	case ANX7688_OCM_MSG_GOTO_MIN_REQ:
> +		dev_info(anx7688->dev, "received GOTO_MIN_REQ\n");
> +		break;
> +
> +	case ANX7688_OCM_MSG_PD_STATUS_REQ:
> +		dev_info(anx7688->dev, "received PD_STATUS_REQ\n");
> +		break;
> +
> +	case ANX7688_OCM_MSG_GET_DP_SNK_CAP:
> +		dev_info(anx7688->dev, "received GET_DP_SNK_CAP\n");
> +		break;
> +
> +	case ANX7688_OCM_MSG_DP_SNK_CFG:
> +		dev_info(anx7688->dev, "received DP_SNK_CFG\n");
> +		break;
> +
> +	default:
> +		dev_info(anx7688->dev, "received unknown message 0x%02x\n", cmd);
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static int anx7688_receive_msg(struct anx7688 *anx7688)
> +{
> +	u8 pkt[32], checksum = 0;
> +	int i, ret;
> +
> +	ret = i2c_smbus_read_i2c_block_data(anx7688->client_tcpc,
> +					    ANX7688_TCPC_REG_INTERFACE_RECV,
> +					    32, pkt);
> +	if (ret < 0) {
> +		dev_err(anx7688->dev, "failed to read pd msg\n");
> +		return ret;
> +	}
> +
> +	ret = anx7688_tcpc_reg_write(anx7688, ANX7688_TCPC_REG_INTERFACE_RECV, 0);
> +	if (ret)
> +		dev_warn(anx7688->dev, "failed to clear recv FIFO\n");
> +
> +	if (pkt[0] == 0 || pkt[0] > sizeof(pkt) - 2) {
> +		dev_err(anx7688->dev, "received invalid pd message: %*ph\n",
> +			(int)sizeof(pkt), pkt);
> +		return -EINVAL;
> +	}
> +
> +	dev_dbg(anx7688->dev, "recv ocm message cmd=0x%02x %*ph\n",
> +		pkt[1], pkt[0] + 2, pkt);
> +
> +	for (i = 0; i < pkt[0] + 2; i++)
> +		checksum += pkt[i];
> +
> +	if (checksum != 0) {
> +		dev_err(anx7688->dev, "bad checksum on received message\n");
> +		return -EINVAL;
> +	}
> +
> +	anx7688_handle_pd_message(anx7688, pkt[1], pkt + 2, pkt[0] - 1);
> +	return 0;
> +}
> +
> +static const char *anx7688_cc_status_string(unsigned int v)
> +{
> +	switch (v) {
> +	case 0: return "SRC.Open";
> +	case 1: return "SRC.Rd";
> +	case 2: return "SRC.Ra";
> +	case 4: return "SNK.Default";
> +	case 8: return "SNK.Power1.5";
> +	case 12: return "SNK.Power3.0";
> +	default: return "UNK";
> +	}
> +}
> +
> +static int anx7688_update_status(struct anx7688 *anx7688)
> +{
> +	struct device *dev = anx7688->dev;
> +	bool vbus_on, vconn_on, dr_dfp;
> +	int status, cc_status, dp_state, dp_substate, ret;
> +
> +	status = anx7688_reg_read(anx7688, ANX7688_REG_STATUS);
> +	if (status < 0)
> +		return status;
> +
> +	cc_status = anx7688_reg_read(anx7688, ANX7688_REG_CC_STATUS);
> +	if (cc_status < 0)
> +		return cc_status;
> +
> +	dp_state = anx7688_tcpc_reg_read(anx7688, 0x87);
> +	if (dp_state < 0)
> +		return dp_state;
> +
> +	dp_substate = anx7688_tcpc_reg_read(anx7688, 0x88);
> +	if (dp_substate < 0)
> +		return dp_substate;
> +
> +	anx7688_set_hdmi_hpd(anx7688, dp_state >= 3);
> +
> +	dp_state = (dp_state << 8) | dp_substate;
> +
> +	if (anx7688->last_status == -1 || anx7688->last_status != status) {
> +		anx7688->last_status = status;
> +		dev_dbg(dev, "status changed to 0x%02x\n", status);
> +	}
> +
> +	if (anx7688->last_cc_status == -1 || anx7688->last_cc_status != cc_status) {
> +		anx7688->last_cc_status = cc_status;
> +		dev_dbg(dev, "cc_status changed to CC1 = %s CC2 = %s\n",
> +			anx7688_cc_status_string(cc_status & 0xf),
> +			anx7688_cc_status_string((cc_status >> 4) & 0xf));
> +	}
> +
> +	if (anx7688->last_dp_state == -1 || anx7688->last_dp_state != dp_state) {
> +		anx7688->last_dp_state = dp_state;
> +		dev_dbg(dev, "DP state changed to 0x%04x\n", dp_state);
> +	}
> +
> +	vbus_on = !!(status & ANX7688_VBUS_STATUS);
> +	vconn_on = !!(status & ANX7688_VCONN_STATUS);
> +	dr_dfp = !!(status & ANX7688_DATA_ROLE_STATUS);
> +
> +	if (anx7688->vbus_on != vbus_on) {
> +		dev_dbg(anx7688->dev, "POWER role change to %s\n",
> +			vbus_on ? "SOURCE" : "SINK");
> +
> +		if (vbus_on) {
> +			ret = regulator_enable(anx7688->supplies[ANX7688_VBUS_INDEX].consumer);
> +			if (ret) {
> +				dev_err(anx7688->dev, "failed to enable vbus\n");
> +				return ret;
> +			}
> +		} else {
> +			ret = regulator_disable(anx7688->supplies[ANX7688_VBUS_INDEX].consumer);
> +			if (ret) {
> +				dev_err(anx7688->dev, "failed to disable vbus\n");
> +				return ret;
> +			}
> +		}
> +
> +		anx7688->pwr_role = vbus_on ? TYPEC_SOURCE : TYPEC_SINK;
> +		typec_set_pwr_role(anx7688->port, anx7688->pwr_role);
> +		anx7688->vbus_on = vbus_on;
> +	}
> +
> +	if (anx7688->vconn_on != vconn_on) {
> +		dev_dbg(anx7688->dev, "VCONN role change to %s\n",
> +			vconn_on ? "SOURCE" : "SINK");
> +
> +		if (vconn_on) {
> +			ret = regulator_enable(anx7688->supplies[ANX7688_VCONN_INDEX].consumer);
> +			if (ret) {
> +				dev_err(anx7688->dev, "failed to enable vconn\n");
> +				return ret;
> +			}
> +		} else {
> +			ret = regulator_disable(anx7688->supplies[ANX7688_VCONN_INDEX].consumer);
> +			if (ret) {
> +				dev_err(anx7688->dev, "failed to disable vconn\n");
> +				return ret;
> +			}
> +		}
> +
> +		typec_set_vconn_role(anx7688->port, vconn_on ? TYPEC_SOURCE : TYPEC_SINK);
> +		anx7688->vconn_on = vconn_on;
> +	}
> +
> +	anx7688->data_role = dr_dfp ? TYPEC_HOST : TYPEC_DEVICE;
> +	typec_set_data_role(anx7688->port, anx7688->data_role);
> +
> +	if (usb_role_switch_get_role(anx7688->role_sw) !=
> +	    (dr_dfp ? USB_ROLE_HOST : USB_ROLE_DEVICE)) {
> +		dev_dbg(anx7688->dev, "DATA role change requested to %s\n",
> +			dr_dfp ? "DFP" : "UFP");
> +
> +		ret = usb_role_switch_set_role(anx7688->role_sw,
> +					       dr_dfp ? USB_ROLE_HOST : USB_ROLE_DEVICE);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static irqreturn_t anx7688_irq_status_handler(int irq, void *data)
> +{
> +	struct anx7688 *anx7688 = data;
> +	struct device *dev = anx7688->dev;
> +	int tcpc_status, ext2_status, soft_status;
> +
> +	mutex_lock(&anx7688->lock);
> +
> +	if (!test_bit(ANX7688_F_CONNECTED, anx7688->flags)) {
> +		dev_dbg(dev, "spurious status irq\n");
> +		/*
> +		 * anx chip should be disabled and power off, nothing
> +		 * more to do
> +		 */
> +		goto out_unlock;
> +	}
> +
> +	// clear tcpc interrupt
> +	tcpc_status = anx7688_tcpc_reg_read(anx7688, ANX7688_TCPC_REG_ALERT0);
> +	if (tcpc_status > 0)
> +		anx7688_tcpc_reg_write(anx7688, ANX7688_TCPC_REG_ALERT0, tcpc_status);
> +
> +	ext2_status = anx7688_reg_read(anx7688, ANX7688_REG_IRQ_EXT_SOURCE2);
> +	if (ext2_status & ANX7688_IRQ2_SOFT_INT) {
> +		soft_status = anx7688_reg_read(anx7688, ANX7688_REG_STATUS_INT);
> +		anx7688_reg_write(anx7688, ANX7688_REG_STATUS_INT, 0);
> +
> +		if (soft_status > 0) {
> +			soft_status &= ANX7688_SOFT_INT_MASK;
> +
> +			if (soft_status & ANX7688_IRQS_RECEIVED_MSG)
> +				anx7688_receive_msg(anx7688);
> +
> +			if (soft_status & (ANX7688_IRQS_CC_STATUS_CHANGE |
> +					   ANX7688_IRQS_VBUS_CHANGE |
> +					   ANX7688_IRQS_VCONN_CHANGE |
> +					   ANX7688_IRQS_DATA_ROLE_CHANGE)) {
> +				anx7688_update_status(anx7688);
> +			}
> +		}
> +
> +		anx7688_reg_write(anx7688, ANX7688_REG_IRQ_EXT_SOURCE2, ANX7688_IRQ2_SOFT_INT);
> +	}
> +
> +out_unlock:
> +	mutex_unlock(&anx7688->lock);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int anx7688_dr_set(struct typec_port *port, enum typec_data_role role)
> +{
> +	struct anx7688 *anx7688 = typec_get_drvdata(port);
> +	int ret = 0;
> +
> +	dev_info(anx7688->dev, "data role set %d\n", role);
> +
> +	if (anx7688->data_role != role) {
> +		mutex_lock(&anx7688->lock);
> +		ret = anx7688_send_ocm_message(anx7688, ANX7688_OCM_MSG_DSWAP_REQ, 0, 0);
> +		mutex_unlock(&anx7688->lock);
> +	}
> +
> +	return ret;
> +}
> +
> +static int anx7688_pr_set(struct typec_port *port, enum typec_role role)
> +{
> +	struct anx7688 *anx7688 = typec_get_drvdata(port);
> +	int ret = 0;
> +
> +	dev_info(anx7688->dev, "power role set %d\n", role);
> +
> +	if (anx7688->pwr_role != role) {
> +		mutex_lock(&anx7688->lock);
> +		ret = anx7688_send_ocm_message(anx7688, ANX7688_OCM_MSG_PSWAP_REQ, 0, 0);
> +		mutex_unlock(&anx7688->lock);
> +	}
> +
> +	return ret;
> +}
> +
> +/*
> + * Calls to the EEPROM functions need to be taken under the anx7688 lock.
> + */
> +static int anx7688_eeprom_set_address(struct anx7688 *anx7688, u16 addr)
> +{
> +	int ret;
> +
> +	ret = anx7688_reg_write(anx7688, 0xe0, (addr >> 8) & 0xff);
> +	if (ret < 0)
> +		return ret;
> +
> +	return anx7688_reg_write(anx7688, 0xe1, addr & 0xff);
> +}
> +
> +static int anx7688_eeprom_wait_done(struct anx7688 *anx7688)
> +{
> +	ktime_t timeout;
> +	int ret;
> +
> +	// wait for read to be done
> +	timeout = ktime_add_us(ktime_get(), 50000);
> +	while (true) {
> +		ret = anx7688_reg_read(anx7688, 0xe2);
> +		if (ret < 0)
> +			return ret;
> +
> +		if (ret & BIT(3))
> +			return 0;
> +
> +		if (ktime_after(ktime_get(), timeout)) {
> +			dev_err(anx7688->dev, "timeout waiting for eeprom\n");
> +			return -ETIMEDOUT;
> +		}
> +	}
> +}
> +
> +/* wait for internal FSM of EEPROM to be in a state ready for
> + * programming/reading
> + */
> +static int anx7688_eeprom_wait_ready(struct anx7688 *anx7688)
> +{
> +	ktime_t timeout;
> +	int ret;
> +
> +	// wait until eeprom is ready
> +	timeout = ktime_add_us(ktime_get(), 1000000);
> +	while (true) {
> +		ret = anx7688_reg_read(anx7688, 0x7f);
> +		if (ret < 0)
> +			return ret;
> +
> +		if ((ret & 0x0f) == 7)
> +			return 0;
> +
> +		if (ktime_after(ktime_get(), timeout)) {
> +			dev_err(anx7688->dev, "timeout waiting for eeprom to initialize\n");
> +			return -ETIMEDOUT;
> +		}
> +
> +		msleep(5);
> +	}
> +}
> +
> +static int anx7688_eeprom_read(struct anx7688 *anx7688, unsigned int addr, u8 buf[16])
> +{
> +	int ret;
> +
> +	ret = anx7688_eeprom_set_address(anx7688, addr);
> +	if (ret)
> +		return ret;
> +
> +	// initiate read
> +	ret = anx7688_reg_write(anx7688, 0xe2, 0x06);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = anx7688_eeprom_wait_done(anx7688);
> +	if (ret)
> +		return ret;
> +
> +	ret = i2c_smbus_read_i2c_block_data(anx7688->client, 0xd0, 16, buf);
> +	if (ret < 0) {
> +		dev_err(anx7688->dev,
> +			"failed to read eeprom data (err=%d)\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct typec_operations anx7688_typec_ops = {
> +	.dr_set = anx7688_dr_set,
> +	.pr_set = anx7688_pr_set,
> +};
> +
> +/*
> + * This function has to work when the ANX7688 is active, and when
> + * it is powered down. It power cycles the chip and asserts the OCM
> + * reset, to prevent OCM FW interfering with EEPROM reading.
> + *
> + * After reading EEPROM, the reconnection is scheduled.
> + */
> +static int anx7688_firmware_show(struct seq_file *s, void *data)
> +{
> +	struct anx7688 *anx7688 = s->private;
> +	unsigned int addr;
> +	u8 buf[16];
> +	int ret;
> +
> +	mutex_lock(&anx7688->lock);
> +
> +	if (test_bit(ANX7688_F_CONNECTED, anx7688->flags))
> +		anx7688_disconnect(anx7688);
> +
> +	msleep(20);
> +
> +	anx7688_power_enable(anx7688);
> +
> +	ret = anx7688_reg_update_bits(anx7688, ANX7688_REG_USBC_RESET_CTRL,
> +				      ANX7688_USBC_RESET_CTRL_OCM_RESET,
> +				      ANX7688_USBC_RESET_CTRL_OCM_RESET);
> +	if (ret < 0)
> +		goto out_powerdown;
> +
> +	ret = anx7688_eeprom_wait_ready(anx7688);
> +	if (ret)
> +		goto out_powerdown;
> +
> +	msleep(10);
> +
> +	for (addr = 0x10; addr < 0x10000; addr += 16) {
> +		// set address
> +		ret = anx7688_eeprom_read(anx7688, addr, buf);
> +		if (ret < 0)
> +			goto out_powerdown;
> +
> +		seq_write(s, buf, sizeof(buf));
> +	}
> +
> +out_powerdown:
> +	anx7688_power_disable(anx7688);
> +	schedule_delayed_work(&anx7688->work, 0);
> +	mutex_unlock(&anx7688->lock);
> +
> +	return ret;
> +}
> +DEFINE_SHOW_ATTRIBUTE(anx7688_firmware);
> +
> +static int anx7688_regs_show(struct seq_file *s, void *data)
> +{
> +	struct anx7688 *anx7688 = s->private;
> +	u8 buf[16];
> +	unsigned int i, addr;
> +	int ret = -ENODEV;
> +
> +	mutex_lock(&anx7688->lock);
> +
> +	if (!test_bit(ANX7688_F_POWERED, anx7688->flags))
> +		goto out_unlock;
> +
> +	for (addr = 0; addr < 256; addr += 16) {
> +		ret = i2c_smbus_read_i2c_block_data(anx7688->client, addr,
> +						    sizeof(buf), buf);
> +		if (ret < 0) {
> +			dev_err(anx7688->dev,
> +				"failed to read registers (err=%d)\n", ret);
> +			goto out_unlock;
> +		}
> +
> +		for (i = 0; i < 16; i++)
> +			seq_printf(s, "50%02x: %02x\n", addr + i, buf[i]);
> +	}
> +
> +	for (addr = 0; addr < 256; addr += 16) {
> +		ret = i2c_smbus_read_i2c_block_data(anx7688->client_tcpc, addr,
> +						    sizeof(buf), buf);
> +		if (ret < 0) {
> +			dev_err(anx7688->dev,
> +				"failed to read registers (err=%d)\n", ret);
> +			goto out_unlock;
> +		}
> +
> +		for (i = 0; i < 16; i++)
> +			seq_printf(s, "58%02x: %02x\n", addr + i, buf[i]);
> +	}
> +
> +out_unlock:
> +	mutex_unlock(&anx7688->lock);
> +
> +	return ret;
> +}
> +DEFINE_SHOW_ATTRIBUTE(anx7688_regs);
> +
> +static int anx7688_status_show(struct seq_file *s, void *data)
> +{
> +	struct anx7688 *anx7688 = s->private;
> +
> +	mutex_lock(&anx7688->lock);
> +
> +	seq_puts(s, "not much\n");
> +
> +	mutex_unlock(&anx7688->lock);
> +
> +	return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(anx7688_status);
> +
> +/*
> + * This is just a 1s watchdog checking the state if cabledet pin.
> + */
> +static void anx7688_cabledet_timer_fn(struct timer_list *t)
> +{
> +	struct anx7688 *anx7688 = from_timer(anx7688, t, work_timer);
> +
> +	schedule_delayed_work(&anx7688->work, 0);
> +	mod_timer(t, jiffies + msecs_to_jiffies(1000));
> +}
> +
> +static void anx7688_handle_vbus_in_notify(struct anx7688 *anx7688)
> +{
> +	union power_supply_propval psy_val = {0,};
> +	struct device *dev = anx7688->dev;
> +	int ret;
> +
> +	ret = power_supply_get_property(anx7688->vbus_in_supply,
> +					POWER_SUPPLY_PROP_USB_TYPE,
> +					&psy_val);
> +	if (ret) {
> +		dev_err(dev, "failed to get USB BC1.2 result\n");
> +		return;
> +	}
> +
> +	if (anx7688->last_bc_result == psy_val.intval)
> +		return;
> +
> +	anx7688->last_bc_result = psy_val.intval;
> +
> +	switch (psy_val.intval) {
> +	case POWER_SUPPLY_USB_TYPE_DCP:
> +	case POWER_SUPPLY_USB_TYPE_CDP:
> +		dev_dbg(dev, "BC 1.2 result: DCP or CDP\n");
> +		break;
> +	case POWER_SUPPLY_USB_TYPE_SDP:
> +	default:
> +		dev_dbg(dev, "BC 1.2 result: SDP\n");
> +		break;
> +	}
> +}
> +
> +static int anx7688_cc_status(unsigned int v)
> +{
> +	switch (v) {
> +	case 0: return -1;
> +	case 1: return -1;
> +	case 2: return -1;
> +	case 4: return TYPEC_PWR_MODE_USB;
> +	case 8: return TYPEC_PWR_MODE_1_5A;
> +	case 12: return TYPEC_PWR_MODE_3_0A;
> +	default: return -1;
> +	}
> +}
> +
> +static const char *anx7688_get_power_mode_name(enum typec_pwr_opmode mode)
> +{
> +	switch (mode) {
> +	case TYPEC_PWR_MODE_USB: return "USB";
> +	case TYPEC_PWR_MODE_1_5A: return "1.5A";
> +	case TYPEC_PWR_MODE_3_0A: return "3.0A";
> +	case TYPEC_PWR_MODE_PD: return "PD";
> +	default: return "Unknown";
> +	}
> +}
> +
> +/*
> + * This is called after 500ms after connection when the PD contract should have
> + * been negotiated. We should inspect CC pins or PD status here and decide what
> + * input current limit to set.
> + */
> +static void anx7688_handle_current_update(struct anx7688 *anx7688)
> +{
> +	unsigned int cc_status = anx7688->last_cc_status;
> +	union power_supply_propval val = {0,};
> +	struct device *dev = anx7688->dev;
> +	int pwr_mode, ret, current_limit = 0;
> +
> +	if (anx7688->pd_capable) {
> +		pwr_mode = TYPEC_PWR_MODE_PD;
> +	} else if (cc_status < 0) {
> +		pwr_mode = TYPEC_PWR_MODE_USB;
> +	} else {
> +		pwr_mode = anx7688_cc_status(cc_status & 0xf);
> +		if (pwr_mode < 0)
> +			pwr_mode = anx7688_cc_status((cc_status >> 4) & 0xf);
> +		if (pwr_mode < 0)
> +			pwr_mode = TYPEC_PWR_MODE_USB;
> +	}
> +
> +	if (pwr_mode == TYPEC_PWR_MODE_1_5A)
> +		current_limit = 1500;
> +	else if (pwr_mode == TYPEC_PWR_MODE_3_0A)
> +		current_limit = 3000;
> +	else if (pwr_mode == TYPEC_PWR_MODE_PD)
> +		current_limit = anx7688->pd_current_limit;
> +
> +	anx7688->input_current_limit = current_limit;
> +
> +	dev_info(anx7688->dev, "updating power mode to %s, current limit %dmA (0 => BC1.2)\n",
> +		 anx7688_get_power_mode_name(pwr_mode), current_limit);
> +
> +	if (current_limit) {
> +		/*
> +		 * Disable BC1.2 detection, because we'll be setting
> +		 * a current limit determined by USB-PD
> +		 */
> +		dev_dbg(dev, "disabling USB BC 1.2 detection\n");
> +
> +		val.intval = current_limit * 1000;
> +		dev_dbg(dev, "setting vbus_in current limit to %d mA\n", current_limit);
> +		ret = power_supply_set_property(anx7688->vbus_in_supply,
> +						POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
> +						&val);
> +		if (ret)
> +			dev_err(dev, "failed to set vbus_in current to %d mA\n",
> +				current_limit);
> +	}
> +		/*
> +		 * else use the result of BC1.2 detection performed by PMIC.
> +		 */
> +
> +	/* Turn on VBUS power path inside PMIC. */
> +	val.intval = 1;
> +	dev_dbg(dev, "enabling vbus_in power path\n");
> +	ret = power_supply_set_property(anx7688->vbus_in_supply,
> +					POWER_SUPPLY_PROP_ONLINE,
> +					&val);
> +	if (ret)
> +		dev_err(anx7688->dev, "failed to enable vbus_in\n");
> +
> +	typec_set_pwr_opmode(anx7688->port, pwr_mode);
> +}
> +
> +static int anx7688_vbus_in_notify(struct notifier_block *nb,
> +				  unsigned long val, void *v)
> +{
> +	struct anx7688 *anx7688 = container_of(nb, struct anx7688, vbus_in_nb);
> +	struct power_supply *psy = v;
> +
> +	/* atomic context */
> +	if (val == PSY_EVENT_PROP_CHANGED && psy == anx7688->vbus_in_supply) {
> +		set_bit(ANX7688_F_PWRSUPPLY_CHANGE, anx7688->flags);
> +		schedule_delayed_work(&anx7688->work, 0);
> +	}
> +
> +	return NOTIFY_OK;
> +}
> +
> +static void anx7688_work(struct work_struct *work)
> +{
> +	struct anx7688 *anx7688 = container_of(work, struct anx7688, work.work);
> +
> +	if (test_bit(ANX7688_F_FW_FAILED, anx7688->flags))
> +		return;
> +
> +	mutex_lock(&anx7688->lock);
> +
> +	if (test_and_clear_bit(ANX7688_F_PWRSUPPLY_CHANGE, anx7688->flags))
> +		anx7688_handle_vbus_in_notify(anx7688);
> +
> +	anx7688_handle_cable_change(anx7688);
> +
> +	if (test_bit(ANX7688_F_CONNECTED, anx7688->flags)) {
> +		/*
> +		 * We check status periodically outside of interrupt, just to
> +		 * be sure we didn't miss any status interrupts
> +		 */
> +		anx7688_update_status(anx7688);
> +
> +		if (anx7688->current_update_deadline &&
> +			ktime_after(ktime_get(), anx7688->current_update_deadline)) {
> +			anx7688->current_update_deadline = 0;
> +			anx7688_handle_current_update(anx7688);
> +		}
> +	}
> +
> +	mutex_unlock(&anx7688->lock);
> +}
> +
> +static int anx7688_parse_connector(struct device *dev, struct anx7688 *anx7688, struct typec_capability *cap)
> +{
> +	struct fwnode_handle *fwnode;
> +	const char *buf;
> +	int ret;
> +
> +	fwnode = device_get_named_child_node(dev, "connector");
> +	if (!fwnode)
> +		return -EINVAL;
> +
> +	ret = fwnode_property_read_string(fwnode, "power-role", &buf);
> +	if (ret) {
> +		dev_err(dev, "power-role not found: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = typec_find_port_power_role(buf);
> +	if (ret < 0)
> +		return ret;
> +	cap->type = ret;
> +
> +	ret = fwnode_property_read_string(fwnode, "data-role", &buf);
> +	if (ret) {
> +		dev_err(dev, "data-role not found: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = typec_find_port_data_role(buf);
> +	if (ret < 0)
> +		return ret;
> +	cap->data = ret;
> +
> +	ret = fwnode_property_read_string(fwnode, "try-power-role", &buf);
> +	if (ret) {
> +		dev_err(dev, "try-power-role not found: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = typec_find_power_role(buf);
> +	if (ret < 0)
> +		return ret;
> +	cap->prefer_role = ret;
> +
> +	/* Get source pdos */
> +	ret = fwnode_property_count_u32(fwnode, "source-pdos");
> +	if (ret <= 0)
> +		return -EINVAL;
> +
> +	anx7688->n_src_caps = min_t(u8, ret, ARRAY_SIZE(anx7688->src_caps));
> +	ret = fwnode_property_read_u32_array(fwnode, "source-pdos",
> +					     anx7688->src_caps,
> +					     anx7688->n_src_caps);
> +	if (ret < 0) {
> +		dev_err(dev, "source cap validate failed: %d\n", ret);
> +		return -EINVAL;
> +	}
> +
> +	ret = fwnode_property_count_u32(fwnode, "sink-pdos");
> +	if (ret <= 0)
> +		return -EINVAL;
> +
> +	anx7688->n_snk_caps = min_t(u8, ret, ARRAY_SIZE(anx7688->snk_caps));
> +	ret = fwnode_property_read_u32_array(fwnode, "sink-pdos",
> +					     anx7688->snk_caps,
> +					     anx7688->n_snk_caps);
> +	if (ret < 0) {
> +		dev_err(dev, "sink cap validate failed: %d\n", ret);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int anx7688_i2c_probe(struct i2c_client *client)
> +{
> +	struct anx7688 *anx7688;
> +	struct device *dev = &client->dev;
> +	struct typec_capability typec_cap = { };
> +	int i, vid_h, vid_l;
> +	int irq_cabledet;
> +	int ret = 0;
> +
> +	anx7688 = devm_kzalloc(dev, sizeof(*anx7688), GFP_KERNEL);
> +	if (!anx7688)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, anx7688);
> +	anx7688->client = client;
> +	anx7688->dev = &client->dev;
> +	mutex_init(&anx7688->lock);
> +	INIT_DELAYED_WORK(&anx7688->work, anx7688_work);
> +	anx7688->last_extcon_state = -1;
> +
> +	ret = anx7688_parse_connector(dev, anx7688, &typec_cap);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to parse connector from DT\n");
> +		return ret;
> +	}
> +
> +	for (i = 0; i < ANX7688_NUM_SUPPLIES; i++)
> +		anx7688->supplies[i].supply = anx7688_supply_names[i];
> +	ret = devm_regulator_bulk_get(dev, ANX7688_NUM_SUPPLIES,
> +				      anx7688->supplies);
> +	if (ret)
> +		return ret;
> +
> +	anx7688->vbus_in_supply =
> +		devm_power_supply_get_by_phandle(dev, "vbus-in-supply");
> +	if (IS_ERR(anx7688->vbus_in_supply)) {
> +		dev_err(dev, "Couldn't get the VBUS power supply\n");
> +		return PTR_ERR(anx7688->vbus_in_supply);
> +	}
> +
> +	if (!anx7688->vbus_in_supply)
> +		return -EPROBE_DEFER;
> +
> +	anx7688->gpio_enable = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW);
> +	if (IS_ERR(anx7688->gpio_enable)) {
> +		dev_err(dev, "Could not get enable gpio\n");
> +		return PTR_ERR(anx7688->gpio_enable);
> +	}
> +
> +	anx7688->gpio_reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(anx7688->gpio_reset)) {
> +		dev_err(dev, "Could not get reset gpio\n");
> +		return PTR_ERR(anx7688->gpio_reset);
> +	}
> +
> +	anx7688->gpio_cabledet = devm_gpiod_get(dev, "cabledet", GPIOD_IN);
> +	if (IS_ERR(anx7688->gpio_cabledet)) {
> +		dev_err(dev, "Could not get cabledet gpio\n");
> +		return PTR_ERR(anx7688->gpio_cabledet);
> +	}
> +
> +	irq_cabledet = gpiod_to_irq(anx7688->gpio_cabledet);
> +	if (irq_cabledet < 0) {
> +		dev_err(dev, "Could not get cabledet irq\n");
> +		return irq_cabledet;
> +	}
> +
> +	// Initialize extcon device
> +	anx7688->extcon = devm_extcon_dev_allocate(dev, anx7688_extcon_cable);
> +	if (IS_ERR(anx7688->extcon))
> +		return -ENOMEM;
> +
> +	ret = devm_extcon_dev_register(dev, anx7688->extcon);
> +	if (ret) {
> +		dev_err(dev, "failed to register extcon device\n");
> +		return ret;
> +	}
> +
> +	// Register the TCPC i2c interface as second interface (0x58)
> +	anx7688->client_tcpc = i2c_new_dummy_device(client->adapter, 0x2c);
> +	if (IS_ERR(anx7688->client_tcpc)) {
> +		dev_err(dev, "Could not register tcpc i2c client\n");
> +		return PTR_ERR(anx7688->client_tcpc);
> +	}
> +	i2c_set_clientdata(anx7688->client_tcpc, anx7688);
> +
> +	// powerup and probe the ANX chip
> +
> +	ret = regulator_bulk_enable(ANX7688_NUM_ALWAYS_ON_SUPPLIES,
> +				    anx7688->supplies);
> +	if (ret) {
> +		dev_err(dev, "Could not enable regulators\n");
> +		goto err_dummy_dev;
> +	}
> +
> +	msleep(10);
> +
> +	anx7688_power_enable(anx7688);
> +
> +	vid_l = anx7688_tcpc_reg_read(anx7688, ANX7688_TCPC_REG_VENDOR_ID0);
> +	vid_h = anx7688_tcpc_reg_read(anx7688, ANX7688_TCPC_REG_VENDOR_ID1);
> +	if (vid_l < 0 || vid_h < 0) {
> +		ret = vid_l < 0 ? vid_l : vid_h;
> +		anx7688_power_disable(anx7688);
> +		goto err_disable_reg;
> +	}
> +
> +	dev_info(dev, "Vendor id 0x%04x\n", vid_l | vid_h << 8);
> +
> +	anx7688_power_disable(anx7688);
> +
> +	anx7688->role_sw = usb_role_switch_get(dev);
> +	if (IS_ERR(anx7688->role_sw)) {
> +		dev_err(dev, "Could not get role switch\n");
> +		ret = PTR_ERR(anx7688->role_sw);
> +		goto err_disable_reg;
> +	}
> +
> +	// setup a typec port device
> +	typec_cap.revision = USB_TYPEC_REV_1_2;
> +	typec_cap.pd_revision = 0x200;
> +	ret = -EINVAL;
> +	if (typec_cap.type != TYPEC_PORT_DRP)
> +		goto err_disable_reg;
> +	if (typec_cap.data != TYPEC_PORT_DRD)
> +		goto err_disable_reg;
> +	typec_cap.driver_data = anx7688;
> +	typec_cap.ops = &anx7688_typec_ops;
> +
> +	anx7688->port = typec_register_port(dev, &typec_cap);
> +	if (IS_ERR(anx7688->port)) {
> +		dev_err(dev, "Could not register type-c port\n");
> +		ret = PTR_ERR(anx7688->port);
> +		goto err_role_sw;
> +	}
> +
> +	anx7688->pwr_role = TYPEC_SINK;
> +	anx7688->data_role = TYPEC_DEVICE;
> +	typec_set_pwr_role(anx7688->port, anx7688->pwr_role);
> +	typec_set_data_role(anx7688->port, anx7688->data_role);
> +	typec_set_pwr_opmode(anx7688->port, TYPEC_PWR_MODE_USB);
> +	typec_set_vconn_role(anx7688->port, TYPEC_SINK);
> +
> +	// make sure BC1.2 detection in PMIC is enabled
> +	anx7688->last_bc_result = -1;
> +
> +	dev_dbg(dev, "enabling USB BC 1.2 detection\n");
> +
> +	ret = devm_request_irq(dev, irq_cabledet, anx7688_irq_plug_handler,
> +			       IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> +			       "anx7688-cabledet", anx7688);
> +	if (ret < 0) {
> +		dev_err(dev, "Could not request cabledet irq (%d)\n", ret);
> +		goto err_cport;
> +	}
> +
> +	ret = devm_request_threaded_irq(dev, client->irq,
> +					NULL, anx7688_irq_status_handler,
> +					IRQF_ONESHOT, NULL, anx7688);
> +	if (ret < 0) {
> +		dev_err(dev, "Could not request irq (%d)\n", ret);
> +		goto err_cport;
> +	}
> +
> +	anx7688->vbus_in_nb.notifier_call = anx7688_vbus_in_notify;
> +	anx7688->vbus_in_nb.priority = 0;
> +	ret = power_supply_reg_notifier(&anx7688->vbus_in_nb);
> +	if (ret)
> +		goto err_cport;
> +
> +	anx7688->debug_root = debugfs_create_dir("anx7688", NULL);
> +	debugfs_create_file("firmware", 0444, anx7688->debug_root, anx7688,
> +			    &anx7688_firmware_fops);
> +	debugfs_create_file("regs", 0444, anx7688->debug_root, anx7688,
> +			    &anx7688_regs_fops);
> +	debugfs_create_file("status", 0444, anx7688->debug_root, anx7688,
> +			    &anx7688_status_fops);
> +
> +	schedule_delayed_work(&anx7688->work, msecs_to_jiffies(10));
> +
> +	timer_setup(&anx7688->work_timer, anx7688_cabledet_timer_fn, 0);
> +	mod_timer(&anx7688->work_timer, jiffies + msecs_to_jiffies(1000));
> +	return 0;
> +
> +err_cport:
> +	typec_unregister_port(anx7688->port);
> +err_role_sw:
> +	usb_role_switch_put(anx7688->role_sw);
> +err_disable_reg:
> +	regulator_bulk_disable(ANX7688_NUM_ALWAYS_ON_SUPPLIES, anx7688->supplies);
> +err_dummy_dev:
> +	i2c_unregister_device(anx7688->client_tcpc);
> +	return ret;
> +}
> +
> +static void anx7688_i2c_remove(struct i2c_client *client)
> +{
> +	struct anx7688 *anx7688 = i2c_get_clientdata(client);
> +
> +	mutex_lock(&anx7688->lock);
> +
> +	power_supply_unreg_notifier(&anx7688->vbus_in_nb);
> +
> +	del_timer_sync(&anx7688->work_timer);
> +
> +	cancel_delayed_work_sync(&anx7688->work);
> +
> +	if (test_bit(ANX7688_F_CONNECTED, anx7688->flags))
> +		anx7688_disconnect(anx7688);
> +
> +	typec_unregister_partner(anx7688->partner);
> +	typec_unregister_port(anx7688->port);
> +	usb_role_switch_put(anx7688->role_sw);
> +
> +	regulator_bulk_disable(ANX7688_NUM_ALWAYS_ON_SUPPLIES, anx7688->supplies);
> +	i2c_unregister_device(anx7688->client_tcpc);
> +
> +	debugfs_remove(anx7688->debug_root);
> +
> +	mutex_unlock(&anx7688->lock);
> +}
> +
> +static int __maybe_unused anx7688_suspend(struct device *dev)
> +{
> +	struct anx7688 *anx7688 = i2c_get_clientdata(to_i2c_client(dev));
> +
> +	del_timer_sync(&anx7688->work_timer);
> +	cancel_delayed_work_sync(&anx7688->work);
> +
> +	regulator_disable(anx7688->supplies[ANX7688_I2C_INDEX].consumer);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused anx7688_resume(struct device *dev)
> +{
> +	struct anx7688 *anx7688 = i2c_get_clientdata(to_i2c_client(dev));
> +	int ret;
> +
> +	ret = regulator_enable(anx7688->supplies[ANX7688_I2C_INDEX].consumer);
> +	if (ret)
> +		dev_warn(anx7688->dev,
> +			 "failed to enable I2C regulator (%d)\n", ret);
> +
> +	// check status right after resume, since it could have changed during
> +	// sleep
> +	schedule_delayed_work(&anx7688->work, msecs_to_jiffies(50));
> +	mod_timer(&anx7688->work_timer, jiffies + msecs_to_jiffies(1000));
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops anx7688_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(anx7688_suspend, anx7688_resume)
> +};
> +
> +static const struct i2c_device_id anx7688_ids[] = {
> +	{ "anx7688", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, anx7688_ids);
> +
> +static const struct of_device_id anx7688_of_match_table[] = {
> +	{ .compatible = "analogix,anx7688" },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, anx7688_of_match_table);
> +
> +static struct i2c_driver anx7688_driver = {
> +	.driver = {
> +		.name = "anx7688",
> +		.of_match_table = anx7688_of_match_table,
> +		.pm = &anx7688_pm_ops,
> +	},
> +	.probe = anx7688_i2c_probe,
> +	.remove = anx7688_i2c_remove,
> +	.id_table = anx7688_ids,
> +};
> +
> +module_i2c_driver(anx7688_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Martijn Braam <martijn@brixit.nl>");
> +MODULE_AUTHOR("Ondrej Jirman <megi@xff.cz>");
> +MODULE_DESCRIPTION("Analogix ANX7688 USB-C DisplayPort bridge");
> 
> 
> -- 
> People of Russia, stop Putin before his war on Ukraine escalates.



-- 
heikki

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCHv2 2/2] usb: typec: anx7688: Add driver for ANX7688 USB-C HDMI bridge
  2024-03-12 14:12   ` Heikki Krogerus
@ 2024-03-21 22:10     ` Pavel Machek
  0 siblings, 0 replies; 14+ messages in thread
From: Pavel Machek @ 2024-03-21 22:10 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: phone-devel, kernel list, fiona.klute, martijn, samuel, gregkh,
	linux-usb, megi

[-- Attachment #1: Type: text/plain, Size: 5167 bytes --]

Hi!

> I'm sorry to keep you waiting.

Thanks for comments.

> > +	struct gpio_desc *gpio_reset;
> > +	struct gpio_desc *gpio_cabledet;
> > +
> > +	uint32_t src_caps[8];
> 
> Use u32 instead of uint32_t.

Will replace globally.

> > +static int anx7688_reg_read(struct anx7688 *anx7688, u8 reg_addr)
> > +{
> > +	int ret;
> > +
> > +	ret = i2c_smbus_read_byte_data(anx7688->client, reg_addr);
> > +	if (ret < 0)
> > +		dev_err(anx7688->dev, "i2c read failed at 0x%x (%d)\n",
> > +			reg_addr, ret);
> 
> dev_dbg instead.

I'd prefer not to. i2c functions should not really fail, and if they
do, we want the error log. This is not debugging, this is i2c failing.

> > +static void anx7688_power_enable(struct anx7688 *anx7688)
> > +{
> > +	gpiod_set_value(anx7688->gpio_reset, 1);
> > +	gpiod_set_value(anx7688->gpio_enable, 1);
> > +
> > +	/* wait for power to stabilize and release reset */
> > +	msleep(10);
> 
> So is it okay that the sleep may take up to 20ms?

I don't see how that would be a problem.

> > +	gpiod_set_value(anx7688->gpio_reset, 0);
> > +	udelay(2);
> 
> Use usleep_range() instead.

Can do, but it makes no difference.

> > +	gpiod_set_value(anx7688->gpio_reset, 1);
> > +	msleep(5);
> 
> The same question here, is it a problem if the sleep ends up taking
> 20ms?

Again, I expect that to be ok.

> > +	ret = anx7688_tcpc_reg_read(anx7688, ANX7688_TCPC_REG_INTERFACE_SEND);
> > +	if (ret) {
> > +		dev_err(anx7688->dev,
> > +			"failed to send pd packet (tx buffer full)\n");
> 
> One line should be enought for that one.

That makes it go over 80 columns, but yes, can be one line.

> > +	// wait until the message is processed (30ms max)
> > +	for (i = 0; i < 300; i++) {
> > +		ret = anx7688_tcpc_reg_read(anx7688, ANX7688_TCPC_REG_INTERFACE_SEND);
> > +		if (ret <= 0)
> > +			return ret;
> > +
> > +		udelay(100);
> > +	}
> > +
> > +	dev_err(anx7688->dev, "timeout waiting for the message queue flush\n");
> 
> Maybe dev_dbg for this too.

Let's not hide these. If they happen, we can downgrade them, but they
should not.

> > +	/* wait till the firmware is loaded (typically ~30ms) */
> > +	for (i = 0; i < 100; i++) {
> > +		ret = anx7688_reg_read(anx7688, ANX7688_REG_EEPROM_LOAD_STATUS0);
> > +
> > +		if (ret >= 0 && (ret & ANX7688_EEPROM_FW_LOADED) == ANX7688_EEPROM_FW_LOADED) {
> > +			dev_dbg(anx7688->dev, "eeprom0 = 0x%02x\n", ret);
> > +			dev_info(anx7688->dev, "fw loaded after %d ms\n", i * 10);
> 
> Debugging information. Use dev_dbg.

Ok.

> > +	set_bit(ANX7688_F_FW_FAILED, anx7688->flags);
> > +	dev_err(anx7688->dev, "boot firmware load failed (you may need to flash FW to anx7688 first)\n");
> > +	ret = -ETIMEDOUT;
> > +	goto err_vconoff;
> > +
> > +fw_loaded:
> 
> This label looks a bit messy to me. You could also move that firmware
> loading wait to its own function.

Ok, let me try to refactor this.

> > +static int anx7688_handle_pd_message_response(struct anx7688 *anx7688,
> > +					      u8 to_cmd, u8 resp)
> > +{
...
> > +	return 0;
> > +}
> 
> Noise. Drop this whole function. If you need this kind of information,
> then please consider trace points, or just use some debugfs trick like
> what we have in drivers/usb/typec/tcpm/tcpm.c and few other drivers.

Ok.

> > +	switch (cmd) {
> > +	case ANX7688_OCM_MSG_PWR_SRC_CAP:
> > +		dev_info(anx7688->dev, "received SRC_CAP\n");
> 
> Noise.

Ok, let me convert these to dev_dbg.

> > +
> > +		if (len % 4 != 0) {
> > +			dev_warn(anx7688->dev, "received invalid sized PDO array\n");
> > +			break;
> > +		}
> > +
> > +		/* the partner is PD capable */
> > +		anx7688->pd_capable = true;
> > +
> > +		for (i = 0; i < len / 4; i++) {
> > +			pdo = le32_to_cpu(pdos[i]);
> > +
> > +			if (pdo_type(pdo) == PDO_TYPE_FIXED) {
> > +				unsigned int voltage = pdo_fixed_voltage(pdo);
> > +				unsigned int max_curr = pdo_max_current(pdo);
> > +
> > +				dev_info(anx7688->dev, "SRC_CAP PDO_FIXED (%umV %umA)\n", voltage, max_curr);
> 
> Noise.
> 
> > +			} else if (pdo_type(pdo) == PDO_TYPE_BATT) {
> > +				unsigned int min_volt = pdo_min_voltage(pdo);
> > +				unsigned int max_volt = pdo_max_voltage(pdo);
> > +				unsigned int max_pow = pdo_max_power(pdo);
> > +
> > +				dev_info(anx7688->dev, "SRC_CAP PDO_BATT (%umV-%umV %umW)\n", min_volt, max_volt, max_pow);
> 
> Noise. That line also really should be split in two.
> 
> I'm stopping my review here. This driver is too noisy. All dev_info
> calls need to be dropped. If the driver is working correctly then it
> needs to quiet.
> 
> Most of those prints are useful for debugging only, so I think similar
> debugfs log like the one tcpm.c uses could be a good idea for them
> since you already use debugfs in this driver in any case.

Ok, let me convert the non-error ones to dev_dbg() and split the long
lines. Debug needs to be enabled, so it should not bother anyone, and
it is easier than refactoring driver to use debugfs.

Best regards,
								Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2024-03-21 22:10 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-09 19:51 [PATCH] dt-bindings: usb: typec: anx7688: start a binding document Pavel Machek
2024-02-11 13:56 ` Krzysztof Kozlowski
2024-02-12 11:02   ` Pavel Machek
2024-02-12 11:16     ` Krzysztof Kozlowski
2024-02-21 22:45   ` Pavel Machek
2024-02-22  8:22     ` Krzysztof Kozlowski
2024-02-23 21:28 ` [PATCHv2 1/2] " Pavel Machek
2024-02-23 22:59   ` Rob Herring
2024-03-11 21:22   ` Pavel Machek
2024-03-12 10:51     ` Krzysztof Kozlowski
2024-02-23 21:28 ` [PATCHv2 2/2] usb: typec: anx7688: Add driver for ANX7688 USB-C HDMI bridge Pavel Machek
2024-03-11 21:22   ` Pavel Machek
2024-03-12 14:12   ` Heikki Krogerus
2024-03-21 22:10     ` Pavel Machek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.