Hello Douglas Anderson, Philip Chen, mturney mturney,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/45882
to review the following change.
Change subject: drivers: sn65dsi86: Retry link training up to 5 times ......................................................................
drivers: sn65dsi86: Retry link training up to 5 times
The kernel guys have found that automatic link training from this bridge can occasionally fail and needs to be retried. They think 5 retries should be enough but have added up to 10 to be sure. Let's do the same, but since things are synchronous for us and every try takes 500ms, maybe better restrict it to 5.
BUG=b:169535092
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: I713b6851bd51d3527ed4c6e6407dee6b42d09955 --- M src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c 1 file changed, 9 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/45882/1
diff --git a/src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c b/src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c index e0058c4..91e64aa 100644 --- a/src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c +++ b/src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c @@ -415,15 +415,18 @@ sn65dsi86_bridge_dpcd_request(bus, chip, DP_BRIDGE_CONFIGURATION_SET, 1, DPCD_WRITE, &buf);
- /* semi auto link training mode */ - i2c_writeb(bus, chip, SN_ML_TX_MODE_REG, 0xa); + int i; /* Kernel driver suggests to retry this a few times if it fails. */ + for (i = 0; i < 5; i++) { + i2c_writeb(bus, chip, SN_ML_TX_MODE_REG, SEMI_AUTO_LINK_TRAINING);
- if (!wait_ms(500, - !(i2c_readb(bus, chip, SN_ML_TX_MODE_REG, &buf)) && - (buf & NORMAL_MODE))) { - printk(BIOS_ERR, "ERROR: Link training failed"); + wait_ms(500, !(i2c_readb(bus, chip, SN_ML_TX_MODE_REG, &buf)) && + (buf == NORMAL_MODE || buf == MAIN_LINK_OFF)) + + if (buf == NORMAL_MODE) + return; }
+ printk(BIOS_ERR, "ERROR: Link training failed 5 times\n"); }
static enum cb_err sn65dsi86_bridge_get_plug_in_status(uint8_t bus, uint8_t chip)
Hello Douglas Anderson, Philip Chen, mturney mturney,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45882
to look at the new patch set (#2).
Change subject: drivers: sn65dsi86: Retry link training up to 5 times ......................................................................
drivers: sn65dsi86: Retry link training up to 5 times
The kernel guys have found that automatic link training from this bridge can occasionally fail and needs to be retried. They think 5 retries should be enough but have added up to 10 to be sure. Let's do the same, but since things are synchronous for us and every try takes 500ms, maybe better restrict it to 5.
BUG=b:169535092
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: I713b6851bd51d3527ed4c6e6407dee6b42d09955 --- M src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c 1 file changed, 9 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/45882/2
Hello build bot (Jenkins), Douglas Anderson, Philip Chen, mturney mturney,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45882
to look at the new patch set (#3).
Change subject: drivers: sn65dsi86: Retry link training up to 10 times ......................................................................
drivers: sn65dsi86: Retry link training up to 10 times
The kernel guys have found that automatic link training from this bridge can occasionally fail and needs to be retried. They have added up to 10 retries just to be sure, so let's do the same in coreboot.
BUG=b:169535092
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: I713b6851bd51d3527ed4c6e6407dee6b42d09955 --- M src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c 1 file changed, 11 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/45882/3
Douglas Anderson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45882 )
Change subject: drivers: sn65dsi86: Retry link training up to 10 times ......................................................................
Patch Set 3: Code-Review+1
Douglas Anderson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45882 )
Change subject: drivers: sn65dsi86: Retry link training up to 10 times ......................................................................
Patch Set 3: Code-Review+2
Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45882 )
Change subject: drivers: sn65dsi86: Retry link training up to 10 times ......................................................................
drivers: sn65dsi86: Retry link training up to 10 times
The kernel guys have found that automatic link training from this bridge can occasionally fail and needs to be retried. They have added up to 10 retries just to be sure, so let's do the same in coreboot.
BUG=b:169535092
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: I713b6851bd51d3527ed4c6e6407dee6b42d09955 Reviewed-on: https://review.coreboot.org/c/coreboot/+/45882 Reviewed-by: Douglas Anderson dianders@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c 1 file changed, 11 insertions(+), 6 deletions(-)
Approvals: build bot (Jenkins): Verified Douglas Anderson: Looks good to me, approved
diff --git a/src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c b/src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c index e0058c4..effa841 100644 --- a/src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c +++ b/src/drivers/ti/sn65dsi86bridge/sn65dsi86bridge.c @@ -415,15 +415,20 @@ sn65dsi86_bridge_dpcd_request(bus, chip, DP_BRIDGE_CONFIGURATION_SET, 1, DPCD_WRITE, &buf);
- /* semi auto link training mode */ - i2c_writeb(bus, chip, SN_ML_TX_MODE_REG, 0xa); + int i; /* Kernel driver suggests to retry this up to 10 times if it fails. */ + for (i = 0; i < 10; i++) { + i2c_writeb(bus, chip, SN_ML_TX_MODE_REG, SEMI_AUTO_LINK_TRAINING);
- if (!wait_ms(500, - !(i2c_readb(bus, chip, SN_ML_TX_MODE_REG, &buf)) && - (buf & NORMAL_MODE))) { - printk(BIOS_ERR, "ERROR: Link training failed"); + if (!wait_ms(500, !(i2c_readb(bus, chip, SN_ML_TX_MODE_REG, &buf)) && + (buf == NORMAL_MODE || buf == MAIN_LINK_OFF))) { + printk(BIOS_ERR, "ERROR: unexpected link training state: %#x\n", buf); + return; + } + if (buf == NORMAL_MODE) + return; }
+ printk(BIOS_ERR, "ERROR: Link training failed 10 times\n"); }
static enum cb_err sn65dsi86_bridge_get_plug_in_status(uint8_t bus, uint8_t chip)
-
Douglas Anderson (Code Review) -
Julius Werner (Code Review)