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

Issue 759880 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

ec: Enable boards supporting pd/type-c but not supporting bc1.2

Project Member Reported by philipchen@chromium.org, Aug 28 2017

Issue description

In '/common/charge_manager.c' and '/include/charge_manager.h',
we seem to assume all of the following charge suppliers are in place: 

enum charge_supplier {
        CHARGE_SUPPLIER_PD,
        CHARGE_SUPPLIER_TYPEC,
        CHARGE_SUPPLIER_BC12_DCP,
        CHARGE_SUPPLIER_BC12_CDP,
        CHARGE_SUPPLIER_BC12_SDP,
        CHARGE_SUPPLIER_PROPRIETARY,
        CHARGE_SUPPLIER_OTHER,
        CHARGE_SUPPLIER_VBUS,
        CHARGE_SUPPLIER_COUNT
};

Because we keep track of available charge by:
static struct charge_port_info available_charge[CHARGE_SUPPLIER_COUNT]
                                               [CHARGE_PORT_COUNT];

However, for boards only supporting pd/type-c but not the others as a charging source, we don't have 'usb_charger_task'.
Therefore the charge_port_info for all charge sources except for pd and type-c would never be initialized.

We should clean up these dependencies so that we can support such boards.
 

Comment 1 by sha...@chromium.org, Aug 29 2017

Since we plan to add more boards that only support USB-C / PD charging, we should consider automatically changing `enum charge_supplier` and related charge_manager code to work automatically (and more efficiently, eg. no wasted space in available_charge[][]) when CONFIG_USB_CHARGER is not defined.
Cc: philipchen@chromium.org
Owner: sha...@chromium.org
Looks like Shawn is on it:
https://chromium-review.googlesource.com/#/c/chromiumos/platform/ec/+/663838/
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 28 2017

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

commit c781609bfd19b16737bec5482da5c1a021a6afa6
Author: Shawn Nematbakhsh <shawnn@chromium.org>
Date: Thu Sep 28 18:18:54 2017

charge_manager: Support no-BC1.2 configuration

If BC1.2 isn't supported, don't waste space + time checking for inputs
that don't exist.

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

Change-Id: I47e81451abd79a67a666d1859faf2610ee5c941a
Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/663838
Commit-Ready: Shawn N <shawnn@chromium.org>
Tested-by: Shawn N <shawnn@chromium.org>
Reviewed-by: Shawn N <shawnn@chromium.org>

[modify] https://crrev.com/c781609bfd19b16737bec5482da5c1a021a6afa6/board/glkrvp/board.h
[modify] https://crrev.com/c781609bfd19b16737bec5482da5c1a021a6afa6/include/charge_manager.h
[modify] https://crrev.com/c781609bfd19b16737bec5482da5c1a021a6afa6/board/fizz/board.c
[modify] https://crrev.com/c781609bfd19b16737bec5482da5c1a021a6afa6/common/charge_manager.c
[modify] https://crrev.com/c781609bfd19b16737bec5482da5c1a021a6afa6/board/scarlet/board.c

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

Status: Verified (was: Started)
Project Member

Comment 5 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/+/ee9ae0f08468eac1d9e733670cc9010b1873c11e

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

charge_manager: Support no-BC1.2 configuration

If BC1.2 isn't supported, don't waste space + time checking for inputs
that don't exist.

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

Change-Id: I47e81451abd79a67a666d1859faf2610ee5c941a
Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/740006

[modify] https://crrev.com/ee9ae0f08468eac1d9e733670cc9010b1873c11e/include/charge_manager.h
[modify] https://crrev.com/ee9ae0f08468eac1d9e733670cc9010b1873c11e/common/charge_manager.c

Project Member

Comment 6 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/+/5c4f03ee4ee0fe66676db48c2d5150b8ab76af9e

commit 5c4f03ee4ee0fe66676db48c2d5150b8ab76af9e
Author: Shawn Nematbakhsh <shawnn@chromium.org>
Date: Mon Feb 12 19:49:27 2018

charge_manager: Support no-BC1.2 configuration

If BC1.2 isn't supported, don't waste space + time checking for inputs
that don't exist.

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

Change-Id: I59dfe04d779b14269d398f339566eebdee802b27
Signed-off-by: Duncan Laurie <dlaurie@google.com>
Original-Commit-Id: c781609bfd19b16737bec5482da5c1a021a6afa6
Original-Change-Id: I47e81451abd79a67a666d1859faf2610ee5c941a
Original-Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Original-Reviewed-on: https://chromium-review.googlesource.com/663838
Original-Commit-Ready: Shawn N <shawnn@chromium.org>
Original-Tested-by: Shawn N <shawnn@chromium.org>
Original-Reviewed-by: Shawn N <shawnn@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/914649

[modify] https://crrev.com/5c4f03ee4ee0fe66676db48c2d5150b8ab76af9e/include/charge_manager.h
[modify] https://crrev.com/5c4f03ee4ee0fe66676db48c2d5150b8ab76af9e/board/fizz/board.c
[modify] https://crrev.com/5c4f03ee4ee0fe66676db48c2d5150b8ab76af9e/common/charge_manager.c

Sign in to add a comment