[v2,1/3] usb: typec: tcpm: Notify the tcpc to start connection-detection for SRPs

[v2,1/3] usb: typec: tcpm: Notify the tcpc to start connection-detection for SRPs

[Hans de Goede]

Some tcpc device-drivers need to explicitly be told to watch for connection
events, otherwise the tcpc will not generate any TCPM_CC_EVENTs and devices
being plugged into the Type-C port will not be noticed.

For dual-role ports tcpm_start_drp_toggling() is used to tell the tcpc to
watch for connection events. Sofar we lack a similar callback to the tcpc
for single-role ports. With some tcpc-s such as the fusb302 this means
no TCPM_CC_EVENTs will be generated when the port is configured as a
single-role port.

This commit renames start_drp_toggling to start_toggling and since the
device-properties are parsed by the tcpm-core, adds a port_type parameter
to the start_toggling callback so that the tcpc_dev driver knows the
port-type and can act accordingly when it starts toggling.

The new start_toggling callback now always gets called if defined, instead
of only being called for DRP ports.

To avoid this causing undesirable functional changes all existing
start_drp_toggling implementations are not only renamed to start_toggling,
but also get a port_type check added and return -EOPNOTSUPP when port_type
is not DRP.

Fixes: ea3b4d5523bc("usb: typec: fusb302: Resolve fixed power role ...")
Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Rename start_drp_toggling to start_toggling and make it multi-purpose,
 rather then adding a new start_srp_connection_detect callback
---
 drivers/usb/typec/tcpm/fusb302.c | 10 +++++++---
 drivers/usb/typec/tcpm/tcpci.c   | 10 +++++++---
 drivers/usb/typec/tcpm/tcpm.c    | 32 ++++++++++++++------------------
 drivers/usb/typec/tcpm/wcove.c   | 10 +++++++---
 include/linux/usb/tcpm.h         | 13 +++++++------
 5 files changed, 42 insertions(+), 33 deletions(-)

diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
index 6ea6199caafa..6d83891cc895 100644
--- a/drivers/usb/typec/tcpm/fusb302.c
+++ b/drivers/usb/typec/tcpm/fusb302.c
@@ -876,13 +876,17 @@ static int tcpm_set_roles(struct tcpc_dev *dev, bool attached,
 	return ret;
 }
 
-static int tcpm_start_drp_toggling(struct tcpc_dev *dev,
-				   enum typec_cc_status cc)
+static int tcpm_start_toggling(struct tcpc_dev *dev,
+			       enum typec_port_type port_type,
+			       enum typec_cc_status cc)
 {
 	struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
 						 tcpc_dev);
 	int ret = 0;
 
+	if (port_type != TYPEC_PORT_DRP)
+		return -EOPNOTSUPP;
+
 	mutex_lock(&chip->lock);
 	ret = fusb302_set_src_current(chip, cc_src_current[cc]);
 	if (ret < 0) {
@@ -1094,7 +1098,7 @@ static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev)
 	fusb302_tcpc_dev->set_vbus = tcpm_set_vbus;
 	fusb302_tcpc_dev->set_pd_rx = tcpm_set_pd_rx;
 	fusb302_tcpc_dev->set_roles = tcpm_set_roles;
-	fusb302_tcpc_dev->start_drp_toggling = tcpm_start_drp_toggling;
+	fusb302_tcpc_dev->start_toggling = tcpm_start_toggling;
 	fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit;
 }
 
diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index ac6b418b15f1..c1f7073a56de 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -100,13 +100,17 @@ static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
 	return 0;
 }
 
-static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc,
-				    enum typec_cc_status cc)
+static int tcpci_start_toggling(struct tcpc_dev *tcpc,
+				enum typec_port_type port_type,
+				enum typec_cc_status cc)
 {
 	int ret;
 	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
 	unsigned int reg = TCPC_ROLE_CTRL_DRP;
 
+	if (port_type != TYPEC_PORT_DRP)
+		return -EOPNOTSUPP;
+
 	/* Handle vendor drp toggling */
 	if (tcpci->data->start_drp_toggling) {
 		ret = tcpci->data->start_drp_toggling(tcpci, tcpci->data, cc);
@@ -511,7 +515,7 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
 	tcpci->tcpc.get_cc = tcpci_get_cc;
 	tcpci->tcpc.set_polarity = tcpci_set_polarity;
 	tcpci->tcpc.set_vconn = tcpci_set_vconn;
-	tcpci->tcpc.start_drp_toggling = tcpci_start_drp_toggling;
+	tcpci->tcpc.start_toggling = tcpci_start_toggling;
 
 	tcpci->tcpc.set_pd_rx = tcpci_set_pd_rx;
 	tcpci->tcpc.set_roles = tcpci_set_roles;
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index a2233d72ae7c..fba32d84e578 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -31,7 +31,7 @@
 
 #define FOREACH_STATE(S)			\
 	S(INVALID_STATE),			\
-	S(DRP_TOGGLING),			\
+	S(TOGGLING),			\
 	S(SRC_UNATTACHED),			\
 	S(SRC_ATTACH_WAIT),			\
 	S(SRC_ATTACHED),			\
@@ -472,7 +472,7 @@ static void tcpm_log(struct tcpm_port *port, const char *fmt, ...)
 	/* Do not log while disconnected and unattached */
 	if (tcpm_port_is_disconnected(port) &&
 	    (port->state == SRC_UNATTACHED || port->state == SNK_UNATTACHED ||
-	     port->state == DRP_TOGGLING))
+	     port->state == TOGGLING))
 		return;
 
 	va_start(args, fmt);
@@ -2540,20 +2540,16 @@ static int tcpm_set_charge(struct tcpm_port *port, bool charge)
 	return 0;
 }
 
-static bool tcpm_start_drp_toggling(struct tcpm_port *port,
-				    enum typec_cc_status cc)
+static bool tcpm_start_toggling(struct tcpm_port *port, enum typec_cc_status cc)
 {
 	int ret;
 
-	if (port->tcpc->start_drp_toggling &&
-	    port->port_type == TYPEC_PORT_DRP) {
-		tcpm_log_force(port, "Start DRP toggling");
-		ret = port->tcpc->start_drp_toggling(port->tcpc, cc);
-		if (!ret)
-			return true;
-	}
+	if (!port->tcpc->start_toggling)
+		return false;
 
-	return false;
+	tcpm_log_force(port, "Start toggling");
+	ret = port->tcpc->start_toggling(port->tcpc, port->port_type, cc);
+	return ret == 0;
 }
 
 static void tcpm_set_cc(struct tcpm_port *port, enum typec_cc_status cc)
@@ -2847,15 +2843,15 @@ static void run_state_machine(struct tcpm_port *port)
 
 	port->enter_state = port->state;
 	switch (port->state) {
-	case DRP_TOGGLING:
+	case TOGGLING:
 		break;
 	/* SRC states */
 	case SRC_UNATTACHED:
 		if (!port->non_pd_role_swap)
 			tcpm_swap_complete(port, -ENOTCONN);
 		tcpm_src_detach(port);
-		if (tcpm_start_drp_toggling(port, tcpm_rp_cc(port))) {
-			tcpm_set_state(port, DRP_TOGGLING, 0);
+		if (tcpm_start_toggling(port, tcpm_rp_cc(port))) {
+			tcpm_set_state(port, TOGGLING, 0);
 			break;
 		}
 		tcpm_set_cc(port, tcpm_rp_cc(port));
@@ -3053,8 +3049,8 @@ static void run_state_machine(struct tcpm_port *port)
 			tcpm_swap_complete(port, -ENOTCONN);
 		tcpm_pps_complete(port, -ENOTCONN);
 		tcpm_snk_detach(port);
-		if (tcpm_start_drp_toggling(port, TYPEC_CC_RD)) {
-			tcpm_set_state(port, DRP_TOGGLING, 0);
+		if (tcpm_start_toggling(port, TYPEC_CC_RD)) {
+			tcpm_set_state(port, TOGGLING, 0);
 			break;
 		}
 		tcpm_set_cc(port, TYPEC_CC_RD);
@@ -3621,7 +3617,7 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1,
 						       : "connected");
 
 	switch (port->state) {
-	case DRP_TOGGLING:
+	case TOGGLING:
 		if (tcpm_port_is_debug(port) || tcpm_port_is_audio(port) ||
 		    tcpm_port_is_source(port))
 			tcpm_set_state(port, SRC_ATTACH_WAIT, 0);
diff --git a/drivers/usb/typec/tcpm/wcove.c b/drivers/usb/typec/tcpm/wcove.c
index 6770afd40765..f1f8f45e2f3d 100644
--- a/drivers/usb/typec/tcpm/wcove.c
+++ b/drivers/usb/typec/tcpm/wcove.c
@@ -416,12 +416,16 @@ static int wcove_pd_transmit(struct tcpc_dev *tcpc,
 	return regmap_write(wcove->regmap, USBC_TXCMD, cmd | USBC_TXCMD_START);
 }
 
-static int wcove_start_drp_toggling(struct tcpc_dev *tcpc,
-				    enum typec_cc_status cc)
+static int wcove_start_toggling(struct tcpc_dev *tcpc,
+				enum typec_port_type port_type,
+				enum typec_cc_status cc)
 {
 	struct wcove_typec *wcove = tcpc_to_wcove(tcpc);
 	unsigned int usbc_ctrl;
 
+	if (port_type != TYPEC_PORT_DRP)
+		return -EOPNOTSUPP;
+
 	usbc_ctrl = USBC_CONTROL1_MODE_DRP | USBC_CONTROL1_DRPTOGGLE_RANDOM;
 
 	switch (cc) {
@@ -642,7 +646,7 @@ static int wcove_typec_probe(struct platform_device *pdev)
 	wcove->tcpc.set_polarity = wcove_set_polarity;
 	wcove->tcpc.set_vconn = wcove_set_vconn;
 	wcove->tcpc.set_current_limit = wcove_set_current_limit;
-	wcove->tcpc.start_drp_toggling = wcove_start_drp_toggling;
+	wcove->tcpc.start_toggling = wcove_start_toggling;
 
 	wcove->tcpc.set_pd_rx = wcove_set_pd_rx;
 	wcove->tcpc.set_roles = wcove_set_roles;
diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
index 0c532ca3f079..36a15dcadc53 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -121,10 +121,10 @@ struct tcpc_config {
  *		with partner.
  * @set_pd_rx:	Called to enable or disable reception of PD messages
  * @set_roles:	Called to set power and data roles
- * @start_drp_toggling:
- *		Optional; if supported by hardware, called to start DRP
- *		toggling. DRP toggling is stopped automatically if
- *		a connection is established.
+ * @start_toggling:
+ *		Optional; if supported by hardware, called to start dual-role
+ *		toggling or single-role connection detection. Toggling stops
+ *		automatically if a connection is established.
  * @try_role:	Optional; called to set a preferred role
  * @pd_transmit:Called to transmit PD message
  * @mux:	Pointer to multiplexer data
@@ -147,8 +147,9 @@ struct tcpc_dev {
 	int (*set_pd_rx)(struct tcpc_dev *dev, bool on);
 	int (*set_roles)(struct tcpc_dev *dev, bool attached,
 			 enum typec_role role, enum typec_data_role data);
-	int (*start_drp_toggling)(struct tcpc_dev *dev,
-				  enum typec_cc_status cc);
+	int (*start_toggling)(struct tcpc_dev *dev,
+			      enum typec_port_type port_type,
+			      enum typec_cc_status cc);
 	int (*try_role)(struct tcpc_dev *dev, int role);
 	int (*pd_transmit)(struct tcpc_dev *dev, enum tcpm_transmit_type type,
 			   const struct pd_message *msg);

[PATCH v2 1/3] usb: typec: tcpm: Notify the tcpc to start connection-detection for SRPs

[Hans de Goede]

Some tcpc device-drivers need to explicitly be told to watch for connection
events, otherwise the tcpc will not generate any TCPM_CC_EVENTs and devices
being plugged into the Type-C port will not be noticed.

For dual-role ports tcpm_start_drp_toggling() is used to tell the tcpc to
watch for connection events. Sofar we lack a similar callback to the tcpc
for single-role ports. With some tcpc-s such as the fusb302 this means
no TCPM_CC_EVENTs will be generated when the port is configured as a
single-role port.

This commit renames start_drp_toggling to start_toggling and since the
device-properties are parsed by the tcpm-core, adds a port_type parameter
to the start_toggling callback so that the tcpc_dev driver knows the
port-type and can act accordingly when it starts toggling.

The new start_toggling callback now always gets called if defined, instead
of only being called for DRP ports.

To avoid this causing undesirable functional changes all existing
start_drp_toggling implementations are not only renamed to start_toggling,
but also get a port_type check added and return -EOPNOTSUPP when port_type
is not DRP.

Fixes: ea3b4d5523bc("usb: typec: fusb302: Resolve fixed power role ...")
Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Rename start_drp_toggling to start_toggling and make it multi-purpose,
 rather then adding a new start_srp_connection_detect callback
---
 drivers/usb/typec/tcpm/fusb302.c | 10 +++++++---
 drivers/usb/typec/tcpm/tcpci.c   | 10 +++++++---
 drivers/usb/typec/tcpm/tcpm.c    | 32 ++++++++++++++------------------
 drivers/usb/typec/tcpm/wcove.c   | 10 +++++++---
 include/linux/usb/tcpm.h         | 13 +++++++------
 5 files changed, 42 insertions(+), 33 deletions(-)

diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
index 6ea6199caafa..6d83891cc895 100644
--- a/drivers/usb/typec/tcpm/fusb302.c
+++ b/drivers/usb/typec/tcpm/fusb302.c
@@ -876,13 +876,17 @@ static int tcpm_set_roles(struct tcpc_dev *dev, bool attached,
 	return ret;
 }
 
-static int tcpm_start_drp_toggling(struct tcpc_dev *dev,
-				   enum typec_cc_status cc)
+static int tcpm_start_toggling(struct tcpc_dev *dev,
+			       enum typec_port_type port_type,
+			       enum typec_cc_status cc)
 {
 	struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
 						 tcpc_dev);
 	int ret = 0;
 
+	if (port_type != TYPEC_PORT_DRP)
+		return -EOPNOTSUPP;
+
 	mutex_lock(&chip->lock);
 	ret = fusb302_set_src_current(chip, cc_src_current[cc]);
 	if (ret < 0) {
@@ -1094,7 +1098,7 @@ static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev)
 	fusb302_tcpc_dev->set_vbus = tcpm_set_vbus;
 	fusb302_tcpc_dev->set_pd_rx = tcpm_set_pd_rx;
 	fusb302_tcpc_dev->set_roles = tcpm_set_roles;
-	fusb302_tcpc_dev->start_drp_toggling = tcpm_start_drp_toggling;
+	fusb302_tcpc_dev->start_toggling = tcpm_start_toggling;
 	fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit;
 }
 
diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index ac6b418b15f1..c1f7073a56de 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -100,13 +100,17 @@ static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
 	return 0;
 }
 
-static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc,
-				    enum typec_cc_status cc)
+static int tcpci_start_toggling(struct tcpc_dev *tcpc,
+				enum typec_port_type port_type,
+				enum typec_cc_status cc)
 {
 	int ret;
 	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
 	unsigned int reg = TCPC_ROLE_CTRL_DRP;
 
+	if (port_type != TYPEC_PORT_DRP)
+		return -EOPNOTSUPP;
+
 	/* Handle vendor drp toggling */
 	if (tcpci->data->start_drp_toggling) {
 		ret = tcpci->data->start_drp_toggling(tcpci, tcpci->data, cc);
@@ -511,7 +515,7 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
 	tcpci->tcpc.get_cc = tcpci_get_cc;
 	tcpci->tcpc.set_polarity = tcpci_set_polarity;
 	tcpci->tcpc.set_vconn = tcpci_set_vconn;
-	tcpci->tcpc.start_drp_toggling = tcpci_start_drp_toggling;
+	tcpci->tcpc.start_toggling = tcpci_start_toggling;
 
 	tcpci->tcpc.set_pd_rx = tcpci_set_pd_rx;
 	tcpci->tcpc.set_roles = tcpci_set_roles;
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index a2233d72ae7c..fba32d84e578 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -31,7 +31,7 @@
 
 #define FOREACH_STATE(S)			\
 	S(INVALID_STATE),			\
-	S(DRP_TOGGLING),			\
+	S(TOGGLING),			\
 	S(SRC_UNATTACHED),			\
 	S(SRC_ATTACH_WAIT),			\
 	S(SRC_ATTACHED),			\
@@ -472,7 +472,7 @@ static void tcpm_log(struct tcpm_port *port, const char *fmt, ...)
 	/* Do not log while disconnected and unattached */
 	if (tcpm_port_is_disconnected(port) &&
 	    (port->state == SRC_UNATTACHED || port->state == SNK_UNATTACHED ||
-	     port->state == DRP_TOGGLING))
+	     port->state == TOGGLING))
 		return;
 
 	va_start(args, fmt);
@@ -2540,20 +2540,16 @@ static int tcpm_set_charge(struct tcpm_port *port, bool charge)
 	return 0;
 }
 
-static bool tcpm_start_drp_toggling(struct tcpm_port *port,
-				    enum typec_cc_status cc)
+static bool tcpm_start_toggling(struct tcpm_port *port, enum typec_cc_status cc)
 {
 	int ret;
 
-	if (port->tcpc->start_drp_toggling &&
-	    port->port_type == TYPEC_PORT_DRP) {
-		tcpm_log_force(port, "Start DRP toggling");
-		ret = port->tcpc->start_drp_toggling(port->tcpc, cc);
-		if (!ret)
-			return true;
-	}
+	if (!port->tcpc->start_toggling)
+		return false;
 
-	return false;
+	tcpm_log_force(port, "Start toggling");
+	ret = port->tcpc->start_toggling(port->tcpc, port->port_type, cc);
+	return ret == 0;
 }
 
 static void tcpm_set_cc(struct tcpm_port *port, enum typec_cc_status cc)
@@ -2847,15 +2843,15 @@ static void run_state_machine(struct tcpm_port *port)
 
 	port->enter_state = port->state;
 	switch (port->state) {
-	case DRP_TOGGLING:
+	case TOGGLING:
 		break;
 	/* SRC states */
 	case SRC_UNATTACHED:
 		if (!port->non_pd_role_swap)
 			tcpm_swap_complete(port, -ENOTCONN);
 		tcpm_src_detach(port);
-		if (tcpm_start_drp_toggling(port, tcpm_rp_cc(port))) {
-			tcpm_set_state(port, DRP_TOGGLING, 0);
+		if (tcpm_start_toggling(port, tcpm_rp_cc(port))) {
+			tcpm_set_state(port, TOGGLING, 0);
 			break;
 		}
 		tcpm_set_cc(port, tcpm_rp_cc(port));
@@ -3053,8 +3049,8 @@ static void run_state_machine(struct tcpm_port *port)
 			tcpm_swap_complete(port, -ENOTCONN);
 		tcpm_pps_complete(port, -ENOTCONN);
 		tcpm_snk_detach(port);
-		if (tcpm_start_drp_toggling(port, TYPEC_CC_RD)) {
-			tcpm_set_state(port, DRP_TOGGLING, 0);
+		if (tcpm_start_toggling(port, TYPEC_CC_RD)) {
+			tcpm_set_state(port, TOGGLING, 0);
 			break;
 		}
 		tcpm_set_cc(port, TYPEC_CC_RD);
@@ -3621,7 +3617,7 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1,
 						       : "connected");
 
 	switch (port->state) {
-	case DRP_TOGGLING:
+	case TOGGLING:
 		if (tcpm_port_is_debug(port) || tcpm_port_is_audio(port) ||
 		    tcpm_port_is_source(port))
 			tcpm_set_state(port, SRC_ATTACH_WAIT, 0);
diff --git a/drivers/usb/typec/tcpm/wcove.c b/drivers/usb/typec/tcpm/wcove.c
index 6770afd40765..f1f8f45e2f3d 100644
--- a/drivers/usb/typec/tcpm/wcove.c
+++ b/drivers/usb/typec/tcpm/wcove.c
@@ -416,12 +416,16 @@ static int wcove_pd_transmit(struct tcpc_dev *tcpc,
 	return regmap_write(wcove->regmap, USBC_TXCMD, cmd | USBC_TXCMD_START);
 }
 
-static int wcove_start_drp_toggling(struct tcpc_dev *tcpc,
-				    enum typec_cc_status cc)
+static int wcove_start_toggling(struct tcpc_dev *tcpc,
+				enum typec_port_type port_type,
+				enum typec_cc_status cc)
 {
 	struct wcove_typec *wcove = tcpc_to_wcove(tcpc);
 	unsigned int usbc_ctrl;
 
+	if (port_type != TYPEC_PORT_DRP)
+		return -EOPNOTSUPP;
+
 	usbc_ctrl = USBC_CONTROL1_MODE_DRP | USBC_CONTROL1_DRPTOGGLE_RANDOM;
 
 	switch (cc) {
@@ -642,7 +646,7 @@ static int wcove_typec_probe(struct platform_device *pdev)
 	wcove->tcpc.set_polarity = wcove_set_polarity;
 	wcove->tcpc.set_vconn = wcove_set_vconn;
 	wcove->tcpc.set_current_limit = wcove_set_current_limit;
-	wcove->tcpc.start_drp_toggling = wcove_start_drp_toggling;
+	wcove->tcpc.start_toggling = wcove_start_toggling;
 
 	wcove->tcpc.set_pd_rx = wcove_set_pd_rx;
 	wcove->tcpc.set_roles = wcove_set_roles;
diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
index 0c532ca3f079..36a15dcadc53 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -121,10 +121,10 @@ struct tcpc_config {
  *		with partner.
  * @set_pd_rx:	Called to enable or disable reception of PD messages
  * @set_roles:	Called to set power and data roles
- * @start_drp_toggling:
- *		Optional; if supported by hardware, called to start DRP
- *		toggling. DRP toggling is stopped automatically if
- *		a connection is established.
+ * @start_toggling:
+ *		Optional; if supported by hardware, called to start dual-role
+ *		toggling or single-role connection detection. Toggling stops
+ *		automatically if a connection is established.
  * @try_role:	Optional; called to set a preferred role
  * @pd_transmit:Called to transmit PD message
  * @mux:	Pointer to multiplexer data
@@ -147,8 +147,9 @@ struct tcpc_dev {
 	int (*set_pd_rx)(struct tcpc_dev *dev, bool on);
 	int (*set_roles)(struct tcpc_dev *dev, bool attached,
 			 enum typec_role role, enum typec_data_role data);
-	int (*start_drp_toggling)(struct tcpc_dev *dev,
-				  enum typec_cc_status cc);
+	int (*start_toggling)(struct tcpc_dev *dev,
+			      enum typec_port_type port_type,
+			      enum typec_cc_status cc);
 	int (*try_role)(struct tcpc_dev *dev, int role);
 	int (*pd_transmit)(struct tcpc_dev *dev, enum tcpm_transmit_type type,
 			   const struct pd_message *msg);
-- 
2.21.0

[v2,2/3] usb: typec: fusb302: Implement start_toggling for all port-types

[Hans de Goede]

When in single-role port mode, we must start single-role toggling to
get an interrupt when a device / cable gets plugged into the port.

This commit modifies the fusb302 start_toggling implementation to
start toggling for all port-types, so that connection-detection works
on single-role ports too.

Fixes: ea3b4d5523bc("usb: typec: fusb302: Resolve fixed power role ...")
Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Adjust for the tcpm core renaming start_drp_toggling to start_toggling,
 instead of adding a new start_srp_connection_detect callback
---
 drivers/usb/typec/tcpm/fusb302.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
index 6d83891cc895..ba030b03d156 100644
--- a/drivers/usb/typec/tcpm/fusb302.c
+++ b/drivers/usb/typec/tcpm/fusb302.c
@@ -882,10 +882,20 @@ static int tcpm_start_toggling(struct tcpc_dev *dev,
 {
 	struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
 						 tcpc_dev);
+	enum toggling_mode mode = TOGGLING_MODE_OFF;
 	int ret = 0;
 
-	if (port_type != TYPEC_PORT_DRP)
-		return -EOPNOTSUPP;
+	switch (port_type) {
+	case TYPEC_PORT_SRC:
+		mode = TOGGLING_MODE_SRC;
+		break;
+	case TYPEC_PORT_SNK:
+		mode = TOGGLING_MODE_SNK;
+		break;
+	case TYPEC_PORT_DRP:
+		mode = TOGGLING_MODE_DRP;
+		break;
+	}
 
 	mutex_lock(&chip->lock);
 	ret = fusb302_set_src_current(chip, cc_src_current[cc]);
@@ -894,7 +904,7 @@ static int tcpm_start_toggling(struct tcpc_dev *dev,
 			    typec_cc_status_name[cc], ret);
 		goto done;
 	}
-	ret = fusb302_set_toggling(chip, TOGGLING_MODE_DRP);
+	ret = fusb302_set_toggling(chip, mode);
 	if (ret < 0) {
 		fusb302_log(chip,
 			    "unable to start drp toggling, ret=%d", ret);

[PATCH v2 2/3] usb: typec: fusb302: Implement start_toggling for all port-types

[Hans de Goede]

When in single-role port mode, we must start single-role toggling to
get an interrupt when a device / cable gets plugged into the port.

This commit modifies the fusb302 start_toggling implementation to
start toggling for all port-types, so that connection-detection works
on single-role ports too.

Fixes: ea3b4d5523bc("usb: typec: fusb302: Resolve fixed power role ...")
Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Adjust for the tcpm core renaming start_drp_toggling to start_toggling,
 instead of adding a new start_srp_connection_detect callback
---
 drivers/usb/typec/tcpm/fusb302.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
index 6d83891cc895..ba030b03d156 100644
--- a/drivers/usb/typec/tcpm/fusb302.c
+++ b/drivers/usb/typec/tcpm/fusb302.c
@@ -882,10 +882,20 @@ static int tcpm_start_toggling(struct tcpc_dev *dev,
 {
 	struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
 						 tcpc_dev);
+	enum toggling_mode mode = TOGGLING_MODE_OFF;
 	int ret = 0;
 
-	if (port_type != TYPEC_PORT_DRP)
-		return -EOPNOTSUPP;
+	switch (port_type) {
+	case TYPEC_PORT_SRC:
+		mode = TOGGLING_MODE_SRC;
+		break;
+	case TYPEC_PORT_SNK:
+		mode = TOGGLING_MODE_SNK;
+		break;
+	case TYPEC_PORT_DRP:
+		mode = TOGGLING_MODE_DRP;
+		break;
+	}
 
 	mutex_lock(&chip->lock);
 	ret = fusb302_set_src_current(chip, cc_src_current[cc]);
@@ -894,7 +904,7 @@ static int tcpm_start_toggling(struct tcpc_dev *dev,
 			    typec_cc_status_name[cc], ret);
 		goto done;
 	}
-	ret = fusb302_set_toggling(chip, TOGGLING_MODE_DRP);
+	ret = fusb302_set_toggling(chip, mode);
 	if (ret < 0) {
 		fusb302_log(chip,
 			    "unable to start drp toggling, ret=%d", ret);
-- 
2.21.0

[v2,3/3] usb: typec: fusb302: Revert "Resolve fixed power role contract setup"

[Hans de Goede]

Some tcpc device-drivers need to explicitly be told to watch for connection
events, otherwise the tcpc will not generate any TCPM_CC_EVENTs and devices
being plugged into the Type-C port will not be noticed.

For dual-role ports tcpm_start_drp_toggling() is used to tell the tcpc to
watch for connection events. But for single-role ports we've so far been
falling back to just calling tcpm_set_cc(). For some tcpc-s such as the
fusb302 this is not enough and no TCPM_CC_EVENT will be generated.

Commit ea3b4d5523bc ("usb: typec: fusb302: Resolve fixed power role
contract setup") fixed SRPs not working because of this by making the
fusb302 driver start connection detection on every tcpm_set_cc() call.
It turns out this breaks src->snk power-role swapping because during the
swap we first set the Cc pins to Rp, calling set_cc, and then send a PS_RDY
message. But the fusb302 cannot send PD messages while its toggling engine
is active, so sending the PS_RDY message fails.

Struct tcpc_dev now has a new start_srp_connection_detect callback and
fusb302.c now implements this. This callback gets called when we the
fusb302 needs to start connection detection, fixing fusb302 SRPs not
seeing connected devices.

This allows us to revert the changes to fusb302's set_cc implementation,
making it once again purely setup the Cc-s and matching disconnect
detection, fixing src->snk power-role swapping no longer working.

Note that since the code was refactored in between, codewise this is not a
straight forward revert. Functionality wise this is a straight revert and
the original functionality is fully restored.

Fixes: ea3b4d5523bc ("usb: typec: fusb302: Resolve fixed power role ...")
Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-No changes
---
 drivers/usb/typec/tcpm/fusb302.c | 55 ++++++++++++++++++++++++++------
 1 file changed, 46 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
index ba030b03d156..4328a6cbfb69 100644
--- a/drivers/usb/typec/tcpm/fusb302.c
+++ b/drivers/usb/typec/tcpm/fusb302.c
@@ -606,19 +606,16 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
 			    FUSB_REG_SWITCHES0_CC2_PU_EN |
 			    FUSB_REG_SWITCHES0_CC1_PD_EN |
 			    FUSB_REG_SWITCHES0_CC2_PD_EN;
-	u8 switches0_data = 0x00;
+	u8 rd_mda, switches0_data = 0x00;
 	int ret = 0;
-	enum toggling_mode mode;
 
 	mutex_lock(&chip->lock);
 	switch (cc) {
 	case TYPEC_CC_OPEN:
-		mode = TOGGLING_MODE_OFF;
 		break;
 	case TYPEC_CC_RD:
 		switches0_data |= FUSB_REG_SWITCHES0_CC1_PD_EN |
 				  FUSB_REG_SWITCHES0_CC2_PD_EN;
-		mode = TOGGLING_MODE_SNK;
 		break;
 	case TYPEC_CC_RP_DEF:
 	case TYPEC_CC_RP_1_5:
@@ -626,7 +623,6 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
 		switches0_data |= (chip->cc_polarity == TYPEC_POLARITY_CC1) ?
 				  FUSB_REG_SWITCHES0_CC1_PU_EN :
 				  FUSB_REG_SWITCHES0_CC2_PU_EN;
-		mode = TOGGLING_MODE_SRC;
 		break;
 	default:
 		fusb302_log(chip, "unsupported cc value %s",
@@ -637,6 +633,12 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
 
 	fusb302_log(chip, "cc := %s", typec_cc_status_name[cc]);
 
+	ret = fusb302_set_toggling(chip, TOGGLING_MODE_OFF);
+	if (ret < 0) {
+		fusb302_log(chip, "cannot set toggling mode, ret=%d", ret);
+		goto done;
+	}
+
 	ret = fusb302_i2c_mask_write(chip, FUSB_REG_SWITCHES0,
 				     switches0_mask, switches0_data);
 	if (ret < 0) {
@@ -655,10 +657,45 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
 		goto done;
 	}
 
-	ret = fusb302_set_toggling(chip, mode);
-	if (ret < 0)
-		fusb302_log(chip, "cannot set toggling mode, ret=%d", ret);
-
+	/* enable/disable interrupts, BC_LVL for SNK and COMP_CHNG for SRC */
+	switch (cc) {
+	case TYPEC_CC_RP_DEF:
+	case TYPEC_CC_RP_1_5:
+	case TYPEC_CC_RP_3_0:
+		rd_mda = rd_mda_value[cc_src_current[cc]];
+		ret = fusb302_i2c_write(chip, FUSB_REG_MEASURE, rd_mda);
+		if (ret < 0) {
+			fusb302_log(chip,
+				    "cannot set SRC measure value, ret=%d",
+				    ret);
+			goto done;
+		}
+		ret = fusb302_i2c_mask_write(chip, FUSB_REG_MASK,
+					     FUSB_REG_MASK_BC_LVL |
+					     FUSB_REG_MASK_COMP_CHNG,
+					     FUSB_REG_MASK_COMP_CHNG);
+		if (ret < 0) {
+			fusb302_log(chip, "cannot set SRC interrupt, ret=%d",
+				    ret);
+			goto done;
+		}
+		chip->intr_comp_chng = true;
+		break;
+	case TYPEC_CC_RD:
+		ret = fusb302_i2c_mask_write(chip, FUSB_REG_MASK,
+					     FUSB_REG_MASK_BC_LVL |
+					     FUSB_REG_MASK_COMP_CHNG,
+					     FUSB_REG_MASK_BC_LVL);
+		if (ret < 0) {
+			fusb302_log(chip, "cannot set SRC interrupt, ret=%d",
+				    ret);
+			goto done;
+		}
+		chip->intr_bc_lvl = true;
+		break;
+	default:
+		break;
+	}
 done:
 	mutex_unlock(&chip->lock);
 

[PATCH v2 3/3] usb: typec: fusb302: Revert "Resolve fixed power role contract setup"

[Hans de Goede]

Some tcpc device-drivers need to explicitly be told to watch for connection
events, otherwise the tcpc will not generate any TCPM_CC_EVENTs and devices
being plugged into the Type-C port will not be noticed.

For dual-role ports tcpm_start_drp_toggling() is used to tell the tcpc to
watch for connection events. But for single-role ports we've so far been
falling back to just calling tcpm_set_cc(). For some tcpc-s such as the
fusb302 this is not enough and no TCPM_CC_EVENT will be generated.

Commit ea3b4d5523bc ("usb: typec: fusb302: Resolve fixed power role
contract setup") fixed SRPs not working because of this by making the
fusb302 driver start connection detection on every tcpm_set_cc() call.
It turns out this breaks src->snk power-role swapping because during the
swap we first set the Cc pins to Rp, calling set_cc, and then send a PS_RDY
message. But the fusb302 cannot send PD messages while its toggling engine
is active, so sending the PS_RDY message fails.

Struct tcpc_dev now has a new start_srp_connection_detect callback and
fusb302.c now implements this. This callback gets called when we the
fusb302 needs to start connection detection, fixing fusb302 SRPs not
seeing connected devices.

This allows us to revert the changes to fusb302's set_cc implementation,
making it once again purely setup the Cc-s and matching disconnect
detection, fixing src->snk power-role swapping no longer working.

Note that since the code was refactored in between, codewise this is not a
straight forward revert. Functionality wise this is a straight revert and
the original functionality is fully restored.

Fixes: ea3b4d5523bc ("usb: typec: fusb302: Resolve fixed power role ...")
Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-No changes
---
 drivers/usb/typec/tcpm/fusb302.c | 55 ++++++++++++++++++++++++++------
 1 file changed, 46 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
index ba030b03d156..4328a6cbfb69 100644
--- a/drivers/usb/typec/tcpm/fusb302.c
+++ b/drivers/usb/typec/tcpm/fusb302.c
@@ -606,19 +606,16 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
 			    FUSB_REG_SWITCHES0_CC2_PU_EN |
 			    FUSB_REG_SWITCHES0_CC1_PD_EN |
 			    FUSB_REG_SWITCHES0_CC2_PD_EN;
-	u8 switches0_data = 0x00;
+	u8 rd_mda, switches0_data = 0x00;
 	int ret = 0;
-	enum toggling_mode mode;
 
 	mutex_lock(&chip->lock);
 	switch (cc) {
 	case TYPEC_CC_OPEN:
-		mode = TOGGLING_MODE_OFF;
 		break;
 	case TYPEC_CC_RD:
 		switches0_data |= FUSB_REG_SWITCHES0_CC1_PD_EN |
 				  FUSB_REG_SWITCHES0_CC2_PD_EN;
-		mode = TOGGLING_MODE_SNK;
 		break;
 	case TYPEC_CC_RP_DEF:
 	case TYPEC_CC_RP_1_5:
@@ -626,7 +623,6 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
 		switches0_data |= (chip->cc_polarity == TYPEC_POLARITY_CC1) ?
 				  FUSB_REG_SWITCHES0_CC1_PU_EN :
 				  FUSB_REG_SWITCHES0_CC2_PU_EN;
-		mode = TOGGLING_MODE_SRC;
 		break;
 	default:
 		fusb302_log(chip, "unsupported cc value %s",
@@ -637,6 +633,12 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
 
 	fusb302_log(chip, "cc := %s", typec_cc_status_name[cc]);
 
+	ret = fusb302_set_toggling(chip, TOGGLING_MODE_OFF);
+	if (ret < 0) {
+		fusb302_log(chip, "cannot set toggling mode, ret=%d", ret);
+		goto done;
+	}
+
 	ret = fusb302_i2c_mask_write(chip, FUSB_REG_SWITCHES0,
 				     switches0_mask, switches0_data);
 	if (ret < 0) {
@@ -655,10 +657,45 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
 		goto done;
 	}
 
-	ret = fusb302_set_toggling(chip, mode);
-	if (ret < 0)
-		fusb302_log(chip, "cannot set toggling mode, ret=%d", ret);
-
+	/* enable/disable interrupts, BC_LVL for SNK and COMP_CHNG for SRC */
+	switch (cc) {
+	case TYPEC_CC_RP_DEF:
+	case TYPEC_CC_RP_1_5:
+	case TYPEC_CC_RP_3_0:
+		rd_mda = rd_mda_value[cc_src_current[cc]];
+		ret = fusb302_i2c_write(chip, FUSB_REG_MEASURE, rd_mda);
+		if (ret < 0) {
+			fusb302_log(chip,
+				    "cannot set SRC measure value, ret=%d",
+				    ret);
+			goto done;
+		}
+		ret = fusb302_i2c_mask_write(chip, FUSB_REG_MASK,
+					     FUSB_REG_MASK_BC_LVL |
+					     FUSB_REG_MASK_COMP_CHNG,
+					     FUSB_REG_MASK_COMP_CHNG);
+		if (ret < 0) {
+			fusb302_log(chip, "cannot set SRC interrupt, ret=%d",
+				    ret);
+			goto done;
+		}
+		chip->intr_comp_chng = true;
+		break;
+	case TYPEC_CC_RD:
+		ret = fusb302_i2c_mask_write(chip, FUSB_REG_MASK,
+					     FUSB_REG_MASK_BC_LVL |
+					     FUSB_REG_MASK_COMP_CHNG,
+					     FUSB_REG_MASK_BC_LVL);
+		if (ret < 0) {
+			fusb302_log(chip, "cannot set SRC interrupt, ret=%d",
+				    ret);
+			goto done;
+		}
+		chip->intr_bc_lvl = true;
+		break;
+	default:
+		break;
+	}
 done:
 	mutex_unlock(&chip->lock);
 
-- 
2.21.0

[v2,1/3] usb: typec: tcpm: Notify the tcpc to start connection-detection for SRPs

[Guenter Roeck]

On 4/16/19 1:07 PM, Hans de Goede wrote:
> Some tcpc device-drivers need to explicitly be told to watch for connection
> events, otherwise the tcpc will not generate any TCPM_CC_EVENTs and devices
> being plugged into the Type-C port will not be noticed.
> 
> For dual-role ports tcpm_start_drp_toggling() is used to tell the tcpc to
> watch for connection events. Sofar we lack a similar callback to the tcpc
> for single-role ports. With some tcpc-s such as the fusb302 this means
> no TCPM_CC_EVENTs will be generated when the port is configured as a
> single-role port.
> 
> This commit renames start_drp_toggling to start_toggling and since the
> device-properties are parsed by the tcpm-core, adds a port_type parameter
> to the start_toggling callback so that the tcpc_dev driver knows the
> port-type and can act accordingly when it starts toggling.
> 
> The new start_toggling callback now always gets called if defined, instead
> of only being called for DRP ports.
> 
> To avoid this causing undesirable functional changes all existing
> start_drp_toggling implementations are not only renamed to start_toggling,
> but also get a port_type check added and return -EOPNOTSUPP when port_type
> is not DRP.
> 
> Fixes: ea3b4d5523bc("usb: typec: fusb302: Resolve fixed power role ...")
> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Rename start_drp_toggling to start_toggling and make it multi-purpose,
>   rather then adding a new start_srp_connection_detect callback
> ---
>   drivers/usb/typec/tcpm/fusb302.c | 10 +++++++---
>   drivers/usb/typec/tcpm/tcpci.c   | 10 +++++++---
>   drivers/usb/typec/tcpm/tcpm.c    | 32 ++++++++++++++------------------
>   drivers/usb/typec/tcpm/wcove.c   | 10 +++++++---
>   include/linux/usb/tcpm.h         | 13 +++++++------
>   5 files changed, 42 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
> index 6ea6199caafa..6d83891cc895 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -876,13 +876,17 @@ static int tcpm_set_roles(struct tcpc_dev *dev, bool attached,
>   	return ret;
>   }
>   
> -static int tcpm_start_drp_toggling(struct tcpc_dev *dev,
> -				   enum typec_cc_status cc)
> +static int tcpm_start_toggling(struct tcpc_dev *dev,
> +			       enum typec_port_type port_type,
> +			       enum typec_cc_status cc)
>   {
>   	struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
>   						 tcpc_dev);
>   	int ret = 0;
>   
> +	if (port_type != TYPEC_PORT_DRP)
> +		return -EOPNOTSUPP;
> +
>   	mutex_lock(&chip->lock);
>   	ret = fusb302_set_src_current(chip, cc_src_current[cc]);
>   	if (ret < 0) {
> @@ -1094,7 +1098,7 @@ static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev)
>   	fusb302_tcpc_dev->set_vbus = tcpm_set_vbus;
>   	fusb302_tcpc_dev->set_pd_rx = tcpm_set_pd_rx;
>   	fusb302_tcpc_dev->set_roles = tcpm_set_roles;
> -	fusb302_tcpc_dev->start_drp_toggling = tcpm_start_drp_toggling;
> +	fusb302_tcpc_dev->start_toggling = tcpm_start_toggling;
>   	fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit;
>   }
>   
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index ac6b418b15f1..c1f7073a56de 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -100,13 +100,17 @@ static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
>   	return 0;
>   }
>   
> -static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc,
> -				    enum typec_cc_status cc)
> +static int tcpci_start_toggling(struct tcpc_dev *tcpc,
> +				enum typec_port_type port_type,
> +				enum typec_cc_status cc)
>   {
>   	int ret;
>   	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
>   	unsigned int reg = TCPC_ROLE_CTRL_DRP;
>   
> +	if (port_type != TYPEC_PORT_DRP)
> +		return -EOPNOTSUPP;
> +
>   	/* Handle vendor drp toggling */
>   	if (tcpci->data->start_drp_toggling) {
>   		ret = tcpci->data->start_drp_toggling(tcpci, tcpci->data, cc);

Wonder if we need to change this as well, but then I guess
we can do this if/when actually needed.

Otherwise

Reviewed-by: Guenter Roeck <linux@roeck-us.net>


> @@ -511,7 +515,7 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
>   	tcpci->tcpc.get_cc = tcpci_get_cc;
>   	tcpci->tcpc.set_polarity = tcpci_set_polarity;
>   	tcpci->tcpc.set_vconn = tcpci_set_vconn;
> -	tcpci->tcpc.start_drp_toggling = tcpci_start_drp_toggling;
> +	tcpci->tcpc.start_toggling = tcpci_start_toggling;
>   
>   	tcpci->tcpc.set_pd_rx = tcpci_set_pd_rx;
>   	tcpci->tcpc.set_roles = tcpci_set_roles;
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index a2233d72ae7c..fba32d84e578 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -31,7 +31,7 @@
>   
>   #define FOREACH_STATE(S)			\
>   	S(INVALID_STATE),			\
> -	S(DRP_TOGGLING),			\
> +	S(TOGGLING),			\
>   	S(SRC_UNATTACHED),			\
>   	S(SRC_ATTACH_WAIT),			\
>   	S(SRC_ATTACHED),			\
> @@ -472,7 +472,7 @@ static void tcpm_log(struct tcpm_port *port, const char *fmt, ...)
>   	/* Do not log while disconnected and unattached */
>   	if (tcpm_port_is_disconnected(port) &&
>   	    (port->state == SRC_UNATTACHED || port->state == SNK_UNATTACHED ||
> -	     port->state == DRP_TOGGLING))
> +	     port->state == TOGGLING))
>   		return;
>   
>   	va_start(args, fmt);
> @@ -2540,20 +2540,16 @@ static int tcpm_set_charge(struct tcpm_port *port, bool charge)
>   	return 0;
>   }
>   
> -static bool tcpm_start_drp_toggling(struct tcpm_port *port,
> -				    enum typec_cc_status cc)
> +static bool tcpm_start_toggling(struct tcpm_port *port, enum typec_cc_status cc)
>   {
>   	int ret;
>   
> -	if (port->tcpc->start_drp_toggling &&
> -	    port->port_type == TYPEC_PORT_DRP) {
> -		tcpm_log_force(port, "Start DRP toggling");
> -		ret = port->tcpc->start_drp_toggling(port->tcpc, cc);
> -		if (!ret)
> -			return true;
> -	}
> +	if (!port->tcpc->start_toggling)
> +		return false;
>   
> -	return false;
> +	tcpm_log_force(port, "Start toggling");
> +	ret = port->tcpc->start_toggling(port->tcpc, port->port_type, cc);
> +	return ret == 0;
>   }
>   
>   static void tcpm_set_cc(struct tcpm_port *port, enum typec_cc_status cc)
> @@ -2847,15 +2843,15 @@ static void run_state_machine(struct tcpm_port *port)
>   
>   	port->enter_state = port->state;
>   	switch (port->state) {
> -	case DRP_TOGGLING:
> +	case TOGGLING:
>   		break;
>   	/* SRC states */
>   	case SRC_UNATTACHED:
>   		if (!port->non_pd_role_swap)
>   			tcpm_swap_complete(port, -ENOTCONN);
>   		tcpm_src_detach(port);
> -		if (tcpm_start_drp_toggling(port, tcpm_rp_cc(port))) {
> -			tcpm_set_state(port, DRP_TOGGLING, 0);
> +		if (tcpm_start_toggling(port, tcpm_rp_cc(port))) {
> +			tcpm_set_state(port, TOGGLING, 0);
>   			break;
>   		}
>   		tcpm_set_cc(port, tcpm_rp_cc(port));
> @@ -3053,8 +3049,8 @@ static void run_state_machine(struct tcpm_port *port)
>   			tcpm_swap_complete(port, -ENOTCONN);
>   		tcpm_pps_complete(port, -ENOTCONN);
>   		tcpm_snk_detach(port);
> -		if (tcpm_start_drp_toggling(port, TYPEC_CC_RD)) {
> -			tcpm_set_state(port, DRP_TOGGLING, 0);
> +		if (tcpm_start_toggling(port, TYPEC_CC_RD)) {
> +			tcpm_set_state(port, TOGGLING, 0);
>   			break;
>   		}
>   		tcpm_set_cc(port, TYPEC_CC_RD);
> @@ -3621,7 +3617,7 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1,
>   						       : "connected");
>   
>   	switch (port->state) {
> -	case DRP_TOGGLING:
> +	case TOGGLING:
>   		if (tcpm_port_is_debug(port) || tcpm_port_is_audio(port) ||
>   		    tcpm_port_is_source(port))
>   			tcpm_set_state(port, SRC_ATTACH_WAIT, 0);
> diff --git a/drivers/usb/typec/tcpm/wcove.c b/drivers/usb/typec/tcpm/wcove.c
> index 6770afd40765..f1f8f45e2f3d 100644
> --- a/drivers/usb/typec/tcpm/wcove.c
> +++ b/drivers/usb/typec/tcpm/wcove.c
> @@ -416,12 +416,16 @@ static int wcove_pd_transmit(struct tcpc_dev *tcpc,
>   	return regmap_write(wcove->regmap, USBC_TXCMD, cmd | USBC_TXCMD_START);
>   }
>   
> -static int wcove_start_drp_toggling(struct tcpc_dev *tcpc,
> -				    enum typec_cc_status cc)
> +static int wcove_start_toggling(struct tcpc_dev *tcpc,
> +				enum typec_port_type port_type,
> +				enum typec_cc_status cc)
>   {
>   	struct wcove_typec *wcove = tcpc_to_wcove(tcpc);
>   	unsigned int usbc_ctrl;
>   
> +	if (port_type != TYPEC_PORT_DRP)
> +		return -EOPNOTSUPP;
> +
>   	usbc_ctrl = USBC_CONTROL1_MODE_DRP | USBC_CONTROL1_DRPTOGGLE_RANDOM;
>   
>   	switch (cc) {
> @@ -642,7 +646,7 @@ static int wcove_typec_probe(struct platform_device *pdev)
>   	wcove->tcpc.set_polarity = wcove_set_polarity;
>   	wcove->tcpc.set_vconn = wcove_set_vconn;
>   	wcove->tcpc.set_current_limit = wcove_set_current_limit;
> -	wcove->tcpc.start_drp_toggling = wcove_start_drp_toggling;
> +	wcove->tcpc.start_toggling = wcove_start_toggling;
>   
>   	wcove->tcpc.set_pd_rx = wcove_set_pd_rx;
>   	wcove->tcpc.set_roles = wcove_set_roles;
> diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
> index 0c532ca3f079..36a15dcadc53 100644
> --- a/include/linux/usb/tcpm.h
> +++ b/include/linux/usb/tcpm.h
> @@ -121,10 +121,10 @@ struct tcpc_config {
>    *		with partner.
>    * @set_pd_rx:	Called to enable or disable reception of PD messages
>    * @set_roles:	Called to set power and data roles
> - * @start_drp_toggling:
> - *		Optional; if supported by hardware, called to start DRP
> - *		toggling. DRP toggling is stopped automatically if
> - *		a connection is established.
> + * @start_toggling:
> + *		Optional; if supported by hardware, called to start dual-role
> + *		toggling or single-role connection detection. Toggling stops
> + *		automatically if a connection is established.
>    * @try_role:	Optional; called to set a preferred role
>    * @pd_transmit:Called to transmit PD message
>    * @mux:	Pointer to multiplexer data
> @@ -147,8 +147,9 @@ struct tcpc_dev {
>   	int (*set_pd_rx)(struct tcpc_dev *dev, bool on);
>   	int (*set_roles)(struct tcpc_dev *dev, bool attached,
>   			 enum typec_role role, enum typec_data_role data);
> -	int (*start_drp_toggling)(struct tcpc_dev *dev,
> -				  enum typec_cc_status cc);
> +	int (*start_toggling)(struct tcpc_dev *dev,
> +			      enum typec_port_type port_type,
> +			      enum typec_cc_status cc);
>   	int (*try_role)(struct tcpc_dev *dev, int role);
>   	int (*pd_transmit)(struct tcpc_dev *dev, enum tcpm_transmit_type type,
>   			   const struct pd_message *msg);
>

Re: [PATCH v2 1/3] usb: typec: tcpm: Notify the tcpc to start connection-detection for SRPs

[Guenter Roeck]

On 4/16/19 1:07 PM, Hans de Goede wrote:
> Some tcpc device-drivers need to explicitly be told to watch for connection
> events, otherwise the tcpc will not generate any TCPM_CC_EVENTs and devices
> being plugged into the Type-C port will not be noticed.
> 
> For dual-role ports tcpm_start_drp_toggling() is used to tell the tcpc to
> watch for connection events. Sofar we lack a similar callback to the tcpc
> for single-role ports. With some tcpc-s such as the fusb302 this means
> no TCPM_CC_EVENTs will be generated when the port is configured as a
> single-role port.
> 
> This commit renames start_drp_toggling to start_toggling and since the
> device-properties are parsed by the tcpm-core, adds a port_type parameter
> to the start_toggling callback so that the tcpc_dev driver knows the
> port-type and can act accordingly when it starts toggling.
> 
> The new start_toggling callback now always gets called if defined, instead
> of only being called for DRP ports.
> 
> To avoid this causing undesirable functional changes all existing
> start_drp_toggling implementations are not only renamed to start_toggling,
> but also get a port_type check added and return -EOPNOTSUPP when port_type
> is not DRP.
> 
> Fixes: ea3b4d5523bc("usb: typec: fusb302: Resolve fixed power role ...")
> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Rename start_drp_toggling to start_toggling and make it multi-purpose,
>   rather then adding a new start_srp_connection_detect callback
> ---
>   drivers/usb/typec/tcpm/fusb302.c | 10 +++++++---
>   drivers/usb/typec/tcpm/tcpci.c   | 10 +++++++---
>   drivers/usb/typec/tcpm/tcpm.c    | 32 ++++++++++++++------------------
>   drivers/usb/typec/tcpm/wcove.c   | 10 +++++++---
>   include/linux/usb/tcpm.h         | 13 +++++++------
>   5 files changed, 42 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
> index 6ea6199caafa..6d83891cc895 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -876,13 +876,17 @@ static int tcpm_set_roles(struct tcpc_dev *dev, bool attached,
>   	return ret;
>   }
>   
> -static int tcpm_start_drp_toggling(struct tcpc_dev *dev,
> -				   enum typec_cc_status cc)
> +static int tcpm_start_toggling(struct tcpc_dev *dev,
> +			       enum typec_port_type port_type,
> +			       enum typec_cc_status cc)
>   {
>   	struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
>   						 tcpc_dev);
>   	int ret = 0;
>   
> +	if (port_type != TYPEC_PORT_DRP)
> +		return -EOPNOTSUPP;
> +
>   	mutex_lock(&chip->lock);
>   	ret = fusb302_set_src_current(chip, cc_src_current[cc]);
>   	if (ret < 0) {
> @@ -1094,7 +1098,7 @@ static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev)
>   	fusb302_tcpc_dev->set_vbus = tcpm_set_vbus;
>   	fusb302_tcpc_dev->set_pd_rx = tcpm_set_pd_rx;
>   	fusb302_tcpc_dev->set_roles = tcpm_set_roles;
> -	fusb302_tcpc_dev->start_drp_toggling = tcpm_start_drp_toggling;
> +	fusb302_tcpc_dev->start_toggling = tcpm_start_toggling;
>   	fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit;
>   }
>   
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index ac6b418b15f1..c1f7073a56de 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -100,13 +100,17 @@ static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
>   	return 0;
>   }
>   
> -static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc,
> -				    enum typec_cc_status cc)
> +static int tcpci_start_toggling(struct tcpc_dev *tcpc,
> +				enum typec_port_type port_type,
> +				enum typec_cc_status cc)
>   {
>   	int ret;
>   	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
>   	unsigned int reg = TCPC_ROLE_CTRL_DRP;
>   
> +	if (port_type != TYPEC_PORT_DRP)
> +		return -EOPNOTSUPP;
> +
>   	/* Handle vendor drp toggling */
>   	if (tcpci->data->start_drp_toggling) {
>   		ret = tcpci->data->start_drp_toggling(tcpci, tcpci->data, cc);

Wonder if we need to change this as well, but then I guess
we can do this if/when actually needed.

Otherwise

Reviewed-by: Guenter Roeck <linux@roeck-us.net>


> @@ -511,7 +515,7 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
>   	tcpci->tcpc.get_cc = tcpci_get_cc;
>   	tcpci->tcpc.set_polarity = tcpci_set_polarity;
>   	tcpci->tcpc.set_vconn = tcpci_set_vconn;
> -	tcpci->tcpc.start_drp_toggling = tcpci_start_drp_toggling;
> +	tcpci->tcpc.start_toggling = tcpci_start_toggling;
>   
>   	tcpci->tcpc.set_pd_rx = tcpci_set_pd_rx;
>   	tcpci->tcpc.set_roles = tcpci_set_roles;
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index a2233d72ae7c..fba32d84e578 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -31,7 +31,7 @@
>   
>   #define FOREACH_STATE(S)			\
>   	S(INVALID_STATE),			\
> -	S(DRP_TOGGLING),			\
> +	S(TOGGLING),			\
>   	S(SRC_UNATTACHED),			\
>   	S(SRC_ATTACH_WAIT),			\
>   	S(SRC_ATTACHED),			\
> @@ -472,7 +472,7 @@ static void tcpm_log(struct tcpm_port *port, const char *fmt, ...)
>   	/* Do not log while disconnected and unattached */
>   	if (tcpm_port_is_disconnected(port) &&
>   	    (port->state == SRC_UNATTACHED || port->state == SNK_UNATTACHED ||
> -	     port->state == DRP_TOGGLING))
> +	     port->state == TOGGLING))
>   		return;
>   
>   	va_start(args, fmt);
> @@ -2540,20 +2540,16 @@ static int tcpm_set_charge(struct tcpm_port *port, bool charge)
>   	return 0;
>   }
>   
> -static bool tcpm_start_drp_toggling(struct tcpm_port *port,
> -				    enum typec_cc_status cc)
> +static bool tcpm_start_toggling(struct tcpm_port *port, enum typec_cc_status cc)
>   {
>   	int ret;
>   
> -	if (port->tcpc->start_drp_toggling &&
> -	    port->port_type == TYPEC_PORT_DRP) {
> -		tcpm_log_force(port, "Start DRP toggling");
> -		ret = port->tcpc->start_drp_toggling(port->tcpc, cc);
> -		if (!ret)
> -			return true;
> -	}
> +	if (!port->tcpc->start_toggling)
> +		return false;
>   
> -	return false;
> +	tcpm_log_force(port, "Start toggling");
> +	ret = port->tcpc->start_toggling(port->tcpc, port->port_type, cc);
> +	return ret == 0;
>   }
>   
>   static void tcpm_set_cc(struct tcpm_port *port, enum typec_cc_status cc)
> @@ -2847,15 +2843,15 @@ static void run_state_machine(struct tcpm_port *port)
>   
>   	port->enter_state = port->state;
>   	switch (port->state) {
> -	case DRP_TOGGLING:
> +	case TOGGLING:
>   		break;
>   	/* SRC states */
>   	case SRC_UNATTACHED:
>   		if (!port->non_pd_role_swap)
>   			tcpm_swap_complete(port, -ENOTCONN);
>   		tcpm_src_detach(port);
> -		if (tcpm_start_drp_toggling(port, tcpm_rp_cc(port))) {
> -			tcpm_set_state(port, DRP_TOGGLING, 0);
> +		if (tcpm_start_toggling(port, tcpm_rp_cc(port))) {
> +			tcpm_set_state(port, TOGGLING, 0);
>   			break;
>   		}
>   		tcpm_set_cc(port, tcpm_rp_cc(port));
> @@ -3053,8 +3049,8 @@ static void run_state_machine(struct tcpm_port *port)
>   			tcpm_swap_complete(port, -ENOTCONN);
>   		tcpm_pps_complete(port, -ENOTCONN);
>   		tcpm_snk_detach(port);
> -		if (tcpm_start_drp_toggling(port, TYPEC_CC_RD)) {
> -			tcpm_set_state(port, DRP_TOGGLING, 0);
> +		if (tcpm_start_toggling(port, TYPEC_CC_RD)) {
> +			tcpm_set_state(port, TOGGLING, 0);
>   			break;
>   		}
>   		tcpm_set_cc(port, TYPEC_CC_RD);
> @@ -3621,7 +3617,7 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1,
>   						       : "connected");
>   
>   	switch (port->state) {
> -	case DRP_TOGGLING:
> +	case TOGGLING:
>   		if (tcpm_port_is_debug(port) || tcpm_port_is_audio(port) ||
>   		    tcpm_port_is_source(port))
>   			tcpm_set_state(port, SRC_ATTACH_WAIT, 0);
> diff --git a/drivers/usb/typec/tcpm/wcove.c b/drivers/usb/typec/tcpm/wcove.c
> index 6770afd40765..f1f8f45e2f3d 100644
> --- a/drivers/usb/typec/tcpm/wcove.c
> +++ b/drivers/usb/typec/tcpm/wcove.c
> @@ -416,12 +416,16 @@ static int wcove_pd_transmit(struct tcpc_dev *tcpc,
>   	return regmap_write(wcove->regmap, USBC_TXCMD, cmd | USBC_TXCMD_START);
>   }
>   
> -static int wcove_start_drp_toggling(struct tcpc_dev *tcpc,
> -				    enum typec_cc_status cc)
> +static int wcove_start_toggling(struct tcpc_dev *tcpc,
> +				enum typec_port_type port_type,
> +				enum typec_cc_status cc)
>   {
>   	struct wcove_typec *wcove = tcpc_to_wcove(tcpc);
>   	unsigned int usbc_ctrl;
>   
> +	if (port_type != TYPEC_PORT_DRP)
> +		return -EOPNOTSUPP;
> +
>   	usbc_ctrl = USBC_CONTROL1_MODE_DRP | USBC_CONTROL1_DRPTOGGLE_RANDOM;
>   
>   	switch (cc) {
> @@ -642,7 +646,7 @@ static int wcove_typec_probe(struct platform_device *pdev)
>   	wcove->tcpc.set_polarity = wcove_set_polarity;
>   	wcove->tcpc.set_vconn = wcove_set_vconn;
>   	wcove->tcpc.set_current_limit = wcove_set_current_limit;
> -	wcove->tcpc.start_drp_toggling = wcove_start_drp_toggling;
> +	wcove->tcpc.start_toggling = wcove_start_toggling;
>   
>   	wcove->tcpc.set_pd_rx = wcove_set_pd_rx;
>   	wcove->tcpc.set_roles = wcove_set_roles;
> diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
> index 0c532ca3f079..36a15dcadc53 100644
> --- a/include/linux/usb/tcpm.h
> +++ b/include/linux/usb/tcpm.h
> @@ -121,10 +121,10 @@ struct tcpc_config {
>    *		with partner.
>    * @set_pd_rx:	Called to enable or disable reception of PD messages
>    * @set_roles:	Called to set power and data roles
> - * @start_drp_toggling:
> - *		Optional; if supported by hardware, called to start DRP
> - *		toggling. DRP toggling is stopped automatically if
> - *		a connection is established.
> + * @start_toggling:
> + *		Optional; if supported by hardware, called to start dual-role
> + *		toggling or single-role connection detection. Toggling stops
> + *		automatically if a connection is established.
>    * @try_role:	Optional; called to set a preferred role
>    * @pd_transmit:Called to transmit PD message
>    * @mux:	Pointer to multiplexer data
> @@ -147,8 +147,9 @@ struct tcpc_dev {
>   	int (*set_pd_rx)(struct tcpc_dev *dev, bool on);
>   	int (*set_roles)(struct tcpc_dev *dev, bool attached,
>   			 enum typec_role role, enum typec_data_role data);
> -	int (*start_drp_toggling)(struct tcpc_dev *dev,
> -				  enum typec_cc_status cc);
> +	int (*start_toggling)(struct tcpc_dev *dev,
> +			      enum typec_port_type port_type,
> +			      enum typec_cc_status cc);
>   	int (*try_role)(struct tcpc_dev *dev, int role);
>   	int (*pd_transmit)(struct tcpc_dev *dev, enum tcpm_transmit_type type,
>   			   const struct pd_message *msg);
> 

[v2,2/3] usb: typec: fusb302: Implement start_toggling for all port-types

[Guenter Roeck]

On 4/16/19 1:07 PM, Hans de Goede wrote:
> When in single-role port mode, we must start single-role toggling to
> get an interrupt when a device / cable gets plugged into the port.
> 
> This commit modifies the fusb302 start_toggling implementation to
> start toggling for all port-types, so that connection-detection works
> on single-role ports too.
> 
> Fixes: ea3b4d5523bc("usb: typec: fusb302: Resolve fixed power role ...")
> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> Changes in v2:
> -Adjust for the tcpm core renaming start_drp_toggling to start_toggling,
>   instead of adding a new start_srp_connection_detect callback
> ---
>   drivers/usb/typec/tcpm/fusb302.c | 16 +++++++++++++---
>   1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
> index 6d83891cc895..ba030b03d156 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -882,10 +882,20 @@ static int tcpm_start_toggling(struct tcpc_dev *dev,
>   {
>   	struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
>   						 tcpc_dev);
> +	enum toggling_mode mode = TOGGLING_MODE_OFF;
>   	int ret = 0;
>   
> -	if (port_type != TYPEC_PORT_DRP)
> -		return -EOPNOTSUPP;
> +	switch (port_type) {
> +	case TYPEC_PORT_SRC:
> +		mode = TOGGLING_MODE_SRC;
> +		break;
> +	case TYPEC_PORT_SNK:
> +		mode = TOGGLING_MODE_SNK;
> +		break;
> +	case TYPEC_PORT_DRP:
> +		mode = TOGGLING_MODE_DRP;
> +		break;
> +	}
>   
>   	mutex_lock(&chip->lock);
>   	ret = fusb302_set_src_current(chip, cc_src_current[cc]);
> @@ -894,7 +904,7 @@ static int tcpm_start_toggling(struct tcpc_dev *dev,
>   			    typec_cc_status_name[cc], ret);
>   		goto done;
>   	}
> -	ret = fusb302_set_toggling(chip, TOGGLING_MODE_DRP);
> +	ret = fusb302_set_toggling(chip, mode);
>   	if (ret < 0) {
>   		fusb302_log(chip,
>   			    "unable to start drp toggling, ret=%d", ret);
>

Re: [PATCH v2 2/3] usb: typec: fusb302: Implement start_toggling for all port-types

[Guenter Roeck]

On 4/16/19 1:07 PM, Hans de Goede wrote:
> When in single-role port mode, we must start single-role toggling to
> get an interrupt when a device / cable gets plugged into the port.
> 
> This commit modifies the fusb302 start_toggling implementation to
> start toggling for all port-types, so that connection-detection works
> on single-role ports too.
> 
> Fixes: ea3b4d5523bc("usb: typec: fusb302: Resolve fixed power role ...")
> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> Changes in v2:
> -Adjust for the tcpm core renaming start_drp_toggling to start_toggling,
>   instead of adding a new start_srp_connection_detect callback
> ---
>   drivers/usb/typec/tcpm/fusb302.c | 16 +++++++++++++---
>   1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
> index 6d83891cc895..ba030b03d156 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -882,10 +882,20 @@ static int tcpm_start_toggling(struct tcpc_dev *dev,
>   {
>   	struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
>   						 tcpc_dev);
> +	enum toggling_mode mode = TOGGLING_MODE_OFF;
>   	int ret = 0;
>   
> -	if (port_type != TYPEC_PORT_DRP)
> -		return -EOPNOTSUPP;
> +	switch (port_type) {
> +	case TYPEC_PORT_SRC:
> +		mode = TOGGLING_MODE_SRC;
> +		break;
> +	case TYPEC_PORT_SNK:
> +		mode = TOGGLING_MODE_SNK;
> +		break;
> +	case TYPEC_PORT_DRP:
> +		mode = TOGGLING_MODE_DRP;
> +		break;
> +	}
>   
>   	mutex_lock(&chip->lock);
>   	ret = fusb302_set_src_current(chip, cc_src_current[cc]);
> @@ -894,7 +904,7 @@ static int tcpm_start_toggling(struct tcpc_dev *dev,
>   			    typec_cc_status_name[cc], ret);
>   		goto done;
>   	}
> -	ret = fusb302_set_toggling(chip, TOGGLING_MODE_DRP);
> +	ret = fusb302_set_toggling(chip, mode);
>   	if (ret < 0) {
>   		fusb302_log(chip,
>   			    "unable to start drp toggling, ret=%d", ret);
> 

[v2,3/3] usb: typec: fusb302: Revert "Resolve fixed power role contract setup"

[Guenter Roeck]

On 4/16/19 1:07 PM, Hans de Goede wrote:
> Some tcpc device-drivers need to explicitly be told to watch for connection
> events, otherwise the tcpc will not generate any TCPM_CC_EVENTs and devices
> being plugged into the Type-C port will not be noticed.
> 
> For dual-role ports tcpm_start_drp_toggling() is used to tell the tcpc to
> watch for connection events. But for single-role ports we've so far been
> falling back to just calling tcpm_set_cc(). For some tcpc-s such as the
> fusb302 this is not enough and no TCPM_CC_EVENT will be generated.
> 
> Commit ea3b4d5523bc ("usb: typec: fusb302: Resolve fixed power role
> contract setup") fixed SRPs not working because of this by making the
> fusb302 driver start connection detection on every tcpm_set_cc() call.
> It turns out this breaks src->snk power-role swapping because during the
> swap we first set the Cc pins to Rp, calling set_cc, and then send a PS_RDY
> message. But the fusb302 cannot send PD messages while its toggling engine
> is active, so sending the PS_RDY message fails.
> 
> Struct tcpc_dev now has a new start_srp_connection_detect callback and
> fusb302.c now implements this. This callback gets called when we the
> fusb302 needs to start connection detection, fixing fusb302 SRPs not
> seeing connected devices.
> 
> This allows us to revert the changes to fusb302's set_cc implementation,
> making it once again purely setup the Cc-s and matching disconnect
> detection, fixing src->snk power-role swapping no longer working.
> 
> Note that since the code was refactored in between, codewise this is not a
> straight forward revert. Functionality wise this is a straight revert and
> the original functionality is fully restored.
> 
> Fixes: ea3b4d5523bc ("usb: typec: fusb302: Resolve fixed power role ...")
> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> Changes in v2:
> -No changes
> ---
>   drivers/usb/typec/tcpm/fusb302.c | 55 ++++++++++++++++++++++++++------
>   1 file changed, 46 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
> index ba030b03d156..4328a6cbfb69 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -606,19 +606,16 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
>   			    FUSB_REG_SWITCHES0_CC2_PU_EN |
>   			    FUSB_REG_SWITCHES0_CC1_PD_EN |
>   			    FUSB_REG_SWITCHES0_CC2_PD_EN;
> -	u8 switches0_data = 0x00;
> +	u8 rd_mda, switches0_data = 0x00;
>   	int ret = 0;
> -	enum toggling_mode mode;
>   
>   	mutex_lock(&chip->lock);
>   	switch (cc) {
>   	case TYPEC_CC_OPEN:
> -		mode = TOGGLING_MODE_OFF;
>   		break;
>   	case TYPEC_CC_RD:
>   		switches0_data |= FUSB_REG_SWITCHES0_CC1_PD_EN |
>   				  FUSB_REG_SWITCHES0_CC2_PD_EN;
> -		mode = TOGGLING_MODE_SNK;
>   		break;
>   	case TYPEC_CC_RP_DEF:
>   	case TYPEC_CC_RP_1_5:
> @@ -626,7 +623,6 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
>   		switches0_data |= (chip->cc_polarity == TYPEC_POLARITY_CC1) ?
>   				  FUSB_REG_SWITCHES0_CC1_PU_EN :
>   				  FUSB_REG_SWITCHES0_CC2_PU_EN;
> -		mode = TOGGLING_MODE_SRC;
>   		break;
>   	default:
>   		fusb302_log(chip, "unsupported cc value %s",
> @@ -637,6 +633,12 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
>   
>   	fusb302_log(chip, "cc := %s", typec_cc_status_name[cc]);
>   
> +	ret = fusb302_set_toggling(chip, TOGGLING_MODE_OFF);
> +	if (ret < 0) {
> +		fusb302_log(chip, "cannot set toggling mode, ret=%d", ret);
> +		goto done;
> +	}
> +
>   	ret = fusb302_i2c_mask_write(chip, FUSB_REG_SWITCHES0,
>   				     switches0_mask, switches0_data);
>   	if (ret < 0) {
> @@ -655,10 +657,45 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
>   		goto done;
>   	}
>   
> -	ret = fusb302_set_toggling(chip, mode);
> -	if (ret < 0)
> -		fusb302_log(chip, "cannot set toggling mode, ret=%d", ret);
> -
> +	/* enable/disable interrupts, BC_LVL for SNK and COMP_CHNG for SRC */
> +	switch (cc) {
> +	case TYPEC_CC_RP_DEF:
> +	case TYPEC_CC_RP_1_5:
> +	case TYPEC_CC_RP_3_0:
> +		rd_mda = rd_mda_value[cc_src_current[cc]];
> +		ret = fusb302_i2c_write(chip, FUSB_REG_MEASURE, rd_mda);
> +		if (ret < 0) {
> +			fusb302_log(chip,
> +				    "cannot set SRC measure value, ret=%d",
> +				    ret);
> +			goto done;
> +		}
> +		ret = fusb302_i2c_mask_write(chip, FUSB_REG_MASK,
> +					     FUSB_REG_MASK_BC_LVL |
> +					     FUSB_REG_MASK_COMP_CHNG,
> +					     FUSB_REG_MASK_COMP_CHNG);
> +		if (ret < 0) {
> +			fusb302_log(chip, "cannot set SRC interrupt, ret=%d",
> +				    ret);
> +			goto done;
> +		}
> +		chip->intr_comp_chng = true;
> +		break;
> +	case TYPEC_CC_RD:
> +		ret = fusb302_i2c_mask_write(chip, FUSB_REG_MASK,
> +					     FUSB_REG_MASK_BC_LVL |
> +					     FUSB_REG_MASK_COMP_CHNG,
> +					     FUSB_REG_MASK_BC_LVL);
> +		if (ret < 0) {
> +			fusb302_log(chip, "cannot set SRC interrupt, ret=%d",
> +				    ret);
> +			goto done;
> +		}
> +		chip->intr_bc_lvl = true;
> +		break;
> +	default:
> +		break;
> +	}
>   done:
>   	mutex_unlock(&chip->lock);
>   
>

Re: [PATCH v2 3/3] usb: typec: fusb302: Revert "Resolve fixed power role contract setup"

[Guenter Roeck]

On 4/16/19 1:07 PM, Hans de Goede wrote:
> Some tcpc device-drivers need to explicitly be told to watch for connection
> events, otherwise the tcpc will not generate any TCPM_CC_EVENTs and devices
> being plugged into the Type-C port will not be noticed.
> 
> For dual-role ports tcpm_start_drp_toggling() is used to tell the tcpc to
> watch for connection events. But for single-role ports we've so far been
> falling back to just calling tcpm_set_cc(). For some tcpc-s such as the
> fusb302 this is not enough and no TCPM_CC_EVENT will be generated.
> 
> Commit ea3b4d5523bc ("usb: typec: fusb302: Resolve fixed power role
> contract setup") fixed SRPs not working because of this by making the
> fusb302 driver start connection detection on every tcpm_set_cc() call.
> It turns out this breaks src->snk power-role swapping because during the
> swap we first set the Cc pins to Rp, calling set_cc, and then send a PS_RDY
> message. But the fusb302 cannot send PD messages while its toggling engine
> is active, so sending the PS_RDY message fails.
> 
> Struct tcpc_dev now has a new start_srp_connection_detect callback and
> fusb302.c now implements this. This callback gets called when we the
> fusb302 needs to start connection detection, fixing fusb302 SRPs not
> seeing connected devices.
> 
> This allows us to revert the changes to fusb302's set_cc implementation,
> making it once again purely setup the Cc-s and matching disconnect
> detection, fixing src->snk power-role swapping no longer working.
> 
> Note that since the code was refactored in between, codewise this is not a
> straight forward revert. Functionality wise this is a straight revert and
> the original functionality is fully restored.
> 
> Fixes: ea3b4d5523bc ("usb: typec: fusb302: Resolve fixed power role ...")
> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> Changes in v2:
> -No changes
> ---
>   drivers/usb/typec/tcpm/fusb302.c | 55 ++++++++++++++++++++++++++------
>   1 file changed, 46 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
> index ba030b03d156..4328a6cbfb69 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -606,19 +606,16 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
>   			    FUSB_REG_SWITCHES0_CC2_PU_EN |
>   			    FUSB_REG_SWITCHES0_CC1_PD_EN |
>   			    FUSB_REG_SWITCHES0_CC2_PD_EN;
> -	u8 switches0_data = 0x00;
> +	u8 rd_mda, switches0_data = 0x00;
>   	int ret = 0;
> -	enum toggling_mode mode;
>   
>   	mutex_lock(&chip->lock);
>   	switch (cc) {
>   	case TYPEC_CC_OPEN:
> -		mode = TOGGLING_MODE_OFF;
>   		break;
>   	case TYPEC_CC_RD:
>   		switches0_data |= FUSB_REG_SWITCHES0_CC1_PD_EN |
>   				  FUSB_REG_SWITCHES0_CC2_PD_EN;
> -		mode = TOGGLING_MODE_SNK;
>   		break;
>   	case TYPEC_CC_RP_DEF:
>   	case TYPEC_CC_RP_1_5:
> @@ -626,7 +623,6 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
>   		switches0_data |= (chip->cc_polarity == TYPEC_POLARITY_CC1) ?
>   				  FUSB_REG_SWITCHES0_CC1_PU_EN :
>   				  FUSB_REG_SWITCHES0_CC2_PU_EN;
> -		mode = TOGGLING_MODE_SRC;
>   		break;
>   	default:
>   		fusb302_log(chip, "unsupported cc value %s",
> @@ -637,6 +633,12 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
>   
>   	fusb302_log(chip, "cc := %s", typec_cc_status_name[cc]);
>   
> +	ret = fusb302_set_toggling(chip, TOGGLING_MODE_OFF);
> +	if (ret < 0) {
> +		fusb302_log(chip, "cannot set toggling mode, ret=%d", ret);
> +		goto done;
> +	}
> +
>   	ret = fusb302_i2c_mask_write(chip, FUSB_REG_SWITCHES0,
>   				     switches0_mask, switches0_data);
>   	if (ret < 0) {
> @@ -655,10 +657,45 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
>   		goto done;
>   	}
>   
> -	ret = fusb302_set_toggling(chip, mode);
> -	if (ret < 0)
> -		fusb302_log(chip, "cannot set toggling mode, ret=%d", ret);
> -
> +	/* enable/disable interrupts, BC_LVL for SNK and COMP_CHNG for SRC */
> +	switch (cc) {
> +	case TYPEC_CC_RP_DEF:
> +	case TYPEC_CC_RP_1_5:
> +	case TYPEC_CC_RP_3_0:
> +		rd_mda = rd_mda_value[cc_src_current[cc]];
> +		ret = fusb302_i2c_write(chip, FUSB_REG_MEASURE, rd_mda);
> +		if (ret < 0) {
> +			fusb302_log(chip,
> +				    "cannot set SRC measure value, ret=%d",
> +				    ret);
> +			goto done;
> +		}
> +		ret = fusb302_i2c_mask_write(chip, FUSB_REG_MASK,
> +					     FUSB_REG_MASK_BC_LVL |
> +					     FUSB_REG_MASK_COMP_CHNG,
> +					     FUSB_REG_MASK_COMP_CHNG);
> +		if (ret < 0) {
> +			fusb302_log(chip, "cannot set SRC interrupt, ret=%d",
> +				    ret);
> +			goto done;
> +		}
> +		chip->intr_comp_chng = true;
> +		break;
> +	case TYPEC_CC_RD:
> +		ret = fusb302_i2c_mask_write(chip, FUSB_REG_MASK,
> +					     FUSB_REG_MASK_BC_LVL |
> +					     FUSB_REG_MASK_COMP_CHNG,
> +					     FUSB_REG_MASK_BC_LVL);
> +		if (ret < 0) {
> +			fusb302_log(chip, "cannot set SRC interrupt, ret=%d",
> +				    ret);
> +			goto done;
> +		}
> +		chip->intr_bc_lvl = true;
> +		break;
> +	default:
> +		break;
> +	}
>   done:
>   	mutex_unlock(&chip->lock);
>   
> 

[v2,1/3] usb: typec: tcpm: Notify the tcpc to start connection-detection for SRPs

[Heikki Krogerus]

On Tue, Apr 16, 2019 at 10:07:52PM +0200, Hans de Goede wrote:
> Some tcpc device-drivers need to explicitly be told to watch for connection
> events, otherwise the tcpc will not generate any TCPM_CC_EVENTs and devices
> being plugged into the Type-C port will not be noticed.
> 
> For dual-role ports tcpm_start_drp_toggling() is used to tell the tcpc to
> watch for connection events. Sofar we lack a similar callback to the tcpc
> for single-role ports. With some tcpc-s such as the fusb302 this means
> no TCPM_CC_EVENTs will be generated when the port is configured as a
> single-role port.
> 
> This commit renames start_drp_toggling to start_toggling and since the
> device-properties are parsed by the tcpm-core, adds a port_type parameter
> to the start_toggling callback so that the tcpc_dev driver knows the
> port-type and can act accordingly when it starts toggling.
> 
> The new start_toggling callback now always gets called if defined, instead
> of only being called for DRP ports.
> 
> To avoid this causing undesirable functional changes all existing
> start_drp_toggling implementations are not only renamed to start_toggling,
> but also get a port_type check added and return -EOPNOTSUPP when port_type
> is not DRP.
> 
> Fixes: ea3b4d5523bc("usb: typec: fusb302: Resolve fixed power role ...")
> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> Changes in v2:
> -Rename start_drp_toggling to start_toggling and make it multi-purpose,
>  rather then adding a new start_srp_connection_detect callback
> ---
>  drivers/usb/typec/tcpm/fusb302.c | 10 +++++++---
>  drivers/usb/typec/tcpm/tcpci.c   | 10 +++++++---
>  drivers/usb/typec/tcpm/tcpm.c    | 32 ++++++++++++++------------------
>  drivers/usb/typec/tcpm/wcove.c   | 10 +++++++---
>  include/linux/usb/tcpm.h         | 13 +++++++------
>  5 files changed, 42 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
> index 6ea6199caafa..6d83891cc895 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -876,13 +876,17 @@ static int tcpm_set_roles(struct tcpc_dev *dev, bool attached,
>  	return ret;
>  }
>  
> -static int tcpm_start_drp_toggling(struct tcpc_dev *dev,
> -				   enum typec_cc_status cc)
> +static int tcpm_start_toggling(struct tcpc_dev *dev,
> +			       enum typec_port_type port_type,
> +			       enum typec_cc_status cc)
>  {
>  	struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
>  						 tcpc_dev);
>  	int ret = 0;
>  
> +	if (port_type != TYPEC_PORT_DRP)
> +		return -EOPNOTSUPP;
> +
>  	mutex_lock(&chip->lock);
>  	ret = fusb302_set_src_current(chip, cc_src_current[cc]);
>  	if (ret < 0) {
> @@ -1094,7 +1098,7 @@ static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev)
>  	fusb302_tcpc_dev->set_vbus = tcpm_set_vbus;
>  	fusb302_tcpc_dev->set_pd_rx = tcpm_set_pd_rx;
>  	fusb302_tcpc_dev->set_roles = tcpm_set_roles;
> -	fusb302_tcpc_dev->start_drp_toggling = tcpm_start_drp_toggling;
> +	fusb302_tcpc_dev->start_toggling = tcpm_start_toggling;
>  	fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit;
>  }
>  
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index ac6b418b15f1..c1f7073a56de 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -100,13 +100,17 @@ static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
>  	return 0;
>  }
>  
> -static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc,
> -				    enum typec_cc_status cc)
> +static int tcpci_start_toggling(struct tcpc_dev *tcpc,
> +				enum typec_port_type port_type,
> +				enum typec_cc_status cc)
>  {
>  	int ret;
>  	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
>  	unsigned int reg = TCPC_ROLE_CTRL_DRP;
>  
> +	if (port_type != TYPEC_PORT_DRP)
> +		return -EOPNOTSUPP;
> +
>  	/* Handle vendor drp toggling */
>  	if (tcpci->data->start_drp_toggling) {
>  		ret = tcpci->data->start_drp_toggling(tcpci, tcpci->data, cc);
> @@ -511,7 +515,7 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
>  	tcpci->tcpc.get_cc = tcpci_get_cc;
>  	tcpci->tcpc.set_polarity = tcpci_set_polarity;
>  	tcpci->tcpc.set_vconn = tcpci_set_vconn;
> -	tcpci->tcpc.start_drp_toggling = tcpci_start_drp_toggling;
> +	tcpci->tcpc.start_toggling = tcpci_start_toggling;
>  
>  	tcpci->tcpc.set_pd_rx = tcpci_set_pd_rx;
>  	tcpci->tcpc.set_roles = tcpci_set_roles;
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index a2233d72ae7c..fba32d84e578 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -31,7 +31,7 @@
>  
>  #define FOREACH_STATE(S)			\
>  	S(INVALID_STATE),			\
> -	S(DRP_TOGGLING),			\
> +	S(TOGGLING),			\
>  	S(SRC_UNATTACHED),			\
>  	S(SRC_ATTACH_WAIT),			\
>  	S(SRC_ATTACHED),			\
> @@ -472,7 +472,7 @@ static void tcpm_log(struct tcpm_port *port, const char *fmt, ...)
>  	/* Do not log while disconnected and unattached */
>  	if (tcpm_port_is_disconnected(port) &&
>  	    (port->state == SRC_UNATTACHED || port->state == SNK_UNATTACHED ||
> -	     port->state == DRP_TOGGLING))
> +	     port->state == TOGGLING))
>  		return;
>  
>  	va_start(args, fmt);
> @@ -2540,20 +2540,16 @@ static int tcpm_set_charge(struct tcpm_port *port, bool charge)
>  	return 0;
>  }
>  
> -static bool tcpm_start_drp_toggling(struct tcpm_port *port,
> -				    enum typec_cc_status cc)
> +static bool tcpm_start_toggling(struct tcpm_port *port, enum typec_cc_status cc)
>  {
>  	int ret;
>  
> -	if (port->tcpc->start_drp_toggling &&
> -	    port->port_type == TYPEC_PORT_DRP) {
> -		tcpm_log_force(port, "Start DRP toggling");
> -		ret = port->tcpc->start_drp_toggling(port->tcpc, cc);
> -		if (!ret)
> -			return true;
> -	}
> +	if (!port->tcpc->start_toggling)
> +		return false;
>  
> -	return false;
> +	tcpm_log_force(port, "Start toggling");
> +	ret = port->tcpc->start_toggling(port->tcpc, port->port_type, cc);
> +	return ret == 0;
>  }
>  
>  static void tcpm_set_cc(struct tcpm_port *port, enum typec_cc_status cc)
> @@ -2847,15 +2843,15 @@ static void run_state_machine(struct tcpm_port *port)
>  
>  	port->enter_state = port->state;
>  	switch (port->state) {
> -	case DRP_TOGGLING:
> +	case TOGGLING:
>  		break;
>  	/* SRC states */
>  	case SRC_UNATTACHED:
>  		if (!port->non_pd_role_swap)
>  			tcpm_swap_complete(port, -ENOTCONN);
>  		tcpm_src_detach(port);
> -		if (tcpm_start_drp_toggling(port, tcpm_rp_cc(port))) {
> -			tcpm_set_state(port, DRP_TOGGLING, 0);
> +		if (tcpm_start_toggling(port, tcpm_rp_cc(port))) {
> +			tcpm_set_state(port, TOGGLING, 0);
>  			break;
>  		}
>  		tcpm_set_cc(port, tcpm_rp_cc(port));
> @@ -3053,8 +3049,8 @@ static void run_state_machine(struct tcpm_port *port)
>  			tcpm_swap_complete(port, -ENOTCONN);
>  		tcpm_pps_complete(port, -ENOTCONN);
>  		tcpm_snk_detach(port);
> -		if (tcpm_start_drp_toggling(port, TYPEC_CC_RD)) {
> -			tcpm_set_state(port, DRP_TOGGLING, 0);
> +		if (tcpm_start_toggling(port, TYPEC_CC_RD)) {
> +			tcpm_set_state(port, TOGGLING, 0);
>  			break;
>  		}
>  		tcpm_set_cc(port, TYPEC_CC_RD);
> @@ -3621,7 +3617,7 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1,
>  						       : "connected");
>  
>  	switch (port->state) {
> -	case DRP_TOGGLING:
> +	case TOGGLING:
>  		if (tcpm_port_is_debug(port) || tcpm_port_is_audio(port) ||
>  		    tcpm_port_is_source(port))
>  			tcpm_set_state(port, SRC_ATTACH_WAIT, 0);
> diff --git a/drivers/usb/typec/tcpm/wcove.c b/drivers/usb/typec/tcpm/wcove.c
> index 6770afd40765..f1f8f45e2f3d 100644
> --- a/drivers/usb/typec/tcpm/wcove.c
> +++ b/drivers/usb/typec/tcpm/wcove.c
> @@ -416,12 +416,16 @@ static int wcove_pd_transmit(struct tcpc_dev *tcpc,
>  	return regmap_write(wcove->regmap, USBC_TXCMD, cmd | USBC_TXCMD_START);
>  }
>  
> -static int wcove_start_drp_toggling(struct tcpc_dev *tcpc,
> -				    enum typec_cc_status cc)
> +static int wcove_start_toggling(struct tcpc_dev *tcpc,
> +				enum typec_port_type port_type,
> +				enum typec_cc_status cc)
>  {
>  	struct wcove_typec *wcove = tcpc_to_wcove(tcpc);
>  	unsigned int usbc_ctrl;
>  
> +	if (port_type != TYPEC_PORT_DRP)
> +		return -EOPNOTSUPP;
> +
>  	usbc_ctrl = USBC_CONTROL1_MODE_DRP | USBC_CONTROL1_DRPTOGGLE_RANDOM;
>  
>  	switch (cc) {
> @@ -642,7 +646,7 @@ static int wcove_typec_probe(struct platform_device *pdev)
>  	wcove->tcpc.set_polarity = wcove_set_polarity;
>  	wcove->tcpc.set_vconn = wcove_set_vconn;
>  	wcove->tcpc.set_current_limit = wcove_set_current_limit;
> -	wcove->tcpc.start_drp_toggling = wcove_start_drp_toggling;
> +	wcove->tcpc.start_toggling = wcove_start_toggling;
>  
>  	wcove->tcpc.set_pd_rx = wcove_set_pd_rx;
>  	wcove->tcpc.set_roles = wcove_set_roles;
> diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
> index 0c532ca3f079..36a15dcadc53 100644
> --- a/include/linux/usb/tcpm.h
> +++ b/include/linux/usb/tcpm.h
> @@ -121,10 +121,10 @@ struct tcpc_config {
>   *		with partner.
>   * @set_pd_rx:	Called to enable or disable reception of PD messages
>   * @set_roles:	Called to set power and data roles
> - * @start_drp_toggling:
> - *		Optional; if supported by hardware, called to start DRP
> - *		toggling. DRP toggling is stopped automatically if
> - *		a connection is established.
> + * @start_toggling:
> + *		Optional; if supported by hardware, called to start dual-role
> + *		toggling or single-role connection detection. Toggling stops
> + *		automatically if a connection is established.
>   * @try_role:	Optional; called to set a preferred role
>   * @pd_transmit:Called to transmit PD message
>   * @mux:	Pointer to multiplexer data
> @@ -147,8 +147,9 @@ struct tcpc_dev {
>  	int (*set_pd_rx)(struct tcpc_dev *dev, bool on);
>  	int (*set_roles)(struct tcpc_dev *dev, bool attached,
>  			 enum typec_role role, enum typec_data_role data);
> -	int (*start_drp_toggling)(struct tcpc_dev *dev,
> -				  enum typec_cc_status cc);
> +	int (*start_toggling)(struct tcpc_dev *dev,
> +			      enum typec_port_type port_type,
> +			      enum typec_cc_status cc);
>  	int (*try_role)(struct tcpc_dev *dev, int role);
>  	int (*pd_transmit)(struct tcpc_dev *dev, enum tcpm_transmit_type type,
>  			   const struct pd_message *msg);
> -- 
> 2.21.0

thanks,

Re: [PATCH v2 1/3] usb: typec: tcpm: Notify the tcpc to start connection-detection for SRPs

[Heikki Krogerus]

On Tue, Apr 16, 2019 at 10:07:52PM +0200, Hans de Goede wrote:
> Some tcpc device-drivers need to explicitly be told to watch for connection
> events, otherwise the tcpc will not generate any TCPM_CC_EVENTs and devices
> being plugged into the Type-C port will not be noticed.
> 
> For dual-role ports tcpm_start_drp_toggling() is used to tell the tcpc to
> watch for connection events. Sofar we lack a similar callback to the tcpc
> for single-role ports. With some tcpc-s such as the fusb302 this means
> no TCPM_CC_EVENTs will be generated when the port is configured as a
> single-role port.
> 
> This commit renames start_drp_toggling to start_toggling and since the
> device-properties are parsed by the tcpm-core, adds a port_type parameter
> to the start_toggling callback so that the tcpc_dev driver knows the
> port-type and can act accordingly when it starts toggling.
> 
> The new start_toggling callback now always gets called if defined, instead
> of only being called for DRP ports.
> 
> To avoid this causing undesirable functional changes all existing
> start_drp_toggling implementations are not only renamed to start_toggling,
> but also get a port_type check added and return -EOPNOTSUPP when port_type
> is not DRP.
> 
> Fixes: ea3b4d5523bc("usb: typec: fusb302: Resolve fixed power role ...")
> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> Changes in v2:
> -Rename start_drp_toggling to start_toggling and make it multi-purpose,
>  rather then adding a new start_srp_connection_detect callback
> ---
>  drivers/usb/typec/tcpm/fusb302.c | 10 +++++++---
>  drivers/usb/typec/tcpm/tcpci.c   | 10 +++++++---
>  drivers/usb/typec/tcpm/tcpm.c    | 32 ++++++++++++++------------------
>  drivers/usb/typec/tcpm/wcove.c   | 10 +++++++---
>  include/linux/usb/tcpm.h         | 13 +++++++------
>  5 files changed, 42 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
> index 6ea6199caafa..6d83891cc895 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -876,13 +876,17 @@ static int tcpm_set_roles(struct tcpc_dev *dev, bool attached,
>  	return ret;
>  }
>  
> -static int tcpm_start_drp_toggling(struct tcpc_dev *dev,
> -				   enum typec_cc_status cc)
> +static int tcpm_start_toggling(struct tcpc_dev *dev,
> +			       enum typec_port_type port_type,
> +			       enum typec_cc_status cc)
>  {
>  	struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
>  						 tcpc_dev);
>  	int ret = 0;
>  
> +	if (port_type != TYPEC_PORT_DRP)
> +		return -EOPNOTSUPP;
> +
>  	mutex_lock(&chip->lock);
>  	ret = fusb302_set_src_current(chip, cc_src_current[cc]);
>  	if (ret < 0) {
> @@ -1094,7 +1098,7 @@ static void init_tcpc_dev(struct tcpc_dev *fusb302_tcpc_dev)
>  	fusb302_tcpc_dev->set_vbus = tcpm_set_vbus;
>  	fusb302_tcpc_dev->set_pd_rx = tcpm_set_pd_rx;
>  	fusb302_tcpc_dev->set_roles = tcpm_set_roles;
> -	fusb302_tcpc_dev->start_drp_toggling = tcpm_start_drp_toggling;
> +	fusb302_tcpc_dev->start_toggling = tcpm_start_toggling;
>  	fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit;
>  }
>  
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index ac6b418b15f1..c1f7073a56de 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -100,13 +100,17 @@ static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
>  	return 0;
>  }
>  
> -static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc,
> -				    enum typec_cc_status cc)
> +static int tcpci_start_toggling(struct tcpc_dev *tcpc,
> +				enum typec_port_type port_type,
> +				enum typec_cc_status cc)
>  {
>  	int ret;
>  	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
>  	unsigned int reg = TCPC_ROLE_CTRL_DRP;
>  
> +	if (port_type != TYPEC_PORT_DRP)
> +		return -EOPNOTSUPP;
> +
>  	/* Handle vendor drp toggling */
>  	if (tcpci->data->start_drp_toggling) {
>  		ret = tcpci->data->start_drp_toggling(tcpci, tcpci->data, cc);
> @@ -511,7 +515,7 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
>  	tcpci->tcpc.get_cc = tcpci_get_cc;
>  	tcpci->tcpc.set_polarity = tcpci_set_polarity;
>  	tcpci->tcpc.set_vconn = tcpci_set_vconn;
> -	tcpci->tcpc.start_drp_toggling = tcpci_start_drp_toggling;
> +	tcpci->tcpc.start_toggling = tcpci_start_toggling;
>  
>  	tcpci->tcpc.set_pd_rx = tcpci_set_pd_rx;
>  	tcpci->tcpc.set_roles = tcpci_set_roles;
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index a2233d72ae7c..fba32d84e578 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -31,7 +31,7 @@
>  
>  #define FOREACH_STATE(S)			\
>  	S(INVALID_STATE),			\
> -	S(DRP_TOGGLING),			\
> +	S(TOGGLING),			\
>  	S(SRC_UNATTACHED),			\
>  	S(SRC_ATTACH_WAIT),			\
>  	S(SRC_ATTACHED),			\
> @@ -472,7 +472,7 @@ static void tcpm_log(struct tcpm_port *port, const char *fmt, ...)
>  	/* Do not log while disconnected and unattached */
>  	if (tcpm_port_is_disconnected(port) &&
>  	    (port->state == SRC_UNATTACHED || port->state == SNK_UNATTACHED ||
> -	     port->state == DRP_TOGGLING))
> +	     port->state == TOGGLING))
>  		return;
>  
>  	va_start(args, fmt);
> @@ -2540,20 +2540,16 @@ static int tcpm_set_charge(struct tcpm_port *port, bool charge)
>  	return 0;
>  }
>  
> -static bool tcpm_start_drp_toggling(struct tcpm_port *port,
> -				    enum typec_cc_status cc)
> +static bool tcpm_start_toggling(struct tcpm_port *port, enum typec_cc_status cc)
>  {
>  	int ret;
>  
> -	if (port->tcpc->start_drp_toggling &&
> -	    port->port_type == TYPEC_PORT_DRP) {
> -		tcpm_log_force(port, "Start DRP toggling");
> -		ret = port->tcpc->start_drp_toggling(port->tcpc, cc);
> -		if (!ret)
> -			return true;
> -	}
> +	if (!port->tcpc->start_toggling)
> +		return false;
>  
> -	return false;
> +	tcpm_log_force(port, "Start toggling");
> +	ret = port->tcpc->start_toggling(port->tcpc, port->port_type, cc);
> +	return ret == 0;
>  }
>  
>  static void tcpm_set_cc(struct tcpm_port *port, enum typec_cc_status cc)
> @@ -2847,15 +2843,15 @@ static void run_state_machine(struct tcpm_port *port)
>  
>  	port->enter_state = port->state;
>  	switch (port->state) {
> -	case DRP_TOGGLING:
> +	case TOGGLING:
>  		break;
>  	/* SRC states */
>  	case SRC_UNATTACHED:
>  		if (!port->non_pd_role_swap)
>  			tcpm_swap_complete(port, -ENOTCONN);
>  		tcpm_src_detach(port);
> -		if (tcpm_start_drp_toggling(port, tcpm_rp_cc(port))) {
> -			tcpm_set_state(port, DRP_TOGGLING, 0);
> +		if (tcpm_start_toggling(port, tcpm_rp_cc(port))) {
> +			tcpm_set_state(port, TOGGLING, 0);
>  			break;
>  		}
>  		tcpm_set_cc(port, tcpm_rp_cc(port));
> @@ -3053,8 +3049,8 @@ static void run_state_machine(struct tcpm_port *port)
>  			tcpm_swap_complete(port, -ENOTCONN);
>  		tcpm_pps_complete(port, -ENOTCONN);
>  		tcpm_snk_detach(port);
> -		if (tcpm_start_drp_toggling(port, TYPEC_CC_RD)) {
> -			tcpm_set_state(port, DRP_TOGGLING, 0);
> +		if (tcpm_start_toggling(port, TYPEC_CC_RD)) {
> +			tcpm_set_state(port, TOGGLING, 0);
>  			break;
>  		}
>  		tcpm_set_cc(port, TYPEC_CC_RD);
> @@ -3621,7 +3617,7 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1,
>  						       : "connected");
>  
>  	switch (port->state) {
> -	case DRP_TOGGLING:
> +	case TOGGLING:
>  		if (tcpm_port_is_debug(port) || tcpm_port_is_audio(port) ||
>  		    tcpm_port_is_source(port))
>  			tcpm_set_state(port, SRC_ATTACH_WAIT, 0);
> diff --git a/drivers/usb/typec/tcpm/wcove.c b/drivers/usb/typec/tcpm/wcove.c
> index 6770afd40765..f1f8f45e2f3d 100644
> --- a/drivers/usb/typec/tcpm/wcove.c
> +++ b/drivers/usb/typec/tcpm/wcove.c
> @@ -416,12 +416,16 @@ static int wcove_pd_transmit(struct tcpc_dev *tcpc,
>  	return regmap_write(wcove->regmap, USBC_TXCMD, cmd | USBC_TXCMD_START);
>  }
>  
> -static int wcove_start_drp_toggling(struct tcpc_dev *tcpc,
> -				    enum typec_cc_status cc)
> +static int wcove_start_toggling(struct tcpc_dev *tcpc,
> +				enum typec_port_type port_type,
> +				enum typec_cc_status cc)
>  {
>  	struct wcove_typec *wcove = tcpc_to_wcove(tcpc);
>  	unsigned int usbc_ctrl;
>  
> +	if (port_type != TYPEC_PORT_DRP)
> +		return -EOPNOTSUPP;
> +
>  	usbc_ctrl = USBC_CONTROL1_MODE_DRP | USBC_CONTROL1_DRPTOGGLE_RANDOM;
>  
>  	switch (cc) {
> @@ -642,7 +646,7 @@ static int wcove_typec_probe(struct platform_device *pdev)
>  	wcove->tcpc.set_polarity = wcove_set_polarity;
>  	wcove->tcpc.set_vconn = wcove_set_vconn;
>  	wcove->tcpc.set_current_limit = wcove_set_current_limit;
> -	wcove->tcpc.start_drp_toggling = wcove_start_drp_toggling;
> +	wcove->tcpc.start_toggling = wcove_start_toggling;
>  
>  	wcove->tcpc.set_pd_rx = wcove_set_pd_rx;
>  	wcove->tcpc.set_roles = wcove_set_roles;
> diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
> index 0c532ca3f079..36a15dcadc53 100644
> --- a/include/linux/usb/tcpm.h
> +++ b/include/linux/usb/tcpm.h
> @@ -121,10 +121,10 @@ struct tcpc_config {
>   *		with partner.
>   * @set_pd_rx:	Called to enable or disable reception of PD messages
>   * @set_roles:	Called to set power and data roles
> - * @start_drp_toggling:
> - *		Optional; if supported by hardware, called to start DRP
> - *		toggling. DRP toggling is stopped automatically if
> - *		a connection is established.
> + * @start_toggling:
> + *		Optional; if supported by hardware, called to start dual-role
> + *		toggling or single-role connection detection. Toggling stops
> + *		automatically if a connection is established.
>   * @try_role:	Optional; called to set a preferred role
>   * @pd_transmit:Called to transmit PD message
>   * @mux:	Pointer to multiplexer data
> @@ -147,8 +147,9 @@ struct tcpc_dev {
>  	int (*set_pd_rx)(struct tcpc_dev *dev, bool on);
>  	int (*set_roles)(struct tcpc_dev *dev, bool attached,
>  			 enum typec_role role, enum typec_data_role data);
> -	int (*start_drp_toggling)(struct tcpc_dev *dev,
> -				  enum typec_cc_status cc);
> +	int (*start_toggling)(struct tcpc_dev *dev,
> +			      enum typec_port_type port_type,
> +			      enum typec_cc_status cc);
>  	int (*try_role)(struct tcpc_dev *dev, int role);
>  	int (*pd_transmit)(struct tcpc_dev *dev, enum tcpm_transmit_type type,
>  			   const struct pd_message *msg);
> -- 
> 2.21.0

thanks,

-- 
heikki

[v2,2/3] usb: typec: fusb302: Implement start_toggling for all port-types

[Heikki Krogerus]

On Tue, Apr 16, 2019 at 10:07:53PM +0200, Hans de Goede wrote:
> When in single-role port mode, we must start single-role toggling to
> get an interrupt when a device / cable gets plugged into the port.
> 
> This commit modifies the fusb302 start_toggling implementation to
> start toggling for all port-types, so that connection-detection works
> on single-role ports too.
> 
> Fixes: ea3b4d5523bc("usb: typec: fusb302: Resolve fixed power role ...")
> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> Changes in v2:
> -Adjust for the tcpm core renaming start_drp_toggling to start_toggling,
>  instead of adding a new start_srp_connection_detect callback
> ---
>  drivers/usb/typec/tcpm/fusb302.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
> index 6d83891cc895..ba030b03d156 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -882,10 +882,20 @@ static int tcpm_start_toggling(struct tcpc_dev *dev,
>  {
>  	struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
>  						 tcpc_dev);
> +	enum toggling_mode mode = TOGGLING_MODE_OFF;
>  	int ret = 0;
>  
> -	if (port_type != TYPEC_PORT_DRP)
> -		return -EOPNOTSUPP;
> +	switch (port_type) {
> +	case TYPEC_PORT_SRC:
> +		mode = TOGGLING_MODE_SRC;
> +		break;
> +	case TYPEC_PORT_SNK:
> +		mode = TOGGLING_MODE_SNK;
> +		break;
> +	case TYPEC_PORT_DRP:
> +		mode = TOGGLING_MODE_DRP;
> +		break;
> +	}
>  
>  	mutex_lock(&chip->lock);
>  	ret = fusb302_set_src_current(chip, cc_src_current[cc]);
> @@ -894,7 +904,7 @@ static int tcpm_start_toggling(struct tcpc_dev *dev,
>  			    typec_cc_status_name[cc], ret);
>  		goto done;
>  	}
> -	ret = fusb302_set_toggling(chip, TOGGLING_MODE_DRP);
> +	ret = fusb302_set_toggling(chip, mode);
>  	if (ret < 0) {
>  		fusb302_log(chip,
>  			    "unable to start drp toggling, ret=%d", ret);
> -- 
> 2.21.0

thanks,

Re: [PATCH v2 2/3] usb: typec: fusb302: Implement start_toggling for all port-types

[Heikki Krogerus]

On Tue, Apr 16, 2019 at 10:07:53PM +0200, Hans de Goede wrote:
> When in single-role port mode, we must start single-role toggling to
> get an interrupt when a device / cable gets plugged into the port.
> 
> This commit modifies the fusb302 start_toggling implementation to
> start toggling for all port-types, so that connection-detection works
> on single-role ports too.
> 
> Fixes: ea3b4d5523bc("usb: typec: fusb302: Resolve fixed power role ...")
> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> Changes in v2:
> -Adjust for the tcpm core renaming start_drp_toggling to start_toggling,
>  instead of adding a new start_srp_connection_detect callback
> ---
>  drivers/usb/typec/tcpm/fusb302.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
> index 6d83891cc895..ba030b03d156 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -882,10 +882,20 @@ static int tcpm_start_toggling(struct tcpc_dev *dev,
>  {
>  	struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
>  						 tcpc_dev);
> +	enum toggling_mode mode = TOGGLING_MODE_OFF;
>  	int ret = 0;
>  
> -	if (port_type != TYPEC_PORT_DRP)
> -		return -EOPNOTSUPP;
> +	switch (port_type) {
> +	case TYPEC_PORT_SRC:
> +		mode = TOGGLING_MODE_SRC;
> +		break;
> +	case TYPEC_PORT_SNK:
> +		mode = TOGGLING_MODE_SNK;
> +		break;
> +	case TYPEC_PORT_DRP:
> +		mode = TOGGLING_MODE_DRP;
> +		break;
> +	}
>  
>  	mutex_lock(&chip->lock);
>  	ret = fusb302_set_src_current(chip, cc_src_current[cc]);
> @@ -894,7 +904,7 @@ static int tcpm_start_toggling(struct tcpc_dev *dev,
>  			    typec_cc_status_name[cc], ret);
>  		goto done;
>  	}
> -	ret = fusb302_set_toggling(chip, TOGGLING_MODE_DRP);
> +	ret = fusb302_set_toggling(chip, mode);
>  	if (ret < 0) {
>  		fusb302_log(chip,
>  			    "unable to start drp toggling, ret=%d", ret);
> -- 
> 2.21.0

thanks,

-- 
heikki

[v2,3/3] usb: typec: fusb302: Revert "Resolve fixed power role contract setup"

[Heikki Krogerus]

On Tue, Apr 16, 2019 at 10:07:54PM +0200, Hans de Goede wrote:
> Some tcpc device-drivers need to explicitly be told to watch for connection
> events, otherwise the tcpc will not generate any TCPM_CC_EVENTs and devices
> being plugged into the Type-C port will not be noticed.
> 
> For dual-role ports tcpm_start_drp_toggling() is used to tell the tcpc to
> watch for connection events. But for single-role ports we've so far been
> falling back to just calling tcpm_set_cc(). For some tcpc-s such as the
> fusb302 this is not enough and no TCPM_CC_EVENT will be generated.
> 
> Commit ea3b4d5523bc ("usb: typec: fusb302: Resolve fixed power role
> contract setup") fixed SRPs not working because of this by making the
> fusb302 driver start connection detection on every tcpm_set_cc() call.
> It turns out this breaks src->snk power-role swapping because during the
> swap we first set the Cc pins to Rp, calling set_cc, and then send a PS_RDY
> message. But the fusb302 cannot send PD messages while its toggling engine
> is active, so sending the PS_RDY message fails.
> 
> Struct tcpc_dev now has a new start_srp_connection_detect callback and
> fusb302.c now implements this. This callback gets called when we the
> fusb302 needs to start connection detection, fixing fusb302 SRPs not
> seeing connected devices.
> 
> This allows us to revert the changes to fusb302's set_cc implementation,
> making it once again purely setup the Cc-s and matching disconnect
> detection, fixing src->snk power-role swapping no longer working.
> 
> Note that since the code was refactored in between, codewise this is not a
> straight forward revert. Functionality wise this is a straight revert and
> the original functionality is fully restored.
> 
> Fixes: ea3b4d5523bc ("usb: typec: fusb302: Resolve fixed power role ...")
> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> Changes in v2:
> -No changes
> ---
>  drivers/usb/typec/tcpm/fusb302.c | 55 ++++++++++++++++++++++++++------
>  1 file changed, 46 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
> index ba030b03d156..4328a6cbfb69 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -606,19 +606,16 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
>  			    FUSB_REG_SWITCHES0_CC2_PU_EN |
>  			    FUSB_REG_SWITCHES0_CC1_PD_EN |
>  			    FUSB_REG_SWITCHES0_CC2_PD_EN;
> -	u8 switches0_data = 0x00;
> +	u8 rd_mda, switches0_data = 0x00;
>  	int ret = 0;
> -	enum toggling_mode mode;
>  
>  	mutex_lock(&chip->lock);
>  	switch (cc) {
>  	case TYPEC_CC_OPEN:
> -		mode = TOGGLING_MODE_OFF;
>  		break;
>  	case TYPEC_CC_RD:
>  		switches0_data |= FUSB_REG_SWITCHES0_CC1_PD_EN |
>  				  FUSB_REG_SWITCHES0_CC2_PD_EN;
> -		mode = TOGGLING_MODE_SNK;
>  		break;
>  	case TYPEC_CC_RP_DEF:
>  	case TYPEC_CC_RP_1_5:
> @@ -626,7 +623,6 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
>  		switches0_data |= (chip->cc_polarity == TYPEC_POLARITY_CC1) ?
>  				  FUSB_REG_SWITCHES0_CC1_PU_EN :
>  				  FUSB_REG_SWITCHES0_CC2_PU_EN;
> -		mode = TOGGLING_MODE_SRC;
>  		break;
>  	default:
>  		fusb302_log(chip, "unsupported cc value %s",
> @@ -637,6 +633,12 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
>  
>  	fusb302_log(chip, "cc := %s", typec_cc_status_name[cc]);
>  
> +	ret = fusb302_set_toggling(chip, TOGGLING_MODE_OFF);
> +	if (ret < 0) {
> +		fusb302_log(chip, "cannot set toggling mode, ret=%d", ret);
> +		goto done;
> +	}
> +
>  	ret = fusb302_i2c_mask_write(chip, FUSB_REG_SWITCHES0,
>  				     switches0_mask, switches0_data);
>  	if (ret < 0) {
> @@ -655,10 +657,45 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
>  		goto done;
>  	}
>  
> -	ret = fusb302_set_toggling(chip, mode);
> -	if (ret < 0)
> -		fusb302_log(chip, "cannot set toggling mode, ret=%d", ret);
> -
> +	/* enable/disable interrupts, BC_LVL for SNK and COMP_CHNG for SRC */
> +	switch (cc) {
> +	case TYPEC_CC_RP_DEF:
> +	case TYPEC_CC_RP_1_5:
> +	case TYPEC_CC_RP_3_0:
> +		rd_mda = rd_mda_value[cc_src_current[cc]];
> +		ret = fusb302_i2c_write(chip, FUSB_REG_MEASURE, rd_mda);
> +		if (ret < 0) {
> +			fusb302_log(chip,
> +				    "cannot set SRC measure value, ret=%d",
> +				    ret);
> +			goto done;
> +		}
> +		ret = fusb302_i2c_mask_write(chip, FUSB_REG_MASK,
> +					     FUSB_REG_MASK_BC_LVL |
> +					     FUSB_REG_MASK_COMP_CHNG,
> +					     FUSB_REG_MASK_COMP_CHNG);
> +		if (ret < 0) {
> +			fusb302_log(chip, "cannot set SRC interrupt, ret=%d",
> +				    ret);
> +			goto done;
> +		}
> +		chip->intr_comp_chng = true;
> +		break;
> +	case TYPEC_CC_RD:
> +		ret = fusb302_i2c_mask_write(chip, FUSB_REG_MASK,
> +					     FUSB_REG_MASK_BC_LVL |
> +					     FUSB_REG_MASK_COMP_CHNG,
> +					     FUSB_REG_MASK_BC_LVL);
> +		if (ret < 0) {
> +			fusb302_log(chip, "cannot set SRC interrupt, ret=%d",
> +				    ret);
> +			goto done;
> +		}
> +		chip->intr_bc_lvl = true;
> +		break;
> +	default:
> +		break;
> +	}
>  done:
>  	mutex_unlock(&chip->lock);
>  
> -- 
> 2.21.0

thanks,

Re: [PATCH v2 3/3] usb: typec: fusb302: Revert "Resolve fixed power role contract setup"

[Heikki Krogerus]

On Tue, Apr 16, 2019 at 10:07:54PM +0200, Hans de Goede wrote:
> Some tcpc device-drivers need to explicitly be told to watch for connection
> events, otherwise the tcpc will not generate any TCPM_CC_EVENTs and devices
> being plugged into the Type-C port will not be noticed.
> 
> For dual-role ports tcpm_start_drp_toggling() is used to tell the tcpc to
> watch for connection events. But for single-role ports we've so far been
> falling back to just calling tcpm_set_cc(). For some tcpc-s such as the
> fusb302 this is not enough and no TCPM_CC_EVENT will be generated.
> 
> Commit ea3b4d5523bc ("usb: typec: fusb302: Resolve fixed power role
> contract setup") fixed SRPs not working because of this by making the
> fusb302 driver start connection detection on every tcpm_set_cc() call.
> It turns out this breaks src->snk power-role swapping because during the
> swap we first set the Cc pins to Rp, calling set_cc, and then send a PS_RDY
> message. But the fusb302 cannot send PD messages while its toggling engine
> is active, so sending the PS_RDY message fails.
> 
> Struct tcpc_dev now has a new start_srp_connection_detect callback and
> fusb302.c now implements this. This callback gets called when we the
> fusb302 needs to start connection detection, fixing fusb302 SRPs not
> seeing connected devices.
> 
> This allows us to revert the changes to fusb302's set_cc implementation,
> making it once again purely setup the Cc-s and matching disconnect
> detection, fixing src->snk power-role swapping no longer working.
> 
> Note that since the code was refactored in between, codewise this is not a
> straight forward revert. Functionality wise this is a straight revert and
> the original functionality is fully restored.
> 
> Fixes: ea3b4d5523bc ("usb: typec: fusb302: Resolve fixed power role ...")
> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> Changes in v2:
> -No changes
> ---
>  drivers/usb/typec/tcpm/fusb302.c | 55 ++++++++++++++++++++++++++------
>  1 file changed, 46 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
> index ba030b03d156..4328a6cbfb69 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -606,19 +606,16 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
>  			    FUSB_REG_SWITCHES0_CC2_PU_EN |
>  			    FUSB_REG_SWITCHES0_CC1_PD_EN |
>  			    FUSB_REG_SWITCHES0_CC2_PD_EN;
> -	u8 switches0_data = 0x00;
> +	u8 rd_mda, switches0_data = 0x00;
>  	int ret = 0;
> -	enum toggling_mode mode;
>  
>  	mutex_lock(&chip->lock);
>  	switch (cc) {
>  	case TYPEC_CC_OPEN:
> -		mode = TOGGLING_MODE_OFF;
>  		break;
>  	case TYPEC_CC_RD:
>  		switches0_data |= FUSB_REG_SWITCHES0_CC1_PD_EN |
>  				  FUSB_REG_SWITCHES0_CC2_PD_EN;
> -		mode = TOGGLING_MODE_SNK;
>  		break;
>  	case TYPEC_CC_RP_DEF:
>  	case TYPEC_CC_RP_1_5:
> @@ -626,7 +623,6 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
>  		switches0_data |= (chip->cc_polarity == TYPEC_POLARITY_CC1) ?
>  				  FUSB_REG_SWITCHES0_CC1_PU_EN :
>  				  FUSB_REG_SWITCHES0_CC2_PU_EN;
> -		mode = TOGGLING_MODE_SRC;
>  		break;
>  	default:
>  		fusb302_log(chip, "unsupported cc value %s",
> @@ -637,6 +633,12 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
>  
>  	fusb302_log(chip, "cc := %s", typec_cc_status_name[cc]);
>  
> +	ret = fusb302_set_toggling(chip, TOGGLING_MODE_OFF);
> +	if (ret < 0) {
> +		fusb302_log(chip, "cannot set toggling mode, ret=%d", ret);
> +		goto done;
> +	}
> +
>  	ret = fusb302_i2c_mask_write(chip, FUSB_REG_SWITCHES0,
>  				     switches0_mask, switches0_data);
>  	if (ret < 0) {
> @@ -655,10 +657,45 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum typec_cc_status cc)
>  		goto done;
>  	}
>  
> -	ret = fusb302_set_toggling(chip, mode);
> -	if (ret < 0)
> -		fusb302_log(chip, "cannot set toggling mode, ret=%d", ret);
> -
> +	/* enable/disable interrupts, BC_LVL for SNK and COMP_CHNG for SRC */
> +	switch (cc) {
> +	case TYPEC_CC_RP_DEF:
> +	case TYPEC_CC_RP_1_5:
> +	case TYPEC_CC_RP_3_0:
> +		rd_mda = rd_mda_value[cc_src_current[cc]];
> +		ret = fusb302_i2c_write(chip, FUSB_REG_MEASURE, rd_mda);
> +		if (ret < 0) {
> +			fusb302_log(chip,
> +				    "cannot set SRC measure value, ret=%d",
> +				    ret);
> +			goto done;
> +		}
> +		ret = fusb302_i2c_mask_write(chip, FUSB_REG_MASK,
> +					     FUSB_REG_MASK_BC_LVL |
> +					     FUSB_REG_MASK_COMP_CHNG,
> +					     FUSB_REG_MASK_COMP_CHNG);
> +		if (ret < 0) {
> +			fusb302_log(chip, "cannot set SRC interrupt, ret=%d",
> +				    ret);
> +			goto done;
> +		}
> +		chip->intr_comp_chng = true;
> +		break;
> +	case TYPEC_CC_RD:
> +		ret = fusb302_i2c_mask_write(chip, FUSB_REG_MASK,
> +					     FUSB_REG_MASK_BC_LVL |
> +					     FUSB_REG_MASK_COMP_CHNG,
> +					     FUSB_REG_MASK_BC_LVL);
> +		if (ret < 0) {
> +			fusb302_log(chip, "cannot set SRC interrupt, ret=%d",
> +				    ret);
> +			goto done;
> +		}
> +		chip->intr_bc_lvl = true;
> +		break;
> +	default:
> +		break;
> +	}
>  done:
>  	mutex_unlock(&chip->lock);
>  
> -- 
> 2.21.0

thanks,

-- 
heikki

[v2,1/3] usb: typec: tcpm: Notify the tcpc to start connection-detection for SRPs

[Opensource [Adam Thomson]]

On 16 April 2019 21:08, Hans de Goede wrote:

> Some tcpc device-drivers need to explicitly be told to watch for connection
> events, otherwise the tcpc will not generate any TCPM_CC_EVENTs and devices
> being plugged into the Type-C port will not be noticed.
> 
> For dual-role ports tcpm_start_drp_toggling() is used to tell the tcpc to
> watch for connection events. Sofar we lack a similar callback to the tcpc
> for single-role ports. With some tcpc-s such as the fusb302 this means
> no TCPM_CC_EVENTs will be generated when the port is configured as a
> single-role port.
> 
> This commit renames start_drp_toggling to start_toggling and since the
> device-properties are parsed by the tcpm-core, adds a port_type parameter
> to the start_toggling callback so that the tcpc_dev driver knows the
> port-type and can act accordingly when it starts toggling.
> 
> The new start_toggling callback now always gets called if defined, instead
> of only being called for DRP ports.
> 
> To avoid this causing undesirable functional changes all existing
> start_drp_toggling implementations are not only renamed to start_toggling,
> but also get a port_type check added and return -EOPNOTSUPP when port_type
> is not DRP.
> 
> Fixes: ea3b4d5523bc("usb: typec: fusb302: Resolve fixed power role ...")
> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

For DRP & Sink only toggling scenarios, with FUSB302 TCPC:

Tested-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>

> ---
> Changes in v2:
> -Rename start_drp_toggling to start_toggling and make it multi-purpose,
>  rather then adding a new start_srp_connection_detect callback
> ---
>  drivers/usb/typec/tcpm/fusb302.c | 10 +++++++---
>  drivers/usb/typec/tcpm/tcpci.c   | 10 +++++++---
>  drivers/usb/typec/tcpm/tcpm.c    | 32 ++++++++++++++------------------
>  drivers/usb/typec/tcpm/wcove.c   | 10 +++++++---
>  include/linux/usb/tcpm.h         | 13 +++++++------
>  5 files changed, 42 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c
> b/drivers/usb/typec/tcpm/fusb302.c
> index 6ea6199caafa..6d83891cc895 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -876,13 +876,17 @@ static int tcpm_set_roles(struct tcpc_dev *dev, bool
> attached,
>  	return ret;
>  }
> 
> -static int tcpm_start_drp_toggling(struct tcpc_dev *dev,
> -				   enum typec_cc_status cc)
> +static int tcpm_start_toggling(struct tcpc_dev *dev,
> +			       enum typec_port_type port_type,
> +			       enum typec_cc_status cc)
>  {
>  	struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
>  						 tcpc_dev);
>  	int ret = 0;
> 
> +	if (port_type != TYPEC_PORT_DRP)
> +		return -EOPNOTSUPP;
> +
>  	mutex_lock(&chip->lock);
>  	ret = fusb302_set_src_current(chip, cc_src_current[cc]);
>  	if (ret < 0) {
> @@ -1094,7 +1098,7 @@ static void init_tcpc_dev(struct tcpc_dev
> *fusb302_tcpc_dev)
>  	fusb302_tcpc_dev->set_vbus = tcpm_set_vbus;
>  	fusb302_tcpc_dev->set_pd_rx = tcpm_set_pd_rx;
>  	fusb302_tcpc_dev->set_roles = tcpm_set_roles;
> -	fusb302_tcpc_dev->start_drp_toggling = tcpm_start_drp_toggling;
> +	fusb302_tcpc_dev->start_toggling = tcpm_start_toggling;
>  	fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit;
>  }
> 
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index ac6b418b15f1..c1f7073a56de 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -100,13 +100,17 @@ static int tcpci_set_cc(struct tcpc_dev *tcpc, enum
> typec_cc_status cc)
>  	return 0;
>  }
> 
> -static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc,
> -				    enum typec_cc_status cc)
> +static int tcpci_start_toggling(struct tcpc_dev *tcpc,
> +				enum typec_port_type port_type,
> +				enum typec_cc_status cc)
>  {
>  	int ret;
>  	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
>  	unsigned int reg = TCPC_ROLE_CTRL_DRP;
> 
> +	if (port_type != TYPEC_PORT_DRP)
> +		return -EOPNOTSUPP;
> +
>  	/* Handle vendor drp toggling */
>  	if (tcpci->data->start_drp_toggling) {
>  		ret = tcpci->data->start_drp_toggling(tcpci, tcpci->data, cc);
> @@ -511,7 +515,7 @@ struct tcpci *tcpci_register_port(struct device *dev, struct
> tcpci_data *data)
>  	tcpci->tcpc.get_cc = tcpci_get_cc;
>  	tcpci->tcpc.set_polarity = tcpci_set_polarity;
>  	tcpci->tcpc.set_vconn = tcpci_set_vconn;
> -	tcpci->tcpc.start_drp_toggling = tcpci_start_drp_toggling;
> +	tcpci->tcpc.start_toggling = tcpci_start_toggling;
> 
>  	tcpci->tcpc.set_pd_rx = tcpci_set_pd_rx;
>  	tcpci->tcpc.set_roles = tcpci_set_roles;
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index a2233d72ae7c..fba32d84e578 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -31,7 +31,7 @@
> 
>  #define FOREACH_STATE(S)			\
>  	S(INVALID_STATE),			\
> -	S(DRP_TOGGLING),			\
> +	S(TOGGLING),			\
>  	S(SRC_UNATTACHED),			\
>  	S(SRC_ATTACH_WAIT),			\
>  	S(SRC_ATTACHED),			\
> @@ -472,7 +472,7 @@ static void tcpm_log(struct tcpm_port *port, const char
> *fmt, ...)
>  	/* Do not log while disconnected and unattached */
>  	if (tcpm_port_is_disconnected(port) &&
>  	    (port->state == SRC_UNATTACHED || port->state ==
> SNK_UNATTACHED ||
> -	     port->state == DRP_TOGGLING))
> +	     port->state == TOGGLING))
>  		return;
> 
>  	va_start(args, fmt);
> @@ -2540,20 +2540,16 @@ static int tcpm_set_charge(struct tcpm_port *port,
> bool charge)
>  	return 0;
>  }
> 
> -static bool tcpm_start_drp_toggling(struct tcpm_port *port,
> -				    enum typec_cc_status cc)
> +static bool tcpm_start_toggling(struct tcpm_port *port, enum typec_cc_status
> cc)
>  {
>  	int ret;
> 
> -	if (port->tcpc->start_drp_toggling &&
> -	    port->port_type == TYPEC_PORT_DRP) {
> -		tcpm_log_force(port, "Start DRP toggling");
> -		ret = port->tcpc->start_drp_toggling(port->tcpc, cc);
> -		if (!ret)
> -			return true;
> -	}
> +	if (!port->tcpc->start_toggling)
> +		return false;
> 
> -	return false;
> +	tcpm_log_force(port, "Start toggling");
> +	ret = port->tcpc->start_toggling(port->tcpc, port->port_type, cc);
> +	return ret == 0;
>  }
> 
>  static void tcpm_set_cc(struct tcpm_port *port, enum typec_cc_status cc)
> @@ -2847,15 +2843,15 @@ static void run_state_machine(struct tcpm_port
> *port)
> 
>  	port->enter_state = port->state;
>  	switch (port->state) {
> -	case DRP_TOGGLING:
> +	case TOGGLING:
>  		break;
>  	/* SRC states */
>  	case SRC_UNATTACHED:
>  		if (!port->non_pd_role_swap)
>  			tcpm_swap_complete(port, -ENOTCONN);
>  		tcpm_src_detach(port);
> -		if (tcpm_start_drp_toggling(port, tcpm_rp_cc(port))) {
> -			tcpm_set_state(port, DRP_TOGGLING, 0);
> +		if (tcpm_start_toggling(port, tcpm_rp_cc(port))) {
> +			tcpm_set_state(port, TOGGLING, 0);
>  			break;
>  		}
>  		tcpm_set_cc(port, tcpm_rp_cc(port));
> @@ -3053,8 +3049,8 @@ static void run_state_machine(struct tcpm_port *port)
>  			tcpm_swap_complete(port, -ENOTCONN);
>  		tcpm_pps_complete(port, -ENOTCONN);
>  		tcpm_snk_detach(port);
> -		if (tcpm_start_drp_toggling(port, TYPEC_CC_RD)) {
> -			tcpm_set_state(port, DRP_TOGGLING, 0);
> +		if (tcpm_start_toggling(port, TYPEC_CC_RD)) {
> +			tcpm_set_state(port, TOGGLING, 0);
>  			break;
>  		}
>  		tcpm_set_cc(port, TYPEC_CC_RD);
> @@ -3621,7 +3617,7 @@ static void _tcpm_cc_change(struct tcpm_port *port,
> enum typec_cc_status cc1,
>  						       : "connected");
> 
>  	switch (port->state) {
> -	case DRP_TOGGLING:
> +	case TOGGLING:
>  		if (tcpm_port_is_debug(port) || tcpm_port_is_audio(port) ||
>  		    tcpm_port_is_source(port))
>  			tcpm_set_state(port, SRC_ATTACH_WAIT, 0);
> diff --git a/drivers/usb/typec/tcpm/wcove.c b/drivers/usb/typec/tcpm/wcove.c
> index 6770afd40765..f1f8f45e2f3d 100644
> --- a/drivers/usb/typec/tcpm/wcove.c
> +++ b/drivers/usb/typec/tcpm/wcove.c
> @@ -416,12 +416,16 @@ static int wcove_pd_transmit(struct tcpc_dev *tcpc,
>  	return regmap_write(wcove->regmap, USBC_TXCMD, cmd |
> USBC_TXCMD_START);
>  }
> 
> -static int wcove_start_drp_toggling(struct tcpc_dev *tcpc,
> -				    enum typec_cc_status cc)
> +static int wcove_start_toggling(struct tcpc_dev *tcpc,
> +				enum typec_port_type port_type,
> +				enum typec_cc_status cc)
>  {
>  	struct wcove_typec *wcove = tcpc_to_wcove(tcpc);
>  	unsigned int usbc_ctrl;
> 
> +	if (port_type != TYPEC_PORT_DRP)
> +		return -EOPNOTSUPP;
> +
>  	usbc_ctrl = USBC_CONTROL1_MODE_DRP |
> USBC_CONTROL1_DRPTOGGLE_RANDOM;
> 
>  	switch (cc) {
> @@ -642,7 +646,7 @@ static int wcove_typec_probe(struct platform_device
> *pdev)
>  	wcove->tcpc.set_polarity = wcove_set_polarity;
>  	wcove->tcpc.set_vconn = wcove_set_vconn;
>  	wcove->tcpc.set_current_limit = wcove_set_current_limit;
> -	wcove->tcpc.start_drp_toggling = wcove_start_drp_toggling;
> +	wcove->tcpc.start_toggling = wcove_start_toggling;
> 
>  	wcove->tcpc.set_pd_rx = wcove_set_pd_rx;
>  	wcove->tcpc.set_roles = wcove_set_roles;
> diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
> index 0c532ca3f079..36a15dcadc53 100644
> --- a/include/linux/usb/tcpm.h
> +++ b/include/linux/usb/tcpm.h
> @@ -121,10 +121,10 @@ struct tcpc_config {
>   *		with partner.
>   * @set_pd_rx:	Called to enable or disable reception of PD messages
>   * @set_roles:	Called to set power and data roles
> - * @start_drp_toggling:
> - *		Optional; if supported by hardware, called to start DRP
> - *		toggling. DRP toggling is stopped automatically if
> - *		a connection is established.
> + * @start_toggling:
> + *		Optional; if supported by hardware, called to start dual-role
> + *		toggling or single-role connection detection. Toggling stops
> + *		automatically if a connection is established.
>   * @try_role:	Optional; called to set a preferred role
>   * @pd_transmit:Called to transmit PD message
>   * @mux:	Pointer to multiplexer data
> @@ -147,8 +147,9 @@ struct tcpc_dev {
>  	int (*set_pd_rx)(struct tcpc_dev *dev, bool on);
>  	int (*set_roles)(struct tcpc_dev *dev, bool attached,
>  			 enum typec_role role, enum typec_data_role data);
> -	int (*start_drp_toggling)(struct tcpc_dev *dev,
> -				  enum typec_cc_status cc);
> +	int (*start_toggling)(struct tcpc_dev *dev,
> +			      enum typec_port_type port_type,
> +			      enum typec_cc_status cc);
>  	int (*try_role)(struct tcpc_dev *dev, int role);
>  	int (*pd_transmit)(struct tcpc_dev *dev, enum tcpm_transmit_type
> type,
>  			   const struct pd_message *msg);
> --
> 2.21.0

RE: [PATCH v2 1/3] usb: typec: tcpm: Notify the tcpc to start connection-detection for SRPs

[Adam Thomson]

On 16 April 2019 21:08, Hans de Goede wrote:

> Some tcpc device-drivers need to explicitly be told to watch for connection
> events, otherwise the tcpc will not generate any TCPM_CC_EVENTs and devices
> being plugged into the Type-C port will not be noticed.
> 
> For dual-role ports tcpm_start_drp_toggling() is used to tell the tcpc to
> watch for connection events. Sofar we lack a similar callback to the tcpc
> for single-role ports. With some tcpc-s such as the fusb302 this means
> no TCPM_CC_EVENTs will be generated when the port is configured as a
> single-role port.
> 
> This commit renames start_drp_toggling to start_toggling and since the
> device-properties are parsed by the tcpm-core, adds a port_type parameter
> to the start_toggling callback so that the tcpc_dev driver knows the
> port-type and can act accordingly when it starts toggling.
> 
> The new start_toggling callback now always gets called if defined, instead
> of only being called for DRP ports.
> 
> To avoid this causing undesirable functional changes all existing
> start_drp_toggling implementations are not only renamed to start_toggling,
> but also get a port_type check added and return -EOPNOTSUPP when port_type
> is not DRP.
> 
> Fixes: ea3b4d5523bc("usb: typec: fusb302: Resolve fixed power role ...")
> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

For DRP & Sink only toggling scenarios, with FUSB302 TCPC:

Tested-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>

> ---
> Changes in v2:
> -Rename start_drp_toggling to start_toggling and make it multi-purpose,
>  rather then adding a new start_srp_connection_detect callback
> ---
>  drivers/usb/typec/tcpm/fusb302.c | 10 +++++++---
>  drivers/usb/typec/tcpm/tcpci.c   | 10 +++++++---
>  drivers/usb/typec/tcpm/tcpm.c    | 32 ++++++++++++++------------------
>  drivers/usb/typec/tcpm/wcove.c   | 10 +++++++---
>  include/linux/usb/tcpm.h         | 13 +++++++------
>  5 files changed, 42 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c
> b/drivers/usb/typec/tcpm/fusb302.c
> index 6ea6199caafa..6d83891cc895 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -876,13 +876,17 @@ static int tcpm_set_roles(struct tcpc_dev *dev, bool
> attached,
>  	return ret;
>  }
> 
> -static int tcpm_start_drp_toggling(struct tcpc_dev *dev,
> -				   enum typec_cc_status cc)
> +static int tcpm_start_toggling(struct tcpc_dev *dev,
> +			       enum typec_port_type port_type,
> +			       enum typec_cc_status cc)
>  {
>  	struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
>  						 tcpc_dev);
>  	int ret = 0;
> 
> +	if (port_type != TYPEC_PORT_DRP)
> +		return -EOPNOTSUPP;
> +
>  	mutex_lock(&chip->lock);
>  	ret = fusb302_set_src_current(chip, cc_src_current[cc]);
>  	if (ret < 0) {
> @@ -1094,7 +1098,7 @@ static void init_tcpc_dev(struct tcpc_dev
> *fusb302_tcpc_dev)
>  	fusb302_tcpc_dev->set_vbus = tcpm_set_vbus;
>  	fusb302_tcpc_dev->set_pd_rx = tcpm_set_pd_rx;
>  	fusb302_tcpc_dev->set_roles = tcpm_set_roles;
> -	fusb302_tcpc_dev->start_drp_toggling = tcpm_start_drp_toggling;
> +	fusb302_tcpc_dev->start_toggling = tcpm_start_toggling;
>  	fusb302_tcpc_dev->pd_transmit = tcpm_pd_transmit;
>  }
> 
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index ac6b418b15f1..c1f7073a56de 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -100,13 +100,17 @@ static int tcpci_set_cc(struct tcpc_dev *tcpc, enum
> typec_cc_status cc)
>  	return 0;
>  }
> 
> -static int tcpci_start_drp_toggling(struct tcpc_dev *tcpc,
> -				    enum typec_cc_status cc)
> +static int tcpci_start_toggling(struct tcpc_dev *tcpc,
> +				enum typec_port_type port_type,
> +				enum typec_cc_status cc)
>  {
>  	int ret;
>  	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
>  	unsigned int reg = TCPC_ROLE_CTRL_DRP;
> 
> +	if (port_type != TYPEC_PORT_DRP)
> +		return -EOPNOTSUPP;
> +
>  	/* Handle vendor drp toggling */
>  	if (tcpci->data->start_drp_toggling) {
>  		ret = tcpci->data->start_drp_toggling(tcpci, tcpci->data, cc);
> @@ -511,7 +515,7 @@ struct tcpci *tcpci_register_port(struct device *dev, struct
> tcpci_data *data)
>  	tcpci->tcpc.get_cc = tcpci_get_cc;
>  	tcpci->tcpc.set_polarity = tcpci_set_polarity;
>  	tcpci->tcpc.set_vconn = tcpci_set_vconn;
> -	tcpci->tcpc.start_drp_toggling = tcpci_start_drp_toggling;
> +	tcpci->tcpc.start_toggling = tcpci_start_toggling;
> 
>  	tcpci->tcpc.set_pd_rx = tcpci_set_pd_rx;
>  	tcpci->tcpc.set_roles = tcpci_set_roles;
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index a2233d72ae7c..fba32d84e578 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -31,7 +31,7 @@
> 
>  #define FOREACH_STATE(S)			\
>  	S(INVALID_STATE),			\
> -	S(DRP_TOGGLING),			\
> +	S(TOGGLING),			\
>  	S(SRC_UNATTACHED),			\
>  	S(SRC_ATTACH_WAIT),			\
>  	S(SRC_ATTACHED),			\
> @@ -472,7 +472,7 @@ static void tcpm_log(struct tcpm_port *port, const char
> *fmt, ...)
>  	/* Do not log while disconnected and unattached */
>  	if (tcpm_port_is_disconnected(port) &&
>  	    (port->state == SRC_UNATTACHED || port->state ==
> SNK_UNATTACHED ||
> -	     port->state == DRP_TOGGLING))
> +	     port->state == TOGGLING))
>  		return;
> 
>  	va_start(args, fmt);
> @@ -2540,20 +2540,16 @@ static int tcpm_set_charge(struct tcpm_port *port,
> bool charge)
>  	return 0;
>  }
> 
> -static bool tcpm_start_drp_toggling(struct tcpm_port *port,
> -				    enum typec_cc_status cc)
> +static bool tcpm_start_toggling(struct tcpm_port *port, enum typec_cc_status
> cc)
>  {
>  	int ret;
> 
> -	if (port->tcpc->start_drp_toggling &&
> -	    port->port_type == TYPEC_PORT_DRP) {
> -		tcpm_log_force(port, "Start DRP toggling");
> -		ret = port->tcpc->start_drp_toggling(port->tcpc, cc);
> -		if (!ret)
> -			return true;
> -	}
> +	if (!port->tcpc->start_toggling)
> +		return false;
> 
> -	return false;
> +	tcpm_log_force(port, "Start toggling");
> +	ret = port->tcpc->start_toggling(port->tcpc, port->port_type, cc);
> +	return ret == 0;
>  }
> 
>  static void tcpm_set_cc(struct tcpm_port *port, enum typec_cc_status cc)
> @@ -2847,15 +2843,15 @@ static void run_state_machine(struct tcpm_port
> *port)
> 
>  	port->enter_state = port->state;
>  	switch (port->state) {
> -	case DRP_TOGGLING:
> +	case TOGGLING:
>  		break;
>  	/* SRC states */
>  	case SRC_UNATTACHED:
>  		if (!port->non_pd_role_swap)
>  			tcpm_swap_complete(port, -ENOTCONN);
>  		tcpm_src_detach(port);
> -		if (tcpm_start_drp_toggling(port, tcpm_rp_cc(port))) {
> -			tcpm_set_state(port, DRP_TOGGLING, 0);
> +		if (tcpm_start_toggling(port, tcpm_rp_cc(port))) {
> +			tcpm_set_state(port, TOGGLING, 0);
>  			break;
>  		}
>  		tcpm_set_cc(port, tcpm_rp_cc(port));
> @@ -3053,8 +3049,8 @@ static void run_state_machine(struct tcpm_port *port)
>  			tcpm_swap_complete(port, -ENOTCONN);
>  		tcpm_pps_complete(port, -ENOTCONN);
>  		tcpm_snk_detach(port);
> -		if (tcpm_start_drp_toggling(port, TYPEC_CC_RD)) {
> -			tcpm_set_state(port, DRP_TOGGLING, 0);
> +		if (tcpm_start_toggling(port, TYPEC_CC_RD)) {
> +			tcpm_set_state(port, TOGGLING, 0);
>  			break;
>  		}
>  		tcpm_set_cc(port, TYPEC_CC_RD);
> @@ -3621,7 +3617,7 @@ static void _tcpm_cc_change(struct tcpm_port *port,
> enum typec_cc_status cc1,
>  						       : "connected");
> 
>  	switch (port->state) {
> -	case DRP_TOGGLING:
> +	case TOGGLING:
>  		if (tcpm_port_is_debug(port) || tcpm_port_is_audio(port) ||
>  		    tcpm_port_is_source(port))
>  			tcpm_set_state(port, SRC_ATTACH_WAIT, 0);
> diff --git a/drivers/usb/typec/tcpm/wcove.c b/drivers/usb/typec/tcpm/wcove.c
> index 6770afd40765..f1f8f45e2f3d 100644
> --- a/drivers/usb/typec/tcpm/wcove.c
> +++ b/drivers/usb/typec/tcpm/wcove.c
> @@ -416,12 +416,16 @@ static int wcove_pd_transmit(struct tcpc_dev *tcpc,
>  	return regmap_write(wcove->regmap, USBC_TXCMD, cmd |
> USBC_TXCMD_START);
>  }
> 
> -static int wcove_start_drp_toggling(struct tcpc_dev *tcpc,
> -				    enum typec_cc_status cc)
> +static int wcove_start_toggling(struct tcpc_dev *tcpc,
> +				enum typec_port_type port_type,
> +				enum typec_cc_status cc)
>  {
>  	struct wcove_typec *wcove = tcpc_to_wcove(tcpc);
>  	unsigned int usbc_ctrl;
> 
> +	if (port_type != TYPEC_PORT_DRP)
> +		return -EOPNOTSUPP;
> +
>  	usbc_ctrl = USBC_CONTROL1_MODE_DRP |
> USBC_CONTROL1_DRPTOGGLE_RANDOM;
> 
>  	switch (cc) {
> @@ -642,7 +646,7 @@ static int wcove_typec_probe(struct platform_device
> *pdev)
>  	wcove->tcpc.set_polarity = wcove_set_polarity;
>  	wcove->tcpc.set_vconn = wcove_set_vconn;
>  	wcove->tcpc.set_current_limit = wcove_set_current_limit;
> -	wcove->tcpc.start_drp_toggling = wcove_start_drp_toggling;
> +	wcove->tcpc.start_toggling = wcove_start_toggling;
> 
>  	wcove->tcpc.set_pd_rx = wcove_set_pd_rx;
>  	wcove->tcpc.set_roles = wcove_set_roles;
> diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
> index 0c532ca3f079..36a15dcadc53 100644
> --- a/include/linux/usb/tcpm.h
> +++ b/include/linux/usb/tcpm.h
> @@ -121,10 +121,10 @@ struct tcpc_config {
>   *		with partner.
>   * @set_pd_rx:	Called to enable or disable reception of PD messages
>   * @set_roles:	Called to set power and data roles
> - * @start_drp_toggling:
> - *		Optional; if supported by hardware, called to start DRP
> - *		toggling. DRP toggling is stopped automatically if
> - *		a connection is established.
> + * @start_toggling:
> + *		Optional; if supported by hardware, called to start dual-role
> + *		toggling or single-role connection detection. Toggling stops
> + *		automatically if a connection is established.
>   * @try_role:	Optional; called to set a preferred role
>   * @pd_transmit:Called to transmit PD message
>   * @mux:	Pointer to multiplexer data
> @@ -147,8 +147,9 @@ struct tcpc_dev {
>  	int (*set_pd_rx)(struct tcpc_dev *dev, bool on);
>  	int (*set_roles)(struct tcpc_dev *dev, bool attached,
>  			 enum typec_role role, enum typec_data_role data);
> -	int (*start_drp_toggling)(struct tcpc_dev *dev,
> -				  enum typec_cc_status cc);
> +	int (*start_toggling)(struct tcpc_dev *dev,
> +			      enum typec_port_type port_type,
> +			      enum typec_cc_status cc);
>  	int (*try_role)(struct tcpc_dev *dev, int role);
>  	int (*pd_transmit)(struct tcpc_dev *dev, enum tcpm_transmit_type
> type,
>  			   const struct pd_message *msg);
> --
> 2.21.0

[v2,2/3] usb: typec: fusb302: Implement start_toggling for all port-types

[Opensource [Adam Thomson]]

On 16 April 2019 21:08, Hans de Goede wrote:

> When in single-role port mode, we must start single-role toggling to
> get an interrupt when a device / cable gets plugged into the port.
> 
> This commit modifies the fusb302 start_toggling implementation to
> start toggling for all port-types, so that connection-detection works
> on single-role ports too.
> 
> Fixes: ea3b4d5523bc("usb: typec: fusb302: Resolve fixed power role ...")
> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

For DRP & Sink only toggling scenarios, with FUSB302 TCPC:

Tested-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>

> ---
> Changes in v2:
> -Adjust for the tcpm core renaming start_drp_toggling to start_toggling,
>  instead of adding a new start_srp_connection_detect callback
> ---
>  drivers/usb/typec/tcpm/fusb302.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c
> b/drivers/usb/typec/tcpm/fusb302.c
> index 6d83891cc895..ba030b03d156 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -882,10 +882,20 @@ static int tcpm_start_toggling(struct tcpc_dev *dev,
>  {
>  	struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
>  						 tcpc_dev);
> +	enum toggling_mode mode = TOGGLING_MODE_OFF;
>  	int ret = 0;
> 
> -	if (port_type != TYPEC_PORT_DRP)
> -		return -EOPNOTSUPP;
> +	switch (port_type) {
> +	case TYPEC_PORT_SRC:
> +		mode = TOGGLING_MODE_SRC;
> +		break;
> +	case TYPEC_PORT_SNK:
> +		mode = TOGGLING_MODE_SNK;
> +		break;
> +	case TYPEC_PORT_DRP:
> +		mode = TOGGLING_MODE_DRP;
> +		break;
> +	}
> 
>  	mutex_lock(&chip->lock);
>  	ret = fusb302_set_src_current(chip, cc_src_current[cc]);
> @@ -894,7 +904,7 @@ static int tcpm_start_toggling(struct tcpc_dev *dev,
>  			    typec_cc_status_name[cc], ret);
>  		goto done;
>  	}
> -	ret = fusb302_set_toggling(chip, TOGGLING_MODE_DRP);
> +	ret = fusb302_set_toggling(chip, mode);
>  	if (ret < 0) {
>  		fusb302_log(chip,
>  			    "unable to start drp toggling, ret=%d", ret);
> --
> 2.21.0

RE: [PATCH v2 2/3] usb: typec: fusb302: Implement start_toggling for all port-types

[Adam Thomson]

On 16 April 2019 21:08, Hans de Goede wrote:

> When in single-role port mode, we must start single-role toggling to
> get an interrupt when a device / cable gets plugged into the port.
> 
> This commit modifies the fusb302 start_toggling implementation to
> start toggling for all port-types, so that connection-detection works
> on single-role ports too.
> 
> Fixes: ea3b4d5523bc("usb: typec: fusb302: Resolve fixed power role ...")
> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

For DRP & Sink only toggling scenarios, with FUSB302 TCPC:

Tested-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>

> ---
> Changes in v2:
> -Adjust for the tcpm core renaming start_drp_toggling to start_toggling,
>  instead of adding a new start_srp_connection_detect callback
> ---
>  drivers/usb/typec/tcpm/fusb302.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c
> b/drivers/usb/typec/tcpm/fusb302.c
> index 6d83891cc895..ba030b03d156 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -882,10 +882,20 @@ static int tcpm_start_toggling(struct tcpc_dev *dev,
>  {
>  	struct fusb302_chip *chip = container_of(dev, struct fusb302_chip,
>  						 tcpc_dev);
> +	enum toggling_mode mode = TOGGLING_MODE_OFF;
>  	int ret = 0;
> 
> -	if (port_type != TYPEC_PORT_DRP)
> -		return -EOPNOTSUPP;
> +	switch (port_type) {
> +	case TYPEC_PORT_SRC:
> +		mode = TOGGLING_MODE_SRC;
> +		break;
> +	case TYPEC_PORT_SNK:
> +		mode = TOGGLING_MODE_SNK;
> +		break;
> +	case TYPEC_PORT_DRP:
> +		mode = TOGGLING_MODE_DRP;
> +		break;
> +	}
> 
>  	mutex_lock(&chip->lock);
>  	ret = fusb302_set_src_current(chip, cc_src_current[cc]);
> @@ -894,7 +904,7 @@ static int tcpm_start_toggling(struct tcpc_dev *dev,
>  			    typec_cc_status_name[cc], ret);
>  		goto done;
>  	}
> -	ret = fusb302_set_toggling(chip, TOGGLING_MODE_DRP);
> +	ret = fusb302_set_toggling(chip, mode);
>  	if (ret < 0) {
>  		fusb302_log(chip,
>  			    "unable to start drp toggling, ret=%d", ret);
> --
> 2.21.0

[v2,3/3] usb: typec: fusb302: Revert "Resolve fixed power role contract setup"

[Opensource [Adam Thomson]]

On 16 April 2019 21:08, Hans de Goede wrote:

> Some tcpc device-drivers need to explicitly be told to watch for connection
> events, otherwise the tcpc will not generate any TCPM_CC_EVENTs and devices
> being plugged into the Type-C port will not be noticed.
> 
> For dual-role ports tcpm_start_drp_toggling() is used to tell the tcpc to
> watch for connection events. But for single-role ports we've so far been
> falling back to just calling tcpm_set_cc(). For some tcpc-s such as the
> fusb302 this is not enough and no TCPM_CC_EVENT will be generated.
> 
> Commit ea3b4d5523bc ("usb: typec: fusb302: Resolve fixed power role
> contract setup") fixed SRPs not working because of this by making the
> fusb302 driver start connection detection on every tcpm_set_cc() call.
> It turns out this breaks src->snk power-role swapping because during the
> swap we first set the Cc pins to Rp, calling set_cc, and then send a PS_RDY
> message. But the fusb302 cannot send PD messages while its toggling engine
> is active, so sending the PS_RDY message fails.
> 
> Struct tcpc_dev now has a new start_srp_connection_detect callback and
> fusb302.c now implements this. This callback gets called when we the
> fusb302 needs to start connection detection, fixing fusb302 SRPs not
> seeing connected devices.
> 
> This allows us to revert the changes to fusb302's set_cc implementation,
> making it once again purely setup the Cc-s and matching disconnect
> detection, fixing src->snk power-role swapping no longer working.
> 
> Note that since the code was refactored in between, codewise this is not a
> straight forward revert. Functionality wise this is a straight revert and
> the original functionality is fully restored.
> 
> Fixes: ea3b4d5523bc ("usb: typec: fusb302: Resolve fixed power role ...")
> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

For DRP & Sink only toggling scenarios, with FUSB302 TCPC:

Tested-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>

> ---
> Changes in v2:
> -No changes
> ---
>  drivers/usb/typec/tcpm/fusb302.c | 55 ++++++++++++++++++++++++++------
>  1 file changed, 46 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c
> b/drivers/usb/typec/tcpm/fusb302.c
> index ba030b03d156..4328a6cbfb69 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -606,19 +606,16 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum
> typec_cc_status cc)
>  			    FUSB_REG_SWITCHES0_CC2_PU_EN |
>  			    FUSB_REG_SWITCHES0_CC1_PD_EN |
>  			    FUSB_REG_SWITCHES0_CC2_PD_EN;
> -	u8 switches0_data = 0x00;
> +	u8 rd_mda, switches0_data = 0x00;
>  	int ret = 0;
> -	enum toggling_mode mode;
> 
>  	mutex_lock(&chip->lock);
>  	switch (cc) {
>  	case TYPEC_CC_OPEN:
> -		mode = TOGGLING_MODE_OFF;
>  		break;
>  	case TYPEC_CC_RD:
>  		switches0_data |= FUSB_REG_SWITCHES0_CC1_PD_EN |
>  				  FUSB_REG_SWITCHES0_CC2_PD_EN;
> -		mode = TOGGLING_MODE_SNK;
>  		break;
>  	case TYPEC_CC_RP_DEF:
>  	case TYPEC_CC_RP_1_5:
> @@ -626,7 +623,6 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum
> typec_cc_status cc)
>  		switches0_data |= (chip->cc_polarity == TYPEC_POLARITY_CC1) ?
>  				  FUSB_REG_SWITCHES0_CC1_PU_EN :
>  				  FUSB_REG_SWITCHES0_CC2_PU_EN;
> -		mode = TOGGLING_MODE_SRC;
>  		break;
>  	default:
>  		fusb302_log(chip, "unsupported cc value %s",
> @@ -637,6 +633,12 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum
> typec_cc_status cc)
> 
>  	fusb302_log(chip, "cc := %s", typec_cc_status_name[cc]);
> 
> +	ret = fusb302_set_toggling(chip, TOGGLING_MODE_OFF);
> +	if (ret < 0) {
> +		fusb302_log(chip, "cannot set toggling mode, ret=%d", ret);
> +		goto done;
> +	}
> +
>  	ret = fusb302_i2c_mask_write(chip, FUSB_REG_SWITCHES0,
>  				     switches0_mask, switches0_data);
>  	if (ret < 0) {
> @@ -655,10 +657,45 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum
> typec_cc_status cc)
>  		goto done;
>  	}
> 
> -	ret = fusb302_set_toggling(chip, mode);
> -	if (ret < 0)
> -		fusb302_log(chip, "cannot set toggling mode, ret=%d", ret);
> -
> +	/* enable/disable interrupts, BC_LVL for SNK and COMP_CHNG for SRC
> */
> +	switch (cc) {
> +	case TYPEC_CC_RP_DEF:
> +	case TYPEC_CC_RP_1_5:
> +	case TYPEC_CC_RP_3_0:
> +		rd_mda = rd_mda_value[cc_src_current[cc]];
> +		ret = fusb302_i2c_write(chip, FUSB_REG_MEASURE, rd_mda);
> +		if (ret < 0) {
> +			fusb302_log(chip,
> +				    "cannot set SRC measure value, ret=%d",
> +				    ret);
> +			goto done;
> +		}
> +		ret = fusb302_i2c_mask_write(chip, FUSB_REG_MASK,
> +					     FUSB_REG_MASK_BC_LVL |
> +					     FUSB_REG_MASK_COMP_CHNG,
> +					     FUSB_REG_MASK_COMP_CHNG);
> +		if (ret < 0) {
> +			fusb302_log(chip, "cannot set SRC interrupt, ret=%d",
> +				    ret);
> +			goto done;
> +		}
> +		chip->intr_comp_chng = true;
> +		break;
> +	case TYPEC_CC_RD:
> +		ret = fusb302_i2c_mask_write(chip, FUSB_REG_MASK,
> +					     FUSB_REG_MASK_BC_LVL |
> +					     FUSB_REG_MASK_COMP_CHNG,
> +					     FUSB_REG_MASK_BC_LVL);
> +		if (ret < 0) {
> +			fusb302_log(chip, "cannot set SRC interrupt, ret=%d",
> +				    ret);
> +			goto done;
> +		}
> +		chip->intr_bc_lvl = true;
> +		break;
> +	default:
> +		break;
> +	}
>  done:
>  	mutex_unlock(&chip->lock);
> 
> --
> 2.21.0

RE: [PATCH v2 3/3] usb: typec: fusb302: Revert "Resolve fixed power role contract setup"

[Adam Thomson]

On 16 April 2019 21:08, Hans de Goede wrote:

> Some tcpc device-drivers need to explicitly be told to watch for connection
> events, otherwise the tcpc will not generate any TCPM_CC_EVENTs and devices
> being plugged into the Type-C port will not be noticed.
> 
> For dual-role ports tcpm_start_drp_toggling() is used to tell the tcpc to
> watch for connection events. But for single-role ports we've so far been
> falling back to just calling tcpm_set_cc(). For some tcpc-s such as the
> fusb302 this is not enough and no TCPM_CC_EVENT will be generated.
> 
> Commit ea3b4d5523bc ("usb: typec: fusb302: Resolve fixed power role
> contract setup") fixed SRPs not working because of this by making the
> fusb302 driver start connection detection on every tcpm_set_cc() call.
> It turns out this breaks src->snk power-role swapping because during the
> swap we first set the Cc pins to Rp, calling set_cc, and then send a PS_RDY
> message. But the fusb302 cannot send PD messages while its toggling engine
> is active, so sending the PS_RDY message fails.
> 
> Struct tcpc_dev now has a new start_srp_connection_detect callback and
> fusb302.c now implements this. This callback gets called when we the
> fusb302 needs to start connection detection, fixing fusb302 SRPs not
> seeing connected devices.
> 
> This allows us to revert the changes to fusb302's set_cc implementation,
> making it once again purely setup the Cc-s and matching disconnect
> detection, fixing src->snk power-role swapping no longer working.
> 
> Note that since the code was refactored in between, codewise this is not a
> straight forward revert. Functionality wise this is a straight revert and
> the original functionality is fully restored.
> 
> Fixes: ea3b4d5523bc ("usb: typec: fusb302: Resolve fixed power role ...")
> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

For DRP & Sink only toggling scenarios, with FUSB302 TCPC:

Tested-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>

> ---
> Changes in v2:
> -No changes
> ---
>  drivers/usb/typec/tcpm/fusb302.c | 55 ++++++++++++++++++++++++++------
>  1 file changed, 46 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/fusb302.c
> b/drivers/usb/typec/tcpm/fusb302.c
> index ba030b03d156..4328a6cbfb69 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -606,19 +606,16 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum
> typec_cc_status cc)
>  			    FUSB_REG_SWITCHES0_CC2_PU_EN |
>  			    FUSB_REG_SWITCHES0_CC1_PD_EN |
>  			    FUSB_REG_SWITCHES0_CC2_PD_EN;
> -	u8 switches0_data = 0x00;
> +	u8 rd_mda, switches0_data = 0x00;
>  	int ret = 0;
> -	enum toggling_mode mode;
> 
>  	mutex_lock(&chip->lock);
>  	switch (cc) {
>  	case TYPEC_CC_OPEN:
> -		mode = TOGGLING_MODE_OFF;
>  		break;
>  	case TYPEC_CC_RD:
>  		switches0_data |= FUSB_REG_SWITCHES0_CC1_PD_EN |
>  				  FUSB_REG_SWITCHES0_CC2_PD_EN;
> -		mode = TOGGLING_MODE_SNK;
>  		break;
>  	case TYPEC_CC_RP_DEF:
>  	case TYPEC_CC_RP_1_5:
> @@ -626,7 +623,6 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum
> typec_cc_status cc)
>  		switches0_data |= (chip->cc_polarity == TYPEC_POLARITY_CC1) ?
>  				  FUSB_REG_SWITCHES0_CC1_PU_EN :
>  				  FUSB_REG_SWITCHES0_CC2_PU_EN;
> -		mode = TOGGLING_MODE_SRC;
>  		break;
>  	default:
>  		fusb302_log(chip, "unsupported cc value %s",
> @@ -637,6 +633,12 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum
> typec_cc_status cc)
> 
>  	fusb302_log(chip, "cc := %s", typec_cc_status_name[cc]);
> 
> +	ret = fusb302_set_toggling(chip, TOGGLING_MODE_OFF);
> +	if (ret < 0) {
> +		fusb302_log(chip, "cannot set toggling mode, ret=%d", ret);
> +		goto done;
> +	}
> +
>  	ret = fusb302_i2c_mask_write(chip, FUSB_REG_SWITCHES0,
>  				     switches0_mask, switches0_data);
>  	if (ret < 0) {
> @@ -655,10 +657,45 @@ static int tcpm_set_cc(struct tcpc_dev *dev, enum
> typec_cc_status cc)
>  		goto done;
>  	}
> 
> -	ret = fusb302_set_toggling(chip, mode);
> -	if (ret < 0)
> -		fusb302_log(chip, "cannot set toggling mode, ret=%d", ret);
> -
> +	/* enable/disable interrupts, BC_LVL for SNK and COMP_CHNG for SRC
> */
> +	switch (cc) {
> +	case TYPEC_CC_RP_DEF:
> +	case TYPEC_CC_RP_1_5:
> +	case TYPEC_CC_RP_3_0:
> +		rd_mda = rd_mda_value[cc_src_current[cc]];
> +		ret = fusb302_i2c_write(chip, FUSB_REG_MEASURE, rd_mda);
> +		if (ret < 0) {
> +			fusb302_log(chip,
> +				    "cannot set SRC measure value, ret=%d",
> +				    ret);
> +			goto done;
> +		}
> +		ret = fusb302_i2c_mask_write(chip, FUSB_REG_MASK,
> +					     FUSB_REG_MASK_BC_LVL |
> +					     FUSB_REG_MASK_COMP_CHNG,
> +					     FUSB_REG_MASK_COMP_CHNG);
> +		if (ret < 0) {
> +			fusb302_log(chip, "cannot set SRC interrupt, ret=%d",
> +				    ret);
> +			goto done;
> +		}
> +		chip->intr_comp_chng = true;
> +		break;
> +	case TYPEC_CC_RD:
> +		ret = fusb302_i2c_mask_write(chip, FUSB_REG_MASK,
> +					     FUSB_REG_MASK_BC_LVL |
> +					     FUSB_REG_MASK_COMP_CHNG,
> +					     FUSB_REG_MASK_BC_LVL);
> +		if (ret < 0) {
> +			fusb302_log(chip, "cannot set SRC interrupt, ret=%d",
> +				    ret);
> +			goto done;
> +		}
> +		chip->intr_bc_lvl = true;
> +		break;
> +	default:
> +		break;
> +	}
>  done:
>  	mutex_unlock(&chip->lock);
> 
> --
> 2.21.0