[PATCH 0/4] RFC: USB C extcon patchset for the tps6598x

[PATCH 0/4] RFC: USB C extcon patchset for the tps6598x

[Angus Ainslie]

We have a complex set of hardware components to manage our USB C data and
power. For these to work together we decided to use extcon to communicate
the system changes as various cables and devices are plugged in/out. We did
look at usb_roleswitch and the charging framework but thought it would be
preferable to keep all of the information together in one system.

The components we have in the system are:

1) TPS65982 type USB type C controller
2) dwc3 IP in the imx8mq
3) BQ25895 battery charger

I'll break this into 2 parts the data role and the power role.

For the data role the TPS65982 senses connect and disconnect as well as data
source/sink. It is also controlling the USB 3 data lanes. The display port and
USB 3 muxing is handled by a different chip and we'll submit patches for that
later on. The dwc3 controls the USB 2 data lanes.

On the power side there are even more moving pieces. The TPS65982 negotiates
the power delivery contract, the dwc3 senses the BC1.2 charging current and the
BQ25895 sets whether we are sinking or sourcing current and what the current
limit is of the sink and source.

For both the data and power roles no single chip has all of the required
information. Is using extcon the correct way of doing this and if not what
are the alternatives ?

The extcon extensions allow us to communicate the the power roles amongst
the various chips.

This patch series has been tested with the 5.9-rc4 kernel on the Purism
Librem5 HW. Assuming this is the correct way to use extcon there will be
follow on patches to the BQ25895 and dwc3 drivers.

Strictly speaking only the first 3 patches are needed for extcon support, the
forth patch decodes the power delivery contracts which makes use of the extcon
system.


Angus Ainslie (4):
  extcon: Add USB VBUS properties
  usb: typec: tps6589x: register as an extcon provider
  usb: typec: tps6598x: Add the extcon USB chargers
  usb: typec: tps6598x: Add the power delivery irq

 drivers/usb/typec/tps6598x.c | 488 ++++++++++++++++++++++++++++++++++-
 include/linux/extcon.h       |  17 +-
 2 files changed, 503 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH 1/4] extcon: Add USB VBUS properties

[Angus Ainslie]

USB type C, USB BC1.2 and USB power delivery allow different voltages
and currents for VBUS so we need these additional properties.

Also USB type C allows separate device and power roles so add a VBUS SRC
property.

Signed-off-by: Angus Ainslie <angus@akkea.ca>
---
 include/linux/extcon.h | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index fd183fb9c20f..c4d48f4f74c4 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -117,14 +117,29 @@
  * @type:       integer (intval)
  * @value:      0 (USB/USB2) or 1 (USB3)
  * @default:    0 (USB/USB2)
+ * - EXTCON_PROP_USB_VBUS_SRC
+ * @type:	integer (intval)
+ * @value:	0 (sink) or 1 (source)
+ * @default:	0 (sink)
+ * - EXTCON_PROP_USB_VBUS_VOLTAGE
+ * @type:	integer (intval)
+ * @value:	negotiated vbus voltage in mV
+ * @default:	5000
+ * - EXTCON_PROP_USB_VBUS_CURRENT
+ * @type:	integer (intval)
+ * @value:	negotiated vbus current in mA
+ * @default:	100
  *
  */
 #define EXTCON_PROP_USB_VBUS		0
 #define EXTCON_PROP_USB_TYPEC_POLARITY	1
 #define EXTCON_PROP_USB_SS		2
+#define EXTCON_PROP_USB_VBUS_SRC	3
+#define EXTCON_PROP_USB_VBUS_VOLTAGE	4
+#define EXTCON_PROP_USB_VBUS_CURRENT	5
 
 #define EXTCON_PROP_USB_MIN		0
-#define EXTCON_PROP_USB_MAX		2
+#define EXTCON_PROP_USB_MAX		5
 #define EXTCON_PROP_USB_CNT	(EXTCON_PROP_USB_MAX - EXTCON_PROP_USB_MIN + 1)
 
 /* Properties of EXTCON_TYPE_CHG. */
-- 
2.25.1

[PATCH 2/4] usb: typec: tps6589x: register as an extcon provider

[Angus Ainslie]

The tps6598x type C chip can negotiate the VBUS sink/source status as
well as the VBUS voltage and current. Notify the extcon consumers of
these changes.

Signed-off-by: Angus Ainslie <angus@akkea.ca>
---
 drivers/usb/typec/tps6598x.c | 138 +++++++++++++++++++++++++++++++++++
 1 file changed, 138 insertions(+)

diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index 3db33bb622c3..3b06d43c810d 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -8,6 +8,8 @@
 
 #include <linux/i2c.h>
 #include <linux/acpi.h>
+#include <linux/extcon.h>
+#include <linux/extcon-provider.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
 #include <linux/interrupt.h>
@@ -95,7 +97,12 @@ struct tps6598x {
 	struct typec_port *port;
 	struct typec_partner *partner;
 	struct usb_pd_identity partner_identity;
+
 	struct usb_role_switch *role_sw;
+
+#ifdef CONFIG_EXTCON
+	struct extcon_dev *edev;
+#endif
 };
 
 /*
@@ -209,6 +216,62 @@ static void tps6598x_set_data_role(struct tps6598x *tps,
 	typec_set_data_role(tps->port, role);
 }
 
+#ifdef CONFIG_EXTCON
+static void tps6589x_set_extcon_state(struct tps6598x *tps,
+				      u32 status, u16 pwr_status, bool state)
+{
+	union extcon_property_value val;
+	int max_current;
+
+	if (TPS_STATUS_DATAROLE(status)) {
+		extcon_set_state(tps->edev, EXTCON_USB, false);
+		extcon_set_state(tps->edev, EXTCON_USB_HOST, state);
+	} else {
+		extcon_set_state(tps->edev, EXTCON_USB, state);
+		extcon_set_state(tps->edev, EXTCON_USB_HOST, false);
+	}
+
+	val.intval = TPS_STATUS_PORTROLE(status);
+	extcon_set_property(tps->edev, EXTCON_USB_HOST,
+			    EXTCON_PROP_USB_VBUS_SRC,
+			    val);
+
+	extcon_set_property(tps->edev, EXTCON_USB,
+			    EXTCON_PROP_USB_VBUS_SRC,
+			    val);
+
+	switch (TPS_POWER_STATUS_PWROPMODE(pwr_status)) {
+	case TYPEC_PWR_MODE_USB:
+		max_current = 500;
+		extcon_set_state(tps->edev, EXTCON_CHG_USB_SDP, state);
+		extcon_set_state(tps->edev, EXTCON_CHG_USB_SLOW, state);
+		break;
+	case TYPEC_PWR_MODE_1_5A:
+		max_current = 1500;
+		break;
+	case TYPEC_PWR_MODE_3_0A:
+		max_current = 3000;
+		break;
+	case TYPEC_PWR_MODE_PD:
+		max_current = 500;
+		break;
+	}
+
+	val.intval = max_current;
+	extcon_set_property(tps->edev, EXTCON_USB,
+			    EXTCON_PROP_USB_VBUS_CURRENT,
+			    val);
+	extcon_set_property(tps->edev, EXTCON_USB_HOST,
+			    EXTCON_PROP_USB_VBUS_CURRENT,
+			    val);
+
+	extcon_sync(tps->edev, EXTCON_USB);
+	extcon_sync(tps->edev, EXTCON_USB_HOST);
+	extcon_sync(tps->edev, EXTCON_CHG_USB_SDP);
+	extcon_sync(tps->edev, EXTCON_CHG_USB_SLOW);
+}
+#endif
+
 static int tps6598x_connect(struct tps6598x *tps, u32 status)
 {
 	struct typec_partner_desc desc;
@@ -248,18 +311,41 @@ static int tps6598x_connect(struct tps6598x *tps, u32 status)
 	if (desc.identity)
 		typec_partner_set_identity(tps->partner);
 
+#ifdef CONFIG_EXTCON
+	tps6598x_set_extcon_state(tps, status, pwr_status, true);
+#endif
+
 	return 0;
 }
 
 static void tps6598x_disconnect(struct tps6598x *tps, u32 status)
 {
+	enum typec_pwr_opmode mode;
+	u16 pwr_status;
+	int ret;
+
 	if (!IS_ERR(tps->partner))
 		typec_unregister_partner(tps->partner);
 	tps->partner = NULL;
 	typec_set_pwr_opmode(tps->port, TYPEC_PWR_MODE_USB);
 	typec_set_pwr_role(tps->port, TPS_STATUS_PORTROLE(status));
 	typec_set_vconn_role(tps->port, TPS_STATUS_VCONN(status));
+	typec_set_data_role(tps->port, TPS_STATUS_DATAROLE(status));
 	tps6598x_set_data_role(tps, TPS_STATUS_DATAROLE(status), false);
+
+	ret = tps6598x_read16(tps, TPS_REG_POWER_STATUS, &pwr_status);
+	if (ret < 0)
+		return;
+
+	mode = TPS_POWER_STATUS_PWROPMODE(pwr_status);
+
+	dev_dbg(tps->dev, "%s: mode 0x%x pwr_role %d vconn_role %d data_role %d\n",
+		__func__, mode, TPS_STATUS_PORTROLE(status),
+		TPS_STATUS_VCONN(status), TPS_STATUS_DATAROLE(status));
+
+#ifdef CONFIG_EXTCON
+	tps6598x_set_extcon_state(tps, status, 0, false);
+#endif
 }
 
 static int tps6598x_exec_cmd(struct tps6598x *tps, const char *cmd,
@@ -407,6 +493,9 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
 		goto err_unlock;
 	}
 
+	dev_dbg(tps->dev, "Received irq event: 0x%llx: 0x%x 0x%x", event1,
+	       (u32)((event1 & 0xFFFFFFFF00000000) >> 32), (u32)(event1 & 0xFFFFFFFF));
+
 	ret = tps6598x_read32(tps, TPS_REG_STATUS, &status);
 	if (ret) {
 		dev_err(tps->dev, "%s: failed to read status\n", __func__);
@@ -467,6 +556,18 @@ static const struct regmap_config tps6598x_regmap_config = {
 	.max_register = 0x7F,
 };
 
+#ifdef CONFIG_EXTCON
+/* List of detectable cables */
+static const unsigned int tps6598x_extcon_cable[] = {
+	EXTCON_USB,
+	EXTCON_USB_HOST,
+	EXTCON_CHG_USB_SDP,
+	EXTCON_CHG_USB_SLOW,
+	EXTCON_CHG_USB_FAST,
+	EXTCON_NONE,
+};
+#endif
+
 static int tps6598x_probe(struct i2c_client *client)
 {
 	struct typec_capability typec_cap = { };
@@ -567,10 +668,47 @@ static int tps6598x_probe(struct i2c_client *client)
 	}
 	fwnode_handle_put(fwnode);
 
+#ifdef CONFIG_EXTCON
+	/* Allocate extcon device */
+	tps->edev = devm_extcon_dev_allocate(tps->dev, tps6598x_extcon_cable);
+	if (IS_ERR(tps->edev)) {
+		dev_err(tps->dev, "failed to allocate memory for extcon\n");
+		typec_unregister_port(tps->port);
+		return -ENOMEM;
+	}
+
+	/* Register extcon device */
+	ret = devm_extcon_dev_register(tps->dev, tps->edev);
+	if (ret) {
+		dev_err(tps->dev, "failed to register extcon device\n");
+		typec_unregister_port(tps->port);
+		return ret;
+	}
+
+	extcon_set_property_capability(tps->edev, EXTCON_USB,
+				       EXTCON_PROP_USB_VBUS);
+	extcon_set_property_capability(tps->edev, EXTCON_USB,
+				       EXTCON_PROP_USB_VBUS_SRC);
+	extcon_set_property_capability(tps->edev, EXTCON_USB,
+				       EXTCON_PROP_USB_VBUS_VOLTAGE);
+	extcon_set_property_capability(tps->edev, EXTCON_USB,
+				       EXTCON_PROP_USB_VBUS_CURRENT);
+	extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
+				       EXTCON_PROP_USB_VBUS);
+	extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
+				       EXTCON_PROP_USB_VBUS_SRC);
+	extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
+				       EXTCON_PROP_USB_VBUS_VOLTAGE);
+	extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
+				       EXTCON_PROP_USB_VBUS_CURRENT);
+#endif
+
 	if (status & TPS_STATUS_PLUG_PRESENT) {
 		ret = tps6598x_connect(tps, status);
 		if (ret)
 			dev_err(&client->dev, "failed to register partner\n");
+	} else {
+		tps6598x_disconnect(tps, status);
 	}
 
 	ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
-- 
2.25.1

[PATCH 3/4] usb: typec: tps6598x: Add the extcon USB chargers

[Angus Ainslie]

Notify extcon devcies that the charge current has changed

Signed-off-by: Angus Ainslie <angus@akkea.ca>
---
 drivers/usb/typec/tps6598x.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index 3b06d43c810d..d5aa58c9da0a 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -240,17 +240,25 @@ static void tps6589x_set_extcon_state(struct tps6598x *tps,
 			    EXTCON_PROP_USB_VBUS_SRC,
 			    val);
 
+	extcon_set_state(tps->edev, EXTCON_CHG_USB_SDP, 0);
+	extcon_set_state(tps->edev, EXTCON_CHG_USB_DCP, 0);
+	extcon_set_state(tps->edev, EXTCON_CHG_USB_CDP, 0);
+	extcon_set_state(tps->edev, EXTCON_CHG_USB_SLOW, 0);
+	extcon_set_state(tps->edev, EXTCON_CHG_USB_FAST, 0);
+
 	switch (TPS_POWER_STATUS_PWROPMODE(pwr_status)) {
 	case TYPEC_PWR_MODE_USB:
-		max_current = 500;
+		max_current = val.intval = 500;
 		extcon_set_state(tps->edev, EXTCON_CHG_USB_SDP, state);
 		extcon_set_state(tps->edev, EXTCON_CHG_USB_SLOW, state);
 		break;
 	case TYPEC_PWR_MODE_1_5A:
-		max_current = 1500;
+		max_current = val.intval = 1500;
+		extcon_set_state(tps->edev, EXTCON_CHG_USB_CDP, state);
 		break;
 	case TYPEC_PWR_MODE_3_0A:
-		max_current = 3000;
+		max_current = val.intval = 3000;
+		extcon_set_state(tps->edev, EXTCON_CHG_USB_CDP, state);
 		break;
 	case TYPEC_PWR_MODE_PD:
 		max_current = 500;
@@ -265,10 +273,14 @@ static void tps6589x_set_extcon_state(struct tps6598x *tps,
 			    EXTCON_PROP_USB_VBUS_CURRENT,
 			    val);
 
-	extcon_sync(tps->edev, EXTCON_USB);
-	extcon_sync(tps->edev, EXTCON_USB_HOST);
 	extcon_sync(tps->edev, EXTCON_CHG_USB_SDP);
+	extcon_sync(tps->edev, EXTCON_CHG_USB_CDP);
+	extcon_sync(tps->edev, EXTCON_CHG_USB_DCP);
 	extcon_sync(tps->edev, EXTCON_CHG_USB_SLOW);
+	extcon_sync(tps->edev, EXTCON_CHG_USB_FAST);
+	/* do these last for extcon notifiers */
+	extcon_sync(tps->edev, EXTCON_USB);
+	extcon_sync(tps->edev, EXTCON_USB_HOST);
 }
 #endif
 
