servo_glados_overlay.xml has some conflicting controls with servo_micro.xml |
||||||||||
Issue descriptionraw_usbpd_uart_pty: servo_glados_overlay uses interface 1 while servo_micro uses interface 2 usbpd_ec3po_interp_connect: is not clobber_ok on servo_glados_overlay Does the servo_glados_overlay.xml control file need these controls set this way? Assigning to Duncan since he added those controls in: https://chromium-review.googlesource.com/#/c/327700/
,
Nov 1 2016
Making this a P0 since this is now blocking deployments of Asuka/Sentry in the prometheus lab.
,
Nov 1 2016
Nick knows how to properly fix the situation with raw_usbpd_uart_pty. usbpd_ec3po_interp_connect has an easy fix in flight already: https://chromium-review.googlesource.com/#/c/406009/
,
Nov 1 2016
,
Nov 1 2016
This is difficult because the glados xml contains a bunch of servo v3 specific configuration. Note that the above usbpd fix removes the warning but won't actually enable the pd uart.
,
Nov 1 2016
Did a bit study. In servod, there are 2 kinds of configs: * one for servo board, like servo_v2_r1.xml, servo_v3_r0.xml, servo_v4.xml; * one for cros board, like servo_glados_overlay.xml. In fact, the problematic raw_usbpd_uart_pty should be defined in servo board config, instead of cros board config. However, servo v2 doesn't support a third-uart, so Glados re-purposes JTAG to be the USB PD UART (Samus and some other boards re-purpose EC SPI (interface 5)). This rework is not a normal case for servo v2. So it doesn't make sense to put it to servo v2 board config. But putting it to cros board causes the conflict with servo v4 which uses different interfaces. There are some ways to solve this issue, but as Nick said they are difficult. I can state if someone is interested. IMO, the servo v2 way is a bit hacky and requires rework. The servo v4 + micro is the ideal approach. We may simply phase out the servo v2 way. If developers want the USB PD UART, use servo v4 + micro. Any comment?
,
Nov 1 2016
I'm all for phasing out the servo v2 way but don't know the process of doing that gracefully (seems like all we can do is to remove it and print a message telling the user to switch to a v4/uServo if they want pdusb).
In the spirit of getting things working again sooner rather than later, would it be all right if I added in a servo_{micro,v4}_interface control into servo_glados_overlay.xml raw_usbpd_uart_pty to get servod working again and unblock deployments until a proper solution is decided upon and implemented?
,
Nov 1 2016
,
Nov 1 2016
Didn't know we can specify a servo_{micro,v4}_interface control. Looks good as a short term fix.
,
Nov 1 2016
From my perspective all *_usbpd_* controls were only intended to be used for USBPD developers or for that matter any additional MCU uarts(sensor-hub?) that fit outside the servo V2 header spec. No automation IMO should EVER use these controls so providing your changes to servod don't break the currently supported USBPD developer access to servo V2 we can proceed and later address continued support.
,
Nov 2 2016
Does anyone have a known-working glados based system I can try this on?
,
Nov 2 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/hdctools/+/b7962863752dd51678fef2823ddb8db96cbb7b31 commit b7962863752dd51678fef2823ddb8db96cbb7b31 Author: Kevin Cheng <kevcheng@chromium.org> Date: Tue Nov 01 20:35:15 2016 servo_glados_overlay: Add in servo_{v4,micro}_interface to raw_usbpd_uart_pty. servo_glados_overlay defines raw_usbpd_uart_pty on an interface specifically for servo v2/v3 (that is reworked) that is not compatible with servo v4/uServo. This is a sticky situation since this control really should belong in the servo v2/v3 overlay but at the same time doesnt since those controls don't really exist without a rework on the v2/v3. A proper solution is still being hashed out but this is blocking servod from starting which is blocking all boards that use this overlay from deploying. Right now just define a servo v4/uServo specific interface for this control so that servod can get going. It's expected that usbpd won't work this way but we're not using it as part of deployments and the majority of testing doesn't as well so we're ok with that. BUG= chromium:660922 TEST=Started servod with change with servo v4/uServo and asuka and did not see the conflict. Change-Id: If444c18460103df0b4d921163f8445424d398f1c Reviewed-on: https://chromium-review.googlesource.com/405757 Commit-Ready: Kevin Cheng <kevcheng@chromium.org> Tested-by: Kevin Cheng <kevcheng@chromium.org> Reviewed-by: Wai-Hong Tam <waihong@google.com> [modify] https://crrev.com/b7962863752dd51678fef2823ddb8db96cbb7b31/servo/data/servo_glados_overlay.xml
,
Nov 2 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/hdctools/+/db7bc11ead408c6a7c132f0bf7dbc87d7a1f6f88 commit db7bc11ead408c6a7c132f0bf7dbc87d7a1f6f88 Author: Kevin Cheng <kevcheng@chromium.org> Date: Tue Nov 01 18:05:13 2016 servo_glados_overlay: Make usbpd_ec3po_interp_connect clobber'able. servo_micro.xml also defines this control and in boards that inherit the servo_glados_overlay (sentry, asuka) this causes issues when starting servod. Both control definitions share the same params so it's safe to clobber. BUG= chromium:660922 TEST=Started up servod and did not complain about control anymore. Change-Id: I0536537eaf57e04efda94480424a4773a538cc65 Reviewed-on: https://chromium-review.googlesource.com/406009 Commit-Ready: Kevin Cheng <kevcheng@chromium.org> Tested-by: Kevin Cheng <kevcheng@chromium.org> Reviewed-by: Todd Broch <tbroch@chromium.org> [modify] https://crrev.com/db7bc11ead408c6a7c132f0bf7dbc87d7a1f6f88/servo/data/servo_glados_overlay.xml
,
Nov 3 2016
The routing for uart3 for samus and glados are as follows, we'll need to make an init function for each: samus: gpioset UART3_TX_SERVO_JTAG_TCK ALT gpioset UART3_RX_JTAG_BUFFER_TO_SERVO_TDO ALT gpioset SPI1_MUX_SEL 0 gpioset SPI1_BUF_EN_L 0 gpioset SPI1_VREF_18 1 glados: gpioset UART3_RX_JTAG_BUFFER_TO_SERVO_TDO ALT gpioset UART3_TX_SERVO_JTAG_TCK ALT gpioset JTAG_BUFIN_EN_L 0 gpioset SERVO_JTAG_TDO_BUFFER_EN 1 gpioset SERVO_JTAG_TDO_SEL 1
,
Nov 7 2016
FYI this bug is blocked on code review of: https://chromium-review.googlesource.com/#/c/407539/
,
Nov 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/hdctools/+/292b645467b46ffdec4fca0dfab954f632731cd6 commit 292b645467b46ffdec4fca0dfab954f632731cd6 Author: Kevin Cheng <kevcheng@chromium.org> Date: Thu Nov 03 06:30:17 2016 servo_lucidcosmos_overlay: Add in servo_{v4,micro}_interface for raw_ec_uart_pty. BUG= chromium:660922 TEST=start servod with v4/micro and servo_lucidcosmos_overlay and it starts up successfully. Change-Id: I1629c2c210b7263c73137812c407e42f546a1a3d Reviewed-on: https://chromium-review.googlesource.com/407052 Commit-Ready: Kevin Cheng <kevcheng@chromium.org> Tested-by: Kevin Cheng <kevcheng@chromium.org> Reviewed-by: Wai-Hong Tam <waihong@google.com> [modify] https://crrev.com/292b645467b46ffdec4fca0dfab954f632731cd6/servo/data/servo_lucidcosmos_overlay.xml
,
Nov 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/hdctools/+/a194d9a5a2292492d782f6b6c1e184addfab3f91 commit a194d9a5a2292492d782f6b6c1e184addfab3f91 Author: Nick Sanders <nsanders@chromium.org> Date: Fri Nov 04 02:50:38 2016 servo_micro: add UART3 routing in servod This provides a driver and control for switching the usbpd uart to spi lines (samus) or jtag lines (glados), allowing pd controller interaction through servod. BUG= chromium:660922 TEST=can access PD uart on samus, chell after usbpd_uart_routing:glados/samus Change-Id: Ie41d5b045a15bf640ee107f4f014b219c08b20a6 Reviewed-on: https://chromium-review.googlesource.com/407539 Commit-Ready: Nick Sanders <nsanders@chromium.org> Tested-by: Nick Sanders <nsanders@chromium.org> Reviewed-by: Kevin Cheng <kevcheng@chromium.org> [modify] https://crrev.com/a194d9a5a2292492d782f6b6c1e184addfab3f91/servo/drv/__init__.py [add] https://crrev.com/a194d9a5a2292492d782f6b6c1e184addfab3f91/servo/drv/ec3po_servo_micro.py [modify] https://crrev.com/a194d9a5a2292492d782f6b6c1e184addfab3f91/servo/data/servo_micro.xml
,
Nov 18 2016
usbpd_uart_routing:glados/samus servo contol is added I believe Kevin is refactoring the xml, and can init this control to the correct value with the servo_micro + glados config xml.
,
Nov 30 2016
Pri-0 bugs are critical regressions or serious emergencies, and this bug has not been updated in three days. Could you please provide an update, or adjust the priority to a more appropriate level if applicable? If a fix is in active development, please set the status to Started. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 15 2016
Pri-0 bugs are critical regressions or serious emergencies, and this bug has not been updated in three days. Could you please provide an update, or adjust the priority to a more appropriate level if applicable? If a fix is in active development, please set the status to Started. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jun 1 2017
Fixed?
,
Jun 2 2017
The conflict was resolved. Per c#18, still need to modify the overlay xml files to set the proper usbpd_uart_routing value to either glados or samus.
,
Jul 17 2017
ChromeOS Infra P1 Bugscrub. P1 Bugs in this component should be important enough to get weekly status updates. Is this already fixed? -> Fixed Is this no longer relevant? -> Archived or WontFix Is this not a P1, based on go/chromeos-infra-bug-slo rubric? -> lower priority. Is this a Feature Request rather than a bug? Type -> Feature Is this missing important information or scope needed to decide how to proceed? -> Ask question on bug, possibly reassign. Does this bug have the wrong owner? -> reassign. Bugs that remain in this state next week will be downgraded to P2.
,
Jul 18 2017
,
Oct 11 2017
,
Jun 8 2018
Hi, this bug has not been updated recently. Please acknowledge the bug and provide status within two weeks (6/22/2018), or the bug will be closed. Thank you.
,
Jun 8 2018
Should be all set here. |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by kevcheng@chromium.org
, Nov 1 2016