From: Alvin Šipraga <[email protected]>
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 <[email protected]>
---
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, ®8);
+ 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, ®8);
- 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
From: Alvin Šipraga <[email protected]>
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 <[email protected]>
---
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
On Fri, Mar 17, 2023 at 11:42:27AM +0100, Alvin Šipraga wrote:
> From: Alvin Šipraga <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 <[email protected]>
> ---
> 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, ®8);
> + 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, ®8);
> - 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
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 <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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 <[email protected]>
Thank you.
On Fri, Mar 17, 2023 at 11:42:28AM +0100, Alvin Šipraga wrote:
> From: Alvin Šipraga <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
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 <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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