@@ -562,6 +574,8 @@ static const unsigned int tps6598x_extcon_cable[] = {
 	EXTCON_USB,
 	EXTCON_USB_HOST,
 	EXTCON_CHG_USB_SDP,
+	EXTCON_CHG_USB_CDP,
+	EXTCON_CHG_USB_DCP,
 	EXTCON_CHG_USB_SLOW,
 	EXTCON_CHG_USB_FAST,
 	EXTCON_NONE,
-- 
2.25.1

[PATCH 4/4] usb: typec: tps6598x: Add the power delivery irq

[Angus Ainslie]

Enable the PD irq and decode the contract.

Signed-off-by: Angus Ainslie <angus@akkea.ca>
---
 drivers/usb/typec/tps6598x.c | 386 ++++++++++++++++++++++++++++++++---
 1 file changed, 360 insertions(+), 26 deletions(-)

diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index d5aa58c9da0a..2eb883ff822c 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -30,11 +30,19 @@
 #define TPS_REG_STATUS			0x1a
 #define TPS_REG_SYSTEM_CONF		0x28
 #define TPS_REG_CTRL_CONF		0x29
+#define TPS_REG_ACTIVE_CONTRACT_PDO	0x34
+#define TPS_REG_ACTIVE_CONTRACT_RDO	0x35
 #define TPS_REG_POWER_STATUS		0x3f
 #define TPS_REG_RX_IDENTITY_SOP		0x48
+#define TPS_REG_DATA_STATUS		0x5f
 
 /* TPS_REG_INT_* bits */
 #define TPS_REG_INT_PLUG_EVENT		BIT(3)
+#define TPS_REG_INT_NEW_CONTRACT_SNK	BIT(12)
+#define TPS_REG_INT_PWR_STATUS_UPDATE	BIT(24)
+#define TPS_REG_INT_DATA_STATUS_UPDATE	BIT(25)
+#define TPS_REG_INT_STATUS_UPDATE	BIT(26)
+#define TPS_REG_INT_PD_STATUS_UPDATE	BIT(27)
 
 /* TPS_REG_STATUS bits */
 #define TPS_STATUS_PLUG_PRESENT		BIT(0)
@@ -57,8 +65,16 @@ enum {
 };
 
 /* TPS_REG_POWER_STATUS bits */
+#define TPS_POWER_STATUS_CONNECTION	BIT(0)
 #define TPS_POWER_STATUS_SOURCESINK	BIT(1)
+#define TPS_POWER_STATUS_BC12_CON	BIT(4)
+#define TPS_POWER_STATUS_BC_SDP		0
+#define TPS_POWER_STATUS_BC_CDP		2
+#define TPS_POWER_STATUS_BC_DCP		3
+
+#define TPS_POWER_STATUS_BCOPMODE(p)	(((p) & GENMASK(6, 5)) >> 5)
 #define TPS_POWER_STATUS_PWROPMODE(p)	(((p) & GENMASK(3, 2)) >> 2)
+#define TPS_POWER_STATUS_SRC(p)		(!(((p) & BIT(1)) >> 1))
 
 /* TPS_REG_RX_IDENTITY_SOP */
 struct tps6598x_rx_identity_reg {
@@ -217,21 +233,237 @@ static void tps6598x_set_data_role(struct tps6598x *tps,
 }
 
 #ifdef CONFIG_EXTCON
-static void tps6589x_set_extcon_state(struct tps6598x *tps,
-				      u32 status, u16 pwr_status, bool state)
+
+/*
+ *   the PDO decoding comes from here
+ *   https://www.ti.com/lit/an/slva842/slva842.pdf
+ */
+
+struct contract_terms {
+	u8 type;
+	bool dr_power;
+	bool dr_data;
+	bool higher_cap;
+	bool ext_powered;
+	bool usb_comms;
+	int max_voltage;
+	int max_current;
+	int max_power;
+};
+
+#define PDO_CONTRACT_TYPE(x)	(x >> 30 & 0x3)
+#define PDO_CONTRACT_FIXED	0
+#define PDO_CONTRACT_BATTERY	1
+#define PDO_CONTRACT_VARIABLE	2
+
+#define PDO_CONTRACT_DR_POWER		BIT(29)
+#define PDO_CONTRACT_HIGHER_CAP		BIT(28)
+#define PDO_CONTRACT_EXTERNAL_PWR	BIT(27)
+#define PDO_CONTRACT_USB_COMMS		BIT(26)
+#define PDO_CONTRACT_DR_DATA		BIT(25)
+
+#define PDO_CONTRACT_VOLTAGE(x)		((x >> 10 & 0x3ff) * 50)
+#define PDO_CONTRACT_MIN_VOLTAGE(x)	PDO_CONTRACT_VOLTAGE(x)
+#define PDO_CONTRACT_MAX_VOLTAGE(x)	((x >> 20 & 0x3ff) * 50)
+#define PDO_CONTRACT_CURRENT(x)		((x & 0x3ff) * 10)
+#define PDO_CONTRACT_POWER(x)		((x & 0x3ff) * 250)
+
+static int tps6598x_decode_pdo_contract(struct tps6598x *tps, u32 contract,
+					struct contract_terms *terms)
+{
+	int min_voltage = 5000; // mV
+
+	memset(terms, 0, sizeof(struct contract_terms));
+
+	dev_dbg(tps->dev, "%s 0x%x\n", __func__, contract);
+
+	switch (PDO_CONTRACT_TYPE(contract)) {
+	case PDO_CONTRACT_FIXED:
+		terms->type = PDO_CONTRACT_FIXED;
+		if (contract & PDO_CONTRACT_DR_POWER) {
+			dev_dbg(tps->dev, "Dual role power\n");
+			terms->dr_power = true;
+		}
+		if (contract & PDO_CONTRACT_DR_DATA) {
+			dev_dbg(tps->dev, "Dual role data\n");
+			terms->dr_data = true;
+		}
+		if (contract & PDO_CONTRACT_HIGHER_CAP) {
+			dev_dbg(tps->dev, "Higher capbility\n");
+			terms->higher_cap = true;
+		}
+		if (contract & PDO_CONTRACT_EXTERNAL_PWR) {
+			dev_dbg(tps->dev, "Externally powered\n");
+			terms->ext_powered = true;
+		}
+		if (contract & PDO_CONTRACT_USB_COMMS) {
+			dev_dbg(tps->dev, "USB communications capable\n");
+			terms->usb_comms = true;
+		}
+		terms->max_voltage = PDO_CONTRACT_VOLTAGE(contract);
+		terms->max_current = PDO_CONTRACT_CURRENT(contract);
+		terms->max_power = (terms->max_voltage * terms->max_current) / 1000;
+		dev_dbg(tps->dev, "Fixed contract uVMIN:uVMAX:uA:uW %d:%d:%d:%d\n",
+			min_voltage, terms->max_voltage, terms->max_current,
+			terms->max_power);
+		break;
+	case PDO_CONTRACT_BATTERY:
+		min_voltage = PDO_CONTRACT_MIN_VOLTAGE(contract);
+		terms->max_voltage = PDO_CONTRACT_MAX_VOLTAGE(contract);
+		terms->max_power = PDO_CONTRACT_POWER(contract);
+		terms->max_current = terms->max_power / (terms->max_voltage / 1000);
+		dev_dbg(tps->dev, "Battery contract uVMIN:uVMAX:uA:uW %d:%d:%d:%d\n",
+			min_voltage, terms->max_voltage, terms->max_current,
+			terms->max_power);
+		break;
+	case PDO_CONTRACT_VARIABLE:
+		min_voltage = PDO_CONTRACT_MIN_VOLTAGE(contract);
+		terms->max_voltage = PDO_CONTRACT_MAX_VOLTAGE(contract);
+		terms->max_current = PDO_CONTRACT_CURRENT(contract);
+		terms->max_power = (terms->max_voltage * terms->max_current) / 1000;
+		dev_dbg(tps->dev, "Variable contract uVMIN:uVMAX:uA:uW %d:%d:%d:%d\n",
+			min_voltage, terms->max_voltage, terms->max_current,
+			terms->max_power);
+		break;
+	default:
+		dev_warn(tps->dev, "Unknown supply\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int tps6598x_pdo_contract(struct tps6598x *tps, u32 contract)
 {
 	union extcon_property_value val;
-	int max_current;
+	struct contract_terms terms;
 
-	if (TPS_STATUS_DATAROLE(status)) {
-		extcon_set_state(tps->edev, EXTCON_USB, false);
-		extcon_set_state(tps->edev, EXTCON_USB_HOST, state);
+	tps6598x_decode_pdo_contract(tps, contract, &terms);
+
+	val.intval = terms.max_current;
+	extcon_set_property(tps->edev, EXTCON_USB,
+			    EXTCON_PROP_USB_VBUS_CURRENT,
+			    val);
+	extcon_set_property(tps->edev, EXTCON_USB_HOST,
+			    EXTCON_PROP_USB_VBUS_CURRENT,
+			    val);
+
+	val.intval = terms.max_voltage;
+	extcon_set_property(tps->edev, EXTCON_USB,
+			    EXTCON_PROP_USB_VBUS_VOLTAGE,
+			    val);
+	extcon_set_property(tps->edev, EXTCON_USB_HOST,
+			    EXTCON_PROP_USB_VBUS_VOLTAGE,
+			    val);
+
+	return terms.max_current;
+}
+
+#define RDO_CONTRACT_OBJ_POSITION		BIT(29)
+#define RDO_CONTRACT_CAPABILITY_MISMATCH	BIT(27)
+#define RDO_CONTRACT_USB_COMMS			BIT(26)
+#define RDO_CONTRACT_NO_SUSPEND			BIT(25)
+
+#define RDO_CONTRACT_CURRENT(x)		((x >> 10 & 0x3ff) * 10)
+#define RDO_CONTRACT_POWER(x)		((x >> 10 & 0x3ff) * 250)
+#define RDO_CONTRACT_MAX_CURRENT(x)	((x & 0x3ff) * 10)
+#define RDO_CONTRACT_MAX_POWER(x)	((x & 0x3ff) * 250)
+
+static int tps6598x_decode_rdo_contract(struct tps6598x *tps, u32 contract, bool battery,
+					struct contract_terms *terms)
+{
+	memset(terms, 0, sizeof(struct contract_terms));
+
+	if (contract & RDO_CONTRACT_OBJ_POSITION)
+		dev_dbg(tps->dev, "Object position\n");
+	if (contract & RDO_CONTRACT_CAPABILITY_MISMATCH)
+		dev_dbg(tps->dev, "Capbility mismatch\n");
+	if (contract & RDO_CONTRACT_USB_COMMS)
+		dev_dbg(tps->dev, "USB communications capable\n");
+	if (contract & RDO_CONTRACT_NO_SUSPEND)
+		dev_dbg(tps->dev, "No USB suspend\n");
+	if (battery) {
+		terms->max_power = RDO_CONTRACT_MAX_POWER(contract);
+		dev_dbg(tps->dev, "Power %d max power %d\n", RDO_CONTRACT_POWER(contract),
+			terms->max_power);
 	} else {
-		extcon_set_state(tps->edev, EXTCON_USB, state);
-		extcon_set_state(tps->edev, EXTCON_USB_HOST, false);
+		terms->max_current = RDO_CONTRACT_MAX_CURRENT(contract); /* mA */
+		dev_dbg(tps->dev, "Current %d max current %d\n", RDO_CONTRACT_CURRENT(contract),
+			terms->max_current);
 	}
 
-	val.intval = TPS_STATUS_PORTROLE(status);
+	return 0;
+}
+
+static int tps6598x_rdo_contract(struct tps6598x *tps, u32 contract, bool battery)
+{
+	union extcon_property_value val;
+	struct contract_terms terms;
+
+	if (battery)
+		dev_dbg(tps->dev, "Battery supply\n");
+
+	tps6598x_decode_rdo_contract(tps, contract, battery, &terms);
+
+	if (terms.max_current > 0) {
+		val.intval = terms.max_current;
+		extcon_set_property(tps->edev, EXTCON_USB,
+				    EXTCON_PROP_USB_VBUS_CURRENT,
+				    val);
+		extcon_set_property(tps->edev, EXTCON_USB_HOST,
+				    EXTCON_PROP_USB_VBUS_CURRENT,
+				    val);
+
+	}
+
+	return terms.max_current;
+}
+
+static int tps6598x_set_extcon_pd_state(struct tps6598x *tps, bool state)
+{
+	u32 pdo_contract, rdo_contract;
+	int ret, max_current;
+
+	ret = tps6598x_read32(tps,
+			      TPS_REG_ACTIVE_CONTRACT_RDO,
+			      &rdo_contract);
+	if (ret) {
+		dev_dbg(tps->dev,
+			"failed to read RDO contract - try PDO contract\n");
+		rdo_contract = 0;
+	}
+
+	if (rdo_contract != 0) {
+		max_current = tps6598x_rdo_contract(tps, rdo_contract, false);
+	} else {
+		ret = tps6598x_read32(tps,
+				      TPS_REG_ACTIVE_CONTRACT_PDO,
+				      &pdo_contract);
+		max_current = tps6598x_pdo_contract(tps, pdo_contract);
+		if (ret || pdo_contract == 0)
+			dev_err(tps->dev, "failed to read PDO contract\n");
+	}
+
+	extcon_set_state(tps->edev, EXTCON_CHG_USB_PD, state);
+	extcon_sync(tps->edev, EXTCON_CHG_USB_PD);
+
+	return max_current;
+}
+
+static void tps6598x_set_extcon_power_state(struct tps6598x *tps, u32 status,
+					    u16 pwr_status)
+{
+	int max_current = 100;
+	union extcon_property_value val;
+	u8 connected;
+
+	/* USB C power source */
+	val.intval = TPS_POWER_STATUS_SRC(pwr_status) &&
+		TPS_STATUS_PORTROLE(status);
+
+	dev_dbg(tps->dev, "Power status 0x%x Source %d\n", pwr_status,
+		val.intval);
+
 	extcon_set_property(tps->edev, EXTCON_USB_HOST,
 			    EXTCON_PROP_USB_VBUS_SRC,
 			    val);
@@ -240,28 +472,55 @@ static void tps6589x_set_extcon_state(struct tps6598x *tps,
 			    EXTCON_PROP_USB_VBUS_SRC,
 			    val);
 
-	extcon_set_state(tps->edev, EXTCON_CHG_USB_SDP, 0);
-	extcon_set_state(tps->edev, EXTCON_CHG_USB_DCP, 0);
-	extcon_set_state(tps->edev, EXTCON_CHG_USB_CDP, 0);
-	extcon_set_state(tps->edev, EXTCON_CHG_USB_SLOW, 0);
-	extcon_set_state(tps->edev, EXTCON_CHG_USB_FAST, 0);
+	connected = pwr_status & TPS_POWER_STATUS_CONNECTION;
+
+	if (pwr_status & TPS_POWER_STATUS_BC12_CON) {
+		switch (TPS_POWER_STATUS_BCOPMODE(pwr_status)) {
+		case TPS_POWER_STATUS_BC_SDP:
+			/* TODO: detect USB3 900mA */
+			extcon_set_state(tps->edev,
+					 EXTCON_CHG_USB_SLOW, connected);
+			dev_dbg(tps->dev, "BC1.2 SDP detected\n");
+			extcon_set_state(tps->edev,
+					 EXTCON_CHG_USB_SDP, connected);
+			break;
+		case TPS_POWER_STATUS_BC_CDP:
+			dev_dbg(tps->dev, "BC1.2 CDP detected\n");
+			extcon_set_state(tps->edev,
+					 EXTCON_CHG_USB_CDP, connected);
+			break;
+		case TPS_POWER_STATUS_BC_DCP:
+			dev_dbg(tps->dev, "BC1.2 DCP detected\n");
+			extcon_set_state(tps->edev,
+					 EXTCON_CHG_USB_DCP, connected);
+			break;
+		default:
+			dev_dbg(tps->dev, "BC1.2 unknown - reserved\n");
+			break;
+
+		}
+	}
 
 	switch (TPS_POWER_STATUS_PWROPMODE(pwr_status)) {
 	case TYPEC_PWR_MODE_USB:
-		max_current = val.intval = 500;
-		extcon_set_state(tps->edev, EXTCON_CHG_USB_SDP, state);
-		extcon_set_state(tps->edev, EXTCON_CHG_USB_SLOW, state);
+		if (max_current < 500)
+			max_current = 500;
+		/* TODO: detect UBS3 900mA */
+		extcon_set_state(tps->edev, EXTCON_CHG_USB_SLOW, connected);
+		extcon_set_state(tps->edev, EXTCON_CHG_USB_SDP, connected);
 		break;
 	case TYPEC_PWR_MODE_1_5A:
-		max_current = val.intval = 1500;
-		extcon_set_state(tps->edev, EXTCON_CHG_USB_CDP, state);
+		if (max_current < 1500)
+			max_current = 1500;
+		extcon_set_state(tps->edev, EXTCON_CHG_USB_CDP, connected);
 		break;
 	case TYPEC_PWR_MODE_3_0A:
-		max_current = val.intval = 3000;
-		extcon_set_state(tps->edev, EXTCON_CHG_USB_CDP, state);
+		if (max_current < 3000)
+			max_current = 3000;
 		break;
 	case TYPEC_PWR_MODE_PD:
-		max_current = 500;
+		extcon_set_state(tps->edev, EXTCON_CHG_USB_PD, connected);
+		max_current = tps6598x_set_extcon_pd_state(tps, connected);
 		break;
 	}
 
@@ -272,6 +531,38 @@ static void tps6589x_set_extcon_state(struct tps6598x *tps,
 	extcon_set_property(tps->edev, EXTCON_USB_HOST,
 			    EXTCON_PROP_USB_VBUS_CURRENT,
 			    val);
+}
+
+static void tps6598x_set_extcon_state(struct tps6598x *tps,
+				      u32 status, u16 pwr_status, bool state)
+{
+	union extcon_property_value val;
+
+	if (TPS_STATUS_DATAROLE(status)) {
+		extcon_set_state(tps->edev, EXTCON_USB, false);
+		extcon_set_state(tps->edev, EXTCON_USB_HOST, state);
+	} else {
+		extcon_set_state(tps->edev, EXTCON_USB, state);
+		extcon_set_state(tps->edev, EXTCON_USB_HOST, false);
+	}
+
+	val.intval = TPS_STATUS_PORTROLE(status);
+	extcon_set_property(tps->edev, EXTCON_USB_HOST,
+			    EXTCON_PROP_USB_VBUS,
+			    val);
+
+	extcon_set_property(tps->edev, EXTCON_USB,
+			    EXTCON_PROP_USB_VBUS,
+			    val);
+
+	extcon_set_state(tps->edev, EXTCON_CHG_USB_SDP, 0);
+	extcon_set_state(tps->edev, EXTCON_CHG_USB_DCP, 0);
+	extcon_set_state(tps->edev, EXTCON_CHG_USB_CDP, 0);
+	extcon_set_state(tps->edev, EXTCON_CHG_USB_SLOW, 0);
+	extcon_set_state(tps->edev, EXTCON_CHG_USB_FAST, 0);
+	extcon_set_state(tps->edev, EXTCON_CHG_USB_PD, 0);
+
+	tps6598x_set_extcon_power_state(tps, status, pwr_status);
 
 	extcon_sync(tps->edev, EXTCON_CHG_USB_SDP);
 	extcon_sync(tps->edev, EXTCON_CHG_USB_CDP);
@@ -323,9 +614,9 @@ static int tps6598x_connect(struct tps6598x *tps, u32 status)
 	if (desc.identity)
 		typec_partner_set_identity(tps->partner);
 
-#ifdef CONFIG_EXTCON
-	tps6598x_set_extcon_state(tps, status, pwr_status, true);
-#endif
+	dev_dbg(tps->dev, "%s: mode 0x%x pwr_role %d vconn_role %d data_role %d\n",
+		__func__, mode, TPS_STATUS_PORTROLE(status),
+		TPS_STATUS_VCONN(status), TPS_STATUS_DATAROLE(status));
 
 	return 0;
 }
@@ -494,7 +785,12 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
 	u64 event1;
 	u64 event2;
 	u32 status;
+	u32 data_status;
+	u16 power_status;
 	int ret;
+#ifdef CONFIG_EXTCON
+	bool connected;
+#endif
 
 	mutex_lock(&tps->lock);
 
@@ -514,8 +810,20 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
 		goto err_clear_ints;
 	}
 
+	connected = status & TPS_STATUS_PLUG_PRESENT;
+	ret = tps6598x_read16(tps, TPS_REG_POWER_STATUS, &power_status);
+	if (ret < 0)
+		goto err_clear_ints;
+
+	ret = tps6598x_read32(tps, TPS_REG_DATA_STATUS, &data_status);
+	if (ret < 0)
+		goto err_clear_ints;
+
+	dev_dbg(tps->dev, "Status: 0x%x data: 0x%x power: 0x%x", status,
+		data_status, power_status);
+
 	/* Handle plug insert or removal */
-	if ((event1 | event2) & TPS_REG_INT_PLUG_EVENT) {
+	if ((event1 | event2) & (TPS_REG_INT_PLUG_EVENT)) {
 		if (status & TPS_STATUS_PLUG_PRESENT) {
 			ret = tps6598x_connect(tps, status);
 			if (ret)
@@ -526,6 +834,19 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
 		}
 	}
 
+#ifdef CONFIG_EXTCON
+	/* updated power or data status register */
+	/* power bits sometimes change without triggering an interrupt */
+	if ((event1 | event2) & (TPS_REG_INT_STATUS_UPDATE |
+				 TPS_REG_INT_PWR_STATUS_UPDATE |
+				 TPS_REG_INT_DATA_STATUS_UPDATE |
+				 TPS_REG_INT_NEW_CONTRACT_SNK |
+				 TPS_REG_INT_PD_STATUS_UPDATE)) {
+		dev_dbg(tps->dev, "%s: update extcon\n", __func__);
+		tps6598x_set_extcon_state(tps, status, power_status, connected);
+	}
+#endif
+
 err_clear_ints:
 	tps6598x_write64(tps, TPS_REG_INT_CLEAR1, event1);
 	tps6598x_write64(tps, TPS_REG_INT_CLEAR2, event2);
@@ -591,6 +912,7 @@ static int tps6598x_probe(struct i2c_client *client)
 	u32 conf;
 	u32 vid;
 	int ret;
+	u64 mask;
 
 	tps = devm_kzalloc(&client->dev, sizeof(*tps), GFP_KERNEL);
 	if (!tps)
@@ -735,6 +1057,18 @@ static int tps6598x_probe(struct i2c_client *client)
 		goto err_role_put;
 	}
 
+	mask = TPS_REG_INT_NEW_CONTRACT_SNK
+		| TPS_REG_INT_PLUG_EVENT
+		| TPS_REG_INT_PWR_STATUS_UPDATE
+		| TPS_REG_INT_DATA_STATUS_UPDATE
+		| TPS_REG_INT_STATUS_UPDATE
+		| TPS_REG_INT_PD_STATUS_UPDATE;
+
+	ret = tps6598x_write64(tps, TPS_REG_INT_MASK1, mask);
+	ret |= tps6598x_write64(tps, TPS_REG_INT_MASK2, mask);
+	if (ret < 0)
+		dev_err(&client->dev, "failed to set irq mask %d\n", ret);
+
 	i2c_set_clientdata(client, tps);
 
 	return 0;
-- 
2.25.1

Re: [PATCH 2/4] usb: typec: tps6589x: register as an extcon provider

[Chanwoo Choi]

Hi,

On 9/15/20 1:46 AM, Angus Ainslie wrote:
> The tps6598x type C chip can negotiate the VBUS sink/source status as
> well as the VBUS voltage and current. Notify the extcon consumers of
> these changes.
> 
> Signed-off-by: Angus Ainslie <angus@akkea.ca>
> ---
>  drivers/usb/typec/tps6598x.c | 138 +++++++++++++++++++++++++++++++++++
>  1 file changed, 138 insertions(+)
> 
> diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
> index 3db33bb622c3..3b06d43c810d 100644
> --- a/drivers/usb/typec/tps6598x.c
> +++ b/drivers/usb/typec/tps6598x.c
> @@ -8,6 +8,8 @@
>  
>  #include <linux/i2c.h>
>  #include <linux/acpi.h>
> +#include <linux/extcon.h>
> +#include <linux/extcon-provider.h>
>  #include <linux/module.h>
>  #include <linux/regmap.h>
>  #include <linux/interrupt.h>
> @@ -95,7 +97,12 @@ struct tps6598x {
>  	struct typec_port *port;
>  	struct typec_partner *partner;
>  	struct usb_pd_identity partner_identity;
> +
>  	struct usb_role_switch *role_sw;
> +
> +#ifdef CONFIG_EXTCON

Is it necessary of 'ifdef CONFIG_EXTCON'?
If you just add 'select EXTCON' for this driver,
you don't need to check CONFIG_EXTCON.

If any device driver need some framework, 
we can add the 'select EXTCON'.

> +	struct extcon_dev *edev;
> +#endif
>  };
>  
>  /*
> @@ -209,6 +216,62 @@ static void tps6598x_set_data_role(struct tps6598x *tps,
>  	typec_set_data_role(tps->port, role);
>  }
>  
> +#ifdef CONFIG_EXTCON
> +static void tps6589x_set_extcon_state(struct tps6598x *tps,
> +				      u32 status, u16 pwr_status, bool state)
> +{
> +	union extcon_property_value val;
> +	int max_current;
> +
> +	if (TPS_STATUS_DATAROLE(status)) {
> +		extcon_set_state(tps->edev, EXTCON_USB, false);
> +		extcon_set_state(tps->edev, EXTCON_USB_HOST, state);
> +	} else {
> +		extcon_set_state(tps->edev, EXTCON_USB, state);
> +		extcon_set_state(tps->edev, EXTCON_USB_HOST, false);
> +	}
> +
> +	val.intval = TPS_STATUS_PORTROLE(status);
> +	extcon_set_property(tps->edev, EXTCON_USB_HOST,
> +			    EXTCON_PROP_USB_VBUS_SRC,
> +			    val);
> +
> +	extcon_set_property(tps->edev, EXTCON_USB,
> +			    EXTCON_PROP_USB_VBUS_SRC,
> +			    val);
> +
> +	switch (TPS_POWER_STATUS_PWROPMODE(pwr_status)) {
> +	case TYPEC_PWR_MODE_USB:
> +		max_current = 500;
> +		extcon_set_state(tps->edev, EXTCON_CHG_USB_SDP, state);
> +		extcon_set_state(tps->edev, EXTCON_CHG_USB_SLOW, state);
> +		break;
> +	case TYPEC_PWR_MODE_1_5A:
> +		max_current = 1500;
> +		break;
> +	case TYPEC_PWR_MODE_3_0A:
> +		max_current = 3000;
> +		break;
> +	case TYPEC_PWR_MODE_PD:
> +		max_current = 500;
> +		break;
> +	}
> +
> +	val.intval = max_current;
> +	extcon_set_property(tps->edev, EXTCON_USB,
> +			    EXTCON_PROP_USB_VBUS_CURRENT,
> +			    val);
> +	extcon_set_property(tps->edev, EXTCON_USB_HOST,
> +			    EXTCON_PROP_USB_VBUS_CURRENT,
> +			    val);
> +
> +	extcon_sync(tps->edev, EXTCON_USB);
> +	extcon_sync(tps->edev, EXTCON_USB_HOST);
> +	extcon_sync(tps->edev, EXTCON_CHG_USB_SDP);
> +	extcon_sync(tps->edev, EXTCON_CHG_USB_SLOW);
> +}
> +#endif
> +
>  static int tps6598x_connect(struct tps6598x *tps, u32 status)
>  {
>  	struct typec_partner_desc desc;
> @@ -248,18 +311,41 @@ static int tps6598x_connect(struct tps6598x *tps, u32 status)
>  	if (desc.identity)
>  		typec_partner_set_identity(tps->partner);
>  
> +#ifdef CONFIG_EXTCON
> +	tps6598x_set_extcon_state(tps, status, pwr_status, true);
> +#endif
> +
>  	return 0;
>  }
>  
>  static void tps6598x_disconnect(struct tps6598x *tps, u32 status)
>  {
> +	enum typec_pwr_opmode mode;
> +	u16 pwr_status;
> +	int ret;
> +
>  	if (!IS_ERR(tps->partner))
>  		typec_unregister_partner(tps->partner);
>  	tps->partner = NULL;
>  	typec_set_pwr_opmode(tps->port, TYPEC_PWR_MODE_USB);
>  	typec_set_pwr_role(tps->port, TPS_STATUS_PORTROLE(status));
>  	typec_set_vconn_role(tps->port, TPS_STATUS_VCONN(status));
> +	typec_set_data_role(tps->port, TPS_STATUS_DATAROLE(status));
>  	tps6598x_set_data_role(tps, TPS_STATUS_DATAROLE(status), false);
> +
> +	ret = tps6598x_read16(tps, TPS_REG_POWER_STATUS, &pwr_status);
> +	if (ret < 0)
> +		return;
> +
> +	mode = TPS_POWER_STATUS_PWROPMODE(pwr_status);
> +
> +	dev_dbg(tps->dev, "%s: mode 0x%x pwr_role %d vconn_role %d data_role %d\n",
> +		__func__, mode, TPS_STATUS_PORTROLE(status),
> +		TPS_STATUS_VCONN(status), TPS_STATUS_DATAROLE(status));
> +
> +#ifdef CONFIG_EXTCON
> +	tps6598x_set_extcon_state(tps, status, 0, false);
> +#endif
>  }
>  
>  static int tps6598x_exec_cmd(struct tps6598x *tps, const char *cmd,
> @@ -407,6 +493,9 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
>  		goto err_unlock;
>  	}
>  
> +	dev_dbg(tps->dev, "Received irq event: 0x%llx: 0x%x 0x%x", event1,
> +	       (u32)((event1 & 0xFFFFFFFF00000000) >> 32), (u32)(event1 & 0xFFFFFFFF));
> +
>  	ret = tps6598x_read32(tps, TPS_REG_STATUS, &status);
>  	if (ret) {
>  		dev_err(tps->dev, "%s: failed to read status\n", __func__);
> @@ -467,6 +556,18 @@ static const struct regmap_config tps6598x_regmap_config = {
>  	.max_register = 0x7F,
>  };
>  
> +#ifdef CONFIG_EXTCON
> +/* List of detectable cables */
> +static const unsigned int tps6598x_extcon_cable[] = {
> +	EXTCON_USB,
> +	EXTCON_USB_HOST,
> +	EXTCON_CHG_USB_SDP,
> +	EXTCON_CHG_USB_SLOW,
> +	EXTCON_CHG_USB_FAST,
> +	EXTCON_NONE,
> +};
> +#endif
> +
>  static int tps6598x_probe(struct i2c_client *client)
>  {
>  	struct typec_capability typec_cap = { };
> @@ -567,10 +668,47 @@ static int tps6598x_probe(struct i2c_client *client)
>  	}
>  	fwnode_handle_put(fwnode);
>  
> +#ifdef CONFIG_EXTCON
> +	/* Allocate extcon device */
> +	tps->edev = devm_extcon_dev_allocate(tps->dev, tps6598x_extcon_cable);
> +	if (IS_ERR(tps->edev)) {
> +		dev_err(tps->dev, "failed to allocate memory for extcon\n");
> +		typec_unregister_port(tps->port);
> +		return -ENOMEM;
> +	}
> +
> +	/* Register extcon device */
> +	ret = devm_extcon_dev_register(tps->dev, tps->edev);
> +	if (ret) {
> +		dev_err(tps->dev, "failed to register extcon device\n");
> +		typec_unregister_port(tps->port);
> +		return ret;
> +	}
> +
> +	extcon_set_property_capability(tps->edev, EXTCON_USB,
> +				       EXTCON_PROP_USB_VBUS);
> +	extcon_set_property_capability(tps->edev, EXTCON_USB,
> +				       EXTCON_PROP_USB_VBUS_SRC);
> +	extcon_set_property_capability(tps->edev, EXTCON_USB,
> +				       EXTCON_PROP_USB_VBUS_VOLTAGE);
> +	extcon_set_property_capability(tps->edev, EXTCON_USB,
> +				       EXTCON_PROP_USB_VBUS_CURRENT);
> +	extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
> +				       EXTCON_PROP_USB_VBUS);
> +	extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
> +				       EXTCON_PROP_USB_VBUS_SRC);
> +	extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
> +				       EXTCON_PROP_USB_VBUS_VOLTAGE);
> +	extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
> +				       EXTCON_PROP_USB_VBUS_CURRENT);
> +#endif
> +
>  	if (status & TPS_STATUS_PLUG_PRESENT) {
>  		ret = tps6598x_connect(tps, status);
>  		if (ret)
>  			dev_err(&client->dev, "failed to register partner\n");
> +	} else {
> +		tps6598x_disconnect(tps, status);
>  	}
>  
>  	ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH 1/4] extcon: Add USB VBUS properties

[Chanwoo Choi]

Hi,

On 9/15/20 1:46 AM, Angus Ainslie wrote:
> USB type C, USB BC1.2 and USB power delivery allow different voltages
> and currents for VBUS so we need these additional properties.
> 
> Also USB type C allows separate device and power roles so add a VBUS SRC
> property.
> 
> Signed-off-by: Angus Ainslie <angus@akkea.ca>
> ---
>  include/linux/extcon.h | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/extcon.h b/include/linux/extcon.h
> index fd183fb9c20f..c4d48f4f74c4 100644
> --- a/include/linux/extcon.h
> +++ b/include/linux/extcon.h
> @@ -117,14 +117,29 @@
>   * @type:       integer (intval)
>   * @value:      0 (USB/USB2) or 1 (USB3)
>   * @default:    0 (USB/USB2)
> + * - EXTCON_PROP_USB_VBUS_SRC

Could you explain more correct meaning of both sink and source?

> + * @type:	integer (intval)
> + * @value:	0 (sink) or 1 (source)
> + * @default:	0 (sink)
> + * - EXTCON_PROP_USB_VBUS_VOLTAGE
> + * @type:	integer (intval)
> + * @value:	negotiated vbus voltage in mV
> + * @default:	5000

Could you suggest the data why do you set default value as 5000?

> + * - EXTCON_PROP_USB_VBUS_CURRENT
> + * @type:	integer (intval)
> + * @value:	negotiated vbus current in mA
> + * @default:	100

ditto. Why default value is 100?

>   *
>   */
>  #define EXTCON_PROP_USB_VBUS		0
>  #define EXTCON_PROP_USB_TYPEC_POLARITY	1
>  #define EXTCON_PROP_USB_SS		2
> +#define EXTCON_PROP_USB_VBUS_SRC	3
> +#define EXTCON_PROP_USB_VBUS_VOLTAGE	4
> +#define EXTCON_PROP_USB_VBUS_CURRENT	5
>  
>  #define EXTCON_PROP_USB_MIN		0
> -#define EXTCON_PROP_USB_MAX		2
> +#define EXTCON_PROP_USB_MAX		5
>  #define EXTCON_PROP_USB_CNT	(EXTCON_PROP_USB_MAX - EXTCON_PROP_USB_MIN + 1)
>  
>  /* Properties of EXTCON_TYPE_CHG. */
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH 1/4] extcon: Add USB VBUS properties

