On 9/19/19 3:48 AM, Kyle Tso wrote:
Ping! Anyone still reviewing this patch?
Which patch ? Hans' series was long since pushed.
Guenter
I have another change related to AMS.
I will group them as a set and re-send it.
Regards,
Kyle Tso
On Mon, Apr 15, 2019 at 6:03 PM Adam Thomson
<Adam.Thomson.Opensource@xxxxxxxxxxx> wrote:
On 13 April 2019 21:39, Hans de Goede wrote:
On 10-04-19 18:38, Hans de Goede wrote:
On 10-04-19 18:14, Adam Thomson wrote:
On 10 April 2019 16:45, Hans de Goede wrote:
<snip>
Starting toggling from tcpm_set_cc() just feels wrong; and currently
power role swapping is broken with the fusb302, which IIRC used to
work. I suspect this is related.
I plan to write a patch tomorrow to functionally take tcpm_set_cc()
back to the way it was before. This should fix your case and I hope
this also fixes power-role swapping.
This will re-introduce Adam Thomson's problem, but I have a feeling
that that actually needs a fix in the tcpm.c code rather then at the fusb302
level.
To be clear here, the names TOGGLING_MODE_SNK and
TOGGLING_MODE_SRC
are a misnomer from the HW spec for fusb302. The device isn't
toggling anything as far as I'm aware, so I don't necessarily agree with your
point.
If I understand the datasheet correctly:
"The FUSB302 has the capability to do autonomous DRP toggle. In
autonomous toggle the FUSB302 internally controls the PDWN1, PDWN2,
PU_EN1 and PU_EN2, MEAS_CC1 and MEAS_CC2 and implements a fixed DRP
toggle between presenting as a SRC and presenting as a SNK.
Alternately, it can present as a SRC or SNK only and poll CC1 and CC2
continuously."
It is still attaching Rp resp Rd to CC1 or CC2 one at a time to detect
polarity, so it is still toggling, it just is not doing dual-role
toggling. This is also expected behavior for a sink, a sink may not
present Rd on both CC pins at the same time, otherwise the source
cannot detect the polarity and the source also cannot detect if Vconn
is necessary.
It's a mechanism to
have the HW report when the CC line changes on connection. Without
that we have no reporting from the HW for the fixed role scenarios.
Not just connection, also polarity detection. Notice that the tcpm
framework / the driver also has a start_drp_toggling() method. I think
we may also need a start_srp_toggling function just like it and call
that from the SNK_UNATTACHED and SRC_UNATTACHED states for single-role
ports. I agree that we need to start toggling when in those states,
but tcpm_set_cc gets called in a lot of other places where AFAIK we
should NOT restart toggling and your patch causes us to restart
toggling in those cases.
Ok, so as I suspected, commit ea3b4d5523bc ("usb: typec: fusb302:
Resolve fixed power role contract setup") is what caused the power-role
swapping breakage I've been seeing.
Apologies for the breakage. Annoyed I didn't catch that when submitting that
patch. Thanks for looking to resolve this and will review your updates shortly.
So I've prepared a 3 patch series:
1) Add a new start_srp_connection_detect function which, when implemented
by the tcpc_dev, gets called instead of start_drp_toggling for single role ports
(SRPs)
2) Implement 1. for fusb302 to fix the SRP issue Adam was seeing, without
depending on set_cc starting "toggling"
or something like it for the fix
3) Revert commit ea3b4d5523bc, restoring power-role swap functionality.
This should also fix the issue Kyle Tso was seeing when trying to change from one
Rp setting to another.
I'll send out the series right after this email.
Regards,
Hans
