ec: pd: Fix data role assignment on soft / hard reset |
|||||||
Issue descriptionOn successful soft reset, we should not be re-assigning data / vconn role to power-role default: Protocol Errors are handled by a Soft_Reset Message issued by either Port Partner, that resets counters, timers and states, but does not change the negotiated voltage and current or the Port’s role (e.g. Sink, DFP/UFP, V CONN Source) and does not cause an exit from Modal Operation. We're currently re-assigning data role, see calls to pd_set_data_role() in *_DISCONNECTED_DEBOUCE. On cold reset, we do need to re-assign data role to power-role default, and we're not doing that, when we're in source role.
,
Feb 8 2018
,
Feb 20 2018
AFAICT Aseda is working on it
,
Mar 14 2018
,
Mar 14 2018
It's in my patch series[0], but I need to revisit it soon. [0] - https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/905390
,
Apr 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/36980ec169795231ba5ddaba86decb2e05512581 commit 36980ec169795231ba5ddaba86decb2e05512581 Author: Aseda Aboagye <aaboagye@google.com> Date: Thu Apr 26 06:00:01 2018 pd: Properly assign data role on reset According to PD spec: - Data role shall not be reset on soft reset. - Data role shall be reset to power-role default on hard reset. Implement the above. Even if both ports follow spec, it's still possible for a data role conflict to occur if, for example, data role swap occurs (data role mismatches power role default) followed by a hardware reset of one port (such that data role gets reset to power role default). Handle such cases by taking error recovery actions. BUG=b:71333840,chromium:805040 TEST=Connect scarlet to powered Apple accessory, verify scarlet comes up in SNK-DFP after soft reset and issuing "reboot" on EC console. After issuing a hard reset, the port comes up in SNK-UFP (which is the power-role default). BRANCH=None Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org> Signed-off-by: Aseda Aboagye <aaboagye@google.com> Change-Id: I65139f277d59a0612f8323d711080f52425ff5e7 Reviewed-on: https://chromium-review.googlesource.com/885462 Commit-Ready: Aseda Aboagye <aaboagye@chromium.org> Tested-by: Aseda Aboagye <aaboagye@chromium.org> Reviewed-by: Jett Rink <jettrink@chromium.org> Reviewed-by: Vincent Palatin <vpalatin@chromium.org> [modify] https://crrev.com/36980ec169795231ba5ddaba86decb2e05512581/include/usb_pd.h [modify] https://crrev.com/36980ec169795231ba5ddaba86decb2e05512581/common/usb_pd_protocol.c
,
Apr 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/a112d2495bbbd8da64659387b258f6608c65aa9e commit a112d2495bbbd8da64659387b258f6608c65aa9e Author: Aseda Aboagye <aaboagye@google.com> Date: Thu Apr 26 06:00:00 2018 pd: Save power role in BBRAM. In order to re-initialize our PD state variables properly following a reset, we need to save our current power role. This commit adds a bit in the BBRAM PD flags for the power role. BUG=b:71333840,chromium:805040 BRANCH=None TEST=Add code to save data role and restore both roles, verify that both are saved accordingly. Change-Id: I156ae8179c8e12c63322132d1f0078990bd215f8 Signed-off-by: Aseda Aboagye <aaboagye@google.com> Reviewed-on: https://chromium-review.googlesource.com/979264 Commit-Ready: Aseda Aboagye <aaboagye@chromium.org> Tested-by: Aseda Aboagye <aaboagye@chromium.org> Reviewed-by: Vincent Palatin <vpalatin@chromium.org> [modify] https://crrev.com/a112d2495bbbd8da64659387b258f6608c65aa9e/include/usb_pd.h [modify] https://crrev.com/a112d2495bbbd8da64659387b258f6608c65aa9e/common/usb_pd_protocol.c
,
Apr 27 2018
This is fixed in ToT, but should probably make its way to firmware branches.
,
Apr 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/6493e4316ada26c1dba7a3bad0e3093ff010f87c commit 6493e4316ada26c1dba7a3bad0e3093ff010f87c Author: Aseda Aboagye <aaboagye@google.com> Date: Fri Apr 27 22:58:18 2018 pd: Save power role in BBRAM. In order to re-initialize our PD state variables properly following a reset, we need to save our current power role. This commit adds a bit in the BBRAM PD flags for the power role. BUG=b:71333840,chromium:805040 BRANCH=None TEST=Add code to save data role and restore both roles, verify that both are saved accordingly. Change-Id: I156ae8179c8e12c63322132d1f0078990bd215f8 Signed-off-by: Aseda Aboagye <aaboagye@google.com> Reviewed-on: https://chromium-review.googlesource.com/979264 Commit-Ready: Aseda Aboagye <aaboagye@chromium.org> Tested-by: Aseda Aboagye <aaboagye@chromium.org> Reviewed-by: Vincent Palatin <vpalatin@chromium.org> (cherry picked from commit a112d2495bbbd8da64659387b258f6608c65aa9e) Reviewed-on: https://chromium-review.googlesource.com/1033365 Reviewed-by: Philip Chen <philipchen@chromium.org> Commit-Queue: Philip Chen <philipchen@chromium.org> Tested-by: Philip Chen <philipchen@chromium.org> [modify] https://crrev.com/6493e4316ada26c1dba7a3bad0e3093ff010f87c/include/usb_pd.h [modify] https://crrev.com/6493e4316ada26c1dba7a3bad0e3093ff010f87c/common/usb_pd_protocol.c
,
Apr 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/130d0b37e2a3e5e9596b1abdb212bdd34444a49d commit 130d0b37e2a3e5e9596b1abdb212bdd34444a49d Author: Aseda Aboagye <aaboagye@google.com> Date: Fri Apr 27 23:01:28 2018 pd: Properly assign data role on reset According to PD spec: - Data role shall not be reset on soft reset. - Data role shall be reset to power-role default on hard reset. Implement the above. Even if both ports follow spec, it's still possible for a data role conflict to occur if, for example, data role swap occurs (data role mismatches power role default) followed by a hardware reset of one port (such that data role gets reset to power role default). Handle such cases by taking error recovery actions. BUG=b:71333840,chromium:805040 TEST=Connect scarlet to powered Apple accessory, verify scarlet comes up in SNK-DFP after soft reset and issuing "reboot" on EC console. After issuing a hard reset, the port comes up in SNK-UFP (which is the power-role default). BRANCH=None Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org> Signed-off-by: Aseda Aboagye <aaboagye@google.com> Change-Id: I65139f277d59a0612f8323d711080f52425ff5e7 Reviewed-on: https://chromium-review.googlesource.com/885462 Commit-Ready: Aseda Aboagye <aaboagye@chromium.org> Tested-by: Aseda Aboagye <aaboagye@chromium.org> Reviewed-by: Jett Rink <jettrink@chromium.org> Reviewed-by: Vincent Palatin <vpalatin@chromium.org> (cherry picked from commit 36980ec169795231ba5ddaba86decb2e05512581) Reviewed-on: https://chromium-review.googlesource.com/1033366 Reviewed-by: Philip Chen <philipchen@chromium.org> Commit-Queue: Philip Chen <philipchen@chromium.org> Tested-by: Philip Chen <philipchen@chromium.org> [modify] https://crrev.com/130d0b37e2a3e5e9596b1abdb212bdd34444a49d/include/usb_pd.h [modify] https://crrev.com/130d0b37e2a3e5e9596b1abdb212bdd34444a49d/common/usb_pd_protocol.c
,
May 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/a19e540ce9508e4bc5e8b4a1e7bb9c081d8a9765 commit a19e540ce9508e4bc5e8b4a1e7bb9c081d8a9765 Author: Aseda Aboagye <aaboagye@google.com> Date: Tue May 01 15:49:57 2018 pd: Properly assign data role on reset According to PD spec: - Data role shall not be reset on soft reset. - Data role shall be reset to power-role default on hard reset. Implement the above. Even if both ports follow spec, it's still possible for a data role conflict to occur if, for example, data role swap occurs (data role mismatches power role default) followed by a hardware reset of one port (such that data role gets reset to power role default). Handle such cases by taking error recovery actions. BUG=b:71333840,chromium:805040 TEST=Connect scarlet to powered Apple accessory, verify scarlet comes up in SNK-DFP after soft reset and issuing "reboot" on EC console. After issuing a hard reset, the port comes up in SNK-UFP (which is the power-role default). BRANCH=None Signed-off-by: Duncan Laurie <dlaurie@google.com> Original-Commit-Id: 36980ec169795231ba5ddaba86decb2e05512581 Original-Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org> Original-Signed-off-by: Aseda Aboagye <aaboagye@google.com> Original-Change-Id: I65139f277d59a0612f8323d711080f52425ff5e7 Original-Reviewed-on: https://chromium-review.googlesource.com/885462 Original-Commit-Ready: Aseda Aboagye <aaboagye@chromium.org> Original-Tested-by: Aseda Aboagye <aaboagye@chromium.org> Original-Reviewed-by: Jett Rink <jettrink@chromium.org> Original-Reviewed-by: Vincent Palatin <vpalatin@chromium.org> Original-(cherry picked from commit 36980ec169795231ba5ddaba86decb2e05512581) Change-Id: Id79e1d915acc0275ab24990d1c3a29d9d78515e2 Reviewed-on: https://chromium-review.googlesource.com/1036929 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/a19e540ce9508e4bc5e8b4a1e7bb9c081d8a9765/include/usb_pd.h [modify] https://crrev.com/a19e540ce9508e4bc5e8b4a1e7bb9c081d8a9765/common/usb_pd_protocol.c |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by sha...@chromium.org
, Jan 23 2018