[Angus Ainslie]

Hi,

On 2020-09-14 18:40, Chanwoo Choi wrote:
> Hi,
> 
> On 9/15/20 1:46 AM, Angus Ainslie wrote:
>> USB type C, USB BC1.2 and USB power delivery allow different voltages
>> and currents for VBUS so we need these additional properties.
>> 
>> Also USB type C allows separate device and power roles so add a VBUS 
>> SRC
>> property.
>> 
>> Signed-off-by: Angus Ainslie <angus@akkea.ca>
>> ---
>>  include/linux/extcon.h | 17 ++++++++++++++++-
>>  1 file changed, 16 insertions(+), 1 deletion(-)
>> 
>> diff --git a/include/linux/extcon.h b/include/linux/extcon.h
>> index fd183fb9c20f..c4d48f4f74c4 100644
>> --- a/include/linux/extcon.h
>> +++ b/include/linux/extcon.h
>> @@ -117,14 +117,29 @@
>>   * @type:       integer (intval)
>>   * @value:      0 (USB/USB2) or 1 (USB3)
>>   * @default:    0 (USB/USB2)
>> + * - EXTCON_PROP_USB_VBUS_SRC
> 
> Could you explain more correct meaning of both sink and source?
> 

Sure I can add some comments.

>> + * @type:	integer (intval)
>> + * @value:	0 (sink) or 1 (source)
>> + * @default:	0 (sink)
>> + * - EXTCON_PROP_USB_VBUS_VOLTAGE
>> + * @type:	integer (intval)
>> + * @value:	negotiated vbus voltage in mV
>> + * @default:	5000
> 
> Could you suggest the data why do you set default value as 5000?
> 

The lowest USB VBUS is 5V so I can add that to the comments.

>> + * - EXTCON_PROP_USB_VBUS_CURRENT
>> + * @type:	integer (intval)
>> + * @value:	negotiated vbus current in mA
>> + * @default:	100
> 
> ditto. Why default value is 100?
> 

USB spec says that until the current is negotiated the max that anything 
can draw is 100mA. I can add a comment to that effect.

Thanks
Angus

>>   *
>>   */
>>  #define EXTCON_PROP_USB_VBUS		0
>>  #define EXTCON_PROP_USB_TYPEC_POLARITY	1
>>  #define EXTCON_PROP_USB_SS		2
>> +#define EXTCON_PROP_USB_VBUS_SRC	3
>> +#define EXTCON_PROP_USB_VBUS_VOLTAGE	4
>> +#define EXTCON_PROP_USB_VBUS_CURRENT	5
>> 
>>  #define EXTCON_PROP_USB_MIN		0
>> -#define EXTCON_PROP_USB_MAX		2
>> +#define EXTCON_PROP_USB_MAX		5
>>  #define EXTCON_PROP_USB_CNT	(EXTCON_PROP_USB_MAX - 
>> EXTCON_PROP_USB_MIN + 1)
>> 
>>  /* Properties of EXTCON_TYPE_CHG. */
>> 

Re: [PATCH 2/4] usb: typec: tps6589x: register as an extcon provider

[Angus Ainslie]

Hi,

On 2020-09-14 18:19, Chanwoo Choi wrote:
> Hi,
> 
> On 9/15/20 1:46 AM, Angus Ainslie wrote:
>> The tps6598x type C chip can negotiate the VBUS sink/source status as
>> well as the VBUS voltage and current. Notify the extcon consumers of
>> these changes.
>> 
>> Signed-off-by: Angus Ainslie <angus@akkea.ca>
>> ---
>>  drivers/usb/typec/tps6598x.c | 138 
>> +++++++++++++++++++++++++++++++++++
>>  1 file changed, 138 insertions(+)
>> 
>> diff --git a/drivers/usb/typec/tps6598x.c 
>> b/drivers/usb/typec/tps6598x.c
>> index 3db33bb622c3..3b06d43c810d 100644
>> --- a/drivers/usb/typec/tps6598x.c
>> +++ b/drivers/usb/typec/tps6598x.c
>> @@ -8,6 +8,8 @@
>> 
>>  #include <linux/i2c.h>
>>  #include <linux/acpi.h>
>> +#include <linux/extcon.h>
>> +#include <linux/extcon-provider.h>
>>  #include <linux/module.h>
>>  #include <linux/regmap.h>
>>  #include <linux/interrupt.h>
>> @@ -95,7 +97,12 @@ struct tps6598x {
>>  	struct typec_port *port;
>>  	struct typec_partner *partner;
>>  	struct usb_pd_identity partner_identity;
>> +
>>  	struct usb_role_switch *role_sw;
>> +
>> +#ifdef CONFIG_EXTCON
> 
> Is it necessary of 'ifdef CONFIG_EXTCON'?
> If you just add 'select EXTCON' for this driver,
> you don't need to check CONFIG_EXTCON.
> 
> If any device driver need some framework,
> we can add the 'select EXTCON'.
> 

Sure I can change that for v2

Angus

>> +	struct extcon_dev *edev;
>> +#endif
>>  };
>> 
>>  /*
>> @@ -209,6 +216,62 @@ static void tps6598x_set_data_role(struct 
>> tps6598x *tps,
>>  	typec_set_data_role(tps->port, role);
>>  }
>> 
>> +#ifdef CONFIG_EXTCON
>> +static void tps6589x_set_extcon_state(struct tps6598x *tps,
>> +				      u32 status, u16 pwr_status, bool state)
>> +{
>> +	union extcon_property_value val;
>> +	int max_current;
>> +
>> +	if (TPS_STATUS_DATAROLE(status)) {
>> +		extcon_set_state(tps->edev, EXTCON_USB, false);
>> +		extcon_set_state(tps->edev, EXTCON_USB_HOST, state);
>> +	} else {
>> +		extcon_set_state(tps->edev, EXTCON_USB, state);
>> +		extcon_set_state(tps->edev, EXTCON_USB_HOST, false);
>> +	}
>> +
>> +	val.intval = TPS_STATUS_PORTROLE(status);
>> +	extcon_set_property(tps->edev, EXTCON_USB_HOST,
>> +			    EXTCON_PROP_USB_VBUS_SRC,
>> +			    val);
>> +
>> +	extcon_set_property(tps->edev, EXTCON_USB,
>> +			    EXTCON_PROP_USB_VBUS_SRC,
>> +			    val);
>> +
>> +	switch (TPS_POWER_STATUS_PWROPMODE(pwr_status)) {
>> +	case TYPEC_PWR_MODE_USB:
>> +		max_current = 500;
>> +		extcon_set_state(tps->edev, EXTCON_CHG_USB_SDP, state);
>> +		extcon_set_state(tps->edev, EXTCON_CHG_USB_SLOW, state);
>> +		break;
>> +	case TYPEC_PWR_MODE_1_5A:
>> +		max_current = 1500;
>> +		break;
>> +	case TYPEC_PWR_MODE_3_0A:
>> +		max_current = 3000;
>> +		break;
>> +	case TYPEC_PWR_MODE_PD:
>> +		max_current = 500;
>> +		break;
>> +	}
>> +
>> +	val.intval = max_current;
>> +	extcon_set_property(tps->edev, EXTCON_USB,
>> +			    EXTCON_PROP_USB_VBUS_CURRENT,
>> +			    val);
>> +	extcon_set_property(tps->edev, EXTCON_USB_HOST,
>> +			    EXTCON_PROP_USB_VBUS_CURRENT,
>> +			    val);
>> +
>> +	extcon_sync(tps->edev, EXTCON_USB);
>> +	extcon_sync(tps->edev, EXTCON_USB_HOST);
>> +	extcon_sync(tps->edev, EXTCON_CHG_USB_SDP);
>> +	extcon_sync(tps->edev, EXTCON_CHG_USB_SLOW);
>> +}
>> +#endif
>> +
>>  static int tps6598x_connect(struct tps6598x *tps, u32 status)
>>  {
>>  	struct typec_partner_desc desc;
>> @@ -248,18 +311,41 @@ static int tps6598x_connect(struct tps6598x 
>> *tps, u32 status)
>>  	if (desc.identity)
>>  		typec_partner_set_identity(tps->partner);
>> 
>> +#ifdef CONFIG_EXTCON
>> +	tps6598x_set_extcon_state(tps, status, pwr_status, true);
>> +#endif
>> +
>>  	return 0;
>>  }
>> 
>>  static void tps6598x_disconnect(struct tps6598x *tps, u32 status)
>>  {
>> +	enum typec_pwr_opmode mode;
>> +	u16 pwr_status;
>> +	int ret;
>> +
>>  	if (!IS_ERR(tps->partner))
>>  		typec_unregister_partner(tps->partner);
>>  	tps->partner = NULL;
>>  	typec_set_pwr_opmode(tps->port, TYPEC_PWR_MODE_USB);
>>  	typec_set_pwr_role(tps->port, TPS_STATUS_PORTROLE(status));
>>  	typec_set_vconn_role(tps->port, TPS_STATUS_VCONN(status));
>> +	typec_set_data_role(tps->port, TPS_STATUS_DATAROLE(status));
>>  	tps6598x_set_data_role(tps, TPS_STATUS_DATAROLE(status), false);
>> +
>> +	ret = tps6598x_read16(tps, TPS_REG_POWER_STATUS, &pwr_status);
>> +	if (ret < 0)
>> +		return;
>> +
>> +	mode = TPS_POWER_STATUS_PWROPMODE(pwr_status);
>> +
>> +	dev_dbg(tps->dev, "%s: mode 0x%x pwr_role %d vconn_role %d data_role 
>> %d\n",
>> +		__func__, mode, TPS_STATUS_PORTROLE(status),
>> +		TPS_STATUS_VCONN(status), TPS_STATUS_DATAROLE(status));
>> +
>> +#ifdef CONFIG_EXTCON
>> +	tps6598x_set_extcon_state(tps, status, 0, false);
>> +#endif
>>  }
>> 
>>  static int tps6598x_exec_cmd(struct tps6598x *tps, const char *cmd,
>> @@ -407,6 +493,9 @@ static irqreturn_t tps6598x_interrupt(int irq, 
>> void *data)
>>  		goto err_unlock;
>>  	}
>> 
>> +	dev_dbg(tps->dev, "Received irq event: 0x%llx: 0x%x 0x%x", event1,
>> +	       (u32)((event1 & 0xFFFFFFFF00000000) >> 32), (u32)(event1 & 
>> 0xFFFFFFFF));
>> +
>>  	ret = tps6598x_read32(tps, TPS_REG_STATUS, &status);
>>  	if (ret) {
>>  		dev_err(tps->dev, "%s: failed to read status\n", __func__);
>> @@ -467,6 +556,18 @@ static const struct regmap_config 
>> tps6598x_regmap_config = {
>>  	.max_register = 0x7F,
>>  };
>> 
>> +#ifdef CONFIG_EXTCON
>> +/* List of detectable cables */
>> +static const unsigned int tps6598x_extcon_cable[] = {
>> +	EXTCON_USB,
>> +	EXTCON_USB_HOST,
>> +	EXTCON_CHG_USB_SDP,
>> +	EXTCON_CHG_USB_SLOW,
>> +	EXTCON_CHG_USB_FAST,
>> +	EXTCON_NONE,
>> +};
>> +#endif
>> +
>>  static int tps6598x_probe(struct i2c_client *client)
>>  {
>>  	struct typec_capability typec_cap = { };
>> @@ -567,10 +668,47 @@ static int tps6598x_probe(struct i2c_client 
>> *client)
>>  	}
>>  	fwnode_handle_put(fwnode);
>> 
>> +#ifdef CONFIG_EXTCON
>> +	/* Allocate extcon device */
>> +	tps->edev = devm_extcon_dev_allocate(tps->dev, 
>> tps6598x_extcon_cable);
>> +	if (IS_ERR(tps->edev)) {
>> +		dev_err(tps->dev, "failed to allocate memory for extcon\n");
>> +		typec_unregister_port(tps->port);
>> +		return -ENOMEM;
>> +	}
>> +
>> +	/* Register extcon device */
>> +	ret = devm_extcon_dev_register(tps->dev, tps->edev);
>> +	if (ret) {
>> +		dev_err(tps->dev, "failed to register extcon device\n");
>> +		typec_unregister_port(tps->port);
>> +		return ret;
>> +	}
>> +
>> +	extcon_set_property_capability(tps->edev, EXTCON_USB,
>> +				       EXTCON_PROP_USB_VBUS);
>> +	extcon_set_property_capability(tps->edev, EXTCON_USB,
>> +				       EXTCON_PROP_USB_VBUS_SRC);
>> +	extcon_set_property_capability(tps->edev, EXTCON_USB,
>> +				       EXTCON_PROP_USB_VBUS_VOLTAGE);
>> +	extcon_set_property_capability(tps->edev, EXTCON_USB,
>> +				       EXTCON_PROP_USB_VBUS_CURRENT);
>> +	extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
>> +				       EXTCON_PROP_USB_VBUS);
>> +	extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
>> +				       EXTCON_PROP_USB_VBUS_SRC);
>> +	extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
>> +				       EXTCON_PROP_USB_VBUS_VOLTAGE);
>> +	extcon_set_property_capability(tps->edev, EXTCON_USB_HOST,
>> +				       EXTCON_PROP_USB_VBUS_CURRENT);
>> +#endif
>> +
>>  	if (status & TPS_STATUS_PLUG_PRESENT) {
>>  		ret = tps6598x_connect(tps, status);
>>  		if (ret)
>>  			dev_err(&client->dev, "failed to register partner\n");
>> +	} else {
>> +		tps6598x_disconnect(tps, status);
>>  	}
>> 
>>  	ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
>> 

