Looks like PD negotiation issue on servo v4.

Nick and Scott, any updates on this? 

I haven't looked at it yet. But, I do want to clarify what DUT was being used in this test. Was it Pyro?

One quick update. I tested with a Samus unit that I have. I can reproduce the log above if CC2 (on Servo v4) is the connected polarity. If I flip the connector to use CC1 as the polarity, then it connects.

Using polarity CC1

> C1 st16
C1 st19
C1 st20
C1 st15
[39.629119 CCD: TypeC detach, no change to SBU mux]
C1 st16
C1 st19
C1 st20
C1 st21
Requested 5000 V 500 mA (for 500/500 mA)
C1 st22
C1 st23
C1 st24
C1 st25
C1 st26
C1 st25
pd 1 state
Port C1 CC1, Ena - Role: SRC-UFP State: SRC_READY, Flags: 0x415e>


Using polarity CC2

> C1 st16
C1 st19
C1 st20
C1 st15
[27.152927 CCD: TypeC detach, no change to SBU mux]
C1 st16
C1 st19
C1 st20
[27.429589 PD TMOUT RX 1/1]
RXERR1 Preamble
[27.541024 PD TMOUT RX 1/1]
RXERR1 Preamble
[27.652460 PD TMOUT RX 1/1]
RXERR1 Preamble

My comment in #4 is slightly incorrect.

Servo_v4 by default acts as a DTS (device test system) and therefore applies an Rp value to both CC lines. The spec requires that CC1 > CC2 in terms of voltage and that allows the DUT to know which CC line to use for PD messages.

I believe the reason that the Samus connection works in one cable orientation and not the other is that Samus is not handling the polarity determination correctly when connected to a DTS (servo_v4 in this test). Since servo_v4 always assumes CC1, the same CC line is always used by servo for PD messaging. If the orientation of the cable is such that it matches Samus's polarity determination, then the connection is successful. However, when the cable orientation is flipped then Samus is looking for PD messages on the wrong CC line.

The DUTs which use 3rd party TCPCs (Analogix, parade, etc) appear to do the correct polarity determination and so the cable orientation doesn't matter. (not sure about Pyro as that's a Reef derivative so I expect it would have worked).

