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

Issue 765369 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

link: LIghtbar probe fails

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

Issue description

Lightbar probe fails on link due to this CL:

https://chromium-review.googlesource.com/#/c/chromiumos/third_party/kernel/+/240272/

	param.cmd = LIGHTBAR_CMD_VERSION;
	ret = cros_ec_cmd_xfer(ec, &msg);
	if (ret < 0)
		return 0;

	switch (msg.result) {
	case EC_RES_INVALID_PARAM:
		/* Pixel had no version command. */
		if (ver_ptr)
			*ver_ptr = 0;
		if (flg_ptr)
			*flg_ptr = 0;
		return 1;

	case EC_RES_SUCCESS:
		/* Future devices w/lightbars should implement this command */
		if (ver_ptr)
			*ver_ptr = resp.version.num;
		if (flg_ptr)
			*flg_ptr = resp.version.flags;
		return 1;
	}

	/* Anything else (ie, EC_RES_INVALID_COMMAND) - no lightbar */
	return 0;
}

cros_ec_cmd_xfer will return -EBADMSG on msg->result == EC_RES_INVALID_PARAM, so "case EC_RES_INVALID_PARAM" will never be reached. This is contained to 3.8 kernel, I'll put up a quick fix.
 
Components: OS>Kernel
I'm curious, does this have any effect in practice? Does anyone talk to the lightbar from kernel/AP side, or is it all controlled by the EC?

Comment 2 by sha...@chromium.org, Sep 14 2017

I don't think this has any effect in practice. The only code I can find that references cros_ec/lightbar is ec/util/lbplay.c, and that looks like a demo / test tool.

Some folks are interested in seeing how link lightbar works for research purposes, which is what prompted this crbug.
Thanks, SGTM. Reviewing now.
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 15 2017

Labels: merge-merged-chromeos-3.8
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/702bf220da21eaf6fa98e0f399f9443c0719033f

commit 702bf220da21eaf6fa98e0f399f9443c0719033f
Author: Shawn Nematbakhsh <shawnn@chromium.org>
Date: Fri Sep 15 01:01:52 2017

HACK: cros_ec_lightbar: Fix probe with EC_RES_INVALID_PARAM

cros_ec_cmd_xfer() will return -EBADMSG when the result of a host
command is EC_RES_INVALID_PARAM, so take that into account when checking
for an unsupported sub-command of EC_CMD_LIGHTBAR_CMD.

cros_ec_cmd_xfer() behaves differently (returns EC_SUCCESS and leaves
interpretation of the EC-supplied command result up to the caller) on
all future kernels, so this CL is a one-off for 3.8.

BUG= chromium:765369 
TEST=Verify /sys/devices/virtual/chromeos/cros_ec/lightbar exists on
link.

Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Change-Id: If45b9912e55b7c3b663b670a68fc65193cc7008e
Reviewed-on: https://chromium-review.googlesource.com/667869
Commit-Ready: Shawn N <shawnn@chromium.org>
Tested-by: Shawn N <shawnn@chromium.org>
Reviewed-by: Bill Richardson <wfrichar@chromium.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>

[modify] https://crrev.com/702bf220da21eaf6fa98e0f399f9443c0719033f/drivers/mfd/cros_ec_lightbar.c

Comment 5 by sha...@chromium.org, Sep 21 2017

Status: Fixed (was: Untriaged)
Status: Verified (was: Fixed)

Sign in to add a comment