Change in ...coreboot[master]: src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller
Xiang Wang has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31059
Change subject: src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller ......................................................................
src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller
Initialize the clock of Gigabit Ethernet Controller and reset the PHY.
Change-Id: I172dc518c9b48c122289bba5a65beece925410d4 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/romstage.c M src/soc/sifive/fu540/clock.c 2 files changed, 61 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/31059/1
diff --git a/src/mainboard/sifive/hifive-unleashed/romstage.c b/src/mainboard/sifive/hifive-unleashed/romstage.c index 17d1aef..4eb7d55 100644 --- a/src/mainboard/sifive/hifive-unleashed/romstage.c +++ b/src/mainboard/sifive/hifive-unleashed/romstage.c @@ -27,6 +27,7 @@ #include <symbols.h> #include <cbfs.h> #include <soc/otp.h> +#include <soc/addressmap.h>
static void update_dtb(void) { @@ -52,6 +53,30 @@ OTHER_HLS(i)->fdt = (void *)dtb_target; }
+static void nsleep(long nsec) +{ + long setp = 600; + while(*(volatile long*)&nsec > 0) + *(volatile long*)&nsec -= setp; +} + +#define GPIO_REG(n) (*(uint32_t*)(FU540_GPIO + (n))) +#define GPIO_OUTPUT_EN 0x08 +#define GPIO_OUTPUT_VAL 0x0c + +static void phy_init(void) +{ +#define PHY_NRESET 0x1000 + nsleep(2000000); + __sync_fetch_and_or(&GPIO_REG(GPIO_OUTPUT_VAL), PHY_NRESET); + __sync_fetch_and_or(&GPIO_REG(GPIO_OUTPUT_EN), PHY_NRESET); + nsleep(100); + __sync_fetch_and_and(&GPIO_REG(GPIO_OUTPUT_VAL), ~PHY_NRESET); + nsleep(100); + __sync_fetch_and_or(&GPIO_REG(GPIO_OUTPUT_VAL), PHY_NRESET); + nsleep(15000000); +} + void main(void) { console_init(); @@ -71,6 +96,7 @@ uart_init(CONFIG_UART_FOR_CONSOLE);
sdram_init(); + phy_init();
cbmem_initialize_empty();
diff --git a/src/soc/sifive/fu540/clock.c b/src/soc/sifive/fu540/clock.c index 597b206..7194c3c 100644 --- a/src/soc/sifive/fu540/clock.c +++ b/src/soc/sifive/fu540/clock.c @@ -59,6 +59,8 @@
#define PRCI_DDRPLLCFG1_MASK (1u << 31)
+#define PRCI_GEMGXLPPLCFG1_MASK (1u << 31) + #define PRCI_CORECLKSEL_CORECLKSEL 1
#define PRCI_DEVICESRESET_DDR_CTRL_RST_N(x) (((x) & 0x1) << 0) @@ -141,6 +143,15 @@ .fse = 1, };
+static const struct pll_settings gemgxlpll_settings = { + .divr = 0, + .divf = 59, + .divq = 5, + .range = 4, + .bypass = 0, + .fse = 1, +}; + static void init_coreclk(void) { // switch coreclk to input reference frequency before modifying PLL @@ -168,6 +179,19 @@ write32(&prci->ddrpllcfg1, cfg1); }
+static void init_gemgxlclk(void) +{ + u32 cfg1 = read32(&prci->gemgxlpllcfg1); + clrbits_le32(&cfg1, PRCI_GEMGXLPPLCFG1_MASK); + write32(&prci->gemgxlpllcfg1,cfg1); + + configure_pll(&prci->gemgxlpllcfg0, &gemgxlpll_settings); + + setbits_le32(&cfg1, PRCI_GEMGXLPPLCFG1_MASK); + write32(&prci->gemgxlpllcfg1,cfg1); +} + + #define FU540_UART_DEVICES 2 #define FU540_UART_REG_DIV 0x18 #define FU540_UART_DIV_VAL 4 @@ -227,6 +251,17 @@ // device? for (int i = 0; i < 256; i++) asm volatile ("nop"); + + init_gemgxlclk(); + + write32(&prci->devicesresetreg, + PRCI_DEVICESRESET_DDR_CTRL_RST_N(1) | + PRCI_DEVICESRESET_DDR_AXI_RST_N(1) | + PRCI_DEVICESRESET_DDR_AHB_RST_N(1) | + PRCI_DEVICESRESET_DDR_PHY_RST_N(1) | + PRCI_DEVICESRESET_GEMGXL_RST_N(1)); + + asm volatile ("fence"); } #endif /* ENV_ROMSTAGE */
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31059 )
Change subject: src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/#/c/31059/1/src/mainboard/sifive/hifive-unleashe... File src/mainboard/sifive/hifive-unleashed/romstage.c:
https://review.coreboot.org/#/c/31059/1/src/mainboard/sifive/hifive-unleashe... PS1, Line 59: while(*(volatile long*)&nsec > 0) space required before the open parenthesis '('
https://review.coreboot.org/#/c/31059/1/src/mainboard/sifive/hifive-unleashe... PS1, Line 63: #define GPIO_REG(n) (*(uint32_t*)(FU540_GPIO + (n))) "(foo*)" should be "(foo *)"
https://review.coreboot.org/#/c/31059/1/src/soc/sifive/fu540/clock.c File src/soc/sifive/fu540/clock.c:
https://review.coreboot.org/#/c/31059/1/src/soc/sifive/fu540/clock.c@186 PS1, Line 186: write32(&prci->gemgxlpllcfg1,cfg1); space required after that ',' (ctx:VxV)
https://review.coreboot.org/#/c/31059/1/src/soc/sifive/fu540/clock.c@191 PS1, Line 191: write32(&prci->gemgxlpllcfg1,cfg1); space required after that ',' (ctx:VxV)
Hello ron minnich, Jonathan Neuschäfer, Philipp Hug,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31059
to look at the new patch set (#2).
Change subject: src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller ......................................................................
src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller
Initialize the clock of Gigabit Ethernet Controller and reset the PHY.
Change-Id: I172dc518c9b48c122289bba5a65beece925410d4 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/romstage.c M src/soc/sifive/fu540/clock.c 2 files changed, 61 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/31059/2
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31059 )
Change subject: src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/31059/2/src/mainboard/sifive/hifive-unleashe... File src/mainboard/sifive/hifive-unleashed/romstage.c:
https://review.coreboot.org/#/c/31059/2/src/mainboard/sifive/hifive-unleashe... PS2, Line 63: #define GPIO_REG(n) (*(uint32_t*)(FU540_GPIO + (n))) "(foo*)" should be "(foo *)"
Hello ron minnich, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31059
to look at the new patch set (#3).
Change subject: src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller ......................................................................
src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller
Initialize the clock of Gigabit Ethernet Controller and reset the PHY.
Change-Id: I172dc518c9b48c122289bba5a65beece925410d4 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/romstage.c M src/soc/sifive/fu540/clock.c 2 files changed, 61 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/31059/3
Hello ron minnich, Shawn C, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31059
to look at the new patch set (#6).
Change subject: src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller ......................................................................
src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller
Initialize the clock of Gigabit Ethernet Controller and reset the PHY.
Change-Id: I172dc518c9b48c122289bba5a65beece925410d4 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/romstage.c M src/soc/sifive/fu540/clock.c 2 files changed, 61 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/31059/6
Philipp Hug has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31059 )
Change subject: src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller ......................................................................
Patch Set 6:
(1 comment)
Thanks. Please drop the nsleep function and use mdelay instead. Is this change tested on hw?
https://review.coreboot.org/#/c/31059/6/src/mainboard/sifive/hifive-unleashe... File src/mainboard/sifive/hifive-unleashed/romstage.c:
https://review.coreboot.org/#/c/31059/6/src/mainboard/sifive/hifive-unleashe... PS6, Line 56: static void nsleep(long nsec) use mdelay instead?
Philipp Hug has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31059 )
Change subject: src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller ......................................................................
Patch Set 9: Code-Review-1
Could you please replace nsleep?
Philipp Hug has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31059 )
Change subject: src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/#/c/31059/9/src/mainboard/sifive/hifive-unleashe... File src/mainboard/sifive/hifive-unleashed/romstage.c:
https://review.coreboot.org/#/c/31059/9/src/mainboard/sifive/hifive-unleashe... PS9, Line 69: #define PHY_NRESET 0x1000 please move this up
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31059 )
Change subject: src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller ......................................................................
Patch Set 9:
(2 comments)
unless this ethernet is causing a problem I don't see the reason to do it in the romstage.
https://review.coreboot.org/#/c/31059/9/src/mainboard/sifive/hifive-unleashe... File src/mainboard/sifive/hifive-unleashed/romstage.c:
https://review.coreboot.org/#/c/31059/9/src/mainboard/sifive/hifive-unleashe... PS9, Line 59: while (*(volatile long *)&nsec > 0) setp -> step I think?
https://review.coreboot.org/#/c/31059/9/src/mainboard/sifive/hifive-unleashe... PS9, Line 99: phy_init(); normally this sort of thing goes in the ramstage; is there a need to do it here?
Jonathan Neuschäfer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31059 )
Change subject: src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller ......................................................................
Patch Set 9: Code-Review+1
(1 comment)
The clock stuff looks fine, but I'm unsure whether the PHY reset is required
https://review.coreboot.org/#/c/31059/9/src/mainboard/sifive/hifive-unleashe... File src/mainboard/sifive/hifive-unleashed/romstage.c:
https://review.coreboot.org/#/c/31059/9/src/mainboard/sifive/hifive-unleashe... PS9, Line 70: nsleep(2000000); : __sync_fetch_and_or(&GPIO_REG(GPIO_OUTPUT_VAL), PHY_NRESET); : __sync_fetch_and_or(&GPIO_REG(GPIO_OUTPUT_EN), PHY_NRESET); : nsleep(100); : __sync_fetch_and_and(&GPIO_REG(GPIO_OUTPUT_VAL), ~PHY_NRESET); : nsleep(100); : __sync_fetch_and_or(&GPIO_REG(GPIO_OUTPUT_VAL), PHY_NRESET); : nsleep(15000000); : } Is it necessary to do the PHY reset in coreboot, at all? I think Linux (or other OSes) should be able to reset the PHY.
Hello ron minnich, Shawn C, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31059
to look at the new patch set (#10).
Change subject: src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller ......................................................................
src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller
Initialize the clock of Gigabit Ethernet Controller and reset the PHY.
Change-Id: I172dc518c9b48c122289bba5a65beece925410d4 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/romstage.c M src/soc/sifive/fu540/clock.c 2 files changed, 61 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/31059/10
Hello ron minnich, Shawn C, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31059
to look at the new patch set (#11).
Change subject: src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller ......................................................................
src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller
Initialize the clock of Gigabit Ethernet Controller and reset the PHY.
Change-Id: I172dc518c9b48c122289bba5a65beece925410d4 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/romstage.c M src/soc/sifive/fu540/clock.c 2 files changed, 62 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/31059/11
Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31059 )
Change subject: src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/#/c/31059/9/src/mainboard/sifive/hifive-unleashe... File src/mainboard/sifive/hifive-unleashed/romstage.c:
https://review.coreboot.org/#/c/31059/9/src/mainboard/sifive/hifive-unleashe... PS9, Line 70: nsleep(2000000); : __sync_fetch_and_or(&GPIO_REG(GPIO_OUTPUT_VAL), PHY_NRESET); : __sync_fetch_and_or(&GPIO_REG(GPIO_OUTPUT_EN), PHY_NRESET); : nsleep(100); : __sync_fetch_and_and(&GPIO_REG(GPIO_OUTPUT_VAL), ~PHY_NRESET); : nsleep(100); : __sync_fetch_and_or(&GPIO_REG(GPIO_OUTPUT_VAL), PHY_NRESET); : nsleep(15000000); : }
Is it necessary to do the PHY reset in coreboot, at all? […]
This is necessary, if you remove the code sifive provides linux can not identify the network card
Philipp Hug has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31059 )
Change subject: src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/#/c/31059/9/src/mainboard/sifive/hifive-unleashe... File src/mainboard/sifive/hifive-unleashed/romstage.c:
https://review.coreboot.org/#/c/31059/9/src/mainboard/sifive/hifive-unleashe... PS9, Line 70: nsleep(2000000); : __sync_fetch_and_or(&GPIO_REG(GPIO_OUTPUT_VAL), PHY_NRESET); : __sync_fetch_and_or(&GPIO_REG(GPIO_OUTPUT_EN), PHY_NRESET); : nsleep(100); : __sync_fetch_and_and(&GPIO_REG(GPIO_OUTPUT_VAL), ~PHY_NRESET); : nsleep(100); : __sync_fetch_and_or(&GPIO_REG(GPIO_OUTPUT_VAL), PHY_NRESET); : nsleep(15000000); : }
This is necessary, if you remove the code sifive provides linux can not identify the network card
It "should" not be necessary anymore. It has been fixed in the linux kernel recently.
Philipp Hug has uploaded a new patch set (#12) to the change originally created by Xiang Wang. ( https://review.coreboot.org/c/coreboot/+/31059 )
Change subject: src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller ......................................................................
src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller
Initialize the clock of the Gigabit Ethernet Controller.
Change-Id: I172dc518c9b48c122289bba5a65beece925410d4 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/romstage.c 1 file changed, 27 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/31059/12
Philipp Hug has uploaded a new patch set (#13) to the change originally created by Xiang Wang. ( https://review.coreboot.org/c/coreboot/+/31059 )
Change subject: src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller ......................................................................
src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller
Initialize the clock of the Gigabit Ethernet Controller.
Change-Id: I172dc518c9b48c122289bba5a65beece925410d4 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/soc/sifive/fu540/clock.c 1 file changed, 35 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/31059/13
Philipp Hug has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31059 )
Change subject: src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller ......................................................................
Patch Set 13: Code-Review+1
I updated the change and dropped the PHY init.
Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31059 )
Change subject: src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller ......................................................................
Patch Set 13: Code-Review+1
Philipp Hug has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31059 )
Change subject: src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller ......................................................................
Patch Set 13: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31059 )
Change subject: src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller ......................................................................
src/mb/sifive/hifive-unleashed: initialize Gigabit Ethernet Controller
Initialize the clock of the Gigabit Ethernet Controller.
Change-Id: I172dc518c9b48c122289bba5a65beece925410d4 Signed-off-by: Xiang Wang wxjstz@126.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/31059 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Philipp Hug philipp@hug.cx --- M src/soc/sifive/fu540/clock.c 1 file changed, 35 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Xiang Wang: Looks good to me, but someone else must approve Philipp Hug: Looks good to me, approved
diff --git a/src/soc/sifive/fu540/clock.c b/src/soc/sifive/fu540/clock.c index 4b52c56..60a8a13 100644 --- a/src/soc/sifive/fu540/clock.c +++ b/src/soc/sifive/fu540/clock.c @@ -59,6 +59,8 @@
#define PRCI_DDRPLLCFG1_MASK (1u << 31)
+#define PRCI_GEMGXLPPLCFG1_MASK (1u << 31) + #define PRCI_CORECLKSEL_CORECLKSEL 1
#define PRCI_DEVICESRESET_DDR_CTRL_RST_N(x) (((x) & 0x1) << 0) @@ -141,6 +143,15 @@ .fse = 1, };
+static const struct pll_settings gemgxlpll_settings = { + .divr = 0, + .divf = 59, + .divq = 5, + .range = 4, + .bypass = 0, + .fse = 1, +}; + static void init_coreclk(void) { // switch coreclk to input reference frequency before modifying PLL @@ -168,6 +179,19 @@ write32(&prci->ddrpllcfg1, cfg1); }
+static void init_gemgxlclk(void) +{ + u32 cfg1 = read32(&prci->gemgxlpllcfg1); + clrbits_le32(&cfg1, PRCI_GEMGXLPPLCFG1_MASK); + write32(&prci->gemgxlpllcfg1, cfg1); + + configure_pll(&prci->gemgxlpllcfg0, &gemgxlpll_settings); + + setbits_le32(&cfg1, PRCI_GEMGXLPPLCFG1_MASK); + write32(&prci->gemgxlpllcfg1, cfg1); +} + + #define FU540_UART_DEVICES 2 #define FU540_UART_REG_DIV 0x18 #define FU540_UART_DIV_VAL 4 @@ -227,6 +251,17 @@ // device? for (int i = 0; i < 256; i++) asm volatile ("nop"); + + init_gemgxlclk(); + + write32(&prci->devicesresetreg, + PRCI_DEVICESRESET_DDR_CTRL_RST_N(1) | + PRCI_DEVICESRESET_DDR_AXI_RST_N(1) | + PRCI_DEVICESRESET_DDR_AHB_RST_N(1) | + PRCI_DEVICESRESET_DDR_PHY_RST_N(1) | + PRCI_DEVICESRESET_GEMGXL_RST_N(1)); + + asm volatile ("fence"); } #endif /* ENV_ROMSTAGE */
-
build bot (Jenkins) (Code Review) -
Jonathan Neuschäfer (Code Review)
-
Patrick Georgi (Code Review)
-
Philipp Hug (Code Review)
-
ron minnich (Code Review)
-
Xiang Wang (Code Review)