I tested with a Pixel phone and it seems to never pick the correct polarity which is why it seems to fail regardless of cable orientation. (That's my guess, but I don't have any way to know what's going in the Pixel phone).

Servo_v4 has a console command 'dts on|off' which can be used to control whether servo acts as a normal PD device or a DTS. When I set 'dts off', then servo_v4 can connect to both Samus and the Pixel phone in both orientations.

I have confirmed the fix.

Should we consider "dts off" as default setting? 

+jrbarnette

What I describe in #4 isn't a "fix". Instead it should be viewed as a description of behavior with servo_v4 and either Samus and possibly Pyro.

As far as my understanding, servo_v4 DUT port's main use case is to enable Faft testing via CCD. For this mode to be operation 'dts mode' must be 'on'. If only one CC line is pulled up, then the H1 on the DUT will not enabled CCD and then there is no AP/EC console access.

Here are some additional insights to the behavior described in #5

TLDR: I don't believe there is any issue with servo_v4 and even the log messages above can be expected on some DUTs. Updating both Samus and Pyro to latest EC image resulted in similar behavior as Reef/Eve devices.

There is no guarantee that a DUT will support PD messaging when a port is connected as a debug accessory (the 'dts on' case). If messaging is not supported for this mode, then the DUT port will remain in PD STATE 4 (PD_STATE_SNK_ACCESSORY). When servo_v4 in dts mode, it does not remain in the state PD_STATE_SRC_ACCESSORY. Instead it will advance to PD_STATE_SRC_DISCOVERY. In this state it sends what are called SRC CAPs to the DUT and is expecting a message in response. When it does not get a response, then messages 

[27.429589 PD TMOUT RX 1/1]
RXERR1 Preamble

will follow each SRC CAP sent attempt. 

Therefore, the servo_v4 console log shown above is actually expected if the DUT does not advance beyond the SNK_ACCESSORY state.

In more recent PD code (from the m/master branch) what I observe is that when SRC CAPs are sent, that triggers a PD hard reset, which allows makes the DUT transition from SNK_ACCESSORY to a state that is expecting to get SRC CAP messages. Then the DUT can advance to SNK_READY (normal steady state for a port acting as a sink).

Older PD code doesn't have this exact same behavior. Once I updated my Samus PD to the lastest TOT image and a Pyro system that I tested, then I found that they behaved similar to Reef/Eve systems I had experience with.

The behavior is related to my original finding which is no Ethernet interface shows on DUT site. 

If there is no issue on the Servo V4, then do you think there are something else causing the NIC issue? 

Is there a separate issue open for the ethernet interface not working?

I thought this is related to interface issue. Since I was asking to flash firmware otherwise NIC won't show up. Also, I notice once I set dts off the interface comes up. 

With DTS mode on, can you after connecting to the DUT please issue the EC console command on servo_v4 console 'pd 1 state'.  Then can you do the same when you have DTS mode off. I'd like to see what data roles servo_v4 is in under both circumstances. I think the reason that the ethernet port on servo_v4 is not acting correctly is because servo_v4 is acting as a power source starts and that gives it a default data role of DFP. When servo_v4 gets to SRC_READY state (state 25), it will check the data role and attempt to swap to UFP. If servo_v4 is not getting to SRC_READY, then it's going to remain as a DFP and the DUT will remain as a UFP. The DUT port needs to be a DPF in order for it to connect is internal USBC mux correctly so the USB-A port on servo_v4 is accessible.

DTS mode on: 
> pd 1 state
Port C1 CC1, Ena - Role: SRC-DFP State: SRC_DISCOVERY, Flags: 0x>

DTS mode off:
> pd 1 state
Port C1 CC1, Ena - Role: SRC-UFP State: SRC_READY, Flags: 0x41de>

which DUT did you test this with? I am finding that I get a different behavior when I test with Electro vs Pyro. With Electro the servo_v4 is able to data role swap to UFP, but that's not happening with Pyro

I test it on a Samus device. 

Any updates on this bug? 

From earlier conversations with Scott, this appears to be a samus EC bug. Can you file an issue vs. Samus in buganizer and retest on reef?

I test on a pyro  device. the DTS mode is default on. 
> dts
dts mode: on

And I still don't see the NIC from DUT side. 

Hmm, it works on my electro w/ TOT. DTS on is fine end expected. However, it seems not working in the release branch firmware-servo-9040.B

Scott, is there some cherry pick missing?

Re #21: DTS mode on doesn't guarantee that the servo_v4 DUT port will end up as a UFP. There is still the unresolved issue that data role swaps are not allowed when the EC is executing RO.

I didn't update my findings form before, but the Samus issue here is that Samus doesn't deal properly when connected to a DTS (debug test accessory). It so happens that it works in one polarity and not the other. The reason for that is that it's always going to assume that CC2 is the active CC line for PD messaging when both CC1 and CC2 are connected as is the case when connected to servo_v4.

By disabling DTS mode, then only 1 CC line is attached and there is no question about which CC line is being used for PD messaging.

Re #24 There does appear to be at least one cherry-pick missing. The function void pd_check_dr_role(int port, int dr_role, int flags) is not attempting to switch to UFP. I will double check the CLs to make sure all the USB-PDs were added correctly to the firmware branch.

This CL was missing from TOT https://chromium-review.googlesource.com/c/452725. I have a CL for the firmware branch to pick up this change.

What's the status of this bug?

Samus doesn't support CCD, full fledged PD, or the explicit Chrome OS lab support in PD, and shouldn't use the USB-C servo v4. It should use type-A servo v4.

I don't think we want to do another EC/PD RO release for Samus just to support servo v4 charge though and role reversal.