Re: [PATCH 0/4] RFC: USB C extcon patchset for the tps6598x

[Heikki Krogerus]

On Mon, Sep 14, 2020 at 09:46:35AM -0700, Angus Ainslie wrote:
> We have a complex set of hardware components to manage our USB C data and
> power. For these to work together we decided to use extcon to communicate
> the system changes as various cables and devices are plugged in/out. We did
> look at usb_roleswitch and the charging framework but thought it would be
> preferable to keep all of the information together in one system.
> 
> The components we have in the system are:
> 
> 1) TPS65982 type USB type C controller
> 2) dwc3 IP in the imx8mq
> 3) BQ25895 battery charger
> 
> I'll break this into 2 parts the data role and the power role.
> 
> For the data role the TPS65982 senses connect and disconnect as well as data
> source/sink. It is also controlling the USB 3 data lanes. The display port and
> USB 3 muxing is handled by a different chip and we'll submit patches for that
> later on. The dwc3 controls the USB 2 data lanes.
> 
> On the power side there are even more moving pieces. The TPS65982 negotiates
> the power delivery contract, the dwc3 senses the BC1.2 charging current and the
> BQ25895 sets whether we are sinking or sourcing current and what the current
> limit is of the sink and source.
> 
> For both the data and power roles no single chip has all of the required
> information. Is using extcon the correct way of doing this and if not what
> are the alternatives ?

Do not use extcon with the Type-C drivers unless you have some really
good reason for not using the dedicated frameworks for each thing. The
reason why we even have some of the dedicated frameworks in the first
place, for example the USB role switch class, is because extcon simply
could not be made to work on every type of hardware architecture.

So you will need to register a power supply in tps6598x.c just like
the other USB Type-C drivers like tcpm.c and ucsi.c if the TPS65982
does not communicated directly with the BQ25895. That can be one
of "supplied_from" (and also "supplied_to" if needed for sourcing) for
the bq25890_changer. You probable only need to implement the
external_power_changed() hook for it if it's missing in order to make
it work. You can also register a power supply in dwc3 and use it as a
second supply for bq25890 if you still really need to handle BC1.2.

The data role should already be handled for you. dwc3 already
registers an USB role switch, and tps6598x.c already configures one.
For data role you should not need any additional code.

Please note that there is also framework for the alt mode muxes.


> The extcon extensions allow us to communicate the the power roles amongst
> the various chips.
> 
> This patch series has been tested with the 5.9-rc4 kernel on the Purism
> Librem5 HW. Assuming this is the correct way to use extcon there will be
> follow on patches to the BQ25895 and dwc3 drivers.
> 
> Strictly speaking only the first 3 patches are needed for extcon support, the
> forth patch decodes the power delivery contracts which makes use of the extcon
> system.
> 
> 
> Angus Ainslie (4):
>   extcon: Add USB VBUS properties
>   usb: typec: tps6589x: register as an extcon provider
>   usb: typec: tps6598x: Add the extcon USB chargers
>   usb: typec: tps6598x: Add the power delivery irq
> 
>  drivers/usb/typec/tps6598x.c | 488 ++++++++++++++++++++++++++++++++++-
>  include/linux/extcon.h       |  17 +-
>  2 files changed, 503 insertions(+), 2 deletions(-)
> 
> -- 
> 2.25.1

thanks,

-- 
heikki

Re: [PATCH 0/4] RFC: USB C extcon patchset for the tps6598x

[Angus Ainslie]

Hi Heikki,

On 2020-09-21 07:37, Heikki Krogerus wrote:
> On Mon, Sep 14, 2020 at 09:46:35AM -0700, Angus Ainslie wrote:
>> We have a complex set of hardware components to manage our USB C data 
>> and
>> power. For these to work together we decided to use extcon to 
>> communicate
>> the system changes as various cables and devices are plugged in/out. 
>> We did
>> look at usb_roleswitch and the charging framework but thought it would 
>> be
>> preferable to keep all of the information together in one system.
>> 
>> The components we have in the system are:
>> 
>> 1) TPS65982 type USB type C controller
>> 2) dwc3 IP in the imx8mq
>> 3) BQ25895 battery charger
>> 
>> I'll break this into 2 parts the data role and the power role.
>> 
>> For the data role the TPS65982 senses connect and disconnect as well 
>> as data
>> source/sink. It is also controlling the USB 3 data lanes. The display 
>> port and
>> USB 3 muxing is handled by a different chip and we'll submit patches 
>> for that
>> later on. The dwc3 controls the USB 2 data lanes.
>> 
>> On the power side there are even more moving pieces. The TPS65982 
>> negotiates
>> the power delivery contract, the dwc3 senses the BC1.2 charging 
>> current and the
>> BQ25895 sets whether we are sinking or sourcing current and what the 
>> current
>> limit is of the sink and source.
>> 
>> For both the data and power roles no single chip has all of the 
>> required
>> information. Is using extcon the correct way of doing this and if not 
>> what
>> are the alternatives ?
> 
> Do not use extcon with the Type-C drivers unless you have some really
> good reason for not using the dedicated frameworks for each thing. The
> reason why we even have some of the dedicated frameworks in the first
> place, for example the USB role switch class, is because extcon simply
> could not be made to work on every type of hardware architecture.
> 
> So you will need to register a power supply in tps6598x.c just like
> the other USB Type-C drivers like tcpm.c and ucsi.c if the TPS65982
> does not communicated directly with the BQ25895. That can be one
> of "supplied_from" (and also "supplied_to" if needed for sourcing) for
> the bq25890_changer. You probable only need to implement the
> external_power_changed() hook for it if it's missing in order to make
> it work. You can also register a power supply in dwc3 and use it as a
> second supply for bq25890 if you still really need to handle BC1.2.
> 
> The data role should already be handled for you. dwc3 already
> registers an USB role switch, and tps6598x.c already configures one.
> For data role you should not need any additional code.
> 
> Please note that there is also framework for the alt mode muxes.
> 

Thanks for looking this over. I'll investigate the power supply 
framework.

Angus

> 
>> The extcon extensions allow us to communicate the the power roles 
>> amongst
>> the various chips.
>> 
>> This patch series has been tested with the 5.9-rc4 kernel on the 
>> Purism
>> Librem5 HW. Assuming this is the correct way to use extcon there will 
>> be
>> follow on patches to the BQ25895 and dwc3 drivers.
>> 
>> Strictly speaking only the first 3 patches are needed for extcon 
>> support, the
>> forth patch decodes the power delivery contracts which makes use of 
>> the extcon
>> system.
>> 
>> 
>> Angus Ainslie (4):
>>   extcon: Add USB VBUS properties
>>   usb: typec: tps6589x: register as an extcon provider
>>   usb: typec: tps6598x: Add the extcon USB chargers
>>   usb: typec: tps6598x: Add the power delivery irq
>> 
>>  drivers/usb/typec/tps6598x.c | 488 
>> ++++++++++++++++++++++++++++++++++-
>>  include/linux/extcon.h       |  17 +-
>>  2 files changed, 503 insertions(+), 2 deletions(-)
>> 
>> --
>> 2.25.1
> 
> thanks,

Re: [PATCH 4/4] usb: typec: tps6598x: Add the power delivery irq

[kernel test robot]

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

Hi Angus,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on chanwoo-extcon/extcon-next v5.9-rc6 next-20200924]
[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]

url:    https://github.com/0day-ci/linux/commits/Angus-Ainslie/RFC-USB-C-extcon-patchset-for-the-tps6598x/20200915-010019
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: riscv-randconfig-r034-20200923 (attached as .config)
compiler: riscv32-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=riscv 

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

All errors (new ones prefixed by >>):

   drivers/usb/typec/tps6598x.c: In function 'tps6598x_interrupt':
>> drivers/usb/typec/tps6598x.c:813:2: error: 'connected' undeclared (first use in this function)
     813 |  connected = status & TPS_STATUS_PLUG_PRESENT;
         |  ^~~~~~~~~
   drivers/usb/typec/tps6598x.c:813:2: note: each undeclared identifier is reported only once for each function it appears in

# https://github.com/0day-ci/linux/commit/2ddad1e8ed61dd0c0c003e334ae0085519a7d44b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Angus-Ainslie/RFC-USB-C-extcon-patchset-for-the-tps6598x/20200915-010019
git checkout 2ddad1e8ed61dd0c0c003e334ae0085519a7d44b
vim +/connected +813 drivers/usb/typec/tps6598x.c

   794	
   795		mutex_lock(&tps->lock);
   796	
   797		ret = tps6598x_read64(tps, TPS_REG_INT_EVENT1, &event1);
   798		ret |= tps6598x_read64(tps, TPS_REG_INT_EVENT2, &event2);
   799		if (ret) {
   800			dev_err(tps->dev, "%s: failed to read events\n", __func__);
   801			goto err_unlock;
   802		}
   803	
   804		dev_dbg(tps->dev, "Received irq event: 0x%llx: 0x%x 0x%x", event1,
   805		       (u32)((event1 & 0xFFFFFFFF00000000) >> 32), (u32)(event1 & 0xFFFFFFFF));
   806	
   807		ret = tps6598x_read32(tps, TPS_REG_STATUS, &status);
   808		if (ret) {
   809			dev_err(tps->dev, "%s: failed to read status\n", __func__);
   810			goto err_clear_ints;
   811		}
   812	
 > 813		connected = status & TPS_STATUS_PLUG_PRESENT;
   814		ret = tps6598x_read16(tps, TPS_REG_POWER_STATUS, &power_status);
   815		if (ret < 0)
   816			goto err_clear_ints;
   817	
   818		ret = tps6598x_read32(tps, TPS_REG_DATA_STATUS, &data_status);
   819		if (ret < 0)
   820			goto err_clear_ints;
   821	
   822		dev_dbg(tps->dev, "Status: 0x%x data: 0x%x power: 0x%x", status,
   823			data_status, power_status);
   824	
   825		/* Handle plug insert or removal */
   826		if ((event1 | event2) & (TPS_REG_INT_PLUG_EVENT)) {
   827			if (status & TPS_STATUS_PLUG_PRESENT) {
   828				ret = tps6598x_connect(tps, status);
   829				if (ret)
   830					dev_err(tps->dev,
   831						"failed to register partner\n");
   832			} else {
   833				tps6598x_disconnect(tps, status);
   834			}
   835		}
   836	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 26586 bytes --]