servo_v4: Change DUT voltage transition sequence |
|||||
Issue descriptionWith current TOT code, servo_v4 will negotiate PD with its charge port, possibly negotiating > 5V. Subsequently and separately, PD negotiation will occur with the DUT port, advertising (1) 5V and (2) the same voltage / current negotiated with the charge port. If (2) is selected, a gpio will be toggled to switch the DUT VBUS source from 5V internal to > 5V, directly from the charge port. Switch-over is not seamless and VBUS may drop during the transition, and in that case, DUT power source will switch back to 5V internal, and negotiation with the DUT will start anew, often leading to repeating cyclical failure. We will change this to work as follows: 1) When no source is attached to the CHG port, DUT power source will be 5V internal. 2) As soon as a source is attached to the CHG port, DUT power source will switch over to CHG VBUS (this will initially be 5V). This may cause DUT VBUS to glitch, but that's ok, because it will only happen once, it won't be a cyclic / recurring failure. 3) CHG port will not negotiate > 5V yet, instead we will check CHG source caps, and advertise those source caps to the DUT. 4) If DUT selects a source cap > 5V, we will negotiate with the CHG source, once that negotiation / step-up completes, we will finish negotiation with the DUT.
,
Oct 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/ce2377414aeabddba8bfb109a9eb6aeb5303850f commit ce2377414aeabddba8bfb109a9eb6aeb5303850f Author: Shawn Nematbakhsh <shawnn@chromium.org> Date: Thu Oct 19 21:56:08 2017 servo_v4: Add CHG / DUT port voltage synchronization The previous method of negotiating > 5V on CHG port followed by asynchronously negotiating > 5V on DUT port (and then switching DUT VBUS source from internal 5V to DUT) will often not function correctly due to VBUS glitching on the switchover. Instead, immediately switch VBUS source to CHG when detected, and negotiate > 5V on CHG only after DUT has requested a transition. BUG= chromium:775542 BRANCH=servo TEST=Attach {kevin, scarlet} to servo_v4, attach Apple charger to servo_v4, verify DUTs charge at 9V and consoles are reachable on scarlet. Change-Id: I1e53f1e74e5ab1cd32a252243c23faaa8fce107b Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/724683 Commit-Ready: Shawn N <shawnn@chromium.org> Tested-by: Shawn N <shawnn@chromium.org> Reviewed-by: Vincent Palatin <vpalatin@chromium.org> [modify] https://crrev.com/ce2377414aeabddba8bfb109a9eb6aeb5303850f/board/servo_v4/board.c [modify] https://crrev.com/ce2377414aeabddba8bfb109a9eb6aeb5303850f/board/servo_v4/usb_pd_policy.c
,
Oct 21 2017
I see charging on Scarlet rev2 doesn't work well. Building with head at a3931810f, here is the log when I plug in guppy: C0 st3 [370.616859 event set 0x08000000] C0 st5 charger_set_input_current iin = 512(0x08) [370.875161 event set 0x00400000] C0 st34 [371.242264 New chg p0] charger_set_input_current iin = 512(0x08) C0 st35 [371.244218 CL: p0 s2 i500 v5000] C0 HARD RST TX C0 st4 [371.244694 event set 0x00200000] C0 st34 C0 st35 C0 HARD RST TX C0 st4 C0 st5 [372.617612 event set 0x00200000]
,
Oct 21 2017
With the same setup as #3, cherry-picking CL:730877 on top of a3931810f, I see the pd state changing back and forth between st2 and st3: C0 st3 C0 st2 C0 st3 C0 st2 C0 st3 C0 st2 C0 st3 C0 st2 C0 st3 C0 st2 C0 st3
,
Oct 21 2017
I only have Scarlet/Dru rev2 on hand. I'm not sure if the issue I saw can be reproduced on rev1 or any other boards.
,
Oct 21 2017
(3) shows that we're failing to negotiate PD. We'll charge at 500ma for a non-PD DTS source, since we have no ramp and we can't rely on the advertised USB-C current to be truthful (eg. suqy-qable). Taking servo_v4 out of the picture, does PD negotiation work on your board with just the charger attached? (4) doesn't make sense, I don't understand how my pending CL could cause us to bounce between SNK_DISCONNECTED and SNK_DISCONNECTED_DEBOUNCE, but maybe I'm missing something.
,
Oct 21 2017
,
Oct 21 2017
Sorry, I think something wrong with my board. So My finding in #4 is invalid. But CL:730877 still doesn't help - The pd transition is the same as #3 (w/i or w/o 730877). I only tested guppy, don't have servo_v4 at hand. More info: > pd 0 state Port C0 CC1, Ena - Role: SNK-UFP State: SNK_DISCOVERY, Flags: 0x0608 > charger Name: rt9467 Option: 0000000000000000 (0x0000) Man id: (unsupported) Dev id: 0x0090 V_batt: 4350 (3900 - 4710, 10) I_batt: 2000 ( 100 - 5000, 100) I_in: 500 ( 100 - 3250, 50) I_dptf: disabled Hint: I see charging works after reverting 165f7d6f3b.
,
Oct 23 2017
Digging deeper, I found this line limits the charge current to 500mA: https://chromium.googlesource.com/chromiumos/platform/ec/+/master/common/charge_manager.c#861 Commenting out this line works for me. Again, I tested with guppy and Scarlet rev2. Not sure what DTS stands for but I think the charge source should be type C.
,
Oct 23 2017
> Not sure what DTS stands for but I think the charge source should be type C. in the USB type-C spec, DTS means Debug Test System (used for the Debug accessory Mode), it's a special combination of resistors on CC pins telling the other side we have debugging capability (CCD for us). Servo v4 definitely wants to enable CCD / uses this.
,
Oct 23 2017
IIUC guppy shouldn't be recognized as DTS.
,
Oct 23 2017
> guppy shouldn't be recognized as DTS. In the code you are referring to, guppy is not, servo v4 is a DTS(as expected) and your DUT is the TS. By the way, can you use a real USB-PD power supply ? (I don't think guppy is one, it's just a crappy USB-C 15W adapter. )
,
Oct 23 2017
Sorry, this is my fault:
static typec_current_t get_typec_current_limit(int polarity, int cc1, int cc2)
{
...
if (cc_alt != TYPEC_CC_OPEN)
charge |= TYPEC_CURRENT_DTS_MASK;
...
}
TYPEC_CC_OPEN should be TYPEC_CC_VOLT_OPEN. This will fix it:
https://chromium-review.googlesource.com/#/c/chromiumos/platform/ec/+/733220/
,
Oct 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/8a909ba35dc9f7787d489ffa4c6567169f10fbb4 commit 8a909ba35dc9f7787d489ffa4c6567169f10fbb4 Author: Shawn Nematbakhsh <shawnn@chromium.org> Date: Mon Oct 23 20:08:31 2017 pd: Fix false USB-C DTS detection tcpm_get_cc() returns TYPEC_CC_VOLT_*, not TYPEC_CC_*. Check for RP rather than non-open to match USB-C spec (Table B-2 Rp/Rp Charging Current Values for a DTS Source). BUG= chromium:775542 BRANCH=servo TEST=Verify donette and guppy are not detected as DTS, verify suzy-qable is detected as DTS, on kevin DUT. Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org> Change-Id: I0ff5550e9171ff86b42b489525044bf63827240c Reviewed-on: https://chromium-review.googlesource.com/733220 Commit-Ready: Shawn N <shawnn@chromium.org> Tested-by: Shawn N <shawnn@chromium.org> Reviewed-by: Vincent Palatin <vpalatin@chromium.org> [modify] https://crrev.com/8a909ba35dc9f7787d489ffa4c6567169f10fbb4/common/usb_pd_protocol.c
,
Oct 23 2017
,
Oct 24 2017
I verified both type-c and pd charger work for Scarlet rev2 now. Thanks.
,
Oct 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/34086a6b71a9babd15f73668b12b04433a8165a5 commit 34086a6b71a9babd15f73668b12b04433a8165a5 Author: Shawn Nematbakhsh <shawnn@chromium.org> Date: Mon Oct 30 17:05:11 2017 cleanup: pd: Make PDO find / extract functions non-static Allow other modules to call pd_find_pdo_index() / pd_extract_pdo_power() in order to get information about current PDOs. BUG= chromium:775542 TEST=Manual on kevin, verify 20V negotiation with zinger still works. BRANCH=servo Change-Id: I1861a0226501bda13e7d576d0971d841da9d2b74 Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/724682 Commit-Ready: Shawn N <shawnn@chromium.org> Tested-by: Shawn N <shawnn@chromium.org> Reviewed-by: Vincent Palatin <vpalatin@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/740003 [modify] https://crrev.com/34086a6b71a9babd15f73668b12b04433a8165a5/common/usb_pd_policy.c [modify] https://crrev.com/34086a6b71a9babd15f73668b12b04433a8165a5/include/usb_pd.h [modify] https://crrev.com/34086a6b71a9babd15f73668b12b04433a8165a5/common/usb_pd_protocol.c
,
Oct 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/c2eb6ff7f05d3d43073221d4a466dcea648c6160 commit c2eb6ff7f05d3d43073221d4a466dcea648c6160 Author: Shawn Nematbakhsh <shawnn@chromium.org> Date: Mon Oct 30 17:05:14 2017 servo_v4: Add CHG / DUT port voltage synchronization The previous method of negotiating > 5V on CHG port followed by asynchronously negotiating > 5V on DUT port (and then switching DUT VBUS source from internal 5V to DUT) will often not function correctly due to VBUS glitching on the switchover. Instead, immediately switch VBUS source to CHG when detected, and negotiate > 5V on CHG only after DUT has requested a transition. BUG= chromium:775542 BRANCH=servo TEST=Attach {kevin, scarlet} to servo_v4, attach Apple charger to servo_v4, verify DUTs charge at 9V and consoles are reachable on scarlet. Change-Id: I1e53f1e74e5ab1cd32a252243c23faaa8fce107b Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/724683 Commit-Ready: Shawn N <shawnn@chromium.org> Tested-by: Shawn N <shawnn@chromium.org> Reviewed-by: Vincent Palatin <vpalatin@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/740004 [modify] https://crrev.com/c2eb6ff7f05d3d43073221d4a466dcea648c6160/board/servo_v4/board.c [modify] https://crrev.com/c2eb6ff7f05d3d43073221d4a466dcea648c6160/board/servo_v4/usb_pd_policy.c
,
May 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/d1ac1d5a0bf90d9b0b31613ca2ea4c750b62fa88 commit d1ac1d5a0bf90d9b0b31613ca2ea4c750b62fa88 Author: Shawn Nematbakhsh <shawnn@chromium.org> Date: Tue May 01 19:32:27 2018 pd: Fix false USB-C DTS detection tcpm_get_cc() returns TYPEC_CC_VOLT_*, not TYPEC_CC_*. Check for RP rather than non-open to match USB-C spec (Table B-2 Rp/Rp Charging Current Values for a DTS Source). BUG= chromium:775542 BRANCH=servo TEST=Verify donette and guppy are not detected as DTS, verify suzy-qable is detected as DTS, on kevin DUT. Signed-off-by: Duncan Laurie <dlaurie@google.com> Original-Commit-Id: 8a909ba35dc9f7787d489ffa4c6567169f10fbb4 Original-Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org> Original-Change-Id: I0ff5550e9171ff86b42b489525044bf63827240c Original-Reviewed-on: https://chromium-review.googlesource.com/733220 Original-Commit-Ready: Shawn N <shawnn@chromium.org> Original-Tested-by: Shawn N <shawnn@chromium.org> Original-Reviewed-by: Vincent Palatin <vpalatin@chromium.org> Original-(cherry picked from commit 8a909ba35dc9f7787d489ffa4c6567169f10fbb4) Change-Id: I42804dcf0a1852c71b01ed98ba493edf4a1c5a3e Reviewed-on: https://chromium-review.googlesource.com/1038050 Reviewed-by: Duncan Laurie <dlaurie@google.com> Commit-Queue: Duncan Laurie <dlaurie@google.com> Tested-by: Duncan Laurie <dlaurie@google.com> Trybot-Ready: Duncan Laurie <dlaurie@google.com> [modify] https://crrev.com/d1ac1d5a0bf90d9b0b31613ca2ea4c750b62fa88/common/usb_pd_protocol.c |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by bugdroid1@chromium.org
, Oct 19 2017