Hi,
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.
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
