diff mbox series

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

Message ID 20230315220246.951213-2-alvin@pqrs.dk
State Superseded
Headers show
Series [1/2] extcon: usbc-tusb320: add accessory detection support | expand

Commit Message

Alvin Šipraga March 15, 2023, 10:02 p.m. UTC 
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(+)

Comments

kernel test robot March 16, 2023, 12:30 a.m. UTC | #1 
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
kernel test robot March 16, 2023, 4:46 a.m. UTC | #2 
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
diff mbox series

Patch

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);
 }