Change in coreboot[master]: Revert "mb/clevo/cml-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/+/48328
to review the following change.
Change subject: Revert "mb/clevo/cml-u: drop duplicated configuration of UART pads" ......................................................................
Revert "mb/clevo/cml-u: drop duplicated configuration of UART pads"
This reverts commit 1a0071c7115819302c7df3fa2c07b1ca971e515d.
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: I6022935eaab748f82c6330be0729ff72f4880493 --- M src/mainboard/clevo/cml-u/Makefile.inc A src/mainboard/clevo/cml-u/bootblock.c M src/mainboard/clevo/cml-u/include/variant/gpio.h A src/mainboard/clevo/cml-u/variants/l140cu/gpio_early.c 4 files changed, 31 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/48328/1
diff --git a/src/mainboard/clevo/cml-u/Makefile.inc b/src/mainboard/clevo/cml-u/Makefile.inc index ac150c9..b69c257 100644 --- a/src/mainboard/clevo/cml-u/Makefile.inc +++ b/src/mainboard/clevo/cml-u/Makefile.inc @@ -1,5 +1,8 @@ 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)/gpio.c ramstage-y += variants/$(VARIANT_DIR)/hda_verb.c diff --git a/src/mainboard/clevo/cml-u/bootblock.c b/src/mainboard/clevo/cml-u/bootblock.c new file mode 100644 index 0000000..389a7a9 --- /dev/null +++ b/src/mainboard/clevo/cml-u/bootblock.c @@ -0,0 +1,10 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <bootblock_common.h> +#include <gpio.h> +#include <variant/gpio.h> + +void bootblock_mainboard_init(void) +{ + variant_configure_early_gpios(); +} diff --git a/src/mainboard/clevo/cml-u/include/variant/gpio.h b/src/mainboard/clevo/cml-u/include/variant/gpio.h index 4258325..95d5762 100644 --- a/src/mainboard/clevo/cml-u/include/variant/gpio.h +++ b/src/mainboard/clevo/cml-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/cml-u/variants/l140cu/gpio_early.c b/src/mainboard/clevo/cml-u/variants/l140cu/gpio_early.c new file mode 100644 index 0000000..3ea1c81 --- /dev/null +++ b/src/mainboard/clevo/cml-u/variants/l140cu/gpio_early.c @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <soc/gpio.h> +#include <variant/gpio.h> + +/* Name format: <pad name> / <net/pin name in schematics> */ +static const struct pad_config early_gpio_table[] = { + PAD_CFG_NF(GPP_C20, NONE, DEEP, NF1), /* UART2_RXD */ + PAD_CFG_NF(GPP_C21, NONE, DEEP, NF1), /* UART2_TXD */ + PAD_NC(GPP_C22, UP_20K), + PAD_NC(GPP_C23, UP_20K), +}; + +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/+/48328
to look at the new patch set (#2).
Change subject: Revert "mb/clevo/cml-u: drop duplicated configuration of UART pads" ......................................................................
Revert "mb/clevo/cml-u: drop duplicated configuration of UART pads"
This reverts commit 1a0071c7115819302c7df3fa2c07b1ca971e515d.
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: I6022935eaab748f82c6330be0729ff72f4880493 Signed-off-by: Felix Singer felixsinger@posteo.net --- M src/mainboard/clevo/cml-u/Makefile.inc A src/mainboard/clevo/cml-u/bootblock.c M src/mainboard/clevo/cml-u/include/variant/gpio.h A src/mainboard/clevo/cml-u/variants/l140cu/gpio_early.c 4 files changed, 31 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/48328/2
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48328 )
Change subject: Revert "mb/clevo/cml-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/+/48328 )
Change subject: Revert "mb/clevo/cml-u: drop duplicated configuration of UART pads" ......................................................................
Patch Set 3:
please change the topic to gpio_soc_to_boards
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48328 )
Change subject: Revert "mb/clevo/cml-u: drop duplicated configuration of UART pads" ......................................................................
Patch Set 4:
@Michael: I see the reason why you rebased my patches, but please don't rebase my patches without asking. I don't like to see my patches not building.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48328 )
Change subject: Revert "mb/clevo/cml-u: drop duplicated configuration of UART pads" ......................................................................
Patch Set 5:
Patch Set 4:
@Michael: I see the reason why you rebased my patches, but please don't rebase my patches without asking. I don't like to see my patches not building.
sorry, I pushed my testing branch -.- will rebase my change on top of that now
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48328 )
Change subject: Revert "mb/clevo/cml-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
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48328 )
Change subject: Revert "mb/clevo/cml-u: drop duplicated configuration of UART pads" ......................................................................
Patch Set 5:
same as for CB:48801 - not needed unless common uart gpio config got dropped
Eh what? You know that your linked change does something different than this one, right?
If we want to get rid of common GPIO code on soc/cannonlake, this change _is_ needed and necessary since it moves the pad configuration from SoC to mainboard. There is no way around it. This also applies to CB:48327.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48328 )
Change subject: Revert "mb/clevo/cml-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/+/48328 )
Change subject: Revert "mb/clevo/cml-u: drop duplicated configuration of UART pads" ......................................................................
Revert "mb/clevo/cml-u: drop duplicated configuration of UART pads"
This reverts commit 1a0071c7115819302c7df3fa2c07b1ca971e515d.
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: I6022935eaab748f82c6330be0729ff72f4880493 Signed-off-by: Felix Singer felixsinger@posteo.net Reviewed-on: https://review.coreboot.org/c/coreboot/+/48328 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Michael Niewöhner foss@mniewoehner.de --- M src/mainboard/clevo/cml-u/Makefile.inc A src/mainboard/clevo/cml-u/bootblock.c M src/mainboard/clevo/cml-u/include/variant/gpio.h A src/mainboard/clevo/cml-u/variants/l140cu/gpio_early.c 4 files changed, 31 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Michael Niewöhner: Looks good to me, approved
diff --git a/src/mainboard/clevo/cml-u/Makefile.inc b/src/mainboard/clevo/cml-u/Makefile.inc index ac150c9..b69c257 100644 --- a/src/mainboard/clevo/cml-u/Makefile.inc +++ b/src/mainboard/clevo/cml-u/Makefile.inc @@ -1,5 +1,8 @@ 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)/gpio.c ramstage-y += variants/$(VARIANT_DIR)/hda_verb.c diff --git a/src/mainboard/clevo/cml-u/bootblock.c b/src/mainboard/clevo/cml-u/bootblock.c new file mode 100644 index 0000000..389a7a9 --- /dev/null +++ b/src/mainboard/clevo/cml-u/bootblock.c @@ -0,0 +1,10 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <bootblock_common.h> +#include <gpio.h> +#include <variant/gpio.h> + +void bootblock_mainboard_init(void) +{ + variant_configure_early_gpios(); +} diff --git a/src/mainboard/clevo/cml-u/include/variant/gpio.h b/src/mainboard/clevo/cml-u/include/variant/gpio.h index 4258325..95d5762 100644 --- a/src/mainboard/clevo/cml-u/include/variant/gpio.h +++ b/src/mainboard/clevo/cml-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/cml-u/variants/l140cu/gpio_early.c b/src/mainboard/clevo/cml-u/variants/l140cu/gpio_early.c new file mode 100644 index 0000000..3ea1c81 --- /dev/null +++ b/src/mainboard/clevo/cml-u/variants/l140cu/gpio_early.c @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <soc/gpio.h> +#include <variant/gpio.h> + +/* Name format: <pad name> / <net/pin name in schematics> */ +static const struct pad_config early_gpio_table[] = { + PAD_CFG_NF(GPP_C20, NONE, DEEP, NF1), /* UART2_RXD */ + PAD_CFG_NF(GPP_C21, NONE, DEEP, NF1), /* UART2_TXD */ + PAD_NC(GPP_C22, UP_20K), + PAD_NC(GPP_C23, UP_20K), +}; + +void variant_configure_early_gpios(void) +{ + gpio_configure_pads(early_gpio_table, ARRAY_SIZE(early_gpio_table)); +}
-
Felix Singer (Code Review) -
Michael Niewöhner (Code Review)