[PATCH 1/2] extcon: usbc-tusb320: add accessory detection support

[PATCH 1/2] extcon: usbc-tusb320: add accessory detection support

[Alvin Šipraga]

From: Alvin Šipraga <alsi@bang-olufsen.dk>

The TUSB320 can detect the following types of accessory:

  - Audio Accessory
  - Audio Accessory with charge-thru
  - Debug Accessory (DFP)
  - Debug Accessory (UFP)

Moreover, the typec subsystem can be informed of this through the
typec_set_mode() function. The information will be propagated to any
linked typec muxes. Add the necessary support to the driver.

Note that for the Debug Accessory modes, an educated guess was made that
for the USB data role, DFP implies HOST and UFP implies DEVICE. But this
might want to be made configurable at a later date.

Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
---
v2: no change
---
 drivers/extcon/extcon-usbc-tusb320.c | 90 +++++++++++++++++++++-------
 1 file changed, 68 insertions(+), 22 deletions(-)

diff --git a/drivers/extcon/extcon-usbc-tusb320.c b/drivers/extcon/extcon-usbc-tusb320.c
index 10dff1c512c4..882d1f48495e 100644
--- a/drivers/extcon/extcon-usbc-tusb320.c
+++ b/drivers/extcon/extcon-usbc-tusb320.c
@@ -15,6 +15,7 @@
 #include <linux/module.h>
 #include <linux/regmap.h>
 #include <linux/usb/typec.h>
+#include <linux/usb/typec_altmode.h>
 
 #define TUSB320_REG8				0x8
 #define TUSB320_REG8_CURRENT_MODE_ADVERTISE	GENMASK(7, 6)
@@ -26,16 +27,16 @@
 #define TUSB320_REG8_CURRENT_MODE_DETECT_MED	0x1
 #define TUSB320_REG8_CURRENT_MODE_DETECT_ACC	0x2
 #define TUSB320_REG8_CURRENT_MODE_DETECT_HI	0x3
-#define TUSB320_REG8_ACCESSORY_CONNECTED	GENMASK(3, 2)
+#define TUSB320_REG8_ACCESSORY_CONNECTED	GENMASK(3, 1)
 #define TUSB320_REG8_ACCESSORY_CONNECTED_NONE	0x0
 #define TUSB320_REG8_ACCESSORY_CONNECTED_AUDIO	0x4
-#define TUSB320_REG8_ACCESSORY_CONNECTED_ACC	0x5
-#define TUSB320_REG8_ACCESSORY_CONNECTED_DEBUG	0x6
+#define TUSB320_REG8_ACCESSORY_CONNECTED_ACHRG	0x5
+#define TUSB320_REG8_ACCESSORY_CONNECTED_DBGDFP	0x6
+#define TUSB320_REG8_ACCESSORY_CONNECTED_DBGUFP	0x7
 #define TUSB320_REG8_ACTIVE_CABLE_DETECTION	BIT(0)
 
 #define TUSB320_REG9				0x9
-#define TUSB320_REG9_ATTACHED_STATE_SHIFT	6
-#define TUSB320_REG9_ATTACHED_STATE_MASK	0x3
+#define TUSB320_REG9_ATTACHED_STATE		GENMASK(7, 6)
 #define TUSB320_REG9_CABLE_DIRECTION		BIT(5)
 #define TUSB320_REG9_INTERRUPT_STATUS		BIT(4)
 
