[PATCH 0/3] usb: typec: tcpci_rt1711h: Add compatible with rt1715

[PATCH 0/3] usb: typec: tcpci_rt1711h: Add compatible with rt1715

[Gene Chen]

This patch series add binding document for rt1711h and compatible driver with
rt1715.

Gene Chen (2)
 dt-bindings usb: typec: rt1711h: Add binding for Richtek RT1711H
 usb: typec: tcpci: Add get cc tcpci callback
 usb: typec: tcpci_rt1711h: Add compatible with rt1715

 Documentation/devicetree/bindings/usb/richtek,rt1711h.yaml |   96 +++++++
 drivers/usb/typec/tcpm/tcpci.c                             |    3 
 drivers/usb/typec/tcpm/tcpci.h                             |    2 
 drivers/usb/typec/tcpm/tcpci_rt1711h.c                     |  168 ++++++++++++-
 4 files changed, 262 insertions(+), 7 deletions(-)

[PATCH 1/3] dt-bindings usb: typec: rt1711h: Add binding for Richtek RT1711H

[Gene Chen]

From: Gene Chen <gene_chen@richtek.com>

Add binding for Richtek RT1711H

Signed-off-by: Gene Chen <gene_chen@richtek.com>
---
 .../bindings/usb/richtek,rt1711h.yaml         | 96 +++++++++++++++++++
 1 file changed, 96 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/richtek,rt1711h.yaml

diff --git a/Documentation/devicetree/bindings/usb/richtek,rt1711h.yaml b/Documentation/devicetree/bindings/usb/richtek,rt1711h.yaml
new file mode 100644
index 000000000000..b6daca71429b
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/richtek,rt1711h.yaml
@@ -0,0 +1,96 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/usb/richtek,rt1711h.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Richtek RT1711H Type-C Port Switch and Power Delivery controller DT bindings
+
+maintainers:
+  - Gene Chen <gene_chen@richtek.com>
+
+description: |
+  - Richtek RT1711H TypeC PD Controller.
+
+properties:
+  compatible:
+    oneOf:
+      - enum:
+          - richtek,rt1711h
+          - richtek,rt1715
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  wakeup-source:
+    description: enable IRQ remote wakeup, see power/wakeup-source.txt
+    type: boolean
+
+  connector:
+    type: object
+    $ref: ../connector/usb-connector.yaml#
+    description:
+      Properties for usb c connector.
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - connector
+  - interrupts
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/usb/pd.h>
+    i2c0 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      rt1711h@4e {
+        compatible = "richtek,rt1711h";
+        reg = <0x4e>;
+        interrupts-extended = <&gpio26 3 IRQ_TYPE_LEVEL_LOW>;
+        wakeup-source;
+
+        connector {
+          compatible = "usb-c-connector";
+          label = "USB-C";
+          data-role = "dual";
+          power-role = "dual";
+          try-power-role = "sink";
+          source-pdos = <PDO_FIXED(5000, 2000, PDO_FIXED_DUAL_ROLE | PDO_FIXED_DATA_SWAP)>;
+          sink-pdos = <PDO_FIXED(5000, 2000, PDO_FIXED_DUAL_ROLE | PDO_FIXED_DATA_SWAP)>;
+          op-sink-microwatt = <10000000>;
+
+          ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            port@0 {
+              reg = <0>;
+              endpoint {
+                remote-endpoint = <&usb_hs>;
+              };
+            };
+            port@1 {
+              reg = <1>;
+              endpoint {
+                remote-endpoint = <&usb_ss>;
+              };
+            };
+            port@2 {
+              reg = <2>;
+              endpoint {
+                remote-endpoint = <&dp_aux>;
+              };
+            };
+          };
+        };
+      };
+    };
+...
-- 
2.25.1

Re: [PATCH 1/3] dt-bindings usb: typec: rt1711h: Add binding for Richtek RT1711H

[Rob Herring]

On Fri, Jul 15, 2022 at 06:04:16PM +0800, Gene Chen wrote:
> From: Gene Chen <gene_chen@richtek.com>
> 
> Add binding for Richtek RT1711H
> 
> Signed-off-by: Gene Chen <gene_chen@richtek.com>
> ---
>  .../bindings/usb/richtek,rt1711h.yaml         | 96 +++++++++++++++++++
>  1 file changed, 96 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/richtek,rt1711h.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/richtek,rt1711h.yaml b/Documentation/devicetree/bindings/usb/richtek,rt1711h.yaml
> new file mode 100644
> index 000000000000..b6daca71429b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/richtek,rt1711h.yaml
> @@ -0,0 +1,96 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/usb/richtek,rt1711h.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Richtek RT1711H Type-C Port Switch and Power Delivery controller DT bindings

s/ DT bindings//

> +
> +maintainers:
> +  - Gene Chen <gene_chen@richtek.com>
> +
> +description: |

Don't need '|' if no formatting.

> +  - Richtek RT1711H TypeC PD Controller.

'-' should be dropped. Though if you can't say more than the title, you 
might as well drop entirely.

> +
> +properties:
> +  compatible:
> +    oneOf:

Don't need oneOf with only 1 entry.

> +      - enum:
> +          - richtek,rt1711h
> +          - richtek,rt1715

Perhaps 'description' should detail the differences in these parts.

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  wakeup-source:
> +    description: enable IRQ remote wakeup, see power/wakeup-source.txt

Drop generic descriptions.

> +    type: boolean
> +
> +  connector:
> +    type: object
> +    $ref: ../connector/usb-connector.yaml#

full path:

/schemas/connector/...

> +    description:
> +      Properties for usb c connector.
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - connector
> +  - interrupts
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/usb/pd.h>
> +    i2c0 {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      rt1711h@4e {
> +        compatible = "richtek,rt1711h";
> +        reg = <0x4e>;
> +        interrupts-extended = <&gpio26 3 IRQ_TYPE_LEVEL_LOW>;
> +        wakeup-source;
> +
> +        connector {
> +          compatible = "usb-c-connector";
> +          label = "USB-C";
> +          data-role = "dual";
> +          power-role = "dual";
> +          try-power-role = "sink";
> +          source-pdos = <PDO_FIXED(5000, 2000, PDO_FIXED_DUAL_ROLE | PDO_FIXED_DATA_SWAP)>;
> +          sink-pdos = <PDO_FIXED(5000, 2000, PDO_FIXED_DUAL_ROLE | PDO_FIXED_DATA_SWAP)>;
> +          op-sink-microwatt = <10000000>;
> +
> +          ports {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            port@0 {
> +              reg = <0>;
> +              endpoint {
> +                remote-endpoint = <&usb_hs>;
> +              };
> +            };
> +            port@1 {
> +              reg = <1>;
> +              endpoint {
> +                remote-endpoint = <&usb_ss>;
> +              };
> +            };
> +            port@2 {
> +              reg = <2>;
> +              endpoint {
> +                remote-endpoint = <&dp_aux>;
> +              };
> +            };
> +          };
> +        };
> +      };
> +    };
> +...
> -- 
> 2.25.1
> 
> 

[PATCH 2/3] usb: typec: tcpci: Add get cc tcpci callback

[Gene Chen]

From: Gene Chen <gene_chen@richtek.com>

Add set_vbus tcpci callback for vendor IC workaround.
According to different rp level detected, set corresponding
rx dead zone threshold in order to decode right pd message.

Signed-off-by: Gene Chen <gene_chen@richtek.com>
---
 drivers/usb/typec/tcpm/tcpci.c | 3 +++
 drivers/usb/typec/tcpm/tcpci.h | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index f33e08eb7670..fc2f6191b7d3 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -243,6 +243,9 @@ static int tcpci_get_cc(struct tcpc_dev *tcpc,
 	unsigned int reg, role_control;
 	int ret;
 
+	if (tcpci->data->get_cc)
+		return tcpci->data->get_cc(tcpci, tcpci->data, cc1, cc2);
+
 	ret = regmap_read(tcpci->regmap, TCPC_ROLE_CTRL, &role_control);
 	if (ret < 0)
 		return ret;
diff --git a/drivers/usb/typec/tcpm/tcpci.h b/drivers/usb/typec/tcpm/tcpci.h
index b2edd45f13c6..2cef19e131f8 100644
--- a/drivers/usb/typec/tcpm/tcpci.h
+++ b/drivers/usb/typec/tcpm/tcpci.h
@@ -190,6 +190,8 @@ struct tcpci_data {
 	unsigned char vbus_vsafe0v:1;
 
 	int (*init)(struct tcpci *tcpci, struct tcpci_data *data);
+	int (*get_cc)(struct tcpci *tcpci, struct tcpci_data *data,
+		      enum typec_cc_status *cc1, enum typec_cc_status *cc2);
 	int (*set_vconn)(struct tcpci *tcpci, struct tcpci_data *data,
 			 bool enable);
 	int (*start_drp_toggling)(struct tcpci *tcpci, struct tcpci_data *data,
-- 
2.25.1

Re: [PATCH 2/3] usb: typec: tcpci: Add get cc tcpci callback

[Guenter Roeck]

On 7/15/22 03:04, Gene Chen wrote:
> From: Gene Chen <gene_chen@richtek.com>
> 
> Add set_vbus tcpci callback for vendor IC workaround.
> According to different rp level detected, set corresponding
> rx dead zone threshold in order to decode right pd message.
> 

Looking at the next patch, I dispute the need for this callback.
 From what I can see, the additional code should be implemented
in the driver's interrupt handler whenever CC changes, not when
CC values are read from higher level drivers.

Guenter

> Signed-off-by: Gene Chen <gene_chen@richtek.com>
> ---
>   drivers/usb/typec/tcpm/tcpci.c | 3 +++
>   drivers/usb/typec/tcpm/tcpci.h | 2 ++
>   2 files changed, 5 insertions(+)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index f33e08eb7670..fc2f6191b7d3 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -243,6 +243,9 @@ static int tcpci_get_cc(struct tcpc_dev *tcpc,
>   	unsigned int reg, role_control;
>   	int ret;
>   
> +	if (tcpci->data->get_cc)
> +		return tcpci->data->get_cc(tcpci, tcpci->data, cc1, cc2);
> +
>   	ret = regmap_read(tcpci->regmap, TCPC_ROLE_CTRL, &role_control);
>   	if (ret < 0)
>   		return ret;
> diff --git a/drivers/usb/typec/tcpm/tcpci.h b/drivers/usb/typec/tcpm/tcpci.h
> index b2edd45f13c6..2cef19e131f8 100644
> --- a/drivers/usb/typec/tcpm/tcpci.h
> +++ b/drivers/usb/typec/tcpm/tcpci.h
> @@ -190,6 +190,8 @@ struct tcpci_data {
>   	unsigned char vbus_vsafe0v:1;
>   
>   	int (*init)(struct tcpci *tcpci, struct tcpci_data *data);
> +	int (*get_cc)(struct tcpci *tcpci, struct tcpci_data *data,
> +		      enum typec_cc_status *cc1, enum typec_cc_status *cc2);
>   	int (*set_vconn)(struct tcpci *tcpci, struct tcpci_data *data,
>   			 bool enable);
>   	int (*start_drp_toggling)(struct tcpci *tcpci, struct tcpci_data *data,

[PATCH 3/3] usb: typec: tcpci_rt1711h: Add compatible with rt1715

[Gene Chen]

From: Gene Chen <gene_chen@richtek.com>

Add compatible with rt1715

Signed-off-by: Gene Chen <gene_chen@richtek.com>
---
 drivers/usb/typec/tcpm/tcpci_rt1711h.c | 168 +++++++++++++++++++++++--
 1 file changed, 161 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpci_rt1711h.c b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
index b56a0880a044..1fba98e4ef03 100644
--- a/drivers/usb/typec/tcpm/tcpci_rt1711h.c
+++ b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
@@ -10,22 +10,31 @@
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/gpio/consumer.h>
+#include <linux/regulator/consumer.h>
 #include <linux/usb/tcpm.h>
 #include <linux/regmap.h>
 #include "tcpci.h"
 
 #define RT1711H_VID		0x29CF
 #define RT1711H_PID		0x1711
+#define RT1715_DID		0x2173
 
-#define RT1711H_RTCTRL8		0x9B
+#define RT1711H_PHYCTRL1	0x80
+#define RT1711H_PHYCTRL2	0x81
+
+#define RT1711H_RTCTRL4		0x93
+/* rx threshold of rd/rp: 1b0 for level 0.4V/0.7V, 1b1 for 0.35V/0.75V */
+#define RT1711H_BMCIO_RXDZSEL	BIT(0)
 
+#define RT1711H_RTCTRL8		0x9B
 /* Autoidle timeout = (tout * 2 + 1) * 6.4ms */
 #define RT1711H_RTCTRL8_SET(ck300, ship_off, auto_idle, tout) \
 			    (((ck300) << 7) | ((ship_off) << 5) | \
 			    ((auto_idle) << 3) | ((tout) & 0x07))
+#define RT1711H_AUTOIDLEEN_MASK	BIT(3)
+#define RT1711H_ENEXTMSG_MASK	BIT(4)
 
 #define RT1711H_RTCTRL11	0x9E
-
 /* I2C timeout = (tout + 1) * 12.5ms */
 #define RT1711H_RTCTRL11_SET(en, tout) \
 			     (((en) << 7) | ((tout) & 0x0F))
@@ -35,10 +44,17 @@
 #define RT1711H_RTCTRL15	0xA2
 #define RT1711H_RTCTRL16	0xA3
 
+#define RT1711H_RTCTRL18	0xAF
+/* 1b0 as fixed rx threshold of rd/rp 0.55V, 1b1 depends on RTCRTL4[0] */
+#define BMCIO_RXDZEN_MASK	BIT(0)
+
 struct rt1711h_chip {
 	struct tcpci_data data;
 	struct tcpci *tcpci;
 	struct device *dev;
+	struct regulator *vbus;
+	bool src_en;
+	u16 did;
 };
 
 static int rt1711h_read16(struct rt1711h_chip *chip, unsigned int reg, u16 *val)
@@ -75,8 +91,9 @@ static struct rt1711h_chip *tdata_to_rt1711h(struct tcpci_data *tdata)
 
 static int rt1711h_init(struct tcpci *tcpci, struct tcpci_data *tdata)
 {
-	int ret;
 	struct rt1711h_chip *chip = tdata_to_rt1711h(tdata);
+	struct regmap *regmap = chip->data.regmap;
+	int ret;
 
 	/* CK 300K from 320K, shipping off, auto_idle enable, tout = 32ms */
 	ret = rt1711h_write8(chip, RT1711H_RTCTRL8,
@@ -84,6 +101,14 @@ static int rt1711h_init(struct tcpci *tcpci, struct tcpci_data *tdata)
 	if (ret < 0)
 		return ret;
 
+	/* Enable PD30 extended message for RT1715 */
+	if (chip->did == RT1715_DID) {
+		ret = regmap_update_bits(regmap, RT1711H_RTCTRL8,
+					 RT1711H_ENEXTMSG_MASK, 0xFF);
+		if (ret < 0)
+			return ret;
+	}
+
 	/* I2C reset : (val + 1) * 12.5ms */
 	ret = rt1711h_write8(chip, RT1711H_RTCTRL11,
 			     RT1711H_RTCTRL11_SET(1, 0x0F));
@@ -101,7 +126,37 @@ static int rt1711h_init(struct tcpci *tcpci, struct tcpci_data *tdata)
 		return ret;
 
 	/* dcSRC.DRP : 33% */
-	return rt1711h_write16(chip, RT1711H_RTCTRL16, 330);
+	ret = rt1711h_write16(chip, RT1711H_RTCTRL16, 330);
+	if (ret < 0)
+		return ret;
+
+	/* Enable phy discard retry, retry count 7, rx filter deglitech 100 us */
+	ret = rt1711h_write8(chip, RT1711H_PHYCTRL1, 0xF1);
+	if (ret < 0)
+		return ret;
+
+	/* Decrease wait time of BMC-encoded 1 bit from 2.67us to 2.55us */
+	/* wait time : (val * .4167) us */
+	return rt1711h_write8(chip, RT1711H_PHYCTRL2, 62);
+}
+
+static int rt1711h_set_vbus(struct tcpci *tcpci, struct tcpci_data *tdata,
+			    bool src, bool snk)
+{
+	struct rt1711h_chip *chip = tdata_to_rt1711h(tdata);
+	int ret;
+
+	if (chip->src_en == src)
+		return 1;
+
+	if (src)
+		ret = regulator_enable(chip->vbus);
+	else
+		ret = regulator_disable(chip->vbus);
+
+	if (!ret)
+		chip->src_en = src;
+	return ret ? ret : 1;
 }
 
 static int rt1711h_set_vconn(struct tcpci *tcpci, struct tcpci_data *tdata,
@@ -109,8 +164,93 @@ static int rt1711h_set_vconn(struct tcpci *tcpci, struct tcpci_data *tdata,
 {
 	struct rt1711h_chip *chip = tdata_to_rt1711h(tdata);
 
-	return rt1711h_write8(chip, RT1711H_RTCTRL8,
-			      RT1711H_RTCTRL8_SET(0, 1, !enable, 2));
+	return regmap_update_bits(chip->data.regmap, RT1711H_RTCTRL8,
+				  RT1711H_AUTOIDLEEN_MASK, enable ? 0 : 0xFF);
+}
+
+/*
+ * Selects the CC PHY noise filter voltage level according to the current
+ * CC voltage level.
+ *
+ * @param cc_level The CC voltage level for the port's current role
+ * @return EC_SUCCESS if writes succeed; failure code otherwise
+ */
+static inline int rt1711h_init_cc_params(struct rt1711h_chip *chip,
+	enum typec_cc_status cc1, enum typec_cc_status cc2)
+{
+	u32 rxdz_en = 0, rxdz_sel = 0;
+	int ret;
+
+	if ((cc1 >= TYPEC_CC_RP_1_5 && cc2 < TYPEC_CC_RP_DEF) ||
+	    (cc2 >= TYPEC_CC_RP_1_5 && cc1 < TYPEC_CC_RP_DEF)) {
+		if (chip->did == RT1715_DID) {
+			rxdz_en = 1;
+			rxdz_sel = 1;
+		} else {
+			rxdz_en = 1;
+			rxdz_sel = 0;
+		}
+	} else {
+		rxdz_en = 0;
+		rxdz_sel = 1;
+	}
+
+	ret = regmap_update_bits(chip->data.regmap, RT1711H_RTCTRL18,
+				 BMCIO_RXDZEN_MASK, rxdz_en);
+	if (ret < 0)
+		return ret;
+
+	return regmap_update_bits(chip->data.regmap, RT1711H_RTCTRL4,
+				  RT1711H_BMCIO_RXDZSEL, rxdz_en);
+}
+
+#define tcpc_presenting_rd(reg, cc) \
+	(!(TCPC_ROLE_CTRL_DRP & (reg)) && \
+	 (((reg) & (TCPC_ROLE_CTRL_## cc ##_MASK << TCPC_ROLE_CTRL_## cc ##_SHIFT)) == \
+	  (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_## cc ##_SHIFT)))
+
+static enum typec_cc_status tcpci_to_typec_cc(unsigned int cc, bool sink)
+{
+	switch (cc) {
+	case 0x1:
+		return sink ? TYPEC_CC_RP_DEF : TYPEC_CC_RA;
+	case 0x2:
+		return sink ? TYPEC_CC_RP_1_5 : TYPEC_CC_RD;
+	case 0x3:
+		if (sink)
+			return TYPEC_CC_RP_3_0;
+		fallthrough;
+	case 0x0:
+	default:
+		return TYPEC_CC_OPEN;
+	}
+}
+
+static int rt1711h_get_cc(struct tcpci *tcpci, struct tcpci_data *tdata,
+			  enum typec_cc_status *cc1, enum typec_cc_status *cc2)
+{
+	struct rt1711h_chip *chip = tdata_to_rt1711h(tdata);
+	unsigned int reg, role_control;
+	int ret;
+
+	ret = regmap_read(chip->data.regmap, TCPC_ROLE_CTRL, &role_control);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_read(chip->data.regmap, TCPC_CC_STATUS, &reg);
+	if (ret < 0)
+		return ret;
+
+	*cc1 = tcpci_to_typec_cc((reg >> TCPC_CC_STATUS_CC1_SHIFT) &
+				 TCPC_CC_STATUS_CC1_MASK,
+				 reg & TCPC_CC_STATUS_TERM ||
+				 tcpc_presenting_rd(role_control, CC1));
+	*cc2 = tcpci_to_typec_cc((reg >> TCPC_CC_STATUS_CC2_SHIFT) &
+				 TCPC_CC_STATUS_CC2_MASK,
+				 reg & TCPC_CC_STATUS_TERM ||
+				 tcpc_presenting_rd(role_control, CC2));
+
+	return rt1711h_init_cc_params(chip, *cc1, *cc2);
 }
 
 static int rt1711h_start_drp_toggling(struct tcpci *tcpci,
@@ -209,7 +349,11 @@ static int rt1711h_check_revision(struct i2c_client *i2c)
 		dev_err(&i2c->dev, "pid is not correct, 0x%04x\n", ret);
 		return -ENODEV;
 	}
-	return 0;
+	ret = i2c_smbus_read_word_data(i2c, TCPC_BCD_DEV);
+	if (ret < 0)
+		return ret;
+	dev_info(&i2c->dev, "did is 0x%04x\n", ret);
+	return ret;
 }
 
 static int rt1711h_probe(struct i2c_client *client,
@@ -228,6 +372,8 @@ static int rt1711h_probe(struct i2c_client *client,
 	if (!chip)
 		return -ENOMEM;
 
+	chip->did = ret;
+
 	chip->data.regmap = devm_regmap_init_i2c(client,
 						 &rt1711h_regmap_config);
 	if (IS_ERR(chip->data.regmap))
@@ -245,8 +391,14 @@ static int rt1711h_probe(struct i2c_client *client,
 	if (ret < 0)
 		return ret;
 
+	chip->vbus = devm_regulator_get(&client->dev, "vbus");
+	if (IS_ERR(chip->vbus))
+		return PTR_ERR(chip->vbus);
+
 	chip->data.init = rt1711h_init;
+	chip->data.set_vbus = rt1711h_set_vbus;
 	chip->data.set_vconn = rt1711h_set_vconn;
+	chip->data.get_cc = rt1711h_get_cc;
 	chip->data.start_drp_toggling = rt1711h_start_drp_toggling;
 	chip->tcpci = tcpci_register_port(chip->dev, &chip->data);
 	if (IS_ERR_OR_NULL(chip->tcpci))
@@ -273,6 +425,7 @@ static int rt1711h_remove(struct i2c_client *client)
 
 static const struct i2c_device_id rt1711h_id[] = {
 	{ "rt1711h", 0 },
+	{ "rt1715", 0 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, rt1711h_id);
@@ -280,6 +433,7 @@ MODULE_DEVICE_TABLE(i2c, rt1711h_id);
 #ifdef CONFIG_OF
 static const struct of_device_id rt1711h_of_match[] = {
 	{ .compatible = "richtek,rt1711h", },
+	{ .compatible = "richtek,rt1715", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, rt1711h_of_match);
-- 
2.25.1

Re: [PATCH 3/3] usb: typec: tcpci_rt1711h: Add compatible with rt1715

[Greg KH]

On Fri, Jul 15, 2022 at 06:04:18PM +0800, Gene Chen wrote:
> From: Gene Chen <gene_chen@richtek.com>
> 
> Add compatible with rt1715

I can not understand this changelog text at all.  As my bot would say:

- You did not specify a description of why the patch is needed, or
  possibly, any description at all, in the email body.  Please read the
  section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what is needed in order to
  properly describe the change.

- You did not write a descriptive Subject: for the patch, allowing Greg,
  and everyone else, to know what this patch is all about.  Please read
  the section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what a proper Subject: line should
  look like.

Please fix up.

thanks,

greg k-h

Re: [PATCH 3/3] usb: typec: tcpci_rt1711h: Add compatible with rt1715

[Guenter Roeck]

On 7/15/22 03:04, Gene Chen wrote:
> From: Gene Chen <gene_chen@richtek.com>
> 
> Add compatible with rt1715
> 
> Signed-off-by: Gene Chen <gene_chen@richtek.com>

Besides the lack of usable subject/description as mentioned by Greg,
this patch is making more than one logical change.

- It changes a direct register access to a regmap access
- It adds mandatory regulator support (even for already supported
   chips)
- It changes initialization for all supported chips
- It implements get_cc for all chips, not just the new one,
   and changes some chip parameters along the line.
- It adds support for the new chip

So from my perspective this should be (at least) five separate patches.

Implementation wise, the get_cc function is the same as the get_cc
function in tcpci.c, except it adds a call to rt1711h_init_cc_params()
whenever CC is read. This just seems wrong and will require a detailed
explanation. Why change the chip configuration when _reading_ CC values
and not in rt1711h_irq() whenever TCPC_ALERT_CC_STATUS is reported ?

Guenter

> ---
>   drivers/usb/typec/tcpm/tcpci_rt1711h.c | 168 +++++++++++++++++++++++--
>   1 file changed, 161 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpci_rt1711h.c b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
> index b56a0880a044..1fba98e4ef03 100644
> --- a/drivers/usb/typec/tcpm/tcpci_rt1711h.c
> +++ b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
> @@ -10,22 +10,31 @@
>   #include <linux/i2c.h>
>   #include <linux/interrupt.h>
>   #include <linux/gpio/consumer.h>
> +#include <linux/regulator/consumer.h>
>   #include <linux/usb/tcpm.h>
>   #include <linux/regmap.h>
>   #include "tcpci.h"
>   
>   #define RT1711H_VID		0x29CF
>   #define RT1711H_PID		0x1711
> +#define RT1715_DID		0x2173
>   
> -#define RT1711H_RTCTRL8		0x9B
> +#define RT1711H_PHYCTRL1	0x80
> +#define RT1711H_PHYCTRL2	0x81
> +
> +#define RT1711H_RTCTRL4		0x93
> +/* rx threshold of rd/rp: 1b0 for level 0.4V/0.7V, 1b1 for 0.35V/0.75V */
> +#define RT1711H_BMCIO_RXDZSEL	BIT(0)
>   
> +#define RT1711H_RTCTRL8		0x9B
>   /* Autoidle timeout = (tout * 2 + 1) * 6.4ms */
>   #define RT1711H_RTCTRL8_SET(ck300, ship_off, auto_idle, tout) \
>   			    (((ck300) << 7) | ((ship_off) << 5) | \
>   			    ((auto_idle) << 3) | ((tout) & 0x07))
> +#define RT1711H_AUTOIDLEEN_MASK	BIT(3)
> +#define RT1711H_ENEXTMSG_MASK	BIT(4)
>   
>   #define RT1711H_RTCTRL11	0x9E
> -
>   /* I2C timeout = (tout + 1) * 12.5ms */
>   #define RT1711H_RTCTRL11_SET(en, tout) \
>   			     (((en) << 7) | ((tout) & 0x0F))
> @@ -35,10 +44,17 @@
>   #define RT1711H_RTCTRL15	0xA2
>   #define RT1711H_RTCTRL16	0xA3
>   
> +#define RT1711H_RTCTRL18	0xAF
> +/* 1b0 as fixed rx threshold of rd/rp 0.55V, 1b1 depends on RTCRTL4[0] */
> +#define BMCIO_RXDZEN_MASK	BIT(0)
> +
>   struct rt1711h_chip {
>   	struct tcpci_data data;
>   	struct tcpci *tcpci;
>   	struct device *dev;
> +	struct regulator *vbus;
> +	bool src_en;
> +	u16 did;
>   };
>   
>   static int rt1711h_read16(struct rt1711h_chip *chip, unsigned int reg, u16 *val)
> @@ -75,8 +91,9 @@ static struct rt1711h_chip *tdata_to_rt1711h(struct tcpci_data *tdata)
>   
>   static int rt1711h_init(struct tcpci *tcpci, struct tcpci_data *tdata)
>   {
> -	int ret;
>   	struct rt1711h_chip *chip = tdata_to_rt1711h(tdata);
> +	struct regmap *regmap = chip->data.regmap;
> +	int ret;
>   
>   	/* CK 300K from 320K, shipping off, auto_idle enable, tout = 32ms */
>   	ret = rt1711h_write8(chip, RT1711H_RTCTRL8,
> @@ -84,6 +101,14 @@ static int rt1711h_init(struct tcpci *tcpci, struct tcpci_data *tdata)
>   	if (ret < 0)
>   		return ret;
>   
> +	/* Enable PD30 extended message for RT1715 */
> +	if (chip->did == RT1715_DID) {
> +		ret = regmap_update_bits(regmap, RT1711H_RTCTRL8,
> +					 RT1711H_ENEXTMSG_MASK, 0xFF);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
>   	/* I2C reset : (val + 1) * 12.5ms */
>   	ret = rt1711h_write8(chip, RT1711H_RTCTRL11,
>   			     RT1711H_RTCTRL11_SET(1, 0x0F));
> @@ -101,7 +126,37 @@ static int rt1711h_init(struct tcpci *tcpci, struct tcpci_data *tdata)
>   		return ret;
>   
>   	/* dcSRC.DRP : 33% */
> -	return rt1711h_write16(chip, RT1711H_RTCTRL16, 330);
> +	ret = rt1711h_write16(chip, RT1711H_RTCTRL16, 330);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Enable phy discard retry, retry count 7, rx filter deglitech 100 us */
> +	ret = rt1711h_write8(chip, RT1711H_PHYCTRL1, 0xF1);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Decrease wait time of BMC-encoded 1 bit from 2.67us to 2.55us */
> +	/* wait time : (val * .4167) us */
> +	return rt1711h_write8(chip, RT1711H_PHYCTRL2, 62);
> +}
> +
> +static int rt1711h_set_vbus(struct tcpci *tcpci, struct tcpci_data *tdata,
> +			    bool src, bool snk)
> +{
> +	struct rt1711h_chip *chip = tdata_to_rt1711h(tdata);
> +	int ret;
> +
> +	if (chip->src_en == src)
> +		return 1;
> +
> +	if (src)
> +		ret = regulator_enable(chip->vbus);
> +	else
> +		ret = regulator_disable(chip->vbus);
> +
> +	if (!ret)
> +		chip->src_en = src;
> +	return ret ? ret : 1;
>   }
>   
>   static int rt1711h_set_vconn(struct tcpci *tcpci, struct tcpci_data *tdata,
> @@ -109,8 +164,93 @@ static int rt1711h_set_vconn(struct tcpci *tcpci, struct tcpci_data *tdata,
>   {
>   	struct rt1711h_chip *chip = tdata_to_rt1711h(tdata);
>   
> -	return rt1711h_write8(chip, RT1711H_RTCTRL8,
> -			      RT1711H_RTCTRL8_SET(0, 1, !enable, 2));
> +	return regmap_update_bits(chip->data.regmap, RT1711H_RTCTRL8,
> +				  RT1711H_AUTOIDLEEN_MASK, enable ? 0 : 0xFF);
> +}
> +
> +/*
> + * Selects the CC PHY noise filter voltage level according to the current
> + * CC voltage level.
> + *
> + * @param cc_level The CC voltage level for the port's current role
> + * @return EC_SUCCESS if writes succeed; failure code otherwise
> + */
> +static inline int rt1711h_init_cc_params(struct rt1711h_chip *chip,
> +	enum typec_cc_status cc1, enum typec_cc_status cc2)
> +{
> +	u32 rxdz_en = 0, rxdz_sel = 0;
> +	int ret;
> +
> +	if ((cc1 >= TYPEC_CC_RP_1_5 && cc2 < TYPEC_CC_RP_DEF) ||
> +	    (cc2 >= TYPEC_CC_RP_1_5 && cc1 < TYPEC_CC_RP_DEF)) {
> +		if (chip->did == RT1715_DID) {
> +			rxdz_en = 1;
> +			rxdz_sel = 1;
> +		} else {
> +			rxdz_en = 1;
> +			rxdz_sel = 0;
> +		}
> +	} else {
> +		rxdz_en = 0;
> +		rxdz_sel = 1;
> +	}
> +
> +	ret = regmap_update_bits(chip->data.regmap, RT1711H_RTCTRL18,
> +				 BMCIO_RXDZEN_MASK, rxdz_en);
> +	if (ret < 0)
> +		return ret;
> +
> +	return regmap_update_bits(chip->data.regmap, RT1711H_RTCTRL4,
> +				  RT1711H_BMCIO_RXDZSEL, rxdz_en);
> +}
> +
> +#define tcpc_presenting_rd(reg, cc) \
> +	(!(TCPC_ROLE_CTRL_DRP & (reg)) && \
> +	 (((reg) & (TCPC_ROLE_CTRL_## cc ##_MASK << TCPC_ROLE_CTRL_## cc ##_SHIFT)) == \
> +	  (TCPC_ROLE_CTRL_CC_RD << TCPC_ROLE_CTRL_## cc ##_SHIFT)))
> +
> +static enum typec_cc_status tcpci_to_typec_cc(unsigned int cc, bool sink)
> +{
> +	switch (cc) {
> +	case 0x1:
> +		return sink ? TYPEC_CC_RP_DEF : TYPEC_CC_RA;
> +	case 0x2:
> +		return sink ? TYPEC_CC_RP_1_5 : TYPEC_CC_RD;
> +	case 0x3:
> +		if (sink)
> +			return TYPEC_CC_RP_3_0;
> +		fallthrough;
> +	case 0x0:
> +	default:
> +		return TYPEC_CC_OPEN;
> +	}
> +}
> +
> +static int rt1711h_get_cc(struct tcpci *tcpci, struct tcpci_data *tdata,
> +			  enum typec_cc_status *cc1, enum typec_cc_status *cc2)
> +{
> +	struct rt1711h_chip *chip = tdata_to_rt1711h(tdata);
> +	unsigned int reg, role_control;
> +	int ret;
> +
> +	ret = regmap_read(chip->data.regmap, TCPC_ROLE_CTRL, &role_control);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = regmap_read(chip->data.regmap, TCPC_CC_STATUS, &reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	*cc1 = tcpci_to_typec_cc((reg >> TCPC_CC_STATUS_CC1_SHIFT) &
> +				 TCPC_CC_STATUS_CC1_MASK,
> +				 reg & TCPC_CC_STATUS_TERM ||
> +				 tcpc_presenting_rd(role_control, CC1));
> +	*cc2 = tcpci_to_typec_cc((reg >> TCPC_CC_STATUS_CC2_SHIFT) &
> +				 TCPC_CC_STATUS_CC2_MASK,
> +				 reg & TCPC_CC_STATUS_TERM ||
> +				 tcpc_presenting_rd(role_control, CC2));
> +
> +	return rt1711h_init_cc_params(chip, *cc1, *cc2);
>   }
>   
>   static int rt1711h_start_drp_toggling(struct tcpci *tcpci,
> @@ -209,7 +349,11 @@ static int rt1711h_check_revision(struct i2c_client *i2c)
>   		dev_err(&i2c->dev, "pid is not correct, 0x%04x\n", ret);
>   		return -ENODEV;
>   	}
> -	return 0;
> +	ret = i2c_smbus_read_word_data(i2c, TCPC_BCD_DEV);
> +	if (ret < 0)
> +		return ret;
> +	dev_info(&i2c->dev, "did is 0x%04x\n", ret);
> +	return ret;
>   }
>   
>   static int rt1711h_probe(struct i2c_client *client,
> @@ -228,6 +372,8 @@ static int rt1711h_probe(struct i2c_client *client,
>   	if (!chip)
>   		return -ENOMEM;
>   
> +	chip->did = ret;
> +
>   	chip->data.regmap = devm_regmap_init_i2c(client,
>   						 &rt1711h_regmap_config);
>   	if (IS_ERR(chip->data.regmap))
> @@ -245,8 +391,14 @@ static int rt1711h_probe(struct i2c_client *client,
>   	if (ret < 0)
>   		return ret;
>   
> +	chip->vbus = devm_regulator_get(&client->dev, "vbus");
> +	if (IS_ERR(chip->vbus))
> +		return PTR_ERR(chip->vbus);
> +
>   	chip->data.init = rt1711h_init;
> +	chip->data.set_vbus = rt1711h_set_vbus;
>   	chip->data.set_vconn = rt1711h_set_vconn;
> +	chip->data.get_cc = rt1711h_get_cc;
>   	chip->data.start_drp_toggling = rt1711h_start_drp_toggling;
>   	chip->tcpci = tcpci_register_port(chip->dev, &chip->data);
>   	if (IS_ERR_OR_NULL(chip->tcpci))
> @@ -273,6 +425,7 @@ static int rt1711h_remove(struct i2c_client *client)
>   
>   static const struct i2c_device_id rt1711h_id[] = {
>   	{ "rt1711h", 0 },
> +	{ "rt1715", 0 },
>   	{ }
>   };
>   MODULE_DEVICE_TABLE(i2c, rt1711h_id);
> @@ -280,6 +433,7 @@ MODULE_DEVICE_TABLE(i2c, rt1711h_id);
>   #ifdef CONFIG_OF
>   static const struct of_device_id rt1711h_of_match[] = {
>   	{ .compatible = "richtek,rt1711h", },
> +	{ .compatible = "richtek,rt1715", },
>   	{},
>   };
>   MODULE_DEVICE_TABLE(of, rt1711h_of_match);

Re: [PATCH 3/3] usb: typec: tcpci_rt1711h: Add compatible with rt1715

[kernel test robot]

Hi Gene,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on v5.19-rc6]
[also build test WARNING on linus/master]
[cannot apply to usb/usb-testing next-20220715]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Gene-Chen/usb-typec-tcpci_rt1711h-Add-compatible-with-rt1715/20220717-003638
base:    32346491ddf24599decca06190ebca03ff9de7f8
config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20220717/202207170406.z3r8vDPb-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/694b38f034326c96d5211fc2da10a25f4938cc3b
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Gene-Chen/usb-typec-tcpci_rt1711h-Add-compatible-with-rt1715/20220717-003638
        git checkout 694b38f034326c96d5211fc2da10a25f4938cc3b
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/usb/typec/tcpm/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/usb/typec/tcpm/tcpci_rt1711h.c: In function 'rt1711h_init_cc_params':
>> drivers/usb/typec/tcpm/tcpci_rt1711h.c:181:26: warning: variable 'rxdz_sel' set but not used [-Wunused-but-set-variable]
     181 |         u32 rxdz_en = 0, rxdz_sel = 0;
         |                          ^~~~~~~~


vim +/rxdz_sel +181 drivers/usb/typec/tcpm/tcpci_rt1711h.c

   170	
   171	/*
   172	 * Selects the CC PHY noise filter voltage level according to the current
   173	 * CC voltage level.
   174	 *
   175	 * @param cc_level The CC voltage level for the port's current role
   176	 * @return EC_SUCCESS if writes succeed; failure code otherwise
   177	 */
   178	static inline int rt1711h_init_cc_params(struct rt1711h_chip *chip,
   179		enum typec_cc_status cc1, enum typec_cc_status cc2)
   180	{
 > 181		u32 rxdz_en = 0, rxdz_sel = 0;
   182		int ret;
   183	
   184		if ((cc1 >= TYPEC_CC_RP_1_5 && cc2 < TYPEC_CC_RP_DEF) ||
   185		    (cc2 >= TYPEC_CC_RP_1_5 && cc1 < TYPEC_CC_RP_DEF)) {
   186			if (chip->did == RT1715_DID) {
   187				rxdz_en = 1;
   188				rxdz_sel = 1;
   189			} else {
   190				rxdz_en = 1;
   191				rxdz_sel = 0;
   192			}
   193		} else {
   194			rxdz_en = 0;
   195			rxdz_sel = 1;
   196		}
   197	
   198		ret = regmap_update_bits(chip->data.regmap, RT1711H_RTCTRL18,
   199					 BMCIO_RXDZEN_MASK, rxdz_en);
   200		if (ret < 0)
   201			return ret;
   202	
   203		return regmap_update_bits(chip->data.regmap, RT1711H_RTCTRL4,
   204					  RT1711H_BMCIO_RXDZSEL, rxdz_en);
   205	}
   206	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp