Change in coreboot[master]: Revert "mb/clevo/kbl-u: drop duplicated configuration of UART pads"
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Michael Niewöhner,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/48327
to review the following change.
Change subject: Revert "mb/clevo/kbl-u: drop duplicated configuration of UART pads" ......................................................................
Revert "mb/clevo/kbl-u: drop duplicated configuration of UART pads"
This reverts commit ccceb2250eeb820fccfb62d1f3ab407582d2e79f.
Reason for revert:
UART pad configuration should not be done in common code, since it could cause short circuits if the user configures a wrong UART index.
Change-Id: Idc268debc60a027ed2f5a76e0de8ea2d1cde0fc4 --- M src/mainboard/clevo/kbl-u/Makefile.inc A src/mainboard/clevo/kbl-u/bootblock.c M src/mainboard/clevo/kbl-u/include/variant/gpio.h A src/mainboard/clevo/kbl-u/variants/n13xwu/gpio_early.c 4 files changed, 30 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/48327/1
diff --git a/src/mainboard/clevo/kbl-u/Makefile.inc b/src/mainboard/clevo/kbl-u/Makefile.inc index aeb5844..b424d4d 100644 --- a/src/mainboard/clevo/kbl-u/Makefile.inc +++ b/src/mainboard/clevo/kbl-u/Makefile.inc @@ -2,6 +2,9 @@
CPPFLAGS_common += -I$(src)/mainboard/$(MAINBOARDDIR)/include
+bootblock-y += bootblock.c +bootblock-y += variants/$(VARIANT_DIR)/gpio_early.c + ramstage-y += ramstage.c ramstage-y += variants/$(VARIANT_DIR)/hda_verb.c ramstage-y += variants/$(VARIANT_DIR)/gpio.c diff --git a/src/mainboard/clevo/kbl-u/bootblock.c b/src/mainboard/clevo/kbl-u/bootblock.c new file mode 100644 index 0000000..b351fbd --- /dev/null +++ b/src/mainboard/clevo/kbl-u/bootblock.c @@ -0,0 +1,9 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <bootblock_common.h> +#include <variant/gpio.h> + +void bootblock_mainboard_early_init(void) +{ + variant_configure_early_gpios(); +} diff --git a/src/mainboard/clevo/kbl-u/include/variant/gpio.h b/src/mainboard/clevo/kbl-u/include/variant/gpio.h index 4258325..95d5762 100644 --- a/src/mainboard/clevo/kbl-u/include/variant/gpio.h +++ b/src/mainboard/clevo/kbl-u/include/variant/gpio.h @@ -3,6 +3,7 @@ #ifndef VARIANT_GPIO_H #define VARIANT_GPIO_H
+void variant_configure_early_gpios(void); void variant_configure_gpios(void);
#endif diff --git a/src/mainboard/clevo/kbl-u/variants/n13xwu/gpio_early.c b/src/mainboard/clevo/kbl-u/variants/n13xwu/gpio_early.c new file mode 100644 index 0000000..dc55aae --- /dev/null +++ b/src/mainboard/clevo/kbl-u/variants/n13xwu/gpio_early.c @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <soc/gpio.h> +#include <variant/gpio.h> + +static const struct pad_config early_gpio_table[] = { + // UART2_RXD + _PAD_CFG_STRUCT(GPP_C20, 0x44000702, 0x0), + + // UART2_TXD + _PAD_CFG_STRUCT(GPP_C21, 0x44000700, 0x0) +}; + +void variant_configure_early_gpios(void) +{ + gpio_configure_pads(early_gpio_table, ARRAY_SIZE(early_gpio_table)); +}
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Michael Niewöhner,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48327
to look at the new patch set (#2).
Change subject: Revert "mb/clevo/kbl-u: drop duplicated configuration of UART pads" ......................................................................
Revert "mb/clevo/kbl-u: drop duplicated configuration of UART pads"
This reverts commit ccceb2250eeb820fccfb62d1f3ab407582d2e79f.
Reason for revert: UART pad configuration should not be done in common code, since it could cause short circuits if the user configures a wrong UART index.
Change-Id: Idc268debc60a027ed2f5a76e0de8ea2d1cde0fc4 Signed-off-by: Felix Singer felixsinger@posteo.net --- M src/mainboard/clevo/kbl-u/Makefile.inc A src/mainboard/clevo/kbl-u/bootblock.c M src/mainboard/clevo/kbl-u/include/variant/gpio.h A src/mainboard/clevo/kbl-u/variants/n13xwu/gpio_early.c 4 files changed, 30 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/48327/2
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48327 )
Change subject: Revert "mb/clevo/kbl-u: drop duplicated configuration of UART pads" ......................................................................
Patch Set 2: Code-Review+2
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48327 )
Change subject: Revert "mb/clevo/kbl-u: drop duplicated configuration of UART pads" ......................................................................
Patch Set 3:
please change the topic to gpio_soc_to_boards
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48327 )
Change subject: Revert "mb/clevo/kbl-u: drop duplicated configuration of UART pads" ......................................................................
Patch Set 5: Code-Review+1
same as for CB:48801 - not needed unless common uart gpio config got dropped
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48327 )
Change subject: Revert "mb/clevo/kbl-u: drop duplicated configuration of UART pads" ......................................................................
Patch Set 5: Code-Review+2
as discussed
Michael Niewöhner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48327 )
Change subject: Revert "mb/clevo/kbl-u: drop duplicated configuration of UART pads" ......................................................................
Revert "mb/clevo/kbl-u: drop duplicated configuration of UART pads"
This reverts commit ccceb2250eeb820fccfb62d1f3ab407582d2e79f.
Reason for revert: UART pad configuration should not be done in common code, since it could cause short circuits if the user configures a wrong UART index.
Change-Id: Idc268debc60a027ed2f5a76e0de8ea2d1cde0fc4 Signed-off-by: Felix Singer felixsinger@posteo.net Reviewed-on: https://review.coreboot.org/c/coreboot/+/48327 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Michael Niewöhner foss@mniewoehner.de --- M src/mainboard/clevo/kbl-u/Makefile.inc A src/mainboard/clevo/kbl-u/bootblock.c M src/mainboard/clevo/kbl-u/include/variant/gpio.h A src/mainboard/clevo/kbl-u/variants/n13xwu/gpio_early.c 4 files changed, 30 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Michael Niewöhner: Looks good to me, approved
diff --git a/src/mainboard/clevo/kbl-u/Makefile.inc b/src/mainboard/clevo/kbl-u/Makefile.inc index aeb5844..b424d4d 100644 --- a/src/mainboard/clevo/kbl-u/Makefile.inc +++ b/src/mainboard/clevo/kbl-u/Makefile.inc @@ -2,6 +2,9 @@
CPPFLAGS_common += -I$(src)/mainboard/$(MAINBOARDDIR)/include
+bootblock-y += bootblock.c +bootblock-y += variants/$(VARIANT_DIR)/gpio_early.c + ramstage-y += ramstage.c ramstage-y += variants/$(VARIANT_DIR)/hda_verb.c ramstage-y += variants/$(VARIANT_DIR)/gpio.c diff --git a/src/mainboard/clevo/kbl-u/bootblock.c b/src/mainboard/clevo/kbl-u/bootblock.c new file mode 100644 index 0000000..b351fbd --- /dev/null +++ b/src/mainboard/clevo/kbl-u/bootblock.c @@ -0,0 +1,9 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <bootblock_common.h> +#include <variant/gpio.h> + +void bootblock_mainboard_early_init(void) +{ + variant_configure_early_gpios(); +} diff --git a/src/mainboard/clevo/kbl-u/include/variant/gpio.h b/src/mainboard/clevo/kbl-u/include/variant/gpio.h index 4258325..95d5762 100644 --- a/src/mainboard/clevo/kbl-u/include/variant/gpio.h +++ b/src/mainboard/clevo/kbl-u/include/variant/gpio.h @@ -3,6 +3,7 @@ #ifndef VARIANT_GPIO_H #define VARIANT_GPIO_H
+void variant_configure_early_gpios(void); void variant_configure_gpios(void);
#endif diff --git a/src/mainboard/clevo/kbl-u/variants/n13xwu/gpio_early.c b/src/mainboard/clevo/kbl-u/variants/n13xwu/gpio_early.c new file mode 100644 index 0000000..dc55aae --- /dev/null +++ b/src/mainboard/clevo/kbl-u/variants/n13xwu/gpio_early.c @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <soc/gpio.h> +#include <variant/gpio.h> + +static const struct pad_config early_gpio_table[] = { + // UART2_RXD + _PAD_CFG_STRUCT(GPP_C20, 0x44000702, 0x0), + + // UART2_TXD + _PAD_CFG_STRUCT(GPP_C21, 0x44000700, 0x0) +}; + +void variant_configure_early_gpios(void) +{ + gpio_configure_pads(early_gpio_table, ARRAY_SIZE(early_gpio_table)); +}
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48327 )
Change subject: Revert "mb/clevo/kbl-u: drop duplicated configuration of UART pads" ......................................................................
Patch Set 6:
Landed early because I didn't check the clock....
-
Felix Singer (Code Review) -
Michael Niewöhner (Code Review)