@@ -250,8 +251,7 @@ static void tusb320_extcon_irq_handler(struct tusb320_priv *priv, u8 reg)
 {
 	int state, polarity;
 
-	state = (reg >> TUSB320_REG9_ATTACHED_STATE_SHIFT) &
-		TUSB320_REG9_ATTACHED_STATE_MASK;
+	state = FIELD_GET(TUSB320_REG9_ATTACHED_STATE, reg);
 	polarity = !!(reg & TUSB320_REG9_CABLE_DIRECTION);
 
 	dev_dbg(priv->dev, "attached state: %s, polarity: %d\n",
@@ -277,32 +277,78 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
 {
 	struct typec_port *port = priv->port;
 	struct device *dev = priv->dev;
-	u8 mode, role, state;
+	int typec_mode;
+	enum typec_role pwr_role;
+	enum typec_data_role data_role;
+	u8 state, mode, accessory;
 	int ret, reg8;
 	bool ori;
 
+	ret = regmap_read(priv->regmap, TUSB320_REG8, &reg8);
+	if (ret) {
+		dev_err(dev, "error during reg8 i2c read, ret=%d!\n", ret);
+		return;
+	}
+
 	ori = reg9 & TUSB320_REG9_CABLE_DIRECTION;
 	typec_set_orientation(port, ori ? TYPEC_ORIENTATION_REVERSE :
 					  TYPEC_ORIENTATION_NORMAL);
 
-	state = (reg9 >> TUSB320_REG9_ATTACHED_STATE_SHIFT) &
-		TUSB320_REG9_ATTACHED_STATE_MASK;
-	if (state == TUSB320_ATTACHED_STATE_DFP)
-		role = TYPEC_SOURCE;
-	else
-		role = TYPEC_SINK;
+	state = FIELD_GET(TUSB320_REG9_ATTACHED_STATE, reg9);
+	accessory = FIELD_GET(TUSB320_REG8_ACCESSORY_CONNECTED, reg8);
+
+	switch (state) {
+	case TUSB320_ATTACHED_STATE_DFP:
+		typec_mode = TYPEC_MODE_USB2;
+		pwr_role = TYPEC_SOURCE;
+		data_role = TYPEC_HOST;
+		break;
+	case TUSB320_ATTACHED_STATE_UFP:
+		typec_mode = TYPEC_MODE_USB2;
+		pwr_role = TYPEC_SINK;
+		data_role = TYPEC_DEVICE;
+		break;
+	case TUSB320_ATTACHED_STATE_ACC:
+		/*
+		 * Accessory detected. For debug accessories, just make some
+		 * qualified guesses as to the role for lack of a better option.
+		 */
+		if (accessory == TUSB320_REG8_ACCESSORY_CONNECTED_AUDIO ||
+		    accessory == TUSB320_REG8_ACCESSORY_CONNECTED_ACHRG) {
+			typec_mode = TYPEC_MODE_AUDIO;
+			pwr_role = TYPEC_SINK;
+			data_role = TYPEC_DEVICE;
+			break;
+		} else if (accessory ==
+			   TUSB320_REG8_ACCESSORY_CONNECTED_DBGDFP) {
+			typec_mode = TYPEC_MODE_DEBUG;
+			pwr_role = TYPEC_SOURCE;
+			data_role = TYPEC_HOST;
+			break;
+		} else if (accessory ==
+			   TUSB320_REG8_ACCESSORY_CONNECTED_DBGUFP) {
+			typec_mode = TYPEC_MODE_DEBUG;
+			pwr_role = TYPEC_SINK;
+			data_role = TYPEC_DEVICE;
+			break;
+		}
 
-	typec_set_vconn_role(port, role);
-	typec_set_pwr_role(port, role);
-	typec_set_data_role(port, role == TYPEC_SOURCE ?
-				  TYPEC_HOST : TYPEC_DEVICE);
+		dev_warn(priv->dev, "unexpected ACCESSORY_CONNECTED state %d\n",
+			 accessory);
 
-	ret = regmap_read(priv->regmap, TUSB320_REG8, &reg8);
-	if (ret) {
-		dev_err(dev, "error during reg8 i2c read, ret=%d!\n", ret);
-		return;
+		fallthrough;
+	default:
+		typec_mode = TYPEC_MODE_USB2;
+		pwr_role = TYPEC_SINK;
+		data_role = TYPEC_DEVICE;
+		break;
 	}
 
+	typec_set_vconn_role(port, pwr_role);
+	typec_set_pwr_role(port, pwr_role);
+	typec_set_data_role(port, data_role);
+	typec_set_mode(port, typec_mode);
+
 	mode = FIELD_GET(TUSB320_REG8_CURRENT_MODE_DETECT, reg8);
 	if (mode == TUSB320_REG8_CURRENT_MODE_DETECT_DEF)
 		typec_set_pwr_opmode(port, TYPEC_PWR_MODE_USB);
-- 
2.39.2

[PATCH 2/2] extcon: usbc-tusb320: add usb_role_switch support

[Alvin Šipraga]

From: Alvin Šipraga <alsi@bang-olufsen.dk>

The connector child node of the TUSB320 device might be linked with a
USB OTG controller with USB role switch capability. Add driver support
for detecting a usb_role_switch and setting its state in the typec
interrupt handler. This follows similar practice in other drivers in the
typec subsystem, which this extcon driver can opt-in to.

Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
---
v2: remove unused variable as reported by kernel test robot
---
 drivers/extcon/extcon-usbc-tusb320.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/extcon/extcon-usbc-tusb320.c b/drivers/extcon/extcon-usbc-tusb320.c
index 882d1f48495e..cc2d0ac7c5f6 100644
--- a/drivers/extcon/extcon-usbc-tusb320.c
+++ b/drivers/extcon/extcon-usbc-tusb320.c
@@ -16,6 +16,7 @@
 #include <linux/regmap.h>
 #include <linux/usb/typec.h>
 #include <linux/usb/typec_altmode.h>
+#include <linux/usb/role.h>
 
 #define TUSB320_REG8				0x8
 #define TUSB320_REG8_CURRENT_MODE_ADVERTISE	GENMASK(7, 6)
@@ -80,6 +81,7 @@ struct tusb320_priv {
 	enum typec_port_type port_type;
 	enum typec_pwr_opmode pwr_opmode;
 	struct fwnode_handle *connector_fwnode;
+	struct usb_role_switch *role_sw;
 };
 
 static const char * const tusb_attached_states[] = {
@@ -278,6 +280,7 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
 	struct typec_port *port = priv->port;
 	struct device *dev = priv->dev;
 	int typec_mode;
+	enum usb_role usb_role;
 	enum typec_role pwr_role;
 	enum typec_data_role data_role;
 	u8 state, mode, accessory;
@@ -300,11 +303,13 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
 	switch (state) {
 	case TUSB320_ATTACHED_STATE_DFP:
 		typec_mode = TYPEC_MODE_USB2;
+		usb_role = USB_ROLE_HOST;
 		pwr_role = TYPEC_SOURCE;
 		data_role = TYPEC_HOST;
 		break;
 	case TUSB320_ATTACHED_STATE_UFP:
 		typec_mode = TYPEC_MODE_USB2;
+		usb_role = USB_ROLE_DEVICE;
 		pwr_role = TYPEC_SINK;
 		data_role = TYPEC_DEVICE;
 		break;
@@ -316,6 +321,7 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
 		if (accessory == TUSB320_REG8_ACCESSORY_CONNECTED_AUDIO ||
 		    accessory == TUSB320_REG8_ACCESSORY_CONNECTED_ACHRG) {
 			typec_mode = TYPEC_MODE_AUDIO;
+			usb_role = USB_ROLE_NONE;
 			pwr_role = TYPEC_SINK;
 			data_role = TYPEC_DEVICE;
 			break;
@@ -323,12 +329,14 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
 			   TUSB320_REG8_ACCESSORY_CONNECTED_DBGDFP) {
 			typec_mode = TYPEC_MODE_DEBUG;
 			pwr_role = TYPEC_SOURCE;
+			usb_role = USB_ROLE_HOST;
 			data_role = TYPEC_HOST;
 			break;
 		} else if (accessory ==
 			   TUSB320_REG8_ACCESSORY_CONNECTED_DBGUFP) {
 			typec_mode = TYPEC_MODE_DEBUG;
 			pwr_role = TYPEC_SINK;
+			usb_role = USB_ROLE_DEVICE;
 			data_role = TYPEC_DEVICE;
 			break;
 		}
@@ -339,6 +347,7 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
 		fallthrough;
 	default:
 		typec_mode = TYPEC_MODE_USB2;
+		usb_role = USB_ROLE_NONE;
 		pwr_role = TYPEC_SINK;
 		data_role = TYPEC_DEVICE;
 		break;
@@ -348,6 +357,7 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
 	typec_set_pwr_role(port, pwr_role);
 	typec_set_data_role(port, data_role);
 	typec_set_mode(port, typec_mode);
+	usb_role_switch_set_role(priv->role_sw, usb_role);
 
 	mode = FIELD_GET(TUSB320_REG8_CURRENT_MODE_DETECT, reg8);
 	if (mode == TUSB320_REG8_CURRENT_MODE_DETECT_DEF)
@@ -472,10 +482,20 @@ static int tusb320_typec_probe(struct i2c_client *client,
 		goto err_put;
 	}
 
+	/* Find any optional USB role switch that needs reporting to */
+	priv->role_sw = fwnode_usb_role_switch_get(connector);
+	if (IS_ERR(priv->role_sw)) {
+		ret = PTR_ERR(priv->role_sw);
+		goto err_unreg;
+	}
+
 	priv->connector_fwnode = connector;
 
 	return 0;
 
+err_unreg:
+	typec_unregister_port(priv->port);
+
 err_put:
 	fwnode_handle_put(connector);
 
@@ -484,6 +504,7 @@ static int tusb320_typec_probe(struct i2c_client *client,
 
 static void tusb320_typec_remove(struct tusb320_priv *priv)
 {
+	usb_role_switch_put(priv->role_sw);
 	typec_unregister_port(priv->port);
 	fwnode_handle_put(priv->connector_fwnode);
 }
-- 
2.39.2

Re: [PATCH 2/2] extcon: usbc-tusb320: add usb_role_switch support

[Heikki Krogerus]

On Fri, Mar 17, 2023 at 11:42:28AM +0100, Alvin Šipraga wrote:
> From: Alvin Šipraga <alsi@bang-olufsen.dk>
> 
> The connector child node of the TUSB320 device might be linked with a
> USB OTG controller with USB role switch capability. Add driver support
> for detecting a usb_role_switch and setting its state in the typec
> interrupt handler. This follows similar practice in other drivers in the
> typec subsystem, which this extcon driver can opt-in to.

I'm sorry to be a bit picky here, but I must ask that you don't use
the term USB OTG. It is too confusing - OTG may refer to USB device
controller, USB host controller, or dual-role capable USB controller,
depending on the case, so we need to be specific.

The OTG spec itself is in any case obsolete - USB Type-C and USB PD
specs define their own way of handling the roles, and they are not
compatible with the USB OTG specification(s).

thanks,

> Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> ---
> v2: remove unused variable as reported by kernel test robot
> ---
>  drivers/extcon/extcon-usbc-tusb320.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/extcon/extcon-usbc-tusb320.c b/drivers/extcon/extcon-usbc-tusb320.c
> index 882d1f48495e..cc2d0ac7c5f6 100644
> --- a/drivers/extcon/extcon-usbc-tusb320.c
> +++ b/drivers/extcon/extcon-usbc-tusb320.c
> @@ -16,6 +16,7 @@
>  #include <linux/regmap.h>
>  #include <linux/usb/typec.h>
>  #include <linux/usb/typec_altmode.h>
> +#include <linux/usb/role.h>
>  
>  #define TUSB320_REG8				0x8
>  #define TUSB320_REG8_CURRENT_MODE_ADVERTISE	GENMASK(7, 6)
> @@ -80,6 +81,7 @@ struct tusb320_priv {
>  	enum typec_port_type port_type;
>  	enum typec_pwr_opmode pwr_opmode;
>  	struct fwnode_handle *connector_fwnode;
> +	struct usb_role_switch *role_sw;
>  };
>  
>  static const char * const tusb_attached_states[] = {
> @@ -278,6 +280,7 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
>  	struct typec_port *port = priv->port;
>  	struct device *dev = priv->dev;
>  	int typec_mode;
> +	enum usb_role usb_role;
>  	enum typec_role pwr_role;
>  	enum typec_data_role data_role;
>  	u8 state, mode, accessory;
> @@ -300,11 +303,13 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
>  	switch (state) {
>  	case TUSB320_ATTACHED_STATE_DFP:
>  		typec_mode = TYPEC_MODE_USB2;
> +		usb_role = USB_ROLE_HOST;
>  		pwr_role = TYPEC_SOURCE;
>  		data_role = TYPEC_HOST;
>  		break;
>  	case TUSB320_ATTACHED_STATE_UFP:
>  		typec_mode = TYPEC_MODE_USB2;
> +		usb_role = USB_ROLE_DEVICE;
>  		pwr_role = TYPEC_SINK;
>  		data_role = TYPEC_DEVICE;
>  		break;
> @@ -316,6 +321,7 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
>  		if (accessory == TUSB320_REG8_ACCESSORY_CONNECTED_AUDIO ||
>  		    accessory == TUSB320_REG8_ACCESSORY_CONNECTED_ACHRG) {
>  			typec_mode = TYPEC_MODE_AUDIO;
> +			usb_role = USB_ROLE_NONE;
>  			pwr_role = TYPEC_SINK;
>  			data_role = TYPEC_DEVICE;
>  			break;
> @@ -323,12 +329,14 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
>  			   TUSB320_REG8_ACCESSORY_CONNECTED_DBGDFP) {
>  			typec_mode = TYPEC_MODE_DEBUG;
>  			pwr_role = TYPEC_SOURCE;
> +			usb_role = USB_ROLE_HOST;
>  			data_role = TYPEC_HOST;
>  			break;
>  		} else if (accessory ==
>  			   TUSB320_REG8_ACCESSORY_CONNECTED_DBGUFP) {
>  			typec_mode = TYPEC_MODE_DEBUG;
>  			pwr_role = TYPEC_SINK;
> +			usb_role = USB_ROLE_DEVICE;
>  			data_role = TYPEC_DEVICE;
>  			break;
>  		}
> @@ -339,6 +347,7 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
>  		fallthrough;
>  	default:
>  		typec_mode = TYPEC_MODE_USB2;
> +		usb_role = USB_ROLE_NONE;
>  		pwr_role = TYPEC_SINK;
>  		data_role = TYPEC_DEVICE;
>  		break;
> @@ -348,6 +357,7 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
>  	typec_set_pwr_role(port, pwr_role);
>  	typec_set_data_role(port, data_role);
>  	typec_set_mode(port, typec_mode);
> +	usb_role_switch_set_role(priv->role_sw, usb_role);
>  
>  	mode = FIELD_GET(TUSB320_REG8_CURRENT_MODE_DETECT, reg8);
>  	if (mode == TUSB320_REG8_CURRENT_MODE_DETECT_DEF)
> @@ -472,10 +482,20 @@ static int tusb320_typec_probe(struct i2c_client *client,
>  		goto err_put;
>  	}
>  
> +	/* Find any optional USB role switch that needs reporting to */
> +	priv->role_sw = fwnode_usb_role_switch_get(connector);
> +	if (IS_ERR(priv->role_sw)) {
> +		ret = PTR_ERR(priv->role_sw);
> +		goto err_unreg;
> +	}
> +
>  	priv->connector_fwnode = connector;
>  
>  	return 0;
>  
> +err_unreg:
> +	typec_unregister_port(priv->port);
> +
>  err_put:
>  	fwnode_handle_put(connector);
>  
> @@ -484,6 +504,7 @@ static int tusb320_typec_probe(struct i2c_client *client,
>  
>  static void tusb320_typec_remove(struct tusb320_priv *priv)
>  {
> +	usb_role_switch_put(priv->role_sw);
>  	typec_unregister_port(priv->port);
>  	fwnode_handle_put(priv->connector_fwnode);
>  }
> -- 
> 2.39.2

-- 
heikki

Re: [PATCH 2/2] extcon: usbc-tusb320: add usb_role_switch support

[Alvin Šipraga]

On Fri, Mar 17, 2023 at 01:13:51PM +0200, Heikki Krogerus wrote:
> On Fri, Mar 17, 2023 at 11:42:28AM +0100, Alvin Šipraga wrote:
> > From: Alvin Šipraga <alsi@bang-olufsen.dk>
> > 
> > The connector child node of the TUSB320 device might be linked with a
> > USB OTG controller with USB role switch capability. Add driver support
> > for detecting a usb_role_switch and setting its state in the typec
> > interrupt handler. This follows similar practice in other drivers in the
> > typec subsystem, which this extcon driver can opt-in to.
> 
> I'm sorry to be a bit picky here, but I must ask that you don't use
> the term USB OTG. It is too confusing - OTG may refer to USB device
> controller, USB host controller, or dual-role capable USB controller,
> depending on the case, so we need to be specific.
> 
> The OTG spec itself is in any case obsolete - USB Type-C and USB PD
> specs define their own way of handling the roles, and they are not
> compatible with the USB OTG specification(s).

Sure thing, I will reword the commit. Gives me an excuse to label it v3
as well. Thanks!

Kind regards,
Alvin

> 
> thanks,
> 
> > Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> > ---
> > v2: remove unused variable as reported by kernel test robot
> > ---
> >  drivers/extcon/extcon-usbc-tusb320.c | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> > 
> > diff --git a/drivers/extcon/extcon-usbc-tusb320.c b/drivers/extcon/extcon-usbc-tusb320.c
> > index 882d1f48495e..cc2d0ac7c5f6 100644
> > --- a/drivers/extcon/extcon-usbc-tusb320.c
> > +++ b/drivers/extcon/extcon-usbc-tusb320.c
> > @@ -16,6 +16,7 @@
> >  #include <linux/regmap.h>
> >  #include <linux/usb/typec.h>
> >  #include <linux/usb/typec_altmode.h>
> > +#include <linux/usb/role.h>
> >  
> >  #define TUSB320_REG8				0x8
> >  #define TUSB320_REG8_CURRENT_MODE_ADVERTISE	GENMASK(7, 6)
> > @@ -80,6 +81,7 @@ struct tusb320_priv {
> >  	enum typec_port_type port_type;
> >  	enum typec_pwr_opmode pwr_opmode;
> >  	struct fwnode_handle *connector_fwnode;
> > +	struct usb_role_switch *role_sw;
> >  };
> >  
> >  static const char * const tusb_attached_states[] = {
> > @@ -278,6 +280,7 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
> >  	struct typec_port *port = priv->port;
> >  	struct device *dev = priv->dev;
> >  	int typec_mode;
> > +	enum usb_role usb_role;
> >  	enum typec_role pwr_role;
> >  	enum typec_data_role data_role;
> >  	u8 state, mode, accessory;
> > @@ -300,11 +303,13 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
> >  	switch (state) {
> >  	case TUSB320_ATTACHED_STATE_DFP:
> >  		typec_mode = TYPEC_MODE_USB2;
> > +		usb_role = USB_ROLE_HOST;
> >  		pwr_role = TYPEC_SOURCE;
> >  		data_role = TYPEC_HOST;
> >  		break;
> >  	case TUSB320_ATTACHED_STATE_UFP:
> >  		typec_mode = TYPEC_MODE_USB2;
> > +		usb_role = USB_ROLE_DEVICE;
> >  		pwr_role = TYPEC_SINK;
> >  		data_role = TYPEC_DEVICE;
> >  		break;
> > @@ -316,6 +321,7 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
> >  		if (accessory == TUSB320_REG8_ACCESSORY_CONNECTED_AUDIO ||
> >  		    accessory == TUSB320_REG8_ACCESSORY_CONNECTED_ACHRG) {
> >  			typec_mode = TYPEC_MODE_AUDIO;
> > +			usb_role = USB_ROLE_NONE;
> >  			pwr_role = TYPEC_SINK;
> >  			data_role = TYPEC_DEVICE;
> >  			break;
> > @@ -323,12 +329,14 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
> >  			   TUSB320_REG8_ACCESSORY_CONNECTED_DBGDFP) {
> >  			typec_mode = TYPEC_MODE_DEBUG;
> >  			pwr_role = TYPEC_SOURCE;
> > +			usb_role = USB_ROLE_HOST;
> >  			data_role = TYPEC_HOST;
> >  			break;
> >  		} else if (accessory ==
> >  			   TUSB320_REG8_ACCESSORY_CONNECTED_DBGUFP) {
> >  			typec_mode = TYPEC_MODE_DEBUG;
> >  			pwr_role = TYPEC_SINK;
> > +			usb_role = USB_ROLE_DEVICE;
> >  			data_role = TYPEC_DEVICE;
> >  			break;
> >  		}
> > @@ -339,6 +347,7 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
> >  		fallthrough;
> >  	default:
> >  		typec_mode = TYPEC_MODE_USB2;
> > +		usb_role = USB_ROLE_NONE;
> >  		pwr_role = TYPEC_SINK;
> >  		data_role = TYPEC_DEVICE;
> >  		break;
> > @@ -348,6 +357,7 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
> >  	typec_set_pwr_role(port, pwr_role);
> >  	typec_set_data_role(port, data_role);
> >  	typec_set_mode(port, typec_mode);
> > +	usb_role_switch_set_role(priv->role_sw, usb_role);
> >  
> >  	mode = FIELD_GET(TUSB320_REG8_CURRENT_MODE_DETECT, reg8);
> >  	if (mode == TUSB320_REG8_CURRENT_MODE_DETECT_DEF)
> > @@ -472,10 +482,20 @@ static int tusb320_typec_probe(struct i2c_client *client,
> >  		goto err_put;
> >  	}
> >  
> > +	/* Find any optional USB role switch that needs reporting to */
> > +	priv->role_sw = fwnode_usb_role_switch_get(connector);
> > +	if (IS_ERR(priv->role_sw)) {
> > +		ret = PTR_ERR(priv->role_sw);
> > +		goto err_unreg;
> > +	}
> > +
> >  	priv->connector_fwnode = connector;
> >  
> >  	return 0;
> >  
> > +err_unreg:
> > +	typec_unregister_port(priv->port);
> > +
> >  err_put:
> >  	fwnode_handle_put(connector);
> >  
> > @@ -484,6 +504,7 @@ static int tusb320_typec_probe(struct i2c_client *client,
> >  
> >  static void tusb320_typec_remove(struct tusb320_priv *priv)
> >  {
> > +	usb_role_switch_put(priv->role_sw);
> >  	typec_unregister_port(priv->port);
> >  	fwnode_handle_put(priv->connector_fwnode);
> >  }
> > -- 
> > 2.39.2
> 
> -- 
> heikki

Re: [PATCH 1/2] extcon: usbc-tusb320: add accessory detection support

[Heikki Krogerus]

On Fri, Mar 17, 2023 at 11:42:27AM +0100, Alvin Šipraga wrote:
> From: Alvin Šipraga <alsi@bang-olufsen.dk>
> 
> The TUSB320 can detect the following types of accessory:
> 
>   - Audio Accessory
>   - Audio Accessory with charge-thru
>   - Debug Accessory (DFP)
>   - Debug Accessory (UFP)
> 
> Moreover, the typec subsystem can be informed of this through the
> typec_set_mode() function. The information will be propagated to any
> linked typec muxes. Add the necessary support to the driver.
> 
> Note that for the Debug Accessory modes, an educated guess was made that
> for the USB data role, DFP implies HOST and UFP implies DEVICE. But this
> might want to be made configurable at a later date.
> 
> Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> ---
> v2: no change

Not a big problem, but you forgot to include the version in the
subject. In any case, FWIW:

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

> ---
>  drivers/extcon/extcon-usbc-tusb320.c | 90 +++++++++++++++++++++-------
>  1 file changed, 68 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-usbc-tusb320.c b/drivers/extcon/extcon-usbc-tusb320.c
> index 10dff1c512c4..882d1f48495e 100644
> --- a/drivers/extcon/extcon-usbc-tusb320.c
> +++ b/drivers/extcon/extcon-usbc-tusb320.c
> @@ -15,6 +15,7 @@
>  #include <linux/module.h>
>  #include <linux/regmap.h>
>  #include <linux/usb/typec.h>
> +#include <linux/usb/typec_altmode.h>
>  
>  #define TUSB320_REG8				0x8
>  #define TUSB320_REG8_CURRENT_MODE_ADVERTISE	GENMASK(7, 6)
> @@ -26,16 +27,16 @@
>  #define TUSB320_REG8_CURRENT_MODE_DETECT_MED	0x1
>  #define TUSB320_REG8_CURRENT_MODE_DETECT_ACC	0x2
>  #define TUSB320_REG8_CURRENT_MODE_DETECT_HI	0x3
> -#define TUSB320_REG8_ACCESSORY_CONNECTED	GENMASK(3, 2)
> +#define TUSB320_REG8_ACCESSORY_CONNECTED	GENMASK(3, 1)
>  #define TUSB320_REG8_ACCESSORY_CONNECTED_NONE	0x0
>  #define TUSB320_REG8_ACCESSORY_CONNECTED_AUDIO	0x4
> -#define TUSB320_REG8_ACCESSORY_CONNECTED_ACC	0x5
> -#define TUSB320_REG8_ACCESSORY_CONNECTED_DEBUG	0x6
> +#define TUSB320_REG8_ACCESSORY_CONNECTED_ACHRG	0x5
> +#define TUSB320_REG8_ACCESSORY_CONNECTED_DBGDFP	0x6
> +#define TUSB320_REG8_ACCESSORY_CONNECTED_DBGUFP	0x7
>  #define TUSB320_REG8_ACTIVE_CABLE_DETECTION	BIT(0)
>  
>  #define TUSB320_REG9				0x9
> -#define TUSB320_REG9_ATTACHED_STATE_SHIFT	6
> -#define TUSB320_REG9_ATTACHED_STATE_MASK	0x3
> +#define TUSB320_REG9_ATTACHED_STATE		GENMASK(7, 6)
>  #define TUSB320_REG9_CABLE_DIRECTION		BIT(5)
>  #define TUSB320_REG9_INTERRUPT_STATUS		BIT(4)
>  
> @@ -250,8 +251,7 @@ static void tusb320_extcon_irq_handler(struct tusb320_priv *priv, u8 reg)
>  {
>  	int state, polarity;
>  
> -	state = (reg >> TUSB320_REG9_ATTACHED_STATE_SHIFT) &
> -		TUSB320_REG9_ATTACHED_STATE_MASK;
> +	state = FIELD_GET(TUSB320_REG9_ATTACHED_STATE, reg);
>  	polarity = !!(reg & TUSB320_REG9_CABLE_DIRECTION);
>  
>  	dev_dbg(priv->dev, "attached state: %s, polarity: %d\n",
> @@ -277,32 +277,78 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
>  {
>  	struct typec_port *port = priv->port;
>  	struct device *dev = priv->dev;
> -	u8 mode, role, state;
> +	int typec_mode;
> +	enum typec_role pwr_role;
> +	enum typec_data_role data_role;
> +	u8 state, mode, accessory;
>  	int ret, reg8;
>  	bool ori;
>  
> +	ret = regmap_read(priv->regmap, TUSB320_REG8, &reg8);
> +	if (ret) {
> +		dev_err(dev, "error during reg8 i2c read, ret=%d!\n", ret);
> +		return;
> +	}
> +
>  	ori = reg9 & TUSB320_REG9_CABLE_DIRECTION;
>  	typec_set_orientation(port, ori ? TYPEC_ORIENTATION_REVERSE :
>  					  TYPEC_ORIENTATION_NORMAL);
>  
> -	state = (reg9 >> TUSB320_REG9_ATTACHED_STATE_SHIFT) &
> -		TUSB320_REG9_ATTACHED_STATE_MASK;
> -	if (state == TUSB320_ATTACHED_STATE_DFP)
> -		role = TYPEC_SOURCE;
> -	else
> -		role = TYPEC_SINK;
> +	state = FIELD_GET(TUSB320_REG9_ATTACHED_STATE, reg9);
> +	accessory = FIELD_GET(TUSB320_REG8_ACCESSORY_CONNECTED, reg8);
> +
> +	switch (state) {
> +	case TUSB320_ATTACHED_STATE_DFP:
> +		typec_mode = TYPEC_MODE_USB2;
> +		pwr_role = TYPEC_SOURCE;
> +		data_role = TYPEC_HOST;
> +		break;
> +	case TUSB320_ATTACHED_STATE_UFP:
> +		typec_mode = TYPEC_MODE_USB2;
> +		pwr_role = TYPEC_SINK;
> +		data_role = TYPEC_DEVICE;
> +		break;
> +	case TUSB320_ATTACHED_STATE_ACC:
> +		/*
> +		 * Accessory detected. For debug accessories, just make some
> +		 * qualified guesses as to the role for lack of a better option.
> +		 */
> +		if (accessory == TUSB320_REG8_ACCESSORY_CONNECTED_AUDIO ||
> +		    accessory == TUSB320_REG8_ACCESSORY_CONNECTED_ACHRG) {
> +			typec_mode = TYPEC_MODE_AUDIO;
> +			pwr_role = TYPEC_SINK;
> +			data_role = TYPEC_DEVICE;
> +			break;
> +		} else if (accessory ==
> +			   TUSB320_REG8_ACCESSORY_CONNECTED_DBGDFP) {
> +			typec_mode = TYPEC_MODE_DEBUG;
> +			pwr_role = TYPEC_SOURCE;
> +			data_role = TYPEC_HOST;
> +			break;
> +		} else if (accessory ==
> +			   TUSB320_REG8_ACCESSORY_CONNECTED_DBGUFP) {
> +			typec_mode = TYPEC_MODE_DEBUG;
> +			pwr_role = TYPEC_SINK;
> +			data_role = TYPEC_DEVICE;
> +			break;
> +		}
>  
> -	typec_set_vconn_role(port, role);
> -	typec_set_pwr_role(port, role);
> -	typec_set_data_role(port, role == TYPEC_SOURCE ?
> -				  TYPEC_HOST : TYPEC_DEVICE);
> +		dev_warn(priv->dev, "unexpected ACCESSORY_CONNECTED state %d\n",
> +			 accessory);
>  
> -	ret = regmap_read(priv->regmap, TUSB320_REG8, &reg8);
> -	if (ret) {
> -		dev_err(dev, "error during reg8 i2c read, ret=%d!\n", ret);
> -		return;
> +		fallthrough;
> +	default:
> +		typec_mode = TYPEC_MODE_USB2;
> +		pwr_role = TYPEC_SINK;
> +		data_role = TYPEC_DEVICE;
> +		break;
>  	}
>  
> +	typec_set_vconn_role(port, pwr_role);
> +	typec_set_pwr_role(port, pwr_role);
> +	typec_set_data_role(port, data_role);
> +	typec_set_mode(port, typec_mode);
> +
>  	mode = FIELD_GET(TUSB320_REG8_CURRENT_MODE_DETECT, reg8);
>  	if (mode == TUSB320_REG8_CURRENT_MODE_DETECT_DEF)
>  		typec_set_pwr_opmode(port, TYPEC_PWR_MODE_USB);
> -- 
> 2.39.2

-- 
heikki

Re: [PATCH 1/2] extcon: usbc-tusb320: add accessory detection support

[Alvin Šipraga]

On Fri, Mar 17, 2023 at 12:52:53PM +0200, Heikki Krogerus wrote:
> On Fri, Mar 17, 2023 at 11:42:27AM +0100, Alvin Šipraga wrote:
> > From: Alvin Šipraga <alsi@bang-olufsen.dk>
> > 
> > The TUSB320 can detect the following types of accessory:
> > 
> >   - Audio Accessory
> >   - Audio Accessory with charge-thru
> >   - Debug Accessory (DFP)
> >   - Debug Accessory (UFP)
> > 
> > Moreover, the typec subsystem can be informed of this through the
> > typec_set_mode() function. The information will be propagated to any
> > linked typec muxes. Add the necessary support to the driver.
> > 
> > Note that for the Debug Accessory modes, an educated guess was made that
> > for the USB data role, DFP implies HOST and UFP implies DEVICE. But this
> > might want to be made configurable at a later date.
> > 
> > Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> > ---
> > v2: no change
> 
> Not a big problem, but you forgot to include the version in the
> subject. In any case, FWIW:

Yikes, sorry about that... Someone let me know if I should resend.

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

Thank you.

[PATCH 1/2] extcon: usbc-tusb320: add accessory detection support

[Alvin Šipraga]

From: Alvin Šipraga <alsi@bang-olufsen.dk>

The TUSB320 can detect the following types of accessory:

  - Audio Accessory
  - Audio Accessory with charge-thru
  - Debug Accessory (DFP)
  - Debug Accessory (UFP)

Moreover, the typec subsystem can be informed of this through the
typec_set_mode() function. The information will be propagated to any
linked typec muxes. Add the necessary support to the driver.

Note that for the Debug Accessory modes, an educated guess was made that
for the USB data role, DFP implies HOST and UFP implies DEVICE. But this
might want to be made configurable at a later date.

Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
---
 drivers/extcon/extcon-usbc-tusb320.c | 90 +++++++++++++++++++++-------
 1 file changed, 68 insertions(+), 22 deletions(-)

diff --git a/drivers/extcon/extcon-usbc-tusb320.c b/drivers/extcon/extcon-usbc-tusb320.c
index 10dff1c512c4..882d1f48495e 100644
--- a/drivers/extcon/extcon-usbc-tusb320.c
+++ b/drivers/extcon/extcon-usbc-tusb320.c
@@ -15,6 +15,7 @@
 #include <linux/module.h>
 #include <linux/regmap.h>
 #include <linux/usb/typec.h>
+#include <linux/usb/typec_altmode.h>
 
 #define TUSB320_REG8				0x8
 #define TUSB320_REG8_CURRENT_MODE_ADVERTISE	GENMASK(7, 6)
@@ -26,16 +27,16 @@
 #define TUSB320_REG8_CURRENT_MODE_DETECT_MED	0x1
 #define TUSB320_REG8_CURRENT_MODE_DETECT_ACC	0x2
 #define TUSB320_REG8_CURRENT_MODE_DETECT_HI	0x3
-#define TUSB320_REG8_ACCESSORY_CONNECTED	GENMASK(3, 2)
+#define TUSB320_REG8_ACCESSORY_CONNECTED	GENMASK(3, 1)
 #define TUSB320_REG8_ACCESSORY_CONNECTED_NONE	0x0
 #define TUSB320_REG8_ACCESSORY_CONNECTED_AUDIO	0x4
-#define TUSB320_REG8_ACCESSORY_CONNECTED_ACC	0x5
-#define TUSB320_REG8_ACCESSORY_CONNECTED_DEBUG	0x6
+#define TUSB320_REG8_ACCESSORY_CONNECTED_ACHRG	0x5
+#define TUSB320_REG8_ACCESSORY_CONNECTED_DBGDFP	0x6
+#define TUSB320_REG8_ACCESSORY_CONNECTED_DBGUFP	0x7
 #define TUSB320_REG8_ACTIVE_CABLE_DETECTION	BIT(0)
 
 #define TUSB320_REG9				0x9
-#define TUSB320_REG9_ATTACHED_STATE_SHIFT	6
-#define TUSB320_REG9_ATTACHED_STATE_MASK	0x3
+#define TUSB320_REG9_ATTACHED_STATE		GENMASK(7, 6)
 #define TUSB320_REG9_CABLE_DIRECTION		BIT(5)
 #define TUSB320_REG9_INTERRUPT_STATUS		BIT(4)
 
@@ -250,8 +251,7 @@ static void tusb320_extcon_irq_handler(struct tusb320_priv *priv, u8 reg)
 {
 	int state, polarity;
 
-	state = (reg >> TUSB320_REG9_ATTACHED_STATE_SHIFT) &
-		TUSB320_REG9_ATTACHED_STATE_MASK;
+	state = FIELD_GET(TUSB320_REG9_ATTACHED_STATE, reg);
 	polarity = !!(reg & TUSB320_REG9_CABLE_DIRECTION);
 
 	dev_dbg(priv->dev, "attached state: %s, polarity: %d\n",
@@ -277,32 +277,78 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
 {
 	struct typec_port *port = priv->port;
 	struct device *dev = priv->dev;
-	u8 mode, role, state;
+	int typec_mode;
+	enum typec_role pwr_role;
+	enum typec_data_role data_role;
+	u8 state, mode, accessory;
 	int ret, reg8;
 	bool ori;
 
+	ret = regmap_read(priv->regmap, TUSB320_REG8, &reg8);
+	if (ret) {
+		dev_err(dev, "error during reg8 i2c read, ret=%d!\n", ret);
+		return;
+	}
+
 	ori = reg9 & TUSB320_REG9_CABLE_DIRECTION;
 	typec_set_orientation(port, ori ? TYPEC_ORIENTATION_REVERSE :
 					  TYPEC_ORIENTATION_NORMAL);
 
-	state = (reg9 >> TUSB320_REG9_ATTACHED_STATE_SHIFT) &
-		TUSB320_REG9_ATTACHED_STATE_MASK;
-	if (state == TUSB320_ATTACHED_STATE_DFP)
-		role = TYPEC_SOURCE;
-	else
-		role = TYPEC_SINK;
+	state = FIELD_GET(TUSB320_REG9_ATTACHED_STATE, reg9);
+	accessory = FIELD_GET(TUSB320_REG8_ACCESSORY_CONNECTED, reg8);
+
+	switch (state) {
+	case TUSB320_ATTACHED_STATE_DFP:
+		typec_mode = TYPEC_MODE_USB2;
+		pwr_role = TYPEC_SOURCE;
+		data_role = TYPEC_HOST;
+		break;
+	case TUSB320_ATTACHED_STATE_UFP:
+		typec_mode = TYPEC_MODE_USB2;
+		pwr_role = TYPEC_SINK;
+		data_role = TYPEC_DEVICE;
+		break;
+	case TUSB320_ATTACHED_STATE_ACC:
+		/*
+		 * Accessory detected. For debug accessories, just make some
+		 * qualified guesses as to the role for lack of a better option.
+		 */
+		if (accessory == TUSB320_REG8_ACCESSORY_CONNECTED_AUDIO ||
+		    accessory == TUSB320_REG8_ACCESSORY_CONNECTED_ACHRG) {
+			typec_mode = TYPEC_MODE_AUDIO;
+			pwr_role = TYPEC_SINK;
+			data_role = TYPEC_DEVICE;
+			break;
+		} else if (accessory ==
+			   TUSB320_REG8_ACCESSORY_CONNECTED_DBGDFP) {
+			typec_mode = TYPEC_MODE_DEBUG;
+			pwr_role = TYPEC_SOURCE;
+			data_role = TYPEC_HOST;
+			break;
+		} else if (accessory ==
+			   TUSB320_REG8_ACCESSORY_CONNECTED_DBGUFP) {
+			typec_mode = TYPEC_MODE_DEBUG;
+			pwr_role = TYPEC_SINK;
+			data_role = TYPEC_DEVICE;
+			break;
+		}
 
-	typec_set_vconn_role(port, role);
-	typec_set_pwr_role(port, role);
-	typec_set_data_role(port, role == TYPEC_SOURCE ?
-				  TYPEC_HOST : TYPEC_DEVICE);
+		dev_warn(priv->dev, "unexpected ACCESSORY_CONNECTED state %d\n",
+			 accessory);
 
-	ret = regmap_read(priv->regmap, TUSB320_REG8, &reg8);
-	if (ret) {
-		dev_err(dev, "error during reg8 i2c read, ret=%d!\n", ret);
-		return;
+		fallthrough;
+	default:
+		typec_mode = TYPEC_MODE_USB2;
+		pwr_role = TYPEC_SINK;
+		data_role = TYPEC_DEVICE;
+		break;
 	}
 
+	typec_set_vconn_role(port, pwr_role);
+	typec_set_pwr_role(port, pwr_role);
+	typec_set_data_role(port, data_role);
+	typec_set_mode(port, typec_mode);
+
 	mode = FIELD_GET(TUSB320_REG8_CURRENT_MODE_DETECT, reg8);
 	if (mode == TUSB320_REG8_CURRENT_MODE_DETECT_DEF)
 		typec_set_pwr_opmode(port, TYPEC_PWR_MODE_USB);
-- 
2.39.2

[PATCH 2/2] extcon: usbc-tusb320: add usb_role_switch support

[Alvin Šipraga]

From: Alvin Šipraga <alsi@bang-olufsen.dk>

The connector child node of the TUSB320 device might be linked with a
USB OTG controller with USB role switch capability. Add driver support
for detecting a usb_role_switch and setting its state in the typec
interrupt handler. This follows similar practice in other drivers in the
typec subsystem, which this extcon driver can opt-in to.

Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
---
 drivers/extcon/extcon-usbc-tusb320.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/extcon/extcon-usbc-tusb320.c b/drivers/extcon/extcon-usbc-tusb320.c
index 882d1f48495e..44b55650b6b4 100644
--- a/drivers/extcon/extcon-usbc-tusb320.c
+++ b/drivers/extcon/extcon-usbc-tusb320.c
@@ -16,6 +16,7 @@
 #include <linux/regmap.h>
 #include <linux/usb/typec.h>
 #include <linux/usb/typec_altmode.h>
+#include <linux/usb/role.h>
 
 #define TUSB320_REG8				0x8
 #define TUSB320_REG8_CURRENT_MODE_ADVERTISE	GENMASK(7, 6)
@@ -80,6 +81,7 @@ struct tusb320_priv {
 	enum typec_port_type port_type;
 	enum typec_pwr_opmode pwr_opmode;
 	struct fwnode_handle *connector_fwnode;
+	struct usb_role_switch *role_sw;
 };
 
 static const char * const tusb_attached_states[] = {
@@ -275,9 +277,11 @@ static void tusb320_extcon_irq_handler(struct tusb320_priv *priv, u8 reg)
 
 static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
 {
+	struct usb_role_switch *role_sw = priv->role_sw;
 	struct typec_port *port = priv->port;
 	struct device *dev = priv->dev;
 	int typec_mode;
+	enum usb_role usb_role;
 	enum typec_role pwr_role;
 	enum typec_data_role data_role;
 	u8 state, mode, accessory;
@@ -300,11 +304,13 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
 	switch (state) {
 	case TUSB320_ATTACHED_STATE_DFP:
 		typec_mode = TYPEC_MODE_USB2;
+		usb_role = USB_ROLE_HOST;
 		pwr_role = TYPEC_SOURCE;
 		data_role = TYPEC_HOST;
 		break;
 	case TUSB320_ATTACHED_STATE_UFP:
 		typec_mode = TYPEC_MODE_USB2;
+		usb_role = USB_ROLE_DEVICE;
 		pwr_role = TYPEC_SINK;
 		data_role = TYPEC_DEVICE;
 		break;
@@ -316,6 +322,7 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
 		if (accessory == TUSB320_REG8_ACCESSORY_CONNECTED_AUDIO ||
 		    accessory == TUSB320_REG8_ACCESSORY_CONNECTED_ACHRG) {
 			typec_mode = TYPEC_MODE_AUDIO;
+			usb_role = USB_ROLE_NONE;
 			pwr_role = TYPEC_SINK;
 			data_role = TYPEC_DEVICE;
 			break;
@@ -323,12 +330,14 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
 			   TUSB320_REG8_ACCESSORY_CONNECTED_DBGDFP) {
 			typec_mode = TYPEC_MODE_DEBUG;
 			pwr_role = TYPEC_SOURCE;
+			usb_role = USB_ROLE_HOST;
 			data_role = TYPEC_HOST;
 			break;
 		} else if (accessory ==
 			   TUSB320_REG8_ACCESSORY_CONNECTED_DBGUFP) {
 			typec_mode = TYPEC_MODE_DEBUG;
 			pwr_role = TYPEC_SINK;
+			usb_role = USB_ROLE_DEVICE;
 			data_role = TYPEC_DEVICE;
 			break;
 		}
@@ -339,6 +348,7 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
 		fallthrough;
 	default:
 		typec_mode = TYPEC_MODE_USB2;
+		usb_role = USB_ROLE_NONE;
 		pwr_role = TYPEC_SINK;
 		data_role = TYPEC_DEVICE;
 		break;
@@ -348,6 +358,7 @@ static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
 	typec_set_pwr_role(port, pwr_role);
 	typec_set_data_role(port, data_role);
 	typec_set_mode(port, typec_mode);
+	usb_role_switch_set_role(priv->role_sw, usb_role);
 
 	mode = FIELD_GET(TUSB320_REG8_CURRENT_MODE_DETECT, reg8);
 	if (mode == TUSB320_REG8_CURRENT_MODE_DETECT_DEF)
@@ -472,10 +483,20 @@ static int tusb320_typec_probe(struct i2c_client *client,
 		goto err_put;
 	}
 
+	/* Find any optional USB role switch that needs reporting to */
+	priv->role_sw = fwnode_usb_role_switch_get(connector);
+	if (IS_ERR(priv->role_sw)) {
+		ret = PTR_ERR(priv->role_sw);
+		goto err_unreg;
+	}
+
 	priv->connector_fwnode = connector;
 
 	return 0;
 
+err_unreg:
+	typec_unregister_port(priv->port);
+
 err_put:
 	fwnode_handle_put(connector);
 
@@ -484,6 +505,7 @@ static int tusb320_typec_probe(struct i2c_client *client,
 
 static void tusb320_typec_remove(struct tusb320_priv *priv)
 {
+	usb_role_switch_put(priv->role_sw);
 	typec_unregister_port(priv->port);
 	fwnode_handle_put(priv->connector_fwnode);
 }
-- 
2.39.2

Re: [PATCH 2/2] extcon: usbc-tusb320: add usb_role_switch support

[kernel test robot]

Hi Alvin,

I love your patch! Perhaps something to improve:

[auto build test WARNING on chanwoo-extcon/extcon-next]
[cannot apply to linus/master v6.3-rc2 next-20230315]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Alvin-ipraga/extcon-usbc-tusb320-add-usb_role_switch-support/20230316-060433
base:   https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git extcon-next
patch link:    https://lore.kernel.org/r/20230315220246.951213-2-alvin%40pqrs.dk
patch subject: [PATCH 2/2] extcon: usbc-tusb320: add usb_role_switch support
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20230316/202303160838.j3Q18WnL-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/8ed7905410ebc9e2de0bd58d4cdd0a8225529f42
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Alvin-ipraga/extcon-usbc-tusb320-add-usb_role_switch-support/20230316-060433
        git checkout 8ed7905410ebc9e2de0bd58d4cdd0a8225529f42
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 olddefconfig
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303160838.j3Q18WnL-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/extcon/extcon-usbc-tusb320.c: In function 'tusb320_typec_irq_handler':
>> drivers/extcon/extcon-usbc-tusb320.c:280:33: warning: unused variable 'role_sw' [-Wunused-variable]
     280 |         struct usb_role_switch *role_sw = priv->role_sw;
         |                                 ^~~~~~~


vim +/role_sw +280 drivers/extcon/extcon-usbc-tusb320.c

   277	
   278	static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
   279	{
 > 280		struct usb_role_switch *role_sw = priv->role_sw;
   281		struct typec_port *port = priv->port;
   282		struct device *dev = priv->dev;
   283		int typec_mode;
   284		enum usb_role usb_role;
   285		enum typec_role pwr_role;
   286		enum typec_data_role data_role;
   287		u8 state, mode, accessory;
   288		int ret, reg8;
   289		bool ori;
   290	
   291		ret = regmap_read(priv->regmap, TUSB320_REG8, &reg8);
   292		if (ret) {
   293			dev_err(dev, "error during reg8 i2c read, ret=%d!\n", ret);
   294			return;
   295		}
   296	
   297		ori = reg9 & TUSB320_REG9_CABLE_DIRECTION;
   298		typec_set_orientation(port, ori ? TYPEC_ORIENTATION_REVERSE :
   299						  TYPEC_ORIENTATION_NORMAL);
   300	
   301		state = FIELD_GET(TUSB320_REG9_ATTACHED_STATE, reg9);
   302		accessory = FIELD_GET(TUSB320_REG8_ACCESSORY_CONNECTED, reg8);
   303	
   304		switch (state) {
   305		case TUSB320_ATTACHED_STATE_DFP:
   306			typec_mode = TYPEC_MODE_USB2;
   307			usb_role = USB_ROLE_HOST;
   308			pwr_role = TYPEC_SOURCE;
   309			data_role = TYPEC_HOST;
   310			break;
   311		case TUSB320_ATTACHED_STATE_UFP:
   312			typec_mode = TYPEC_MODE_USB2;
   313			usb_role = USB_ROLE_DEVICE;
   314			pwr_role = TYPEC_SINK;
   315			data_role = TYPEC_DEVICE;
   316			break;
   317		case TUSB320_ATTACHED_STATE_ACC:
   318			/*
   319			 * Accessory detected. For debug accessories, just make some
   320			 * qualified guesses as to the role for lack of a better option.
   321			 */
   322			if (accessory == TUSB320_REG8_ACCESSORY_CONNECTED_AUDIO ||
   323			    accessory == TUSB320_REG8_ACCESSORY_CONNECTED_ACHRG) {
   324				typec_mode = TYPEC_MODE_AUDIO;
   325				usb_role = USB_ROLE_NONE;
   326				pwr_role = TYPEC_SINK;
   327				data_role = TYPEC_DEVICE;
   328				break;
   329			} else if (accessory ==
   330				   TUSB320_REG8_ACCESSORY_CONNECTED_DBGDFP) {
   331				typec_mode = TYPEC_MODE_DEBUG;
   332				pwr_role = TYPEC_SOURCE;
   333				usb_role = USB_ROLE_HOST;
   334				data_role = TYPEC_HOST;
   335				break;
   336			} else if (accessory ==
   337				   TUSB320_REG8_ACCESSORY_CONNECTED_DBGUFP) {
   338				typec_mode = TYPEC_MODE_DEBUG;
   339				pwr_role = TYPEC_SINK;
   340				usb_role = USB_ROLE_DEVICE;
   341				data_role = TYPEC_DEVICE;
   342				break;
   343			}
   344	
   345			dev_warn(priv->dev, "unexpected ACCESSORY_CONNECTED state %d\n",
   346				 accessory);
   347	
   348			fallthrough;
   349		default:
   350			typec_mode = TYPEC_MODE_USB2;
   351			usb_role = USB_ROLE_NONE;
   352			pwr_role = TYPEC_SINK;
   353			data_role = TYPEC_DEVICE;
   354			break;
   355		}
   356	
   357		typec_set_vconn_role(port, pwr_role);
   358		typec_set_pwr_role(port, pwr_role);
   359		typec_set_data_role(port, data_role);
   360		typec_set_mode(port, typec_mode);
   361		usb_role_switch_set_role(priv->role_sw, usb_role);
   362	
   363		mode = FIELD_GET(TUSB320_REG8_CURRENT_MODE_DETECT, reg8);
   364		if (mode == TUSB320_REG8_CURRENT_MODE_DETECT_DEF)
   365			typec_set_pwr_opmode(port, TYPEC_PWR_MODE_USB);
   366		else if (mode == TUSB320_REG8_CURRENT_MODE_DETECT_MED)
   367			typec_set_pwr_opmode(port, TYPEC_PWR_MODE_1_5A);
   368		else if (mode == TUSB320_REG8_CURRENT_MODE_DETECT_HI)
   369			typec_set_pwr_opmode(port, TYPEC_PWR_MODE_3_0A);
   370		else	/* Charge through accessory */
   371			typec_set_pwr_opmode(port, TYPEC_PWR_MODE_USB);
   372	}
   373	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH 2/2] extcon: usbc-tusb320: add usb_role_switch support

[kernel test robot]

Hi Alvin,

I love your patch! Perhaps something to improve:

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

url:    https://github.com/intel-lab-lkp/linux/commits/Alvin-ipraga/extcon-usbc-tusb320-add-usb_role_switch-support/20230316-060433
base:   https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git extcon-next
patch link:    https://lore.kernel.org/r/20230315220246.951213-2-alvin%40pqrs.dk
patch subject: [PATCH 2/2] extcon: usbc-tusb320: add usb_role_switch support
config: i386-randconfig-a013-20230313 (https://download.01.org/0day-ci/archive/20230316/202303161221.vGdSsAr9-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/8ed7905410ebc9e2de0bd58d4cdd0a8225529f42
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Alvin-ipraga/extcon-usbc-tusb320-add-usb_role_switch-support/20230316-060433
        git checkout 8ed7905410ebc9e2de0bd58d4cdd0a8225529f42
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/extcon/ fs/ksmbd/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303161221.vGdSsAr9-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/extcon/extcon-usbc-tusb320.c:280:26: warning: unused variable 'role_sw' [-Wunused-variable]
           struct usb_role_switch *role_sw = priv->role_sw;
                                   ^
   1 warning generated.


vim +/role_sw +280 drivers/extcon/extcon-usbc-tusb320.c

   277	
   278	static void tusb320_typec_irq_handler(struct tusb320_priv *priv, u8 reg9)
   279	{
 > 280		struct usb_role_switch *role_sw = priv->role_sw;
   281		struct typec_port *port = priv->port;
   282		struct device *dev = priv->dev;
   283		int typec_mode;
   284		enum usb_role usb_role;
   285		enum typec_role pwr_role;
   286		enum typec_data_role data_role;
   287		u8 state, mode, accessory;
   288		int ret, reg8;
   289		bool ori;
   290	
   291		ret = regmap_read(priv->regmap, TUSB320_REG8, &reg8);
   292		if (ret) {
   293			dev_err(dev, "error during reg8 i2c read, ret=%d!\n", ret);
   294			return;
   295		}
   296	
   297		ori = reg9 & TUSB320_REG9_CABLE_DIRECTION;
   298		typec_set_orientation(port, ori ? TYPEC_ORIENTATION_REVERSE :
   299						  TYPEC_ORIENTATION_NORMAL);
   300	
   301		state = FIELD_GET(TUSB320_REG9_ATTACHED_STATE, reg9);
   302		accessory = FIELD_GET(TUSB320_REG8_ACCESSORY_CONNECTED, reg8);
   303	
   304		switch (state) {
   305		case TUSB320_ATTACHED_STATE_DFP:
   306			typec_mode = TYPEC_MODE_USB2;
   307			usb_role = USB_ROLE_HOST;
   308			pwr_role = TYPEC_SOURCE;
   309			data_role = TYPEC_HOST;
   310			break;
   311		case TUSB320_ATTACHED_STATE_UFP:
   312			typec_mode = TYPEC_MODE_USB2;
   313			usb_role = USB_ROLE_DEVICE;
   314			pwr_role = TYPEC_SINK;
   315			data_role = TYPEC_DEVICE;
   316			break;
   317		case TUSB320_ATTACHED_STATE_ACC:
   318			/*
   319			 * Accessory detected. For debug accessories, just make some
   320			 * qualified guesses as to the role for lack of a better option.
   321			 */
   322			if (accessory == TUSB320_REG8_ACCESSORY_CONNECTED_AUDIO ||
   323			    accessory == TUSB320_REG8_ACCESSORY_CONNECTED_ACHRG) {
   324				typec_mode = TYPEC_MODE_AUDIO;
   325				usb_role = USB_ROLE_NONE;
   326				pwr_role = TYPEC_SINK;
   327				data_role = TYPEC_DEVICE;
   328				break;
   329			} else if (accessory ==
   330				   TUSB320_REG8_ACCESSORY_CONNECTED_DBGDFP) {
   331				typec_mode = TYPEC_MODE_DEBUG;
   332				pwr_role = TYPEC_SOURCE;
   333				usb_role = USB_ROLE_HOST;
   334				data_role = TYPEC_HOST;
   335				break;
   336			} else if (accessory ==
   337				   TUSB320_REG8_ACCESSORY_CONNECTED_DBGUFP) {
   338				typec_mode = TYPEC_MODE_DEBUG;
   339				pwr_role = TYPEC_SINK;
   340				usb_role = USB_ROLE_DEVICE;
   341				data_role = TYPEC_DEVICE;
   342				break;
   343			}
   344	
   345			dev_warn(priv->dev, "unexpected ACCESSORY_CONNECTED state %d\n",
   346				 accessory);
   347	
   348			fallthrough;
   349		default:
   350			typec_mode = TYPEC_MODE_USB2;
   351			usb_role = USB_ROLE_NONE;
   352			pwr_role = TYPEC_SINK;
   353			data_role = TYPEC_DEVICE;
   354			break;
   355		}
   356	
   357		typec_set_vconn_role(port, pwr_role);
   358		typec_set_pwr_role(port, pwr_role);
   359		typec_set_data_role(port, data_role);
   360		typec_set_mode(port, typec_mode);
   361		usb_role_switch_set_role(priv->role_sw, usb_role);
   362	
   363		mode = FIELD_GET(TUSB320_REG8_CURRENT_MODE_DETECT, reg8);
   364		if (mode == TUSB320_REG8_CURRENT_MODE_DETECT_DEF)
   365			typec_set_pwr_opmode(port, TYPEC_PWR_MODE_USB);
   366		else if (mode == TUSB320_REG8_CURRENT_MODE_DETECT_MED)
   367			typec_set_pwr_opmode(port, TYPEC_PWR_MODE_1_5A);
   368		else if (mode == TUSB320_REG8_CURRENT_MODE_DETECT_HI)
   369			typec_set_pwr_opmode(port, TYPEC_PWR_MODE_3_0A);
   370		else	/* Charge through accessory */
   371			typec_set_pwr_opmode(port, TYPEC_PWR_MODE_USB);
   372	}
   373	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests