> -----Original Message-----
> From: Hans de Goede <hdegoede@xxxxxxxxxx>
> Sent: Tuesday, June 20, 2023 5:52 PM
> To: Xu Yang <xu.yang_2@xxxxxxx>; 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
>
> 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
>
>
> Hi,
>
> On 6/20/23 11:38, Xu Yang wrote:
> > Hi Hans,
> >
> >> -----Original Message-----
> >> From: Hans de Goede <hdegoede@xxxxxxxxxx>
> >> Sent: Tuesday, June 20, 2023 4:35 PM
> >> To: Xu Yang <xu.yang_2@xxxxxxx>; 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
> >>
> >> 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
> >>
> >>
> >> Hi all,
> >>
> >> On 6/20/23 05:10, Xu Yang wrote:
> >>> ++ 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.
> >>
> >> Thank you for Cc-ing me. This situation is a bit different from the one fixed by the above patches.
> >>
> >> The above patches where for pure single-role ports.
> >>
> >> Where as what we seem to have here is a dual-data-role, single-power-role port.
> >>
> >> I assume that for this port at least the 5v boost output can be turned on/off. But I guess that it cannot be used to charge
> >> the device ?
> >>
> >> To me it sounds like that to make this work, even with dumb (just a resistor on 1 Cc line) USB-C - USB-A cables, the port
> >> should simply be declared as a dual-role port, because that is wat it actually is (it actually is dual-role).
> >>
> >> And then when configured as sink, it can operate in the default device-mode sink data role and just not consume the
> >> provided 5V.
> >>
> >> Note if the 5V boost output can not be disabled that that would be a problem but that would really be out of spec, you
> >> cannot just unconditionally output 5V from a Type-C port.
> >
> > Thanks for your comments.
> >
> > I looked through your patches in detail. It seems that your issue is the
> > TOGDONE interrupt cannot be asserted even the default CC termination is
> > set for SRP when Type-C cable attached. The reason is a step to enable
> > SNK or SRC Toggle autonomous functionality is missed. When this step is
> > added, TOGDONE interrupt will come as expected. However, it has issue to
> > put this step to tcpm_set_cc() when doing power role swap. So your case
> > should be a different issue from mine.
> >
> > Yes, the 5V output from source-only port can be turned on/off. The tcpc
> > does support dual power role. But subject to our usecase, we need to
> > restrict it to be a source-only port with below properties and avoid sinking
> > vbus via hw design:
> >
> > power-role = "source";
> > data-role = "dual";
> >
> > So I think this is a normal usecase. The software might need to make a
> > little change to support it.
>
> Right, but in order to be able to use device-mode with a passive
> USB-C <-> USB-A cable your tcpc still needs to use dual-role toggling,
> otherwise it will not even generate an IRQ when plugging in the
> passive USB-C <-> USB-A cable and you thus will not even get
> an insertion event.
>
> Which I see is more or less what your original patch tries to do.
>
> So I guess your original patch does seem to be something like
> what is necessary but for the UCSI bit it should be limited to
> only still allow dual-role toggling when the data-role is "dual".
Seem a good idea. : )
>
> Also it should be split into separate patches for the UCSI
> and TCPM parts.
>
> Note I've not looked at the TCPM part of the patch in detail,
> I'm not sure that part is correct.
Yeah, thank you very much.
Best Regards,
Xu Yang
>
> Regards,
>
> Hans
>
>
>
>
> >>>>>> 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:
> >>>
> >
