Rizwan Qureshi has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37948 )
Change subject: mb/google/puff: [WIP] Add power sequencing for LSPCON ......................................................................
mb/google/puff: [WIP] Add power sequencing for LSPCON
Toggle power down and reset pins for LSPCON.
BUG=None BRANCH=None TEST=ChromeOS display and firmware screens are displayed on HDMI display connected to the HDMI (LSPCON) port1 on puff.
Change-Id: I336d8b72fcd44a0d03ee7f277eb5e838e4005804 Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com --- M src/mainboard/google/hatch/variants/puff/Makefile.inc M src/mainboard/google/hatch/variants/puff/gpio.c A src/mainboard/google/hatch/variants/puff/ramstage.c 3 files changed, 38 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/37948/1
diff --git a/src/mainboard/google/hatch/variants/puff/Makefile.inc b/src/mainboard/google/hatch/variants/puff/Makefile.inc index 2d1440e..65476bc 100644 --- a/src/mainboard/google/hatch/variants/puff/Makefile.inc +++ b/src/mainboard/google/hatch/variants/puff/Makefile.inc @@ -15,3 +15,4 @@ ramstage-y += gpio.c ramstage-y += mainboard.c bootblock-y += gpio.c +ramstage-y += ramstage.c diff --git a/src/mainboard/google/hatch/variants/puff/gpio.c b/src/mainboard/google/hatch/variants/puff/gpio.c index 57327fe..e726f9d 100644 --- a/src/mainboard/google/hatch/variants/puff/gpio.c +++ b/src/mainboard/google/hatch/variants/puff/gpio.c @@ -107,6 +107,8 @@ PAD_CFG_NF(GPP_B18, NONE, DEEP, NF1), /* C14 : BT_DISABLE_L */ PAD_CFG_GPO(GPP_C14, 0, DEEP), + PAD_CFG_GPO(GPP_C11, 0, PLTRST), + PAD_CFG_GPO(GPP_C10, 0, PLTRST), /* PCH_WP_OD */ PAD_CFG_GPI(GPP_C20, NONE, DEEP), /* C21 : H1_PCH_INT_ODL */ diff --git a/src/mainboard/google/hatch/variants/puff/ramstage.c b/src/mainboard/google/hatch/variants/puff/ramstage.c new file mode 100644 index 0000000..0371865 --- /dev/null +++ b/src/mainboard/google/hatch/variants/puff/ramstage.c @@ -0,0 +1,35 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2019 Google LLC + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <delay.h> +#include <gpio.h> +#include <baseboard/variants.h> +#include <soc/gpio.h> + +void variant_ramstage_init(void) +{ + /* + * Enable power to FPMCU, wait for power rail to stabilize, + * and then deassert FPMCU reset. + * Waiting for the power rail to stabilize can take a while, + * a minimum of 400us on Kohaku. + */ + gpio_output(GPP_C11, 0); + gpio_output(GPP_C10, 0); + mdelay(5); + gpio_output(GPP_C11, 1); + mdelay(2); + gpio_output(GPP_C10, 1); +}
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37948 )
Change subject: mb/google/puff: [WIP] Add power sequencing for LSPCON ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37948/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/puff/gpio.c:
https://review.coreboot.org/c/coreboot/+/37948/1/src/mainboard/google/hatch/... PS1, Line 108: /* C14 : BT_DISABLE_L */ Can you sort these, so:
``` /* C10 : .. */ .. /* C11 : .. */ .. /* C14 : BT_DISABLE_L */ PAD_CFG_GPO(GPP_C14, 0, DEEP), ```
https://review.coreboot.org/c/coreboot/+/37948/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/puff/ramstage.c:
https://review.coreboot.org/c/coreboot/+/37948/1/src/mainboard/google/hatch/... PS1, Line 24: * Enable power to FPMCU, wait for power rail to stabilize, : * and then deassert FPMCU reset. : * Waiting for the power rail to stabilize can take a while, : * a minimum of 400us on Kohaku. I guess this doesn't make sense however I see what you are trying to do with lspcon.
Hello Edward O'Callaghan, Kangheui Won, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37948
to look at the new patch set (#2).
Change subject: mb/google/puff: [WIP] Add power sequencing for LSPCON ......................................................................
mb/google/puff: [WIP] Add power sequencing for LSPCON
Toggle power down and reset pins for LSPCON.
BUG=None BRANCH=None TEST=ChromeOS display and firmware screens are displayed on HDMI display connected to the HDMI (LSPCON) port1 on puff.
Change-Id: I336d8b72fcd44a0d03ee7f277eb5e838e4005804 Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com --- M src/mainboard/google/hatch/variants/puff/Makefile.inc A src/mainboard/google/hatch/variants/puff/ramstage.c 2 files changed, 35 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/37948/2
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37948 )
Change subject: mb/google/puff: [WIP] Add power sequencing for LSPCON ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37948/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/puff/gpio.c:
https://review.coreboot.org/c/coreboot/+/37948/1/src/mainboard/google/hatch/... PS1, Line 108: /* C14 : BT_DISABLE_L */
Can you sort these, so: […]
This is not required
https://review.coreboot.org/c/coreboot/+/37948/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/puff/ramstage.c:
https://review.coreboot.org/c/coreboot/+/37948/1/src/mainboard/google/hatch/... PS1, Line 24: * Enable power to FPMCU, wait for power rail to stabilize, : * and then deassert FPMCU reset. : * Waiting for the power rail to stabilize can take a while, : * a minimum of 400us on Kohaku.
I guess this doesn't make sense however I see what you are trying to do with lspcon.
Done
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37948 )
Change subject: mb/google/puff: [WIP] Add power sequencing for LSPCON ......................................................................
Patch Set 2:
I do not have the data sheet for PS175 LSPCON, I have just put the delays arbitrarily. Besides that the FW screens do not appear across warm reset. will have to fix that.
Usha P has uploaded a new patch set (#3) to the change originally created by Rizwan Qureshi. ( https://review.coreboot.org/c/coreboot/+/37948 )
Change subject: mb/google/puff: [WIP] Add power sequencing for LSPCON ......................................................................
mb/google/puff: [WIP] Add power sequencing for LSPCON
Toggle power down and reset pins for LSPCON.
BUG=None BRANCH=None TEST=ChromeOS display and firmware screens are displayed on HDMI display connected to the HDMI (LSPCON) port1 on puff.
Change-Id: I336d8b72fcd44a0d03ee7f277eb5e838e4005804 Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com --- M src/mainboard/google/hatch/variants/puff/Makefile.inc A src/mainboard/google/hatch/variants/puff/ramstage.c 2 files changed, 37 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/37948/3
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37948 )
Change subject: mb/google/puff: [WIP] Add power sequencing for LSPCON ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37948/3/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/puff/ramstage.c:
https://review.coreboot.org/c/coreboot/+/37948/3/src/mainboard/google/hatch/... PS3, Line 35: mdelay(400); //allow enough time for firmware to initialize before display init in fsp kicks in. line over 96 characters
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37948 )
Change subject: mb/google/puff: [WIP] Add power sequencing for LSPCON ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37948/3/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/puff/ramstage.c:
https://review.coreboot.org/c/coreboot/+/37948/3/src/mainboard/google/hatch/... PS3, Line 35: firmware to initialize which part of the firmware are we waiting for here? the lspcon silicon to settle specifically?
Usha P has uploaded a new patch set (#4) to the change originally created by Rizwan Qureshi. ( https://review.coreboot.org/c/coreboot/+/37948 )
Change subject: mb/google/puff: [WIP] Add power sequencing for LSPCON ......................................................................
mb/google/puff: [WIP] Add power sequencing for LSPCON
Toggle power down and reset pins for LSPCON.
BUG=None BRANCH=None TEST=ChromeOS display and firmware screens are displayed on HDMI display connected to the HDMI (LSPCON) port1 on puff.
Change-Id: I336d8b72fcd44a0d03ee7f277eb5e838e4005804 Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com --- M src/mainboard/google/hatch/variants/puff/Makefile.inc A src/mainboard/google/hatch/variants/puff/ramstage.c 2 files changed, 38 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/37948/4
Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37948 )
Change subject: mb/google/puff: [WIP] Add power sequencing for LSPCON ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37948/3/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/puff/ramstage.c:
https://review.coreboot.org/c/coreboot/+/37948/3/src/mainboard/google/hatch/... PS3, Line 35: firmware to initialize
which part of the firmware are we waiting for here? the lspcon silicon to settle specifically?
Yes, the LSPCON Firmware. One other solution is to move this sequence to bootblock, so that we have enough time before fsp tries to initialize display.
Usha P has uploaded a new patch set (#5) to the change originally created by Rizwan Qureshi. ( https://review.coreboot.org/c/coreboot/+/37948 )
Change subject: mb/google/puff: [WIP] Add power sequencing for LSPCON ......................................................................
mb/google/puff: [WIP] Add power sequencing for LSPCON
Toggle power down and reset pins for LSPCON.
BUG=None BRANCH=None TEST=ChromeOS display and firmware screens are displayed on HDMI display connected to the HDMI (LSPCON) port1 on puff.
Change-Id: I336d8b72fcd44a0d03ee7f277eb5e838e4005804 Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com Signed-off-by: Usha P usha.p@intel.com --- M src/mainboard/google/hatch/variants/puff/Makefile.inc A src/mainboard/google/hatch/variants/puff/ramstage.c 2 files changed, 41 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/37948/5
Usha P has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37948 )
Change subject: mb/google/puff: [WIP] Add power sequencing for LSPCON ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37948/3/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/puff/ramstage.c:
https://review.coreboot.org/c/coreboot/+/37948/3/src/mainboard/google/hatch/... PS3, Line 35: mdelay(400); //allow enough time for firmware to initialize before display init in fsp kicks in.
line over 96 characters
Done
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37948 )
Change subject: mb/google/puff: [WIP] Add power sequencing for LSPCON ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37948/3/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/puff/ramstage.c:
https://review.coreboot.org/c/coreboot/+/37948/3/src/mainboard/google/hatch/... PS3, Line 35: firmware to initialize
Yes, the LSPCON Firmware. […]
OK, make that explicit that you mean LSPCON fw. What about early_gpio tables and setting it to be low first then high later?
Rizwan Qureshi has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/37948 )
Change subject: mb/google/puff: [WIP] Add power sequencing for LSPCON ......................................................................
Abandoned
I tried on both puff SKUs, without this patch. The display works fine. Looks like the board that i was working on has an issue.
-
build bot (Jenkins) (Code Review) -
Edward O'Callaghan (Code Review)
-
Rizwan Qureshi (Code Review)
-
Usha P (Code Review)