++ Hans de Goede > -----Original Message----- > From: Xu Yang > Sent: Friday, June 9, 2023 10:15 AM > To: Guenter Roeck <linux@xxxxxxxxxxxx>; heikki.krogerus@xxxxxxxxxxxxxxx > Cc: gregkh@xxxxxxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>; linux-usb@xxxxxxxxxxxxxxx; Jun Li <jun.li@xxxxxxx>; > Zhipeng Wang <zhipeng.wang_1@xxxxxxx>; Faqiang Zhu <faqiang.zhu@xxxxxxx> > Subject: RE: [EXT] Re: [PATCH] usb: typec: tcpci: allow drp toggling for non-drp port > > Hi Guenter, > > > -----Original Message----- > > From: Guenter Roeck <groeck7@xxxxxxxxx> On Behalf Of Guenter Roeck > > Sent: Thursday, June 8, 2023 9:24 PM > > To: Xu Yang <xu.yang_2@xxxxxxx>; heikki.krogerus@xxxxxxxxxxxxxxx > > Cc: gregkh@xxxxxxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>; linux-usb@xxxxxxxxxxxxxxx; Jun Li > <jun.li@xxxxxxx>; > > Zhipeng Wang <zhipeng.wang_1@xxxxxxx>; Faqiang Zhu <faqiang.zhu@xxxxxxx> > > Subject: [EXT] Re: [PATCH] usb: typec: tcpci: allow drp toggling for non-drp port > > > > Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the > > message using the 'Report this email' button > > > > > > On 6/8/23 04:28, Xu Yang wrote: > > > Some single power role Type-C port with dual data role, this kind of > > > port connects non Type-C port for usb data will need tcpm to work to > > > get polarity for orientation change and let Type-C port keep at fake > > > power role to provide another non-default data role, so remove the drp > > > port condition for now. > > > > > > Has anyone encountered this use case? How do you handle this limitation > > > in current tcpm with a better way? Please kindly share your thoughts. > > > > > > > Have you ? This is an odd comment to make in the patch description. > > Sorry for this. It's not a formal patch. > > > > > Either case, I don't understand why one would need to enable toggling > > under any circumstances if the port is not DRP. The description does > > not explain how "need tcpm to work" correlates to enabling toggling on > > non-DRP ports. > > My case is a source-only PD capable port with dual data role, connect to > legacy PC host via A-to-C to work as USB device mode. Under current tcpm > mechanism, the PC will not recognize the source-only PD capable port and > the usb controller has no chance to work as device mode. > > However, if I enable CC toggling, the PD port can re-work as sink role and > the USB controller can function as device mode too. Since it's USB3 port, > it can work only after the SS link has correctly established. Thus, we also > need tcpm to set correct orientation. > > So, it seems a limitation to let single power role Type-C port with dual > data role to work as non-default data role when connected to non Type-C > port. I do see below patches from Hans to fix the same issue as ours. 48242e30532b ("usb: typec: fusb302: Revert "Resolve fixed power role contract setup"") 6258db14d78c ("usb: typec: fusb302: Implement start_toggling for all port-types") 7893f9e1c26d ("usb: typec: tcpm: Notify the tcpc to start connection-detection for SRPs") So I thinks it's really a limitation for SRP to work correctly with tcpci driver. Maybe an improvement is needed for tcpci driver. Thanks, Xu Yang > > Thanks, > Xu Yang > > > > > Guenter > > > > > Signed-off-by: Xu Yang <xu.yang_2@xxxxxxx> > > > Signed-off-by: Li Jun <jun.li@xxxxxxx> > > > --- > > > drivers/usb/typec/tcpm/tcpci.c | 3 --- > > > drivers/usb/typec/tcpm/tcpm.c | 6 +++++- > > > 2 files changed, 5 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c > > > index fc708c289a73..88559e749120 100644 > > > --- a/drivers/usb/typec/tcpm/tcpci.c > > > +++ b/drivers/usb/typec/tcpm/tcpci.c > > > @@ -175,9 +175,6 @@ static int tcpci_start_toggling(struct tcpc_dev *tcpc, > > > struct tcpci *tcpci = tcpc_to_tcpci(tcpc); > > > unsigned int reg = TCPC_ROLE_CTRL_DRP; > > > > > > - if (port_type != TYPEC_PORT_DRP) > > > - return -EOPNOTSUPP; > > > - > > > /* Handle vendor drp toggling */ > > > if (tcpci->data->start_drp_toggling) { > > > ret = tcpci->data->start_drp_toggling(tcpci, tcpci->data, cc); > > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c > > > index 3c6b0c8e2d3a..6aa62132e69a 100644 > > > --- a/drivers/usb/typec/tcpm/tcpm.c > > > +++ b/drivers/usb/typec/tcpm/tcpm.c > > > @@ -4274,7 +4274,11 @@ static void run_state_machine(struct tcpm_port *port) > > > ret = tcpm_snk_attach(port); > > > if (ret < 0) > > > tcpm_set_state(port, SNK_UNATTACHED, 0); > > > - else > > > + else if (port->port_type == TYPEC_PORT_SRC && > > > + port->typec_caps.data == TYPEC_PORT_DRD) { > > > + tcpm_typec_connect(port); > > > + tcpm_log(port, "Keep at SNK_ATTACHED for USB data."); > > > + } else > > > tcpm_set_state(port, SNK_STARTUP, 0); > > > break; > > > case SNK_STARTUP: