RE: [EXT] Re: [PATCH] usb: typec: tcpci: allow drp toggling for non-drp port

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



++ 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:





[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux