Ravi kumar has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
herobrine : Provide initial mainboard support
Change-Id: I428cf1a461ee63215f5683abbfed90202d1b2a88 Signed-off-by: Ravi Kumar Bokka rbokka@codeaurora.org --- M payloads/libpayload/Kconfig A payloads/libpayload/configs/config.herobrine M payloads/libpayload/drivers/Makefile.inc A payloads/libpayload/drivers/serial/sc7280.c A src/mainboard/google/herobrine/Kconfig A src/mainboard/google/herobrine/Kconfig.name A src/mainboard/google/herobrine/Makefile.inc A src/mainboard/google/herobrine/board.h A src/mainboard/google/herobrine/board_info.txt A src/mainboard/google/herobrine/boardid.c A src/mainboard/google/herobrine/bootblock.c A src/mainboard/google/herobrine/chromeos.c A src/mainboard/google/herobrine/chromeos.fmd A src/mainboard/google/herobrine/devicetree.cb A src/mainboard/google/herobrine/mainboard.c A src/mainboard/google/herobrine/reset.c A src/mainboard/google/herobrine/romstage.c 17 files changed, 319 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/45206/1
diff --git a/payloads/libpayload/Kconfig b/payloads/libpayload/Kconfig index b5dc9a3..43ba6ab 100644 --- a/payloads/libpayload/Kconfig +++ b/payloads/libpayload/Kconfig @@ -256,6 +256,11 @@ depends on SERIAL_CONSOLE default n
+config SC7280_SERIAL_CONSOLE + bool "SC7280 SOC compatible serial port driver" + depends on SERIAL_CONSOLE + default n + config QUALCOMM_QUPV3_SERIAL_CONSOLE bool "Qualcomm QUPV3 serial port driver" depends on SERIAL_CONSOLE diff --git a/payloads/libpayload/configs/config.herobrine b/payloads/libpayload/configs/config.herobrine new file mode 100644 index 0000000..a9d576a --- /dev/null +++ b/payloads/libpayload/configs/config.herobrine @@ -0,0 +1,5 @@ +CONFIG_LP_CHROMEOS=y +CONFIG_LP_ARCH_ARM64=y +CONFIG_LP_TIMER_ARM64_ARCH=y +CONFIG_LP_SERIAL_CONSOLE=y +CONFIG_LP_SC7280_SERIAL_CONSOLE=y diff --git a/payloads/libpayload/drivers/Makefile.inc b/payloads/libpayload/drivers/Makefile.inc index c4f7bf6..6c54c49 100644 --- a/payloads/libpayload/drivers/Makefile.inc +++ b/payloads/libpayload/drivers/Makefile.inc @@ -42,6 +42,8 @@ libc-$(CONFIG_LP_PC_MOUSE) += i8042/mouse.c libc-$(CONFIG_LP_PC_I8042) += i8042/i8042.c
+libc-$(CONFIG_LP_SC7280_SERIAL_CONSOLE) += serial/sc7280.c serial/serial.c + libc-$(CONFIG_LP_CBMEM_CONSOLE) += cbmem_console.c
libc-$(CONFIG_LP_MOUSE_CURSOR) += mouse_cursor.c diff --git a/payloads/libpayload/drivers/serial/sc7280.c b/payloads/libpayload/drivers/serial/sc7280.c new file mode 100644 index 0000000..0d7f5cf --- /dev/null +++ b/payloads/libpayload/drivers/serial/sc7280.c @@ -0,0 +1,9 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <libpayload.h> + +/* For simplicity sake let's rely on coreboot initalizing the UART. */ +void serial_console_init(void) +{ + +} diff --git a/src/mainboard/google/herobrine/Kconfig b/src/mainboard/google/herobrine/Kconfig new file mode 100644 index 0000000..92f77db --- /dev/null +++ b/src/mainboard/google/herobrine/Kconfig @@ -0,0 +1,55 @@ + +config BOARD_GOOGLE_HEROBRINE_COMMON # Umbrella option to be selected by variants + def_bool n + +if BOARD_GOOGLE_HEROBRINE_COMMON + +config BOARD_SPECIFIC_OPTIONS + def_bool y + select BOARD_ROMSIZE_KB_8192 + select COMMON_CBFS_SPI_WRAPPER + select EC_GOOGLE_CHROMEEC + select EC_GOOGLE_CHROMEEC_RTC + select EC_GOOGLE_CHROMEEC_SPI + select RTC + select SOC_QUALCOMM_SC7280 + select SPI_FLASH + select SPI_FLASH_WINBOND + select MAINBOARD_HAS_CHROMEOS + +config VBOOT + select EC_GOOGLE_CHROMEEC_SWITCHES + select VBOOT_VBNV_FLASH + select GBB_FLAG_DISABLE_EC_SOFTWARE_SYNC + select VBOOT_MOCK_SECDATA + +config MAINBOARD_DIR + string + default "google/herobrine" + +config MAINBOARD_VENDOR + string + default "Google" + +config DRIVER_TPM_SPI_BUS + hex + default 0x5 + +config EC_GOOGLE_CHROMEEC_SPI_BUS + hex + default 0xa + +########################################################## +#### Update below when adding a new derivative board. #### +########################################################## + +config MAINBOARD_PART_NUMBER + string + default "Herobrine" if BOARD_GOOGLE_HEROBRINE + +config GBB_HWID + string + depends on CHROMEOS + default "HEROBRINE TEST 1859" if BOARD_GOOGLE_HEROBRINE + +endif # BOARD_GOOGLE_HEROBRINE_COMMON diff --git a/src/mainboard/google/herobrine/Kconfig.name b/src/mainboard/google/herobrine/Kconfig.name new file mode 100644 index 0000000..13dc56a --- /dev/null +++ b/src/mainboard/google/herobrine/Kconfig.name @@ -0,0 +1,4 @@ + +config BOARD_GOOGLE_HEROBRINE + bool "Herobrine" + select BOARD_GOOGLE_HEROBRINE_COMMON diff --git a/src/mainboard/google/herobrine/Makefile.inc b/src/mainboard/google/herobrine/Makefile.inc new file mode 100644 index 0000000..949d775 --- /dev/null +++ b/src/mainboard/google/herobrine/Makefile.inc @@ -0,0 +1,20 @@ +## SPDX-License-Identifier: GPL-2.0-only + +bootblock-y += boardid.c +bootblock-y += chromeos.c +bootblock-y += bootblock.c +bootblock-y += reset.c + +verstage-y += boardid.c +verstage-y += chromeos.c +verstage-y += reset.c + +romstage-y += boardid.c +romstage-y += chromeos.c +romstage-y += romstage.c +romstage-y += reset.c + +ramstage-y += boardid.c +ramstage-y += chromeos.c +ramstage-y += mainboard.c +ramstage-y += reset.c diff --git a/src/mainboard/google/herobrine/board.h b/src/mainboard/google/herobrine/board.h new file mode 100644 index 0000000..c69d5d2 --- /dev/null +++ b/src/mainboard/google/herobrine/board.h @@ -0,0 +1,25 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef _COREBOOT_SRC_MAINBOARD_GOOGLE_TROGDOR_BOARD_H_ +#define _COREBOOT_SRC_MAINBOARD_GOOGLE_TROGDOR_BOARD_H_ + +#include <boardid.h> +#include <gpio.h> +#include <soc/gpio.h> + +#define GPIO_EC_IN_RW GPIO(118) +#define GPIO_AP_EC_INT GPIO(94) +#define GPIO_AP_SUSPEND GPIO(20) +#define GPIO_WP_STATE GPIO(42) +#define GPIO_H1_AP_INT (CONFIG(TROGDOR_REV0) ? GPIO(21) : GPIO(42)) +#define GPIO_SD_CD_L GPIO(69) +#define GPIO_AMP_ENABLE GPIO(23) + +/* Display specific GPIOS */ +#define GPIO_BACKLIGHT_ENABLE GPIO(12) +#define GPIO_EDP_BRIDGE_ENABLE (CONFIG(TROGDOR_REV0) ? GPIO(14) : GPIO(104)) +#define GPIO_EN_PP3300_DX_EDP (CONFIG(TROGDOR_REV0) ? GPIO(106) : GPIO(30)) + +void setup_chromeos_gpios(void); + +#endif /* _COREBOOT_SRC_MAINBOARD_GOOGLE_TROGDOR_BOARD_H_ */ diff --git a/src/mainboard/google/herobrine/board_info.txt b/src/mainboard/google/herobrine/board_info.txt new file mode 100644 index 0000000..a670d6e --- /dev/null +++ b/src/mainboard/google/herobrine/board_info.txt @@ -0,0 +1,6 @@ +Vendor name: Google +Board name: Herobrine Qualcomm sc7280 reference board +Category: eval +ROM protocol: SPI +ROM socketed: n +Flashrom support: y diff --git a/src/mainboard/google/herobrine/boardid.c b/src/mainboard/google/herobrine/boardid.c new file mode 100644 index 0000000..7ff648e --- /dev/null +++ b/src/mainboard/google/herobrine/boardid.c @@ -0,0 +1,43 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <boardid.h> +#include <gpio.h> + +uint32_t board_id(void) +{ + static uint32_t id = UNDEFINED_STRAPPING_ID; +#if 0 + /* TODO: update gpios for Herobrine */ + const gpio_t pins[] = {[2] = GPIO(31), [1] = GPIO(93), [0] = GPIO(33)}; + + if (id == UNDEFINED_STRAPPING_ID) + id = gpio_base2_value(pins, ARRAY_SIZE(pins)); +#endif + return id; +} + +uint32_t ram_code(void) +{ + static uint32_t id = UNDEFINED_STRAPPING_ID; +#if 0 + /* TODO: update gpios for Herobrine */ + const gpio_t pins[] = {[2] = GPIO(13), [1] = GPIO(19), [0] = GPIO(29)}; + + if (id == UNDEFINED_STRAPPING_ID) + id = gpio_base2_value(pins, ARRAY_SIZE(pins)); +#endif + return id; +} + +uint32_t sku_id(void) +{ + static uint32_t id = UNDEFINED_STRAPPING_ID; +#if 0 + /* TODO: update gpios for Herobrine */ + const gpio_t pins[] = {[2] = GPIO(20), [1] = GPIO(90), [0] = GPIO(105)}; + + if (id == UNDEFINED_STRAPPING_ID) + id = gpio_base2_value(pins, ARRAY_SIZE(pins)); +#endif + return id; +} diff --git a/src/mainboard/google/herobrine/bootblock.c b/src/mainboard/google/herobrine/bootblock.c new file mode 100644 index 0000000..05e53a6 --- /dev/null +++ b/src/mainboard/google/herobrine/bootblock.c @@ -0,0 +1,9 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <bootblock_common.h> +#include "board.h" + +void bootblock_mainboard_init(void) +{ + setup_chromeos_gpios(); +} diff --git a/src/mainboard/google/herobrine/chromeos.c b/src/mainboard/google/herobrine/chromeos.c new file mode 100644 index 0000000..dba1c09 --- /dev/null +++ b/src/mainboard/google/herobrine/chromeos.c @@ -0,0 +1,47 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <boot/coreboot_tables.h> +#include <bootmode.h> +#include "board.h" + +int get_write_protect_state(void) +{ +#if 0 + return !gpio_get(GPIO_WP_STATE); +#else + return 0; +#endif +} + +void setup_chromeos_gpios(void) +{ +#if 0 + /* Example of what needs to be done here */ + gpio_input_pullup(GPIO_EC_IN_RW); + gpio_input_pullup(GPIO_AP_EC_INT); + gpio_input_pullup(GPIO_SD_CD_L); + gpio_input_irq(GPIO_H1_AP_INT, IRQ_TYPE_RISING_EDGE, GPIO_PULL_UP); + gpio_output(GPIO_AMP_ENABLE, 0); +#endif +} + +void fill_lb_gpios(struct lb_gpios *gpios) +{ +#if 0 + /* Example of what needs to be done here */ + struct lb_gpio chromeos_gpios[] = { + {GPIO_EC_IN_RW.addr, ACTIVE_LOW, gpio_get(GPIO_EC_IN_RW), + "EC in RW"}, + {GPIO_AP_EC_INT.addr, ACTIVE_LOW, gpio_get(GPIO_AP_EC_INT), + "EC interrupt"}, + {GPIO_H1_AP_INT.addr, ACTIVE_LOW, gpio_get(GPIO_H1_AP_INT), + "TPM interrupt"}, + {GPIO_SD_CD_L.addr, ACTIVE_LOW, gpio_get(GPIO_SD_CD_L), + "SD card detect"}, + {GPIO_AMP_ENABLE.addr, ACTIVE_HIGH, gpio_get(GPIO_AMP_ENABLE), + "speaker enable"}, + }; + + lb_add_gpios(gpios, chromeos_gpios, ARRAY_SIZE(chromeos_gpios)); +#endif +} diff --git a/src/mainboard/google/herobrine/chromeos.fmd b/src/mainboard/google/herobrine/chromeos.fmd new file mode 100644 index 0000000..a44a638 --- /dev/null +++ b/src/mainboard/google/herobrine/chromeos.fmd @@ -0,0 +1,41 @@ +## SPDX-License-Identifier: GPL-2.0-only + +# TODO: update for Herobrine +FLASH@0x0 8M { + WP_RO 4M { + RO_SECTION 0x3c4000 { + BOOTBLOCK 96K + COREBOOT(CBFS) + FMAP@0x3c0000 0x1000 + GBB 0x2f00 + RO_FRID 0x100 + } + RO_VPD(PRESERVE) 228K + RO_DDR_TRAINING(PRESERVE) 8K + RO_LIMITS_CFG(PRESERVE) 4K + } + + RW_VPD(PRESERVE) 32K + RW_NVRAM(PRESERVE) 16K + RW_DDR_TRAINING(PRESERVE) 8K + RW_LIMITS_CFG(PRESERVE) 4K + RW_ELOG(PRESERVE) 4K + RW_SHARED 4K { + SHARED_DATA + } + + RW_SECTION_A 1280K { + VBLOCK_A 8K + FW_MAIN_A(CBFS) + RW_FWID_A 256 + } + + + RW_SECTION_B 1280K { + VBLOCK_B 8K + FW_MAIN_B(CBFS) + RW_FWID_B 256 + } + + RW_LEGACY(CBFS) +} diff --git a/src/mainboard/google/herobrine/devicetree.cb b/src/mainboard/google/herobrine/devicetree.cb new file mode 100644 index 0000000..e23782f --- /dev/null +++ b/src/mainboard/google/herobrine/devicetree.cb @@ -0,0 +1,5 @@ +## SPDX-License-Identifier: GPL-2.0-only + +chip soc/qualcomm/sc7280 + device cpu_cluster 0 on end +end diff --git a/src/mainboard/google/herobrine/mainboard.c b/src/mainboard/google/herobrine/mainboard.c new file mode 100644 index 0000000..31deb77 --- /dev/null +++ b/src/mainboard/google/herobrine/mainboard.c @@ -0,0 +1,22 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <device/device.h> +#include <bootblock_common.h> +#include <arch/mmio.h> +#include <gpio.h> +#include <timestamp.h> + +static void mainboard_init(struct device *dev) +{ + +} + +static void mainboard_enable(struct device *dev) +{ + dev->ops->init = &mainboard_init; +} + +struct chip_operations mainboard_ops = { + .name = CONFIG_MAINBOARD_PART_NUMBER, + .enable_dev = mainboard_enable, +}; diff --git a/src/mainboard/google/herobrine/reset.c b/src/mainboard/google/herobrine/reset.c new file mode 100644 index 0000000..9b5810f --- /dev/null +++ b/src/mainboard/google/herobrine/reset.c @@ -0,0 +1,11 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <ec/google/chromeec/ec.h> +#include <reset.h> + +/* Can't do a "real" reset before the PMIC is initialized in QcLib (romstage), + but this works well enough for our purposes. */ +void do_board_reset(void) +{ + google_chromeec_reboot(0, EC_REBOOT_COLD, 0); +} diff --git a/src/mainboard/google/herobrine/romstage.c b/src/mainboard/google/herobrine/romstage.c new file mode 100644 index 0000000..8844f18 --- /dev/null +++ b/src/mainboard/google/herobrine/romstage.c @@ -0,0 +1,10 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <arch/stages.h> +#include <soc/qclib_common.h> + +void platform_romstage_main(void) +{ + /* QCLib: DDR init & train */ + qclib_load_and_run(); +}
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45206/1/payloads/libpayload/drivers... File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/1/payloads/libpayload/drivers... PS1, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45206/2/payloads/libpayload/drivers... File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/2/payloads/libpayload/drivers... PS2, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/45206/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45206/2//COMMIT_MSG@7 PS2, Line 7: herobrine : Please remove the space before the colon.
https://review.coreboot.org/c/coreboot/+/45206/2//COMMIT_MSG@7 PS2, Line 7: herobrine : Provide initial mainboard support Please mention the SoC in the commit message.
https://review.coreboot.org/c/coreboot/+/45206/2//COMMIT_MSG@10 PS2, Line 10: Signed-off-by: Ravi One space is enough.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45206/3/payloads/libpayload/drivers... File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/3/payloads/libpayload/drivers... PS3, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45206/4/payloads/libpayload/drivers... File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/4/payloads/libpayload/drivers... PS4, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45206/5/payloads/libpayload/drivers... File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/5/payloads/libpayload/drivers... PS5, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45206/6/payloads/libpayload/drivers... File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/6/payloads/libpayload/drivers... PS6, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 6:
(5 comments)
https://review.coreboot.org/c/coreboot/+/45206/6/payloads/libpayload/configs... File payloads/libpayload/configs/config.herobrine:
https://review.coreboot.org/c/coreboot/+/45206/6/payloads/libpayload/configs... PS6, Line 5: CONFIG_LP_SC7280_SERIAL_CONSOLE=y Is the serial driver actually going to be different? I was hoping the QUPs stayed the same between the chips so you can just use the old driver here (and should probably give that a more generic name then).
https://review.coreboot.org/c/coreboot/+/45206/6/src/mainboard/google/herobr... File src/mainboard/google/herobrine/Kconfig:
https://review.coreboot.org/c/coreboot/+/45206/6/src/mainboard/google/herobr... PS6, Line 40: default 0xa Please don't define these before they're known. Just don't select the EC Kconfig for now.
https://review.coreboot.org/c/coreboot/+/45206/6/src/mainboard/google/herobr... PS6, Line 50: config GBB_HWID See the current Trogdor Kconfig, you don't need this option here anymore.
https://review.coreboot.org/c/coreboot/+/45206/6/src/mainboard/google/herobr... File src/mainboard/google/herobrine/board.h:
https://review.coreboot.org/c/coreboot/+/45206/6/src/mainboard/google/herobr... PS6, Line 21: #define GPIO_EN_PP3300_DX_EDP (CONFIG(TROGDOR_REV0) ? GPIO(106) : GPIO(30)) Please don't copy these GPIO tables, they'll be totally different for the new board.
https://review.coreboot.org/c/coreboot/+/45206/6/src/mainboard/google/herobr... File src/mainboard/google/herobrine/chromeos.c:
https://review.coreboot.org/c/coreboot/+/45206/6/src/mainboard/google/herobr... PS6, Line 9: #if 0 Just don't add any of this stuff if you don't have code for it yet.
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45206/6/src/mainboard/google/herobr... File src/mainboard/google/herobrine/board.h:
https://review.coreboot.org/c/coreboot/+/45206/6/src/mainboard/google/herobr... PS6, Line 8: #include <soc/gpio.h> maybe you can remove direct 'include <soc/gpio.h>'
https://review.coreboot.org/c/coreboot/+/45206/6/src/mainboard/google/herobr... File src/mainboard/google/herobrine/mainboard.c:
https://review.coreboot.org/c/coreboot/+/45206/6/src/mainboard/google/herobr... PS6, Line 5: #include <arch/mmio.h> : #include <gpio.h> : #include <timestamp.h> maybe not used
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45206/7/payloads/libpayload/drivers... File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/7/payloads/libpayload/drivers... PS7, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45206/8/payloads/libpayload/drivers... File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/8/payloads/libpayload/drivers... PS8, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45206/9/payloads/libpayload/drivers... File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/9/payloads/libpayload/drivers... PS9, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45206/10/payloads/libpayload/driver... File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/10/payloads/libpayload/driver... PS10, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45206/11/payloads/libpayload/driver... File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/11/payloads/libpayload/driver... PS11, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, mturney mturney,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45206
to look at the new patch set (#12).
Change subject: herobrine : Provide initial mainboard support ......................................................................
herobrine : Provide initial mainboard support
Change-Id: I428cf1a461ee63215f5683abbfed90202d1b2a88 Signed-off-by: Ravi Kumar Bokka rbokka@codeaurora.org --- M payloads/libpayload/Kconfig A payloads/libpayload/configs/config.herobrine M payloads/libpayload/drivers/Makefile.inc A payloads/libpayload/drivers/serial/sc7280.c A src/mainboard/google/herobrine/Kconfig A src/mainboard/google/herobrine/Kconfig.name A src/mainboard/google/herobrine/Makefile.inc A src/mainboard/google/herobrine/board.h A src/mainboard/google/herobrine/board_info.txt A src/mainboard/google/herobrine/boardid.c A src/mainboard/google/herobrine/bootblock.c A src/mainboard/google/herobrine/chromeos.c A src/mainboard/google/herobrine/chromeos.fmd A src/mainboard/google/herobrine/devicetree.cb A src/mainboard/google/herobrine/mainboard.c A src/mainboard/google/herobrine/reset.c A src/mainboard/google/herobrine/romstage.c 17 files changed, 319 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/45206/12
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45206/12/payloads/libpayload/driver... File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/12/payloads/libpayload/driver... PS12, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45206/13/payloads/libpayload/driver... File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/13/payloads/libpayload/driver... PS13, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45206/14/payloads/libpayload/driver... File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/14/payloads/libpayload/driver... PS14, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 15:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45206/15/payloads/libpayload/driver... File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/15/payloads/libpayload/driver... PS15, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 15:
(3 comments)
https://review.coreboot.org/c/coreboot/+/45206/15//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45206/15//COMMIT_MSG@7 PS15, Line 7: herobrine : Please remove the space before the colon.
https://review.coreboot.org/c/coreboot/+/45206/15//COMMIT_MSG@8 PS15, Line 8: 1. Is this code copied? 2. What payload was used? 3. What works?
https://review.coreboot.org/c/coreboot/+/45206/15/payloads/libpayload/Kconfi... File payloads/libpayload/Kconfig:
https://review.coreboot.org/c/coreboot/+/45206/15/payloads/libpayload/Kconfi... PS15, Line 262: default n Please split out the libpayload changes into a separate commit.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 16:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45206/16/payloads/libpayload/driver... File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/16/payloads/libpayload/driver... PS16, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 17:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45206/17/payloads/libpayload/driver... File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/17/payloads/libpayload/driver... PS17, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 18:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45206/18/payloads/libpayload/driver... File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/18/payloads/libpayload/driver... PS18, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 19:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45206/19/payloads/libpayload/driver... File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/19/payloads/libpayload/driver... PS19, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 20:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45206/20/payloads/libpayload/driver... File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/20/payloads/libpayload/driver... PS20, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 21:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45206/21/payloads/libpayload/driver... File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/21/payloads/libpayload/driver... PS21, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 22:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45206/22/payloads/libpayload/driver... File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/22/payloads/libpayload/driver... PS22, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
Attention is currently required from: Ravi kumar. Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 22:
(11 comments)
File payloads/libpayload/Kconfig:
https://review.coreboot.org/c/coreboot/+/45206/comment/21e1cd21_b74132a3 PS22, Line 267: config SC7280_SERIAL_CONSOLE : bool "SC7280 SOC compatible serial port driver" : depends on SERIAL_CONSOLE : default n : Please clarify why we need to add this.
File payloads/libpayload/configs/config.herobrine:
https://review.coreboot.org/c/coreboot/+/45206/comment/5b60f7b4_6bd327b7 PS22, Line 5: CONFIG_LP_SC7280_SERIAL_CONSOLE Do we need an soc specific config for this? I see sc7180 used CONFIG_LP_QUALCOMM_QUPV3_SERIAL_CONSOLE instead.
File payloads/libpayload/drivers/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/45206/comment/3e39a11c_18c350a1 PS22, Line 45: serial/sc7280.c are we able to reuse serial/qcom_qupv3_serial.c instead? If not, what are the differences?
File src/mainboard/google/herobrine/Kconfig:
https://review.coreboot.org/c/coreboot/+/45206/comment/1b980650_6568406f PS22, Line 50: onfig GBB_HWID : string : depends on CHROMEOS : default "HEROBRINE TEST 1859" if BOARD_GOOGLE_HEROBRINE I do not believe that we define HWID in Kconfig anymore, though I'm not sure where we actually do it anymore. Maybe Julius has an idea?
File src/mainboard/google/herobrine/Kconfig.name:
https://review.coreboot.org/c/coreboot/+/45206/comment/522418ef_cb4c7830 PS22, Line 1: Add:
if USE_QC_BLOBS
<add configs here>
endif
comment "(Herobrine requires 'Allow QC blobs repository')" depends on !USE_QC_BLOBS
File src/mainboard/google/herobrine/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/45206/comment/c59cac16_4559eccb PS22, Line 6: bootblock-y += reset.c do we need to add reset.c to every stage?
File src/mainboard/google/herobrine/board.h:
https://review.coreboot.org/c/coreboot/+/45206/comment/ea2fc824_6c1ecba5 PS22, Line 3: #ifndef _COREBOOT_SRC_MAINBOARD_GOOGLE_TROGDOR_BOARD_H_ : #define _COREBOOT_SRC_MAINBOARD_GOOGLE_TROGDOR_BOARD_H_ herobrine
https://review.coreboot.org/c/coreboot/+/45206/comment/4fc367f3_9a7418b4 PS22, Line 10: #define GPIO_EC_IN_RW GPIO(118) : #define GPIO_AP_EC_INT GPIO(94) : #define GPIO_AP_SUSPEND GPIO(20) : #define GPIO_WP_STATE GPIO(42) : #define GPIO_H1_AP_INT (CONFIG(TROGDOR_REV0) ? GPIO(21) : GPIO(42)) : #define GPIO_SD_CD_L GPIO(69) : #define GPIO_AMP_ENABLE GPIO(23) : : /* Display specific GPIOS */ : #define GPIO_BACKLIGHT_ENABLE GPIO(12) : #define GPIO_EDP_BRIDGE_ENABLE (CONFIG(TROGDOR_REV0) ? GPIO(14) : GPIO(104)) : #define GPIO_EN_PP3300_DX_EDP (CONFIG(TROGDOR_REV0) ? GPIO(106) : GPIO(30)) Seems like these have been copied from trodgor? Can you please make sure these are valid for herobrine?
https://review.coreboot.org/c/coreboot/+/45206/comment/58be7164_cea48730 PS22, Line 25: _COREBOOT_SRC_MAINBOARD_GOOGLE_TROGDOR_BOARD_H_ herobrine
File src/mainboard/google/herobrine/boardid.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/48b1d540_068a2df2 PS22, Line 9: #if 0 : /* TODO: update gpios for Herobrine */ : const gpio_t pins[] = {[2] = GPIO(31), [1] = GPIO(93), [0] = GPIO(33)}; : : if (id == UNDEFINED_STRAPPING_ID) : id = gpio_base2_value(pins, ARRAY_SIZE(pins)); : #endif : return id; : } : : uint32_t ram_code(void) : { : static uint32_t id = UNDEFINED_STRAPPING_ID; : #if 0 : /* TODO: update gpios for Herobrine */ : const gpio_t pins[] = {[2] = GPIO(13), [1] = GPIO(19), [0] = GPIO(29)}; : : if (id == UNDEFINED_STRAPPING_ID) : id = gpio_base2_value(pins, ARRAY_SIZE(pins)); : #endif : return id; : } : : uint32_t sku_id(void) : { : static uint32_t id = UNDEFINED_STRAPPING_ID; : #if 0 : /* TODO: update gpios for Herobrine */ : const gpio_t pins[] = {[2] = GPIO(20), [1] = GPIO(90), [0] = GPIO(105)}; : : if (id == UNDEFINED_STRAPPING_ID) : id = gpio_base2_value(pins, ARRAY_SIZE(pins)); : #endif : return id; Just leave these empty instead of using #if 0
File src/mainboard/google/herobrine/chromeos.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/64de7fdc_c08afe68 PS22, Line 9: #if 0 : return !gpio_get(GPIO_WP_STATE); : #else : return 0; : #endif : } : : void setup_chromeos_gpios(void) : { : #if 0 : /* Example of what needs to be done here */ : gpio_input_pullup(GPIO_EC_IN_RW); : gpio_input_pullup(GPIO_AP_EC_INT); : gpio_input_pullup(GPIO_SD_CD_L); : gpio_input_irq(GPIO_H1_AP_INT, IRQ_TYPE_RISING_EDGE, GPIO_PULL_UP); : gpio_output(GPIO_AMP_ENABLE, 0); : #endif : } : : void fill_lb_gpios(struct lb_gpios *gpios) : { : #if 0 : /* Example of what needs to be done here */ : struct lb_gpio chromeos_gpios[] = { : {GPIO_EC_IN_RW.addr, ACTIVE_LOW, gpio_get(GPIO_EC_IN_RW), : "EC in RW"}, : {GPIO_AP_EC_INT.addr, ACTIVE_LOW, gpio_get(GPIO_AP_EC_INT), : "EC interrupt"}, : {GPIO_H1_AP_INT.addr, ACTIVE_LOW, gpio_get(GPIO_H1_AP_INT), : "TPM interrupt"}, : {GPIO_SD_CD_L.addr, ACTIVE_LOW, gpio_get(GPIO_SD_CD_L), : "SD card detect"}, : {GPIO_AMP_ENABLE.addr, ACTIVE_HIGH, gpio_get(GPIO_AMP_ENABLE), : "speaker enable"}, : }; : : lb_add_gpios(gpios, chromeos_gpios, ARRAY_SIZE(chromeos_gpios)); : #endif Same here. Please don't just #if 0 blocks of code that isn't correct. IMHO it's harder to see code mods.
Attention is currently required from: Shelley Chen, Ravi kumar. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 22:
(3 comments)
File src/mainboard/google/herobrine/Kconfig:
https://review.coreboot.org/c/coreboot/+/45206/comment/ced1bae2_50197190 PS22, Line 50: onfig GBB_HWID : string : depends on CHROMEOS : default "HEROBRINE TEST 1859" if BOARD_GOOGLE_HEROBRINE
I do not believe that we define HWID in Kconfig anymore, though I'm not sure where we actually do it […]
Yes, this will get auto-generated correctly now, just leave it out here.
File src/mainboard/google/herobrine/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/45206/comment/4b2518fb_a8d0ecf4 PS22, Line 6: bootblock-y += reset.c
do we need to add reset. […]
For utility functions like board_reset() we often just add it everywhere rather than worry about where exactly it's needed. The linker will figure out where it is actually referenced and not include it in stages where it isn't, so there's no harm in that. It makes things easier if later changes want to call it from a different stage.
BTW, the Makefile system now supports `all-y += reset.c` as a shorthand for adding it to each stage individually, we should use that for files like this.
File src/mainboard/google/herobrine/board.h:
https://review.coreboot.org/c/coreboot/+/45206/comment/73b25208_28f4a45a PS22, Line 10: #define GPIO_EC_IN_RW GPIO(118) : #define GPIO_AP_EC_INT GPIO(94) : #define GPIO_AP_SUSPEND GPIO(20) : #define GPIO_WP_STATE GPIO(42) : #define GPIO_H1_AP_INT (CONFIG(TROGDOR_REV0) ? GPIO(21) : GPIO(42)) : #define GPIO_SD_CD_L GPIO(69) : #define GPIO_AMP_ENABLE GPIO(23) : : /* Display specific GPIOS */ : #define GPIO_BACKLIGHT_ENABLE GPIO(12) : #define GPIO_EDP_BRIDGE_ENABLE (CONFIG(TROGDOR_REV0) ? GPIO(14) : GPIO(104)) : #define GPIO_EN_PP3300_DX_EDP (CONFIG(TROGDOR_REV0) ? GPIO(106) : GPIO(30))
Seems like these have been copied from trodgor? Can you please make sure these are valid for herobr […]
Since there are no herobrine schematics yet (I think?) none of these are known yet. Just leave this file empty or remove it completely instead of copy&pasting stuff we know won't match... it can be added later.
Attention is currently required from: Shelley Chen. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine : Provide initial mainboard support ......................................................................
Patch Set 23:
(1 comment)
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/5197ba00_63406353 PS23, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
Attention is currently required from: Shelley Chen. Hello build bot (Jenkins), Patrick Georgi, Martin Roth, mturney mturney,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45206
to look at the new patch set (#24).
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
herobrine: sc7280: Provide initial mainboard support
Change-Id: I428cf1a461ee63215f5683abbfed90202d1b2a88 Signed-off-by: Ravi Kumar Bokka rbokka@codeaurora.org --- M payloads/libpayload/Kconfig A payloads/libpayload/configs/config.herobrine M payloads/libpayload/drivers/Makefile.inc A payloads/libpayload/drivers/serial/sc7280.c A src/mainboard/google/herobrine/Kconfig A src/mainboard/google/herobrine/Kconfig.name A src/mainboard/google/herobrine/Makefile.inc A src/mainboard/google/herobrine/board.h A src/mainboard/google/herobrine/board_info.txt A src/mainboard/google/herobrine/boardid.c A src/mainboard/google/herobrine/bootblock.c A src/mainboard/google/herobrine/chromeos.c A src/mainboard/google/herobrine/chromeos.fmd A src/mainboard/google/herobrine/devicetree.cb A src/mainboard/google/herobrine/mainboard.c A src/mainboard/google/herobrine/reset.c A src/mainboard/google/herobrine/romstage.c 17 files changed, 247 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/45206/24
Attention is currently required from: Shelley Chen. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
Patch Set 24:
(4 comments)
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/f7daccc1_074881e2 PS24, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
File src/mainboard/google/herobrine/boardid.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/5809bbd7_920071de PS24, Line 9: trailing whitespace
https://review.coreboot.org/c/coreboot/+/45206/comment/1a6a6e0a_135ff82e PS24, Line 16: trailing whitespace
https://review.coreboot.org/c/coreboot/+/45206/comment/973007d4_7867c5ad PS24, Line 23: trailing whitespace
Attention is currently required from: Ravi kumar, Shelley Chen, Paul Menzel, Julius Werner. Ravi Kumar Bokka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
Patch Set 23:
(50 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/45206/comment/ba4dcabb_ebce6f9f PS2, Line 7: herobrine :
Please remove the space before the colon.
Done
https://review.coreboot.org/c/coreboot/+/45206/comment/8d11552f_90efaa89 PS2, Line 7: herobrine : Provide initial mainboard support
Please mention the SoC in the commit message.
Done
https://review.coreboot.org/c/coreboot/+/45206/comment/3f3d43d7_723be067 PS2, Line 10: Signed-off-by: Ravi
One space is enough.
Done
Commit Message:
https://review.coreboot.org/c/coreboot/+/45206/comment/7db188a5_495b8799 PS15, Line 7: herobrine :
Please remove the space before the colon.
Done
File payloads/libpayload/Kconfig:
https://review.coreboot.org/c/coreboot/+/45206/comment/f2bcdb95_59c8c3e7 PS15, Line 262: default n
Please split out the libpayload changes into a separate commit.
This comment address with below gerrit: https://review.coreboot.org/c/coreboot/+/47527/5
File payloads/libpayload/Kconfig:
https://review.coreboot.org/c/coreboot/+/45206/comment/31048a50_4bfdc632 PS22, Line 267: config SC7280_SERIAL_CONSOLE : bool "SC7280 SOC compatible serial port driver" : depends on SERIAL_CONSOLE : default n :
Please clarify why we need to add this.
addressed below: https://review.coreboot.org/c/coreboot/+/47527/5/payloads/libpayload/Kconfig
File payloads/libpayload/configs/config.herobrine:
https://review.coreboot.org/c/coreboot/+/45206/comment/fa1f2018_fe261bee PS6, Line 5: CONFIG_LP_SC7280_SERIAL_CONSOLE=y
Is the serial driver actually going to be different? I was hoping the QUPs stayed the same between t […]
This changes addressed below gerrit: https://review.coreboot.org/c/coreboot/+/47527
File payloads/libpayload/configs/config.herobrine:
https://review.coreboot.org/c/coreboot/+/45206/comment/69526943_a01d3d93 PS22, Line 5: CONFIG_LP_SC7280_SERIAL_CONSOLE
Do we need an soc specific config for this? I see sc7180 used CONFIG_LP_QUALCOMM_QUPV3_SERIAL_CONSO […]
addressed below
https://review.coreboot.org/c/coreboot/+/47527/5/payloads/libpayload/configs...
File payloads/libpayload/drivers/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/45206/comment/c10d78f4_b43e2849 PS22, Line 45: serial/sc7280.c
are we able to reuse serial/qcom_qupv3_serial. […]
addressed this comment with below gerrit: https://review.coreboot.org/c/coreboot/+/47527/5
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/43f88634_5fb25d25 PS1, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */
'initalizing' may be misspelled - perhaps 'initializing'?
done
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/9fa2b3f3_199cf84f PS2, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */
'initalizing' may be misspelled - perhaps 'initializing'?
done
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/8c468ffc_3c36394e PS3, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */
'initalizing' may be misspelled - perhaps 'initializing'?
done
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/893489da_825f5c49 PS4, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */
'initalizing' may be misspelled - perhaps 'initializing'?
done
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/8da2d404_733048c4 PS5, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */
'initalizing' may be misspelled - perhaps 'initializing'?
done
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/e8238c12_56a0f1b9 PS6, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */
'initalizing' may be misspelled - perhaps 'initializing'?
done
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/9fcc762c_8cd27a1b PS7, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */
'initalizing' may be misspelled - perhaps 'initializing'?
done
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/bc3c1209_19c8621c PS8, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */
'initalizing' may be misspelled - perhaps 'initializing'?
done
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/e4a3694f_509f3d2d PS9, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */
'initalizing' may be misspelled - perhaps 'initializing'?
done
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/85eacc22_ff454fd6 PS10, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */
'initalizing' may be misspelled - perhaps 'initializing'?
done
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/7c2522cc_cd6e605e PS11, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */
'initalizing' may be misspelled - perhaps 'initializing'?
done
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/d98a8b86_e7a0a5ac PS12, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */
'initalizing' may be misspelled - perhaps 'initializing'?
done
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/32aa59bc_17ec8dac PS13, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */
'initalizing' may be misspelled - perhaps 'initializing'?
done
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/2cf17a31_2bd4a7d8 PS14, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */
'initalizing' may be misspelled - perhaps 'initializing'?
done
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/a2693d2a_68d8d0f7 PS15, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */
'initalizing' may be misspelled - perhaps 'initializing'?
done
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/2858eb09_7d3c8b76 PS16, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */
'initalizing' may be misspelled - perhaps 'initializing'?
done
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/6b36ccb2_4b418d56 PS17, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */
'initalizing' may be misspelled - perhaps 'initializing'?
done
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/3e3253c7_795825f3 PS18, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */
'initalizing' may be misspelled - perhaps 'initializing'?
done
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/5f2d07db_883baeaf PS19, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */
'initalizing' may be misspelled - perhaps 'initializing'?
done
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/996332be_e25bc828 PS20, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */
'initalizing' may be misspelled - perhaps 'initializing'?
done
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/09e1c6dd_28e3ddd7 PS21, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */
'initalizing' may be misspelled - perhaps 'initializing'?
done
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/c180b98e_d084e2e0 PS22, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */
'initalizing' may be misspelled - perhaps 'initializing'?
done
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/854fa903_0bcf8267 PS23, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */
'initalizing' may be misspelled - perhaps 'initializing'?
done
File src/mainboard/google/herobrine/Kconfig:
https://review.coreboot.org/c/coreboot/+/45206/comment/3ec4edfa_62111b23 PS6, Line 40: default 0xa
Please don't define these before they're known. Just don't select the EC Kconfig for now.
can i use below for disable EC default n
https://review.coreboot.org/c/coreboot/+/45206/comment/457c209e_4f4b0969 PS6, Line 50: config GBB_HWID
See the current Trogdor Kconfig, you don't need this option here anymore.
Done
File src/mainboard/google/herobrine/Kconfig:
https://review.coreboot.org/c/coreboot/+/45206/comment/4e496ab7_24d59184 PS22, Line 50: onfig GBB_HWID : string : depends on CHROMEOS : default "HEROBRINE TEST 1859" if BOARD_GOOGLE_HEROBRINE
Yes, this will get auto-generated correctly now, just leave it out here.
Done
File src/mainboard/google/herobrine/Kconfig.name:
https://review.coreboot.org/c/coreboot/+/45206/comment/e8b99671_5631acd5 PS22, Line 1:
Add: […]
i will address this comment, once discuss with internally
File src/mainboard/google/herobrine/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/45206/comment/55fdb6e4_38b5b27f PS22, Line 6: bootblock-y += reset.c
For utility functions like board_reset() we often just add it everywhere rather than worry about whe […]
Done
File src/mainboard/google/herobrine/board.h:
https://review.coreboot.org/c/coreboot/+/45206/comment/d4db269d_110d2e9f PS6, Line 8: #include <soc/gpio.h>
maybe you can remove direct 'include <soc/gpio. […]
Done
https://review.coreboot.org/c/coreboot/+/45206/comment/90f4e82d_8fa6f599 PS6, Line 21: #define GPIO_EN_PP3300_DX_EDP (CONFIG(TROGDOR_REV0) ? GPIO(106) : GPIO(30))
Please don't copy these GPIO tables, they'll be totally different for the new board.
Done
https://review.coreboot.org/c/coreboot/+/45206/comment/ee0166ce_246a208a PS6, Line 21: #define GPIO_EN_PP3300_DX_EDP (CONFIG(TROGDOR_REV0) ? GPIO(106) : GPIO(30))
Please don't copy these GPIO tables, they'll be totally different for the new board.
Done
File src/mainboard/google/herobrine/board.h:
https://review.coreboot.org/c/coreboot/+/45206/comment/5e3b99ad_0b8385d9 PS22, Line 3: #ifndef _COREBOOT_SRC_MAINBOARD_GOOGLE_TROGDOR_BOARD_H_ : #define _COREBOOT_SRC_MAINBOARD_GOOGLE_TROGDOR_BOARD_H_
herobrine
Done
https://review.coreboot.org/c/coreboot/+/45206/comment/736292a2_59e1fd0e PS22, Line 10: #define GPIO_EC_IN_RW GPIO(118) : #define GPIO_AP_EC_INT GPIO(94) : #define GPIO_AP_SUSPEND GPIO(20) : #define GPIO_WP_STATE GPIO(42) : #define GPIO_H1_AP_INT (CONFIG(TROGDOR_REV0) ? GPIO(21) : GPIO(42)) : #define GPIO_SD_CD_L GPIO(69) : #define GPIO_AMP_ENABLE GPIO(23) : : /* Display specific GPIOS */ : #define GPIO_BACKLIGHT_ENABLE GPIO(12) : #define GPIO_EDP_BRIDGE_ENABLE (CONFIG(TROGDOR_REV0) ? GPIO(14) : GPIO(104)) : #define GPIO_EN_PP3300_DX_EDP (CONFIG(TROGDOR_REV0) ? GPIO(106) : GPIO(30))
Since there are no herobrine schematics yet (I think?) none of these are known yet. […]
Done
https://review.coreboot.org/c/coreboot/+/45206/comment/628066ef_3896f170 PS22, Line 10: #define GPIO_EC_IN_RW GPIO(118) : #define GPIO_AP_EC_INT GPIO(94) : #define GPIO_AP_SUSPEND GPIO(20) : #define GPIO_WP_STATE GPIO(42) : #define GPIO_H1_AP_INT (CONFIG(TROGDOR_REV0) ? GPIO(21) : GPIO(42)) : #define GPIO_SD_CD_L GPIO(69) : #define GPIO_AMP_ENABLE GPIO(23) : : /* Display specific GPIOS */ : #define GPIO_BACKLIGHT_ENABLE GPIO(12) : #define GPIO_EDP_BRIDGE_ENABLE (CONFIG(TROGDOR_REV0) ? GPIO(14) : GPIO(104)) : #define GPIO_EN_PP3300_DX_EDP (CONFIG(TROGDOR_REV0) ? GPIO(106) : GPIO(30))
Since there are no herobrine schematics yet (I think?) none of these are known yet. […]
Done
https://review.coreboot.org/c/coreboot/+/45206/comment/4c3c34a2_48d0bc26 PS22, Line 25: _COREBOOT_SRC_MAINBOARD_GOOGLE_TROGDOR_BOARD_H_
herobrine
Done
File src/mainboard/google/herobrine/boardid.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/b701ab3c_483bd7a1 PS22, Line 9: #if 0 : /* TODO: update gpios for Herobrine */ : const gpio_t pins[] = {[2] = GPIO(31), [1] = GPIO(93), [0] = GPIO(33)}; : : if (id == UNDEFINED_STRAPPING_ID) : id = gpio_base2_value(pins, ARRAY_SIZE(pins)); : #endif : return id; : } : : uint32_t ram_code(void) : { : static uint32_t id = UNDEFINED_STRAPPING_ID; : #if 0 : /* TODO: update gpios for Herobrine */ : const gpio_t pins[] = {[2] = GPIO(13), [1] = GPIO(19), [0] = GPIO(29)}; : : if (id == UNDEFINED_STRAPPING_ID) : id = gpio_base2_value(pins, ARRAY_SIZE(pins)); : #endif : return id; : } : : uint32_t sku_id(void) : { : static uint32_t id = UNDEFINED_STRAPPING_ID; : #if 0 : /* TODO: update gpios for Herobrine */ : const gpio_t pins[] = {[2] = GPIO(20), [1] = GPIO(90), [0] = GPIO(105)}; : : if (id == UNDEFINED_STRAPPING_ID) : id = gpio_base2_value(pins, ARRAY_SIZE(pins)); : #endif : return id;
Just leave these empty instead of using #if 0
Done
https://review.coreboot.org/c/coreboot/+/45206/comment/3330d813_094a3dc8 PS22, Line 9: #if 0 : /* TODO: update gpios for Herobrine */ : const gpio_t pins[] = {[2] = GPIO(31), [1] = GPIO(93), [0] = GPIO(33)}; : : if (id == UNDEFINED_STRAPPING_ID) : id = gpio_base2_value(pins, ARRAY_SIZE(pins)); : #endif : return id; : } : : uint32_t ram_code(void) : { : static uint32_t id = UNDEFINED_STRAPPING_ID; : #if 0 : /* TODO: update gpios for Herobrine */ : const gpio_t pins[] = {[2] = GPIO(13), [1] = GPIO(19), [0] = GPIO(29)}; : : if (id == UNDEFINED_STRAPPING_ID) : id = gpio_base2_value(pins, ARRAY_SIZE(pins)); : #endif : return id; : } : : uint32_t sku_id(void) : { : static uint32_t id = UNDEFINED_STRAPPING_ID; : #if 0 : /* TODO: update gpios for Herobrine */ : const gpio_t pins[] = {[2] = GPIO(20), [1] = GPIO(90), [0] = GPIO(105)}; : : if (id == UNDEFINED_STRAPPING_ID) : id = gpio_base2_value(pins, ARRAY_SIZE(pins)); : #endif : return id;
Just leave these empty instead of using #if 0
Done
File src/mainboard/google/herobrine/chromeos.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/0490794a_c4ff5460 PS6, Line 9: #if 0
Just don't add any of this stuff if you don't have code for it yet.
Done
https://review.coreboot.org/c/coreboot/+/45206/comment/9f8f7000_d4de2c8b PS6, Line 9: #if 0
Just don't add any of this stuff if you don't have code for it yet.
Done
File src/mainboard/google/herobrine/chromeos.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/107dbdc7_96a1a02b PS22, Line 9: #if 0 : return !gpio_get(GPIO_WP_STATE); : #else : return 0; : #endif : } : : void setup_chromeos_gpios(void) : { : #if 0 : /* Example of what needs to be done here */ : gpio_input_pullup(GPIO_EC_IN_RW); : gpio_input_pullup(GPIO_AP_EC_INT); : gpio_input_pullup(GPIO_SD_CD_L); : gpio_input_irq(GPIO_H1_AP_INT, IRQ_TYPE_RISING_EDGE, GPIO_PULL_UP); : gpio_output(GPIO_AMP_ENABLE, 0); : #endif : } : : void fill_lb_gpios(struct lb_gpios *gpios) : { : #if 0 : /* Example of what needs to be done here */ : struct lb_gpio chromeos_gpios[] = { : {GPIO_EC_IN_RW.addr, ACTIVE_LOW, gpio_get(GPIO_EC_IN_RW), : "EC in RW"}, : {GPIO_AP_EC_INT.addr, ACTIVE_LOW, gpio_get(GPIO_AP_EC_INT), : "EC interrupt"}, : {GPIO_H1_AP_INT.addr, ACTIVE_LOW, gpio_get(GPIO_H1_AP_INT), : "TPM interrupt"}, : {GPIO_SD_CD_L.addr, ACTIVE_LOW, gpio_get(GPIO_SD_CD_L), : "SD card detect"}, : {GPIO_AMP_ENABLE.addr, ACTIVE_HIGH, gpio_get(GPIO_AMP_ENABLE), : "speaker enable"}, : }; : : lb_add_gpios(gpios, chromeos_gpios, ARRAY_SIZE(chromeos_gpios)); : #endif
Same here. Please don't just #if 0 blocks of code that isn't correct. […]
Done
File src/mainboard/google/herobrine/mainboard.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/d9412d7c_eb1f6574 PS6, Line 5: #include <arch/mmio.h> : #include <gpio.h> : #include <timestamp.h>
maybe not used
Done
Attention is currently required from: Ravi kumar, Shelley Chen, Paul Menzel, Ravi Kumar Bokka. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
Patch Set 24:
(3 comments)
File payloads/libpayload/configs/config.herobrine:
https://review.coreboot.org/c/coreboot/+/45206/comment/c3fa25be_d4d2751b PS6, Line 5: CONFIG_LP_SC7280_SERIAL_CONSOLE=y
This changes addressed below gerrit: […]
Well, please don't add things in one patch here only to remove it in the next patch down the road again. Rearrange the patch train so the right things are added in the right place from the start.
File src/mainboard/google/herobrine/Kconfig:
https://review.coreboot.org/c/coreboot/+/45206/comment/482d55d2_eb3aafc2 PS6, Line 40: default 0xa
can i use below for disable EC […]
Shelley, can you help figure out which QUP number the EC and TPM are connected to on Herobrine so we can put the right values in here right away? (Just tell Ravi which pins they're connected to, he should be able to figure out the QUP number from that.)
File src/mainboard/google/herobrine/Kconfig.name:
PS24: This should be using a similar USE_QC_BLOBS wrapper as is now in trogdor/Kconfig.name.
Attention is currently required from: Ravi kumar, Paul Menzel, Ravi Kumar Bokka, Julius Werner. Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
Patch Set 24:
(1 comment)
File src/mainboard/google/herobrine/Kconfig:
https://review.coreboot.org/c/coreboot/+/45206/comment/5289fa10_dbfdabbe PS6, Line 40: default 0xa
Shelley, can you help figure out which QUP number the EC and TPM are connected to on Herobrine so we […]
I'm trying to get that info from Jim right now. Instead of blocking this CL waiting on that info, Ravi, can you add a comment here DRIVER_TPM_SPI_BUS and EC_GOOGLE_CHROMEEC_SPI_BUS configs need to be updated as these values are not accurate?
Attention is currently required from: Paul Menzel, Ravi Kumar Bokka, Julius Werner. Hello build bot (Jenkins), Patrick Georgi, Martin Roth, mturney mturney,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45206
to look at the new patch set (#25).
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
herobrine: sc7280: Provide initial mainboard support
Change-Id: I428cf1a461ee63215f5683abbfed90202d1b2a88 Signed-off-by: Ravi Kumar Bokka rbokka@codeaurora.org --- M payloads/libpayload/Kconfig A payloads/libpayload/configs/config.herobrine M payloads/libpayload/drivers/Makefile.inc A payloads/libpayload/drivers/serial/sc7280.c A src/mainboard/google/herobrine/Kconfig A src/mainboard/google/herobrine/Kconfig.name A src/mainboard/google/herobrine/Makefile.inc A src/mainboard/google/herobrine/board.h A src/mainboard/google/herobrine/board_info.txt A src/mainboard/google/herobrine/boardid.c A src/mainboard/google/herobrine/bootblock.c A src/mainboard/google/herobrine/chromeos.c A src/mainboard/google/herobrine/chromeos.fmd A src/mainboard/google/herobrine/devicetree.cb A src/mainboard/google/herobrine/mainboard.c A src/mainboard/google/herobrine/reset.c A src/mainboard/google/herobrine/romstage.c 17 files changed, 246 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/45206/25
Attention is currently required from: Paul Menzel, Ravi Kumar Bokka, Julius Werner. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
Patch Set 25:
(4 comments)
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/ebaf1fb6_e16109f2 PS25, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
File src/mainboard/google/herobrine/boardid.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/7777ce31_7f2971fa PS25, Line 9: trailing whitespace
https://review.coreboot.org/c/coreboot/+/45206/comment/5573cde8_283efbb0 PS25, Line 16: trailing whitespace
https://review.coreboot.org/c/coreboot/+/45206/comment/6053e7cb_0e30bd69 PS25, Line 23: trailing whitespace
Attention is currently required from: Paul Menzel, Ravi Kumar Bokka, Julius Werner, mturney mturney. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
Patch Set 26:
(4 comments)
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/7e5cddb1_d14d9cba PS26, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
File src/mainboard/google/herobrine/boardid.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/785aa44c_25e76d89 PS26, Line 9: trailing whitespace
https://review.coreboot.org/c/coreboot/+/45206/comment/7ad71713_eabe216f PS26, Line 16: trailing whitespace
https://review.coreboot.org/c/coreboot/+/45206/comment/e45b19a7_8eef05dd PS26, Line 23: trailing whitespace
Attention is currently required from: Ravi kumar, Paul Menzel, Ravi Kumar Bokka, Julius Werner, mturney mturney. Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
Patch Set 26:
(2 comments)
File payloads/libpayload/Kconfig:
https://review.coreboot.org/c/coreboot/+/45206/comment/e354c05d_0d51690b PS15, Line 262: default n
This comment address with below gerrit: […]
I think that you should just merge https://review.coreboot.org/c/coreboot/+/47527 into this patch. Is there any reason for you to add the libpayload changes now and remove them later?
File payloads/libpayload/configs/config.herobrine:
https://review.coreboot.org/c/coreboot/+/45206/comment/78c54c3e_045b1203 PS22, Line 5: CONFIG_LP_SC7280_SERIAL_CONSOLE
addressed below […]
Can we just pull this change into this patch?
Attention is currently required from: Ravi kumar, Paul Menzel, Ravi Kumar Bokka, Julius Werner, mturney mturney. Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
Patch Set 26:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/45206/comment/1793f4e2_6dff37b1 PS15, Line 8:
- Is this code copied? […]
In addition to the description Paul is asking for can you please add:
BUG=b:182963902 TEST= (how you tested that this code was correct)
Patchset:
PS26: Also, it seems you need to get rid of the whitespace in order to get build bot to stop complaining:
File src/mainboard/google/herobrine/boardid.c:9: has lines ending with whitespace. File src/mainboard/google/herobrine/boardid.c:16: has lines ending with whitespace. File src/mainboard/google/herobrine/boardid.c:23: has lines ending with whitespace.
Attention is currently required from: Ravi kumar, Paul Menzel, Ravi Kumar Bokka, Julius Werner, mturney mturney. Hello build bot (Jenkins), Patrick Georgi, Martin Roth, mturney mturney,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45206
to look at the new patch set (#27).
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
herobrine: sc7280: Provide initial mainboard support
Change-Id: I428cf1a461ee63215f5683abbfed90202d1b2a88 Signed-off-by: Ravi Kumar Bokka rbokka@codeaurora.org --- M payloads/libpayload/Kconfig A payloads/libpayload/configs/config.herobrine M payloads/libpayload/drivers/Makefile.inc A payloads/libpayload/drivers/serial/sc7280.c A src/mainboard/google/herobrine/Kconfig A src/mainboard/google/herobrine/Kconfig.name A src/mainboard/google/herobrine/Makefile.inc A src/mainboard/google/herobrine/board.h A src/mainboard/google/herobrine/board_info.txt A src/mainboard/google/herobrine/boardid.c A src/mainboard/google/herobrine/bootblock.c A src/mainboard/google/herobrine/chromeos.c A src/mainboard/google/herobrine/chromeos.fmd A src/mainboard/google/herobrine/devicetree.cb A src/mainboard/google/herobrine/mainboard.c A src/mainboard/google/herobrine/reset.c A src/mainboard/google/herobrine/romstage.c 17 files changed, 246 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/45206/27
Attention is currently required from: Paul Menzel, Ravi Kumar Bokka, Julius Werner, mturney mturney. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
Patch Set 27:
(1 comment)
File payloads/libpayload/drivers/serial/sc7280.c:
https://review.coreboot.org/c/coreboot/+/45206/comment/346957a7_3d1ee171 PS27, Line 5: /* For simplicity sake let's rely on coreboot initalizing the UART. */ 'initalizing' may be misspelled - perhaps 'initializing'?
Attention is currently required from: Ravi kumar, Paul Menzel, Ravi Kumar Bokka, Julius Werner, mturney mturney. Hello build bot (Jenkins), Patrick Georgi, Martin Roth, mturney mturney,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45206
to look at the new patch set (#28).
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
herobrine: sc7280: Provide initial mainboard support
Change-Id: I428cf1a461ee63215f5683abbfed90202d1b2a88 Signed-off-by: Ravi Kumar Bokka rbokka@codeaurora.org --- A src/mainboard/google/herobrine/Kconfig A src/mainboard/google/herobrine/Kconfig.name A src/mainboard/google/herobrine/Makefile.inc A src/mainboard/google/herobrine/board.h A src/mainboard/google/herobrine/board_info.txt A src/mainboard/google/herobrine/boardid.c A src/mainboard/google/herobrine/bootblock.c A src/mainboard/google/herobrine/chromeos.c A src/mainboard/google/herobrine/chromeos.fmd A src/mainboard/google/herobrine/devicetree.cb A src/mainboard/google/herobrine/mainboard.c A src/mainboard/google/herobrine/reset.c A src/mainboard/google/herobrine/romstage.c 13 files changed, 226 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/45206/28
Attention is currently required from: Shelley Chen, Ravi kumar, Paul Menzel, Julius Werner, mturney mturney. Ravi Kumar Bokka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
Patch Set 28:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/45206/comment/13d7969e_ad9bf5ce PS15, Line 8:
In addition to the description Paul is asking for can you please add: […]
1. Is this code copied? yes, we have copied from trogdor(sc7180) 2. What payload was used? depthcharge 3. What works? Could you please clarify the question.
File payloads/libpayload/Kconfig:
https://review.coreboot.org/c/coreboot/+/45206/comment/7f0893d8_f9bbe13d PS15, Line 262: default n
I think that you should just merge https://review.coreboot.org/c/coreboot/+/47527 into this patch. […]
This changes has been addressed
File payloads/libpayload/configs/config.herobrine:
https://review.coreboot.org/c/coreboot/+/45206/comment/ae9b8048_68b0aa9a PS22, Line 5: CONFIG_LP_SC7280_SERIAL_CONSOLE
Can we just pull this change into this patch?
This changes has been addressed
File src/mainboard/google/herobrine/Kconfig:
https://review.coreboot.org/c/coreboot/+/45206/comment/1cf9120b_c839b9f6 PS6, Line 40: default 0xa
I'm trying to get that info from Jim right now. […]
DRIVER_TPM_SPI_BUS and EC_GOOGLE_CHROMEEC_SPI_BUS as per legacy sc7180 chipset updated these values.it may not accurate
Attention is currently required from: Shelley Chen, Paul Menzel, Julius Werner, mturney mturney. mturney mturney has uploaded a new patch set (#31) to the change originally created by Ravi kumar. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
herobrine: sc7280: Provide initial mainboard support
Change-Id: I428cf1a461ee63215f5683abbfed90202d1b2a88 Signed-off-by: Ravi Kumar Bokka rbokka@codeaurora.org --- M payloads/libpayload/Kconfig A payloads/libpayload/configs/config.herobrine M payloads/libpayload/drivers/Makefile.inc A payloads/libpayload/drivers/serial/sc7280.c A src/mainboard/google/herobrine/Kconfig A src/mainboard/google/herobrine/Kconfig.name A src/mainboard/google/herobrine/Makefile.inc A src/mainboard/google/herobrine/board.h A src/mainboard/google/herobrine/board_info.txt A src/mainboard/google/herobrine/boardid.c A src/mainboard/google/herobrine/bootblock.c A src/mainboard/google/herobrine/chromeos.c A src/mainboard/google/herobrine/chromeos.fmd A src/mainboard/google/herobrine/devicetree.cb A src/mainboard/google/herobrine/mainboard.c A src/mainboard/google/herobrine/reset.c A src/mainboard/google/herobrine/romstage.c 17 files changed, 246 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/45206/31
Attention is currently required from: Shelley Chen, Paul Menzel, Julius Werner, mturney mturney. mturney mturney has uploaded a new patch set (#32) to the change originally created by Ravi kumar. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
herobrine: sc7280: Provide initial mainboard support
Change-Id: I428cf1a461ee63215f5683abbfed90202d1b2a88 Signed-off-by: Ravi Kumar Bokka rbokka@codeaurora.org --- A src/mainboard/google/herobrine/Kconfig A src/mainboard/google/herobrine/Kconfig.name A src/mainboard/google/herobrine/Makefile.inc A src/mainboard/google/herobrine/board.h A src/mainboard/google/herobrine/board_info.txt A src/mainboard/google/herobrine/boardid.c A src/mainboard/google/herobrine/bootblock.c A src/mainboard/google/herobrine/chromeos.c A src/mainboard/google/herobrine/chromeos.fmd A src/mainboard/google/herobrine/devicetree.cb A src/mainboard/google/herobrine/mainboard.c A src/mainboard/google/herobrine/reset.c A src/mainboard/google/herobrine/romstage.c 13 files changed, 226 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/45206/32
Attention is currently required from: Ravi kumar, Paul Menzel, Ravi Kumar Bokka, Julius Werner, mturney mturney. Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
Patch Set 32:
(5 comments)
File src/mainboard/google/herobrine/Kconfig:
https://review.coreboot.org/c/coreboot/+/45206/comment/71344d88_386aa39e PS6, Line 40: default 0xa
DRIVER_TPM_SPI_BUS and EC_GOOGLE_CHROMEEC_SPI_BUS […]
Ok, let's follow Julius' advice and not define these yet then.
File src/mainboard/google/herobrine/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/45206/comment/529c95b2_103cf8f7 PS32, Line 3: bootblock-y += boardid.c I believe that Julius brought up earlier that you can use `all-y += reset.c` as a shorthand for adding it to each stage individually. I saw the comment had been resolved but I don't see the changes here.
File src/mainboard/google/herobrine/board.h:
https://review.coreboot.org/c/coreboot/+/45206/comment/efdac2bf_e5558b08 PS32, Line 3: TROGDOR HEROBRINE
https://review.coreboot.org/c/coreboot/+/45206/comment/9400722a_84271166 PS32, Line 4: TROGDOR HEROBRINE
https://review.coreboot.org/c/coreboot/+/45206/comment/c8a06ce9_68886241 PS32, Line 11: TROGDOR HEROBRINE
Attention is currently required from: Paul Menzel, Ravi Kumar Bokka, Julius Werner, mturney mturney. mturney mturney has uploaded a new patch set (#33) to the change originally created by Ravi kumar. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
herobrine: sc7280: Provide initial mainboard support
Change-Id: I428cf1a461ee63215f5683abbfed90202d1b2a88 Signed-off-by: Ravi Kumar Bokka rbokka@codeaurora.org --- A src/mainboard/google/herobrine/Kconfig A src/mainboard/google/herobrine/Kconfig.name A src/mainboard/google/herobrine/Makefile.inc A src/mainboard/google/herobrine/board.h A src/mainboard/google/herobrine/board_info.txt A src/mainboard/google/herobrine/boardid.c A src/mainboard/google/herobrine/bootblock.c A src/mainboard/google/herobrine/chromeos.c A src/mainboard/google/herobrine/chromeos.fmd A src/mainboard/google/herobrine/devicetree.cb A src/mainboard/google/herobrine/mainboard.c A src/mainboard/google/herobrine/reset.c A src/mainboard/google/herobrine/romstage.c 13 files changed, 210 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/45206/33
Attention is currently required from: Paul Menzel, Ravi Kumar Bokka, Julius Werner, mturney mturney. Hello build bot (Jenkins), Patrick Georgi, Martin Roth, mturney mturney,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45206
to look at the new patch set (#37).
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
herobrine: sc7280: Provide initial mainboard support
BUG=b:182963902 TEST= validated on qualcomm sc7280 developement board
Change-Id: I428cf1a461ee63215f5683abbfed90202d1b2a88 Signed-off-by: Ravi Kumar Bokka rbokka@codeaurora.org --- A src/mainboard/google/herobrine/Kconfig A src/mainboard/google/herobrine/Kconfig.name A src/mainboard/google/herobrine/Makefile.inc A src/mainboard/google/herobrine/board.h A src/mainboard/google/herobrine/board_info.txt A src/mainboard/google/herobrine/boardid.c A src/mainboard/google/herobrine/bootblock.c A src/mainboard/google/herobrine/chromeos.c A src/mainboard/google/herobrine/chromeos.fmd A src/mainboard/google/herobrine/devicetree.cb A src/mainboard/google/herobrine/mainboard.c A src/mainboard/google/herobrine/reset.c A src/mainboard/google/herobrine/romstage.c 13 files changed, 210 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/45206/37
Attention is currently required from: Shelley Chen, Ravi kumar, Paul Menzel, Julius Werner, mturney mturney. Ravi Kumar Bokka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
Patch Set 37:
(9 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/45206/comment/e1b1330a_2ac6f48a PS15, Line 8:
- Is this code copied? […]
Done
File payloads/libpayload/configs/config.herobrine:
https://review.coreboot.org/c/coreboot/+/45206/comment/5da9065f_c4625a0e PS6, Line 5: CONFIG_LP_SC7280_SERIAL_CONSOLE=y
Well, please don't add things in one patch here only to remove it in the next patch down the road ag […]
Done
File payloads/libpayload/drivers/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/45206/comment/44bc54c3_b38464f8 PS22, Line 45: serial/sc7280.c
addressed this comment with below gerrit: […]
Done
https://review.coreboot.org/c/coreboot/+/45206/comment/abfeb963_ee30fa9c PS22, Line 45: serial/sc7280.c
addressed this comment with below gerrit: […]
Done
File src/mainboard/google/herobrine/Kconfig:
https://review.coreboot.org/c/coreboot/+/45206/comment/8f70e4c6_ddefdce1 PS6, Line 40: default 0xa
Ok, let's follow Julius' advice and not define these yet then.
Done
File src/mainboard/google/herobrine/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/45206/comment/ec71f9d7_b1b08113 PS32, Line 3: bootblock-y += boardid.c
I believe that Julius brought up earlier that you can use `all-y += reset. […]
Done
File src/mainboard/google/herobrine/board.h:
https://review.coreboot.org/c/coreboot/+/45206/comment/ee03355f_0c802521 PS32, Line 3: TROGDOR
HEROBRINE
Done
https://review.coreboot.org/c/coreboot/+/45206/comment/70771261_bb91c9e5 PS32, Line 4: TROGDOR
HEROBRINE
Done
https://review.coreboot.org/c/coreboot/+/45206/comment/185ffe45_0837ed25 PS32, Line 11: TROGDOR
HEROBRINE
Done
Attention is currently required from: Shelley Chen, Ravi kumar, Paul Menzel, Julius Werner, mturney mturney. Hello build bot (Jenkins), Patrick Georgi, Martin Roth, mturney mturney,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45206
to look at the new patch set (#38).
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
herobrine: sc7280: Provide initial mainboard support
BUG=b:182963902 TEST= validated on qualcomm sc7280 developement board
Change-Id: I428cf1a461ee63215f5683abbfed90202d1b2a88 Signed-off-by: Ravi Kumar Bokka rbokka@codeaurora.org --- A src/mainboard/google/herobrine/Kconfig A src/mainboard/google/herobrine/Kconfig.name A src/mainboard/google/herobrine/Makefile.inc A src/mainboard/google/herobrine/board.h A src/mainboard/google/herobrine/board_info.txt A src/mainboard/google/herobrine/boardid.c A src/mainboard/google/herobrine/bootblock.c A src/mainboard/google/herobrine/chromeos.c A src/mainboard/google/herobrine/chromeos.fmd A src/mainboard/google/herobrine/devicetree.cb A src/mainboard/google/herobrine/mainboard.c A src/mainboard/google/herobrine/reset.c A src/mainboard/google/herobrine/romstage.c M src/soc/qualcomm/sc7280/qclib.c 14 files changed, 212 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/45206/38
Attention is currently required from: Shelley Chen, Paul Menzel, Julius Werner, mturney mturney. mturney mturney has uploaded a new patch set (#39) to the change originally created by Ravi kumar. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
herobrine: sc7280: Provide initial mainboard support
BUG=b:182963902 TEST=Validated on qualcomm sc7280 developement board
Change-Id: I428cf1a461ee63215f5683abbfed90202d1b2a88 Signed-off-by: Ravi Kumar Bokka rbokka@codeaurora.org --- A src/mainboard/google/herobrine/Kconfig A src/mainboard/google/herobrine/Kconfig.name A src/mainboard/google/herobrine/Makefile.inc A src/mainboard/google/herobrine/board.h A src/mainboard/google/herobrine/board_info.txt A src/mainboard/google/herobrine/boardid.c A src/mainboard/google/herobrine/bootblock.c A src/mainboard/google/herobrine/chromeos.c A src/mainboard/google/herobrine/chromeos.fmd A src/mainboard/google/herobrine/devicetree.cb A src/mainboard/google/herobrine/mainboard.c A src/mainboard/google/herobrine/reset.c A src/mainboard/google/herobrine/romstage.c 13 files changed, 218 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/45206/39
Attention is currently required from: Ravi kumar, Shelley Chen, Paul Menzel, Ravi Kumar Bokka, Julius Werner. mturney mturney has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
Patch Set 39:
(4 comments)
File src/mainboard/google/herobrine/Kconfig.name:
https://review.coreboot.org/c/coreboot/+/45206/comment/5750fb83_43ec9001 PS22, Line 1:
i will address this comment, once discuss with internally
Followed model use by trogdor, please confirm this is what you intended.
File src/mainboard/google/herobrine/board.h:
https://review.coreboot.org/c/coreboot/+/45206/comment/addc26a1_cbc064fb PS32, Line 3: TROGDOR
HEROBRINE
Ack
https://review.coreboot.org/c/coreboot/+/45206/comment/29f77836_27c13d21 PS32, Line 4: TROGDOR
HEROBRINE
Ack
https://review.coreboot.org/c/coreboot/+/45206/comment/f6f7a02f_344f8b83 PS32, Line 11: TROGDOR
HEROBRINE
Ack
Attention is currently required from: Ravi kumar, Paul Menzel, Ravi Kumar Bokka, Julius Werner, mturney mturney. Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
Patch Set 42:
(2 comments)
File src/mainboard/google/herobrine/Kconfig.name:
https://review.coreboot.org/c/coreboot/+/45206/comment/7a59286a_a95e8a7a PS22, Line 1:
Followed model use by trogdor, please confirm this is what you intended.
Done
File src/mainboard/google/herobrine/Kconfig.name:
PS24:
This should be using a similar USE_QC_BLOBS wrapper as is now in trogdor/Kconfig.name.
Done
Attention is currently required from: Paul Menzel, Ravi Kumar Bokka, Julius Werner, mturney mturney. Hello build bot (Jenkins), Patrick Georgi, Martin Roth, mturney mturney,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45206
to look at the new patch set (#45).
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
herobrine: sc7280: Provide initial mainboard support
BUG=b:182963902 TEST=Validated on qualcomm sc7280 developement board
Change-Id: I428cf1a461ee63215f5683abbfed90202d1b2a88 Signed-off-by: Ravi Kumar Bokka rbokka@codeaurora.org --- A src/mainboard/google/herobrine/Kconfig A src/mainboard/google/herobrine/Kconfig.name A src/mainboard/google/herobrine/Makefile.inc A src/mainboard/google/herobrine/board.h A src/mainboard/google/herobrine/board_info.txt A src/mainboard/google/herobrine/boardid.c A src/mainboard/google/herobrine/bootblock.c A src/mainboard/google/herobrine/chromeos.c A src/mainboard/google/herobrine/chromeos.fmd A src/mainboard/google/herobrine/devicetree.cb A src/mainboard/google/herobrine/mainboard.c A src/mainboard/google/herobrine/reset.c A src/mainboard/google/herobrine/romstage.c 13 files changed, 218 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/45206/45
Attention is currently required from: Ravi kumar, Ravi Kumar Bokka, Julius Werner, mturney mturney. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
Patch Set 45:
(2 comments)
File src/mainboard/google/herobrine/Kconfig:
https://review.coreboot.org/c/coreboot/+/45206/comment/5189401b_3c20a4da PS45, Line 1: Please remove the blank line at the top.
File src/mainboard/google/herobrine/chromeos.fmd:
https://review.coreboot.org/c/coreboot/+/45206/comment/552f4464_702738da PS45, Line 3: # TODO: update for Herobrine Where is the file copied from?
Attention is currently required from: Ravi kumar, Paul Menzel, Ravi Kumar Bokka, Julius Werner. mturney mturney has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
Patch Set 45:
(1 comment)
File src/mainboard/google/herobrine/chromeos.fmd:
https://review.coreboot.org/c/coreboot/+/45206/comment/399e098c_638dbf8b PS45, Line 3: # TODO: update for Herobrine
Where is the file copied from?
All copied files originated in trogdor directory.
Attention is currently required from: Paul Menzel, Ravi Kumar Bokka, Julius Werner. Hello build bot (Jenkins), Patrick Georgi, Martin Roth, mturney mturney,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45206
to look at the new patch set (#46).
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
herobrine: sc7280: Provide initial mainboard support
BUG=b:182963902 TEST=Validated on qualcomm sc7280 developement board
Change-Id: I428cf1a461ee63215f5683abbfed90202d1b2a88 Signed-off-by: Ravi Kumar Bokka rbokka@codeaurora.org --- A src/mainboard/google/herobrine/Kconfig A src/mainboard/google/herobrine/Kconfig.name A src/mainboard/google/herobrine/Makefile.inc A src/mainboard/google/herobrine/board.h A src/mainboard/google/herobrine/board_info.txt A src/mainboard/google/herobrine/boardid.c A src/mainboard/google/herobrine/bootblock.c A src/mainboard/google/herobrine/chromeos.c A src/mainboard/google/herobrine/chromeos.fmd A src/mainboard/google/herobrine/devicetree.cb A src/mainboard/google/herobrine/mainboard.c A src/mainboard/google/herobrine/reset.c A src/mainboard/google/herobrine/romstage.c 13 files changed, 217 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/45206/46
Attention is currently required from: Ravi kumar, Paul Menzel, Julius Werner, mturney mturney. Ravi Kumar Bokka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
Patch Set 46:
(2 comments)
File src/mainboard/google/herobrine/Kconfig:
https://review.coreboot.org/c/coreboot/+/45206/comment/28847268_12e6bf1a PS45, Line 1:
Please remove the blank line at the top.
Done
File src/mainboard/google/herobrine/chromeos.fmd:
https://review.coreboot.org/c/coreboot/+/45206/comment/dab2adec_c717de79 PS45, Line 3: # TODO: update for Herobrine
All copied files originated in trogdor directory.
Ack
Attention is currently required from: Ravi kumar, Paul Menzel, Julius Werner, mturney mturney. Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
Patch Set 46: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45206 )
Change subject: herobrine: sc7280: Provide initial mainboard support ......................................................................
herobrine: sc7280: Provide initial mainboard support
BUG=b:182963902 TEST=Validated on qualcomm sc7280 developement board
Change-Id: I428cf1a461ee63215f5683abbfed90202d1b2a88 Signed-off-by: Ravi Kumar Bokka rbokka@codeaurora.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/45206 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Shelley Chen shchen@google.com --- A src/mainboard/google/herobrine/Kconfig A src/mainboard/google/herobrine/Kconfig.name A src/mainboard/google/herobrine/Makefile.inc A src/mainboard/google/herobrine/board.h A src/mainboard/google/herobrine/board_info.txt A src/mainboard/google/herobrine/boardid.c A src/mainboard/google/herobrine/bootblock.c A src/mainboard/google/herobrine/chromeos.c A src/mainboard/google/herobrine/chromeos.fmd A src/mainboard/google/herobrine/devicetree.cb A src/mainboard/google/herobrine/mainboard.c A src/mainboard/google/herobrine/reset.c A src/mainboard/google/herobrine/romstage.c 13 files changed, 217 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Shelley Chen: Looks good to me, approved
diff --git a/src/mainboard/google/herobrine/Kconfig b/src/mainboard/google/herobrine/Kconfig new file mode 100644 index 0000000..eb1e986 --- /dev/null +++ b/src/mainboard/google/herobrine/Kconfig @@ -0,0 +1,42 @@ +config BOARD_GOOGLE_HEROBRINE_COMMON # Umbrella option to be selected by variants + def_bool n + +if BOARD_GOOGLE_HEROBRINE_COMMON + +config BOARD_SPECIFIC_OPTIONS + def_bool y + select BOARD_ROMSIZE_KB_8192 + select COMMON_CBFS_SPI_WRAPPER + select EC_GOOGLE_CHROMEEC + select EC_GOOGLE_CHROMEEC_RTC + select EC_GOOGLE_CHROMEEC_SPI + select RTC + select SOC_QUALCOMM_SC7280 + select SPI_FLASH + select SPI_FLASH_WINBOND + select MAINBOARD_HAS_CHROMEOS + +config VBOOT + select EC_GOOGLE_CHROMEEC_SWITCHES + select VBOOT_VBNV_FLASH + select GBB_FLAG_DISABLE_EC_SOFTWARE_SYNC + select VBOOT_MOCK_SECDATA + +config MAINBOARD_DIR + string + default "google/herobrine" + +config MAINBOARD_VENDOR + string + default "Google" + + +########################################################## +#### Update below when adding a new derivative board. #### +########################################################## + +config MAINBOARD_PART_NUMBER + string + default "Herobrine" if BOARD_GOOGLE_HEROBRINE + +endif # BOARD_GOOGLE_HEROBRINE_COMMON diff --git a/src/mainboard/google/herobrine/Kconfig.name b/src/mainboard/google/herobrine/Kconfig.name new file mode 100644 index 0000000..51d102a --- /dev/null +++ b/src/mainboard/google/herobrine/Kconfig.name @@ -0,0 +1,12 @@ +comment "Herobrine" + +if USE_QC_BLOBS + +config BOARD_GOOGLE_HEROBRINE + bool "-> Herobrine" + select BOARD_GOOGLE_HEROBRINE_COMMON + +endif + +comment "(Herobrine requires 'Allow QC blobs repository')" + depends on !USE_QC_BLOBS diff --git a/src/mainboard/google/herobrine/Makefile.inc b/src/mainboard/google/herobrine/Makefile.inc new file mode 100644 index 0000000..553634c --- /dev/null +++ b/src/mainboard/google/herobrine/Makefile.inc @@ -0,0 +1,11 @@ +## SPDX-License-Identifier: GPL-2.0-only + +all-y += boardid.c +all-y += chromeos.c +all-y += reset.c + +bootblock-y += bootblock.c + +romstage-y += romstage.c + +ramstage-y += mainboard.c diff --git a/src/mainboard/google/herobrine/board.h b/src/mainboard/google/herobrine/board.h new file mode 100644 index 0000000..c91a238 --- /dev/null +++ b/src/mainboard/google/herobrine/board.h @@ -0,0 +1,11 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef _COREBOOT_SRC_MAINBOARD_GOOGLE_HEROBRINE_BOARD_H_ +#define _COREBOOT_SRC_MAINBOARD_GOOGLE_HEROBRINE_BOARD_H_ + +#include <boardid.h> +#include <gpio.h> + +void setup_chromeos_gpios(void); + +#endif /* _COREBOOT_SRC_MAINBOARD_GOOGLE_HEROBRINE_BOARD_H_ */ diff --git a/src/mainboard/google/herobrine/board_info.txt b/src/mainboard/google/herobrine/board_info.txt new file mode 100644 index 0000000..a670d6e --- /dev/null +++ b/src/mainboard/google/herobrine/board_info.txt @@ -0,0 +1,6 @@ +Vendor name: Google +Board name: Herobrine Qualcomm sc7280 reference board +Category: eval +ROM protocol: SPI +ROM socketed: n +Flashrom support: y diff --git a/src/mainboard/google/herobrine/boardid.c b/src/mainboard/google/herobrine/boardid.c new file mode 100644 index 0000000..eecbeab --- /dev/null +++ b/src/mainboard/google/herobrine/boardid.c @@ -0,0 +1,25 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <boardid.h> +#include <gpio.h> + +uint32_t board_id(void) +{ + static uint32_t id = UNDEFINED_STRAPPING_ID; + + return id; +} + +uint32_t ram_code(void) +{ + static uint32_t id = UNDEFINED_STRAPPING_ID; + + return id; +} + +uint32_t sku_id(void) +{ + static uint32_t id = UNDEFINED_STRAPPING_ID; + + return id; +} diff --git a/src/mainboard/google/herobrine/bootblock.c b/src/mainboard/google/herobrine/bootblock.c new file mode 100644 index 0000000..05e53a6 --- /dev/null +++ b/src/mainboard/google/herobrine/bootblock.c @@ -0,0 +1,9 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <bootblock_common.h> +#include "board.h" + +void bootblock_mainboard_init(void) +{ + setup_chromeos_gpios(); +} diff --git a/src/mainboard/google/herobrine/chromeos.c b/src/mainboard/google/herobrine/chromeos.c new file mode 100644 index 0000000..c46bf7c --- /dev/null +++ b/src/mainboard/google/herobrine/chromeos.c @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <boot/coreboot_tables.h> +#include <bootmode.h> +#include "board.h" + +void setup_chromeos_gpios(void) +{ + +} + +void fill_lb_gpios(struct lb_gpios *gpios) +{ + +} diff --git a/src/mainboard/google/herobrine/chromeos.fmd b/src/mainboard/google/herobrine/chromeos.fmd new file mode 100644 index 0000000..a44a638 --- /dev/null +++ b/src/mainboard/google/herobrine/chromeos.fmd @@ -0,0 +1,41 @@ +## SPDX-License-Identifier: GPL-2.0-only + +# TODO: update for Herobrine +FLASH@0x0 8M { + WP_RO 4M { + RO_SECTION 0x3c4000 { + BOOTBLOCK 96K + COREBOOT(CBFS) + FMAP@0x3c0000 0x1000 + GBB 0x2f00 + RO_FRID 0x100 + } + RO_VPD(PRESERVE) 228K + RO_DDR_TRAINING(PRESERVE) 8K + RO_LIMITS_CFG(PRESERVE) 4K + } + + RW_VPD(PRESERVE) 32K + RW_NVRAM(PRESERVE) 16K + RW_DDR_TRAINING(PRESERVE) 8K + RW_LIMITS_CFG(PRESERVE) 4K + RW_ELOG(PRESERVE) 4K + RW_SHARED 4K { + SHARED_DATA + } + + RW_SECTION_A 1280K { + VBLOCK_A 8K + FW_MAIN_A(CBFS) + RW_FWID_A 256 + } + + + RW_SECTION_B 1280K { + VBLOCK_B 8K + FW_MAIN_B(CBFS) + RW_FWID_B 256 + } + + RW_LEGACY(CBFS) +} diff --git a/src/mainboard/google/herobrine/devicetree.cb b/src/mainboard/google/herobrine/devicetree.cb new file mode 100644 index 0000000..e23782f --- /dev/null +++ b/src/mainboard/google/herobrine/devicetree.cb @@ -0,0 +1,5 @@ +## SPDX-License-Identifier: GPL-2.0-only + +chip soc/qualcomm/sc7280 + device cpu_cluster 0 on end +end diff --git a/src/mainboard/google/herobrine/mainboard.c b/src/mainboard/google/herobrine/mainboard.c new file mode 100644 index 0000000..365fad4 --- /dev/null +++ b/src/mainboard/google/herobrine/mainboard.c @@ -0,0 +1,19 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <device/device.h> +#include <bootblock_common.h> + +static void mainboard_init(struct device *dev) +{ + +} + +static void mainboard_enable(struct device *dev) +{ + dev->ops->init = &mainboard_init; +} + +struct chip_operations mainboard_ops = { + .name = CONFIG_MAINBOARD_PART_NUMBER, + .enable_dev = mainboard_enable, +}; diff --git a/src/mainboard/google/herobrine/reset.c b/src/mainboard/google/herobrine/reset.c new file mode 100644 index 0000000..9b5810f --- /dev/null +++ b/src/mainboard/google/herobrine/reset.c @@ -0,0 +1,11 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <ec/google/chromeec/ec.h> +#include <reset.h> + +/* Can't do a "real" reset before the PMIC is initialized in QcLib (romstage), + but this works well enough for our purposes. */ +void do_board_reset(void) +{ + google_chromeec_reboot(0, EC_REBOOT_COLD, 0); +} diff --git a/src/mainboard/google/herobrine/romstage.c b/src/mainboard/google/herobrine/romstage.c new file mode 100644 index 0000000..8844f18 --- /dev/null +++ b/src/mainboard/google/herobrine/romstage.c @@ -0,0 +1,10 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <arch/stages.h> +#include <soc/qclib_common.h> + +void platform_romstage_main(void) +{ + /* QCLib: DDR init & train */ + qclib_load_and_run(); +}
-
build bot (Jenkins) (Code Review) -
HAOUAS Elyes (Code Review)
-
Julius Werner (Code Review)
-
mturney mturney (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Ravi kumar (Code Review)
-
Ravi Kumar Bokka (Code Review)
-
Shelley Chen (Code Review)