| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 17 Mar 2023 11:57:08 +0000
From: Alvin Šipraga <ALSI@...g-olufsen.dk>
To: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
CC: Alvin Šipraga <alvin@...s.dk>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
MyungJoo Ham <myungjoo.ham@...sung.com>,
Chanwoo Choi <cw00.choi@...sung.com>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] extcon: usbc-tusb320: add usb_role_switch support
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@...g-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@...g-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
Powered by blists - more mailing lists