[PATCH v2 0/1] Add USB role switch to tps6598x

[PATCH v2 0/1] Add USB role switch to tps6598x

[Bryan O'Donoghue]

V2:
- Updates the git commit to include a link to the TI document which
  describes firmware configuration - Andy
- Pads out the description a bit to include information on baking the
  data-role into the firmware with the referenced utility.

V1:
This patch - adds USB role switching to the TI TPS6598x. It has been tested
out with a ChipIdea controller inside a Qualcomm MSM8939.

Right now you need to have configured the TPS firmware with the TI
configuration tool so that the chip knows if it should initiate or accept
data and power role swaps.

https://www.ti.com/lit/an/slva843a/slva843a.pdf

Heikki mentioned that on the ACPI systems the firmware had been
pre-configured to do data/power role swaps. On the hardware I have this is
the case also, which is why I did't invest more time in adding DT bindings
to control data/power roles that I don't need or necessarily support with
the reference hardware.

As-is this code will do role-swappping nicely for me, and I think should be
safe on existing ACPI systems.

Bryan O'Donoghue (1):
  usb: typec: tps6598x: Add USB role switching logic

 drivers/usb/typec/tps6598x.c | 56 +++++++++++++++++++++++++++++++-----
 1 file changed, 49 insertions(+), 7 deletions(-)

-- 
2.25.1

[PATCH v2] usb: typec: tps6598x: Add USB role switching logic

[Bryan O'Donoghue]

This patch adds USB role switch support to the tps6598x.

The setup to initiate or accept a data-role switch is both assumed and
currently required to be baked-into the firmware as described in TI's
document here.

Link: https://www.ti.com/lit/an/slva843a/slva843a.pdf

With this change its possible to use the USB role-switch API to detect and
notify role-switches to downstream consumers.

Tested with a ChipIdea controller on a Qualcomm MSM8939.

Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Gustavo A. R. Silva <garsilva@embeddedor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: linux-usb@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/usb/typec/tps6598x.c | 56 +++++++++++++++++++++++++++++++-----
 1 file changed, 49 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index defa651282b0..b190097c9dba 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -12,6 +12,7 @@
 #include <linux/regmap.h>
 #include <linux/interrupt.h>
 #include <linux/usb/typec.h>
+#include <linux/usb/role.h>
 
 /* Register offsets */
 #define TPS_REG_VID			0x00
@@ -94,6 +95,7 @@ struct tps6598x {
 	struct typec_port *port;
 	struct typec_partner *partner;
 	struct usb_pd_identity partner_identity;
+	struct usb_role_switch *role_sw;
 };
 
 /*
@@ -190,6 +192,23 @@ static int tps6598x_read_partner_identity(struct tps6598x *tps)
 	return 0;
 }
 
+static void tps6598x_set_data_role(struct tps6598x *tps,
+				   enum typec_data_role role, bool connected)
+{
+	enum usb_role role_val;
+
+	if (role == TYPEC_HOST)
+		role_val = USB_ROLE_HOST;
+	else
+		role_val = USB_ROLE_DEVICE;
+
+	if (!connected)
+		role_val = USB_ROLE_NONE;
+
+	usb_role_switch_set_role(tps->role_sw, role_val);
+	typec_set_data_role(tps->port, role);
+}
+
 static int tps6598x_connect(struct tps6598x *tps, u32 status)
 {
 	struct typec_partner_desc desc;
@@ -220,7 +239,7 @@ static int tps6598x_connect(struct tps6598x *tps, u32 status)
 	typec_set_pwr_opmode(tps->port, mode);
 	typec_set_pwr_role(tps->port, TPS_STATUS_PORTROLE(status));
 	typec_set_vconn_role(tps->port, TPS_STATUS_VCONN(status));
-	typec_set_data_role(tps->port, TPS_STATUS_DATAROLE(status));
+	tps6598x_set_data_role(tps, TPS_STATUS_DATAROLE(status), true);
 
 	tps->partner = typec_register_partner(tps->port, &desc);
 	if (IS_ERR(tps->partner))
@@ -240,7 +259,7 @@ static void tps6598x_disconnect(struct tps6598x *tps, u32 status)
 	typec_set_pwr_opmode(tps->port, TYPEC_PWR_MODE_USB);
 	typec_set_pwr_role(tps->port, TPS_STATUS_PORTROLE(status));
 	typec_set_vconn_role(tps->port, TPS_STATUS_VCONN(status));
-	typec_set_data_role(tps->port, TPS_STATUS_DATAROLE(status));
+	tps6598x_set_data_role(tps, TPS_STATUS_DATAROLE(status), false);
 }
 
 static int tps6598x_exec_cmd(struct tps6598x *tps, const char *cmd,
@@ -328,7 +347,7 @@ static int tps6598x_dr_set(struct typec_port *port, enum typec_data_role role)
 		goto out_unlock;
 	}
 
-	typec_set_data_role(tps->port, role);
+	tps6598x_set_data_role(tps, role, true);
 
 out_unlock:
 	mutex_unlock(&tps->lock);
@@ -452,6 +471,7 @@ static int tps6598x_probe(struct i2c_client *client)
 {
 	struct typec_capability typec_cap = { };
 	struct tps6598x *tps;
+	struct fwnode_handle *fwnode;
 	u32 status;
 	u32 conf;
 	u32 vid;
@@ -495,11 +515,21 @@ static int tps6598x_probe(struct i2c_client *client)
 	if (ret < 0)
 		return ret;
 
+	fwnode = device_get_named_child_node(&client->dev, "connector");
+	if (!IS_ERR_OR_NULL(fwnode)) {
+		tps->role_sw = fwnode_usb_role_switch_get(fwnode);
+		if (IS_ERR(tps->role_sw)) {
+			ret = PTR_ERR(tps->role_sw);
+			goto err_fwnode_put;
+		}
+	}
+
 	typec_cap.revision = USB_TYPEC_REV_1_2;
 	typec_cap.pd_revision = 0x200;
 	typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
 	typec_cap.driver_data = tps;
 	typec_cap.ops = &tps6598x_ops;
+	typec_cap.fwnode = fwnode;
 
 	switch (TPS_SYSCONF_PORTINFO(conf)) {
 	case TPS_PORTINFO_SINK_ACCESSORY:
@@ -525,12 +555,16 @@ static int tps6598x_probe(struct i2c_client *client)
 		typec_cap.data = TYPEC_PORT_DFP;
 		break;
 	default:
-		return -ENODEV;
+		ret = -ENODEV;
+		goto err_role_put;
 	}
 
 	tps->port = typec_register_port(&client->dev, &typec_cap);
-	if (IS_ERR(tps->port))
-		return PTR_ERR(tps->port);
+	if (IS_ERR(tps->port)) {
+		ret = PTR_ERR(tps->port);
+		goto err_role_put;
+	}
+	fwnode_handle_put(fwnode);
 
 	if (status & TPS_STATUS_PLUG_PRESENT) {
 		ret = tps6598x_connect(tps, status);
@@ -545,12 +579,19 @@ static int tps6598x_probe(struct i2c_client *client)
 	if (ret) {
 		tps6598x_disconnect(tps, 0);
 		typec_unregister_port(tps->port);
-		return ret;
+		goto err_role_put;
 	}
 
 	i2c_set_clientdata(client, tps);
 
 	return 0;
+
+err_role_put:
+	usb_role_switch_put(tps->role_sw);
+err_fwnode_put:
+	fwnode_handle_put(fwnode);
+
+	return ret;
 }
 
 static int tps6598x_remove(struct i2c_client *client)
@@ -559,6 +600,7 @@ static int tps6598x_remove(struct i2c_client *client)
 
 	tps6598x_disconnect(tps, 0);
 	typec_unregister_port(tps->port);
+	usb_role_switch_put(tps->role_sw);
 
 	return 0;
 }
-- 
2.25.1

Re: [PATCH v2] usb: typec: tps6598x: Add USB role switching logic

[Heikki Krogerus]

Hi,

On Thu, May 07, 2020 at 10:59:38PM +0100, Bryan O'Donoghue wrote:
> @@ -452,6 +471,7 @@ static int tps6598x_probe(struct i2c_client *client)
>  {
>  	struct typec_capability typec_cap = { };
>  	struct tps6598x *tps;
> +	struct fwnode_handle *fwnode;
>  	u32 status;
>  	u32 conf;
>  	u32 vid;
> @@ -495,11 +515,21 @@ static int tps6598x_probe(struct i2c_client *client)
>  	if (ret < 0)
>  		return ret;
>  
> +	fwnode = device_get_named_child_node(&client->dev, "connector");
> +	if (!IS_ERR_OR_NULL(fwnode)) {
> +		tps->role_sw = fwnode_usb_role_switch_get(fwnode);
> +		if (IS_ERR(tps->role_sw)) {
> +			ret = PTR_ERR(tps->role_sw);
> +			goto err_fwnode_put;
> +		}
> +	}

        fwnode = device_get_named_child_node(&client->dev, "connector");
        if (IS_ERR(fwnode))
                return PTR_ERR(fwnode);

        tps->role_sw = fwnode_usb_role_switch_get(fwnode);
        if (IS_ERR(tps->role_sw)) {
        	ret = PTR_ERR(tps->role_sw);
        	goto err_fwnode_put;
        }

>  	typec_cap.revision = USB_TYPEC_REV_1_2;
>  	typec_cap.pd_revision = 0x200;
>  	typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
>  	typec_cap.driver_data = tps;
>  	typec_cap.ops = &tps6598x_ops;
> +	typec_cap.fwnode = fwnode;
>  
>  	switch (TPS_SYSCONF_PORTINFO(conf)) {
>  	case TPS_PORTINFO_SINK_ACCESSORY:
> @@ -525,12 +555,16 @@ static int tps6598x_probe(struct i2c_client *client)
>  		typec_cap.data = TYPEC_PORT_DFP;
>  		break;
>  	default:
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto err_role_put;
>  	}
>  
>  	tps->port = typec_register_port(&client->dev, &typec_cap);
> -	if (IS_ERR(tps->port))
> -		return PTR_ERR(tps->port);
> +	if (IS_ERR(tps->port)) {
> +		ret = PTR_ERR(tps->port);
> +		goto err_role_put;
> +	}
> +	fwnode_handle_put(fwnode);
>  
>  	if (status & TPS_STATUS_PLUG_PRESENT) {
>  		ret = tps6598x_connect(tps, status);
> @@ -545,12 +579,19 @@ static int tps6598x_probe(struct i2c_client *client)
>  	if (ret) {
>  		tps6598x_disconnect(tps, 0);
>  		typec_unregister_port(tps->port);
> -		return ret;
> +		goto err_role_put;
>  	}
>  
>  	i2c_set_clientdata(client, tps);
>  
>  	return 0;
> +
> +err_role_put:
> +	usb_role_switch_put(tps->role_sw);
> +err_fwnode_put:
> +	fwnode_handle_put(fwnode);
> +
> +	return ret;
>  }

thanks,

-- 
heikki