Change in coreboot[master]: soc/amd/picasso/southbridge: make GPP clock outputs configurable
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44889 )
Change subject: soc/amd/picasso/southbridge: make GPP clock outputs configurable ......................................................................
soc/amd/picasso/southbridge: make GPP clock outputs configurable
Make the general purpose PCIe clock outputs configurable to be either permanently enabled, permanently disabled or dynamically enabled via their corresponding external #CLK_REQx pins in the board's devicetree.
BUG=b:149970243 BRANCH=zork
Change-Id: I3f5760c0b869e8a9416ba9b57d182a88a2eb5e44 Signed-off-by: Felix Held felix-coreboot@felixheld.de --- M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/include/soc/southbridge.h M src/soc/amd/picasso/southbridge.c 3 files changed, 53 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/44889/1
diff --git a/src/soc/amd/picasso/chip.h b/src/soc/amd/picasso/chip.h index b641379..ac1a12c 100644 --- a/src/soc/amd/picasso/chip.h +++ b/src/soc/amd/picasso/chip.h @@ -154,6 +154,9 @@ USB_OC_PIN_5 = 0x5, USB_OC_NONE = 0xf, } usb_port_overcurrent_pin[USB_PORT_COUNT]; + + /* The array index is the general purpose PCIe clock output number. */ + enum gpp_clk_req_setting gpp_clk_config[GPP_CLK_OUTPUT_COUNT]; };
typedef struct soc_amd_picasso_config config_t; diff --git a/src/soc/amd/picasso/include/soc/southbridge.h b/src/soc/amd/picasso/include/soc/southbridge.h index b579213..9e5a835 100644 --- a/src/soc/amd/picasso/include/soc/southbridge.h +++ b/src/soc/amd/picasso/include/soc/southbridge.h @@ -245,6 +245,12 @@ /* IO 0xf0 NCP Error */ #define NCP_WARM_BOOT BIT(7) /* Write-once */
+enum gpp_clk_req_setting { + GPP_CLK_ON, /* GPP clock always on; default */ + GPP_CLK_REQ, /* GPP clock controlled by corresponding #CLK_REQx pin */ + GPP_CLK_OFF, /* GPP clk off */ +}; + typedef struct aoac_devs { unsigned int :7; unsigned int ic2e:1; /* 7: I2C2 */ diff --git a/src/soc/amd/picasso/southbridge.c b/src/soc/amd/picasso/southbridge.c index 2a5f822..9772e9b 100644 --- a/src/soc/amd/picasso/southbridge.c +++ b/src/soc/amd/picasso/southbridge.c @@ -307,6 +307,48 @@ write8((void *)(al2ahb_base + AL2AHB_CONTROL_HCLK_OFFSET), al2ahb_val); }
+/* configure the genral purpose PCIe clock outputs according to the devicetree settings */ +static void gpp_clk_setup(void) +{ + const struct soc_amd_picasso_config *cfg = config_of_soc(); + + /* look-up table to be able to iterate over the PCIe clock output settings */ + const uint8_t gpp_clk_shift_lut[GPP_CLK_OUTPUT_COUNT] = { + GPP_CLK0_REQ_SHIFT, + GPP_CLK1_REQ_SHIFT, + GPP_CLK2_REQ_SHIFT, + GPP_CLK3_REQ_SHIFT, + GPP_CLK4_REQ_SHIFT, + GPP_CLK5_REQ_SHIFT, + GPP_CLK6_REQ_SHIFT, + }; + + uint32_t gpp_clk_ctl = misc_read32(GPP_CLK_CNTRL); + + for (int i = 0; i < GPP_CLK_OUTPUT_COUNT; i++) { + gpp_clk_ctl &= ~GPP_CLK_REQ_MASK(gpp_clk_shift_lut[i]); + /* + * The remapping of values is done so that the default of the enum used for the + * devicetree settings is the clock being enabled, so that a missing devicetree + * configuration for this will result in an always active clock and not an + * inactive PCIe clock output. + */ + switch (cfg->gpp_clk_config[i]) { + case GPP_CLK_REQ: + gpp_clk_ctl |= GPP_CLK_REQ_EXT(gpp_clk_shift_lut[i]); + break; + case GPP_CLK_OFF: + gpp_clk_ctl |= GPP_CLK_REQ_OFF(gpp_clk_shift_lut[i]); + break; + case GPP_CLK_ON: + default: + gpp_clk_ctl |= GPP_CLK_REQ_ON(gpp_clk_shift_lut[i]); + } + } + + misc_write32(GPP_CLK_CNTRL, gpp_clk_ctl); +} + void southbridge_init(void *chip_info) { struct chipset_state *state; @@ -322,6 +364,8 @@ acpi_clear_pm_gpe_status();
al2ahb_clock_gate(); + + gpp_clk_setup(); }
void southbridge_final(void *chip_info)
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44889 )
Change subject: soc/amd/picasso/southbridge: make GPP clock outputs configurable ......................................................................
Patch Set 1:
please test this before merging. it didn't break things on mandolin, but i don't have testable pcie devices on mandolin at the moment
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44889
to look at the new patch set (#2).
Change subject: soc/amd/picasso/southbridge: make GPP clock outputs configurable ......................................................................
soc/amd/picasso/southbridge: make GPP clock outputs configurable
Make the general purpose PCIe clock outputs configurable to be either permanently enabled, permanently disabled or dynamically enabled via their corresponding external #CLK_REQx pins in the board's devicetree.
BUG=b:149970243 BRANCH=zork
Change-Id: I3f5760c0b869e8a9416ba9b57d182a88a2eb5e44 Signed-off-by: Felix Held felix-coreboot@felixheld.de --- M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/include/soc/southbridge.h M src/soc/amd/picasso/southbridge.c 3 files changed, 54 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/44889/2
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44889 )
Change subject: soc/amd/picasso/southbridge: make GPP clock outputs configurable ......................................................................
Patch Set 2: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44889 )
Change subject: soc/amd/picasso/southbridge: make GPP clock outputs configurable ......................................................................
soc/amd/picasso/southbridge: make GPP clock outputs configurable
Make the general purpose PCIe clock outputs configurable to be either permanently enabled, permanently disabled or dynamically enabled via their corresponding external #CLK_REQx pins in the board's devicetree.
BUG=b:149970243 BRANCH=zork
Change-Id: I3f5760c0b869e8a9416ba9b57d182a88a2eb5e44 Signed-off-by: Felix Held felix-coreboot@felixheld.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/44889 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Aaron Durbin adurbin@chromium.org --- M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/include/soc/southbridge.h M src/soc/amd/picasso/southbridge.c 3 files changed, 54 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved
diff --git a/src/soc/amd/picasso/chip.h b/src/soc/amd/picasso/chip.h index b641379..ac1a12c 100644 --- a/src/soc/amd/picasso/chip.h +++ b/src/soc/amd/picasso/chip.h @@ -154,6 +154,9 @@ USB_OC_PIN_5 = 0x5, USB_OC_NONE = 0xf, } usb_port_overcurrent_pin[USB_PORT_COUNT]; + + /* The array index is the general purpose PCIe clock output number. */ + enum gpp_clk_req_setting gpp_clk_config[GPP_CLK_OUTPUT_COUNT]; };
typedef struct soc_amd_picasso_config config_t; diff --git a/src/soc/amd/picasso/include/soc/southbridge.h b/src/soc/amd/picasso/include/soc/southbridge.h index b579213..168b2b2 100644 --- a/src/soc/amd/picasso/include/soc/southbridge.h +++ b/src/soc/amd/picasso/include/soc/southbridge.h @@ -245,6 +245,13 @@ /* IO 0xf0 NCP Error */ #define NCP_WARM_BOOT BIT(7) /* Write-once */
+/* this is for the devicetree setting and not the values written to the register */ +enum gpp_clk_req_setting { + GPP_CLK_ON, /* GPP clock always on; default */ + GPP_CLK_REQ, /* GPP clock controlled by corresponding #CLK_REQx pin */ + GPP_CLK_OFF, /* GPP clk off */ +}; + typedef struct aoac_devs { unsigned int :7; unsigned int ic2e:1; /* 7: I2C2 */ diff --git a/src/soc/amd/picasso/southbridge.c b/src/soc/amd/picasso/southbridge.c index 2a5f822..9772e9b 100644 --- a/src/soc/amd/picasso/southbridge.c +++ b/src/soc/amd/picasso/southbridge.c @@ -307,6 +307,48 @@ write8((void *)(al2ahb_base + AL2AHB_CONTROL_HCLK_OFFSET), al2ahb_val); }
+/* configure the genral purpose PCIe clock outputs according to the devicetree settings */ +static void gpp_clk_setup(void) +{ + const struct soc_amd_picasso_config *cfg = config_of_soc(); + + /* look-up table to be able to iterate over the PCIe clock output settings */ + const uint8_t gpp_clk_shift_lut[GPP_CLK_OUTPUT_COUNT] = { + GPP_CLK0_REQ_SHIFT, + GPP_CLK1_REQ_SHIFT, + GPP_CLK2_REQ_SHIFT, + GPP_CLK3_REQ_SHIFT, + GPP_CLK4_REQ_SHIFT, + GPP_CLK5_REQ_SHIFT, + GPP_CLK6_REQ_SHIFT, + }; + + uint32_t gpp_clk_ctl = misc_read32(GPP_CLK_CNTRL); + + for (int i = 0; i < GPP_CLK_OUTPUT_COUNT; i++) { + gpp_clk_ctl &= ~GPP_CLK_REQ_MASK(gpp_clk_shift_lut[i]); + /* + * The remapping of values is done so that the default of the enum used for the + * devicetree settings is the clock being enabled, so that a missing devicetree + * configuration for this will result in an always active clock and not an + * inactive PCIe clock output. + */ + switch (cfg->gpp_clk_config[i]) { + case GPP_CLK_REQ: + gpp_clk_ctl |= GPP_CLK_REQ_EXT(gpp_clk_shift_lut[i]); + break; + case GPP_CLK_OFF: + gpp_clk_ctl |= GPP_CLK_REQ_OFF(gpp_clk_shift_lut[i]); + break; + case GPP_CLK_ON: + default: + gpp_clk_ctl |= GPP_CLK_REQ_ON(gpp_clk_shift_lut[i]); + } + } + + misc_write32(GPP_CLK_CNTRL, gpp_clk_ctl); +} + void southbridge_init(void *chip_info) { struct chipset_state *state; @@ -322,6 +364,8 @@ acpi_clear_pm_gpe_status();
al2ahb_clock_gate(); + + gpp_clk_setup(); }
void southbridge_final(void *chip_info)
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44889 )
Change subject: soc/amd/picasso/southbridge: make GPP clock outputs configurable ......................................................................
Patch Set 3:
Automatic boot test returned (PASS/FAIL/TOTAL): 6/1/7 "QEMU x86 q35/ich9" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/17335 "QEMU x86 q35/ich9" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/17334 "QEMU x86 i440fx/piix4" (x86_64) using payload SeaBIOS : FAIL : https://lava.9esec.io/r/17333 "QEMU x86 i440fx/piix4" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/17332 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/17331 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/17337 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/17336
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Aaron Durbin (Code Review)
-
Felix Held (Code Review)
-
Patrick Georgi (Code Review)