New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 769895 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

ec: Cleanup servo_v4

Project Member Reported by sha...@chromium.org, Sep 28 2017

Issue description

I'd like to create a FW branch for servo_v4 and delete the servo_v4 board from TOT. The main justification for this is that I'd like to remove CONFIG_USB_PD_DTS from TOT EC FW. CONFIG_USB_PD_DTS is basically a proxy for BOARD_SERVO_V4, it's implementing several state machine variations that are necessary for servo V4, but don't really have anything to do with DTS.

I realize that we may have to support servo V4-like boards in the future. But, we can build up support in a cleaner way. For example, servo_v4 has one dedicated source port, one dedicated sink port. We don't have a good way of specifying that in EC FW yet, CONFIG_USB_PD_DUAL_ROLE defines board-wide support for source+sink role (and absence of CONFIG_USB_PD_DUAL_ROLE provides support for source only). We can fix this by maybe adding a config struct for each PD port that specifies supported roles.

We could refactor this piece-by-piece, but it would reduce development and testing time if we could drop servo_v4 from TOT entirely.

Any objection?
 

Comment 1 by sha...@chromium.org, Sep 28 2017

Summary: ec: Consider removing servo_v4 board from TOT (was: ec: Consider branching servo_v4 EC FW)
Scott informs me that we already have a servo_v4 FW branch (firmware-servo-9040.B) so now we just need to decide whether we can delete the servo_v4 board from TOT.
Servo v4 is still in active development so I'd rather not do this just yet.
Servo v4 is still in active development so I'd rather not do this just yet.

Comment 4 by sha...@chromium.org, Oct 11 2017

If we want to keep servo v4 on TOT, I will do some cleanup instead. It will probably involve adding port-level support for sink-only / source-only, and adding charge_manager to your board.
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/33ec4ae3bc4a6c08e78fa6039b8f424627560bdc

commit 33ec4ae3bc4a6c08e78fa6039b8f424627560bdc
Author: Shawn Nematbakhsh <shawnn@chromium.org>
Date: Tue Oct 17 18:07:44 2017

charge_manager: Support no-battery / no-host boards

Boards without batteries and/or without host command support may wish to
use charge_manager.

BUG= chromium:769895 
BRANCH=None
TEST=`make buildall -j`

Change-Id: I2455528de3300a0651791752a05409c888b5f2a3
Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/713943
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/33ec4ae3bc4a6c08e78fa6039b8f424627560bdc/include/usb_pd.h
[modify] https://crrev.com/33ec4ae3bc4a6c08e78fa6039b8f424627560bdc/common/charge_manager.c
[modify] https://crrev.com/33ec4ae3bc4a6c08e78fa6039b8f424627560bdc/test/test_config.h

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/e7dfbf35a403dfa71bb839c3e722a347447be3f5

commit e7dfbf35a403dfa71bb839c3e722a347447be3f5
Author: Shawn Nematbakhsh <shawnn@chromium.org>
Date: Thu Oct 19 19:56:41 2017

servo_v4: Use charge_manager for input port / ILIM selection

BUG= chromium:769895 
BRANCH=servo
TEST=On servo_v4, attach OEM Apple charger to power port and verify
negotiation to 9V and port / ILIM selection from charge_manager. Attach
samus to DUT port and verify 9V charging.

Change-Id: Icf16f6e8c99af4fbb48a83b7a36f550c20f5fd69
Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/713944
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/e7dfbf35a403dfa71bb839c3e722a347447be3f5/common/usb_pd_protocol.c
[modify] https://crrev.com/e7dfbf35a403dfa71bb839c3e722a347447be3f5/board/servo_v4/board.h
[modify] https://crrev.com/e7dfbf35a403dfa71bb839c3e722a347447be3f5/board/servo_v4/usb_pd_policy.c

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/7c2c5a9dc3779587f78a7c602cefeb667d210d41

commit 7c2c5a9dc3779587f78a7c602cefeb667d210d41
Author: Shawn Nematbakhsh <shawnn@chromium.org>
Date: Thu Oct 19 19:56:42 2017

pd: Add "freeze" dual-role policy

Add a new DRP policy to "freeze" the power role of each port, never
toggling automatically, though manual role swaps may still occur.

BUG= chromium:769895 
BRANCH=servo
TEST=On servo_v4, verify DUT port stays in SRC role and POWER port
stays in SNK role while disconnected.

Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Change-Id: Ibff3cd1ffaf0e884b030c231003763a57acbe02e
Reviewed-on: https://chromium-review.googlesource.com/715276
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/7c2c5a9dc3779587f78a7c602cefeb667d210d41/util/ectool.c
[modify] https://crrev.com/7c2c5a9dc3779587f78a7c602cefeb667d210d41/common/usb_pd_protocol.c
[modify] https://crrev.com/7c2c5a9dc3779587f78a7c602cefeb667d210d41/include/usb_pd.h
[modify] https://crrev.com/7c2c5a9dc3779587f78a7c602cefeb667d210d41/include/config.h
[modify] https://crrev.com/7c2c5a9dc3779587f78a7c602cefeb667d210d41/include/ec_commands.h
[modify] https://crrev.com/7c2c5a9dc3779587f78a7c602cefeb667d210d41/board/servo_v4/board.h

Comment 8 by sha...@chromium.org, Oct 23 2017

Status: Verified (was: Untriaged)
Summary: ec: Cleanup servo_v4 (was: ec: Consider removing servo_v4 board from TOT)
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 30 2017

Labels: merge-merged-firmware-servo-9040.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/0e5abb63f262e51a771a54f1b53cc896e4bd9433

commit 0e5abb63f262e51a771a54f1b53cc896e4bd9433
Author: Shawn Nematbakhsh <shawnn@chromium.org>
Date: Mon Oct 30 17:04:34 2017

charge_manager: Support no-battery / no-host boards

Boards without batteries and/or without host command support may wish to
use charge_manager.

BUG= chromium:769895 
BRANCH=None
TEST=`make buildall -j`

Change-Id: I2455528de3300a0651791752a05409c888b5f2a3
Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/713943
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/739991

[modify] https://crrev.com/0e5abb63f262e51a771a54f1b53cc896e4bd9433/include/usb_pd.h
[modify] https://crrev.com/0e5abb63f262e51a771a54f1b53cc896e4bd9433/common/charge_manager.c
[modify] https://crrev.com/0e5abb63f262e51a771a54f1b53cc896e4bd9433/test/test_config.h

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/6d27a437c100f461bbc7ad11ce9b0ab8d30e69ae

commit 6d27a437c100f461bbc7ad11ce9b0ab8d30e69ae
Author: Shawn Nematbakhsh <shawnn@chromium.org>
Date: Mon Oct 30 17:04:54 2017

servo_v4: Use charge_manager for input port / ILIM selection

BUG= chromium:769895 
BRANCH=servo
TEST=On servo_v4, attach OEM Apple charger to power port and verify
negotiation to 9V and port / ILIM selection from charge_manager. Attach
samus to DUT port and verify 9V charging.

Change-Id: Icf16f6e8c99af4fbb48a83b7a36f550c20f5fd69
Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/713944
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/739996

[modify] https://crrev.com/6d27a437c100f461bbc7ad11ce9b0ab8d30e69ae/common/usb_pd_protocol.c
[modify] https://crrev.com/6d27a437c100f461bbc7ad11ce9b0ab8d30e69ae/board/servo_v4/board.h
[modify] https://crrev.com/6d27a437c100f461bbc7ad11ce9b0ab8d30e69ae/board/servo_v4/usb_pd_policy.c

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/e3103182047dd33479bcc4a3b81ce598ffde3efd

commit e3103182047dd33479bcc4a3b81ce598ffde3efd
Author: Shawn Nematbakhsh <shawnn@chromium.org>
Date: Mon Oct 30 17:05:01 2017

pd: Add "freeze" dual-role policy

Add a new DRP policy to "freeze" the power role of each port, never
toggling automatically, though manual role swaps may still occur.

BUG= chromium:769895 
BRANCH=servo
TEST=On servo_v4, verify DUT port stays in SRC role and POWER port
stays in SNK role while disconnected.

Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Change-Id: Ibff3cd1ffaf0e884b030c231003763a57acbe02e
Reviewed-on: https://chromium-review.googlesource.com/715276
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/739999

[modify] https://crrev.com/e3103182047dd33479bcc4a3b81ce598ffde3efd/util/ectool.c
[modify] https://crrev.com/e3103182047dd33479bcc4a3b81ce598ffde3efd/common/usb_pd_protocol.c
[modify] https://crrev.com/e3103182047dd33479bcc4a3b81ce598ffde3efd/include/usb_pd.h
[modify] https://crrev.com/e3103182047dd33479bcc4a3b81ce598ffde3efd/include/config.h
[modify] https://crrev.com/e3103182047dd33479bcc4a3b81ce598ffde3efd/include/ec_commands.h
[modify] https://crrev.com/e3103182047dd33479bcc4a3b81ce598ffde3efd/board/servo_v4/board.h

Project Member

Comment 12 by bugdroid1@chromium.org, Feb 12 2018

Labels: merge-merged-firmware-eve-9584.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/596aee9fcdef1bfe46eb1bb9605a175a1a93665f

commit 596aee9fcdef1bfe46eb1bb9605a175a1a93665f
Author: Shawn Nematbakhsh <shawnn@chromium.org>
Date: Mon Feb 12 19:49:22 2018

pd: Add "freeze" dual-role policy

Add a new DRP policy to "freeze" the power role of each port, never
toggling automatically, though manual role swaps may still occur.

BUG= chromium:769895 
BRANCH=servo
TEST=On servo_v4, verify DUT port stays in SRC role and POWER port
stays in SNK role while disconnected.

Change-Id: Ic4480cb6d241563dee2a848c4b35763ed83cd182
Signed-off-by: Duncan Laurie <dlaurie@google.com>
Original-Commit-Id: 7c2c5a9dc3779587f78a7c602cefeb667d210d41
Original-Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Original-Change-Id: Ibff3cd1ffaf0e884b030c231003763a57acbe02e
Original-Reviewed-on: https://chromium-review.googlesource.com/715276
Original-Commit-Ready: Shawn N <shawnn@chromium.org>
Original-Tested-by: Shawn N <shawnn@chromium.org>
Original-Reviewed-by: Vincent Palatin <vpalatin@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/914645

[modify] https://crrev.com/596aee9fcdef1bfe46eb1bb9605a175a1a93665f/util/ectool.c
[modify] https://crrev.com/596aee9fcdef1bfe46eb1bb9605a175a1a93665f/common/usb_pd_protocol.c
[modify] https://crrev.com/596aee9fcdef1bfe46eb1bb9605a175a1a93665f/include/usb_pd.h
[modify] https://crrev.com/596aee9fcdef1bfe46eb1bb9605a175a1a93665f/include/config.h
[modify] https://crrev.com/596aee9fcdef1bfe46eb1bb9605a175a1a93665f/include/ec_commands.h
[modify] https://crrev.com/596aee9fcdef1bfe46eb1bb9605a175a1a93665f/board/servo_v4/board.h

Sign in to add a comment