Xiang Wang has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30466
Change subject: fu540: add code to initialize flash ......................................................................
fu540: add code to initialize flash
SiFive's ZSBL has initialized flash, but only 16MB of space is available. I fixed it to use all 32MB.
Change-Id: I8cd803369da5526eff90400c15b91bbf6b477c69 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/Makefile.inc A src/mainboard/sifive/hifive-unleashed/bootblock.c A src/mainboard/sifive/hifive-unleashed/flash.c M src/mainboard/sifive/hifive-unleashed/romstage.c M src/soc/sifive/fu540/Makefile.inc A src/soc/sifive/fu540/include/soc/spi.h A src/soc/sifive/fu540/include/soc/spi_flash.h A src/soc/sifive/fu540/spi.c 8 files changed, 506 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/30466/1
diff --git a/src/mainboard/sifive/hifive-unleashed/Makefile.inc b/src/mainboard/sifive/hifive-unleashed/Makefile.inc index 27ddcba..2dac283 100644 --- a/src/mainboard/sifive/hifive-unleashed/Makefile.inc +++ b/src/mainboard/sifive/hifive-unleashed/Makefile.inc @@ -11,6 +11,10 @@ # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # GNU General Public License for more details.
+bootblock-y += flash.c +bootblock-y += bootblock.c + +romstage-y += flash.c romstage-y += romstage.c
bootblock-y += memlayout.ld diff --git a/src/mainboard/sifive/hifive-unleashed/bootblock.c b/src/mainboard/sifive/hifive-unleashed/bootblock.c new file mode 100644 index 0000000..777b2aa --- /dev/null +++ b/src/mainboard/sifive/hifive-unleashed/bootblock.c @@ -0,0 +1,27 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2018 HardenedLinux + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <soc/addressmap.h> +#include <soc/spi.h> +#include <soc/spi_flash.h> +#include <soc/clock.h> + +extern void flash_init(void); +extern void bootblock_mainboard_init(void); + +void bootblock_mainboard_init(void) +{ + flash_init(); +} diff --git a/src/mainboard/sifive/hifive-unleashed/flash.c b/src/mainboard/sifive/hifive-unleashed/flash.c new file mode 100644 index 0000000..a29ac24 --- /dev/null +++ b/src/mainboard/sifive/hifive-unleashed/flash.c @@ -0,0 +1,42 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2018 HardenedLinux + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <soc/addressmap.h> +#include <soc/spi.h> +#include <soc/spi_flash.h> +#include <soc/clock.h> + +extern void flash_init(void); + +const static spi_reg_ffmt ffmt= { + .cmd_en = 1, + .addr_len = 4, + .pad_cnt = 8, + .command_proto = SPI_PROTO_S, + .addr_proto = SPI_PROTO_S, + .data_proto = SPI_PROTO_Q, + .command_code = 0x6c +}; + + +void flash_init(void) +{ + initialize_spi_flash_mmap_quad( + (spi_ctrl*)FU540_QSPI0, + clock_get_tlclk_khz(), + ffmt.raw_bits, + 0x66, + 0x99); +} diff --git a/src/mainboard/sifive/hifive-unleashed/romstage.c b/src/mainboard/sifive/hifive-unleashed/romstage.c index 8277141..1675aca 100644 --- a/src/mainboard/sifive/hifive-unleashed/romstage.c +++ b/src/mainboard/sifive/hifive-unleashed/romstage.c @@ -21,6 +21,8 @@ #include <soc/clock.h> #include <soc/sdram.h>
+extern void flash_init(void); + void main(void) { console_init(); @@ -39,6 +41,7 @@ if (IS_ENABLED(CONFIG_CONSOLE_SERIAL)) uart_init(CONFIG_UART_FOR_CONSOLE);
+ flash_init(); sdram_init();
cbmem_initialize_empty(); diff --git a/src/soc/sifive/fu540/Makefile.inc b/src/soc/sifive/fu540/Makefile.inc index 4f62f3e..83c7b1c 100644 --- a/src/soc/sifive/fu540/Makefile.inc +++ b/src/soc/sifive/fu540/Makefile.inc @@ -18,6 +18,7 @@ bootblock-y += media.c bootblock-y += bootblock.c bootblock-y += clock.c +bootblock-y += spi.c
romstage-y += uart.c romstage-y += clint.c @@ -26,6 +27,7 @@ romstage-y += cbmem.c romstage-y += otp.c romstage-y += clock.c +romstage-y += spi.c
ramstage-y += uart.c ramstage-y += clint.c @@ -34,6 +36,7 @@ ramstage-y += cbmem.c ramstage-y += otp.c ramstage-y += clock.c +ramstage-y += spi.c
CPPFLAGS_common += -Isrc/soc/sifive/fu540/include
diff --git a/src/soc/sifive/fu540/include/soc/spi.h b/src/soc/sifive/fu540/include/soc/spi.h new file mode 100644 index 0000000..c40ae7c --- /dev/null +++ b/src/soc/sifive/fu540/include/soc/spi.h @@ -0,0 +1,266 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2018 SiFive, Inc + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ +#ifndef __SOC_SIFIVE_HIFIVE_U_SPI_H__ +#define __SOC_SIFIVE_HIFIVE_U_SPI_H__ + +#include <stdint.h> + +#define _ASSERT_SIZEOF(type, size) _Static_assert(sizeof(type) == (size), #type " must be " #size " bytes wide") + +typedef union +{ + struct + { + uint32_t pha : 1; + uint32_t pol : 1; + uint32_t reserved : 30; + }; + uint32_t raw_bits; +} spi_reg_sckmode; +_ASSERT_SIZEOF(spi_reg_sckmode, 4); + + +typedef union +{ + struct + { + uint32_t mode : 2; + uint32_t reserved : 30; + }; + uint32_t raw_bits; +} spi_reg_csmode; +_ASSERT_SIZEOF(spi_reg_csmode, 4); + + +typedef union +{ + struct + { + uint32_t cssck : 8; + uint32_t reserved0 : 8; + uint32_t sckcs : 8; + uint32_t reserved1 : 8; + }; + uint32_t raw_bits; +} spi_reg_delay0; +_ASSERT_SIZEOF(spi_reg_delay0, 4); + + +typedef union +{ + struct + { + uint32_t intercs : 8; + uint32_t reserved0 : 8; + uint32_t interxfr : 8; + uint32_t reserved1 : 8; + }; + uint32_t raw_bits; +} spi_reg_delay1; +_ASSERT_SIZEOF(spi_reg_delay1, 4); + + +typedef union +{ + struct + { + uint32_t proto : 2; + uint32_t endian : 1; + uint32_t dir : 1; + uint32_t reserved0 : 12; + uint32_t len : 4; + uint32_t reserved1 : 12; + }; + uint32_t raw_bits; +} spi_reg_fmt; +_ASSERT_SIZEOF(spi_reg_fmt, 4); + + +typedef union +{ + struct + { + uint32_t data : 8; + uint32_t reserved : 23; + uint32_t full : 1; + }; + uint32_t raw_bits; +} spi_reg_txdata; +_ASSERT_SIZEOF(spi_reg_txdata, 4); + + +typedef union +{ + struct + { + uint32_t data : 8; + uint32_t reserved : 23; + uint32_t empty : 1; + }; + uint32_t raw_bits; +} spi_reg_rxdata; +_ASSERT_SIZEOF(spi_reg_rxdata, 4); + + +typedef union +{ + struct + { + uint32_t txmark : 3; + uint32_t reserved : 29; + }; + uint32_t raw_bits; +} spi_reg_txmark; +_ASSERT_SIZEOF(spi_reg_txmark, 4); + + +typedef union +{ + struct + { + uint32_t rxmark : 3; + uint32_t reserved : 29; + }; + uint32_t raw_bits; +} spi_reg_rxmark; +_ASSERT_SIZEOF(spi_reg_rxmark, 4); + + +typedef union +{ + struct + { + uint32_t en : 1; + uint32_t reserved : 31; + }; + uint32_t raw_bits; +} spi_reg_fctrl; +_ASSERT_SIZEOF(spi_reg_fctrl, 4); + + +typedef union +{ + struct + { + uint32_t cmd_en : 1; + uint32_t addr_len : 3; + uint32_t pad_cnt : 4; + uint32_t command_proto : 2; + uint32_t addr_proto : 2; + uint32_t data_proto : 2; + uint32_t reserved : 2; + uint32_t command_code : 8; + uint32_t pad_code : 8; + }; + uint32_t raw_bits; +} spi_reg_ffmt; +_ASSERT_SIZEOF(spi_reg_ffmt, 4); + + +typedef union +{ + struct + { + uint32_t txwm : 1; + uint32_t rxwm : 1; + uint32_t reserved : 30; + }; + uint32_t raw_bits; +} spi_reg_ie; +typedef spi_reg_ie spi_reg_ip; +_ASSERT_SIZEOF(spi_reg_ie, 4); +_ASSERT_SIZEOF(spi_reg_ip, 4); + +#undef _ASSERT_SIZEOF + + +/** + * SPI control register memory map. + * + * All functions take a pointer to a SPI device's control registers. + */ +typedef volatile struct +{ + uint32_t sckdiv; + spi_reg_sckmode sckmode; + uint32_t reserved08; + uint32_t reserved0c; + + uint32_t csid; + uint32_t csdef; + spi_reg_csmode csmode; + uint32_t reserved1c; + + uint32_t reserved20; + uint32_t reserved24; + spi_reg_delay0 delay0; + spi_reg_delay1 delay1; + + uint32_t reserved30; + uint32_t reserved34; + uint32_t reserved38; + uint32_t reserved3c; + + spi_reg_fmt fmt; + uint32_t reserved44; + spi_reg_txdata txdata; + spi_reg_rxdata rxdata; + + spi_reg_txmark txmark; + spi_reg_rxmark rxmark; + uint32_t reserved58; + uint32_t reserved5c; + + spi_reg_fctrl fctrl; + spi_reg_ffmt ffmt; + uint32_t reserved68; + uint32_t reserved6c; + + spi_reg_ie ie; + spi_reg_ip ip; +} spi_ctrl; + + +void spi_tx(spi_ctrl* spictrl, uint8_t in); +uint8_t spi_rx(spi_ctrl* spictrl); +uint8_t spi_txrx(spi_ctrl* spictrl, uint8_t in); + +// Inlining header functions in C +// https://stackoverflow.com/a/23699777/7433423 + +/** + * Get smallest clock divisor that divides input_khz to a quotient less than or + * equal to max_target_khz; + */ +static inline unsigned int spi_min_clk_divisor(unsigned int input_khz, unsigned int max_target_khz) +{ + // f_sck = f_in / (2 * (div + 1)) => div = (f_in / (2*f_sck)) - 1 + // + // The nearest integer solution for div requires rounding up as to not exceed + // max_target_khz. + // + // div = ceil(f_in / (2*f_sck)) - 1 + // = floor((f_in - 1 + 2*f_sck) / (2*f_sck)) - 1 + // + // This should not overflow as long as (f_in - 1 + 2*f_sck) does not exceed + // 2^32 - 1, which is unlikely since we represent frequencies in kHz. + unsigned int quotient = (input_khz + 2 * max_target_khz - 1) / (2 * max_target_khz); + // Avoid underflow + if (quotient == 0) + return 0; + return quotient - 1; +} + +#endif /* __SOC_SIFIVE_HIFIVE_U_SPI_H__ */ diff --git a/src/soc/sifive/fu540/include/soc/spi_flash.h b/src/soc/sifive/fu540/include/soc/spi_flash.h new file mode 100644 index 0000000..9926b82 --- /dev/null +++ b/src/soc/sifive/fu540/include/soc/spi_flash.h @@ -0,0 +1,103 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2018 SiFive, Inc + * Copyright (C) 2018 HardenedLinux + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef __SOC_SIFIVE_HIFIVE_U_SPI_FLASH_H__ +#define __SOC_SIFIVE_HIFIVE_U_SPI_FLASH_H__ + +#include <soc/spi.h> +#include <stdint.h> + +#define SPI_PROTO_S 0 +#define SPI_PROTO_D 1 +#define SPI_PROTO_Q 2 + +/** + * Set up SPI for direct, non-memory-mapped access. + */ + +static inline int initialize_spi_flash_direct( + spi_ctrl* spictrl, + unsigned int spi_clk_input_khz, + unsigned int command_enable, + unsigned int command_reset) +{ + // Max desired SPI clock is 10MHz + spictrl->sckdiv = spi_min_clk_divisor(spi_clk_input_khz, 10000); + + spictrl->fctrl.en = 0; + + spi_txrx(spictrl, command_enable); + spi_txrx(spictrl, command_reset); + + return 0; +} + +static inline int _initialize_spi_flash_mmap( + spi_ctrl* spictrl, + unsigned int spi_clk_input_khz, + uint32_t ffmt_rawbits, + unsigned int command_enable, + unsigned int command_reset) +{ + // Max desired SPI clock is 10MHz + spictrl->sckdiv = spi_min_clk_divisor(spi_clk_input_khz, 10000); + + spictrl->fctrl.en = 0; + + spi_txrx(spictrl, command_enable); + spi_txrx(spictrl, command_reset); + + spictrl->ffmt.raw_bits = ffmt_rawbits; + + spictrl->fctrl.en = 1; + __asm__ __volatile__ ("fence io, io"); + return 0; +} + + +static inline int initialize_spi_flash_mmap_single( + spi_ctrl* spictrl, + unsigned int spi_clk_input_khz, + uint32_t ffmt_rawbits, + unsigned int command_enable, + unsigned int command_reset) +{ + return _initialize_spi_flash_mmap( + spictrl, + spi_clk_input_khz, + ffmt_rawbits, + command_enable, + command_reset); +} + + +static inline int initialize_spi_flash_mmap_quad( + spi_ctrl* spictrl, + unsigned int spi_clk_input_khz, + uint32_t ffmt_rawbits, + unsigned int command_enable, + unsigned int command_reset) +{ + return _initialize_spi_flash_mmap( + spictrl, + spi_clk_input_khz, + ffmt_rawbits, + command_enable, + command_reset); +} + +#endif /* __SOC_SIFIVE_HIFIVE_U_SPI_FLASH_H__ */ + diff --git a/src/soc/sifive/fu540/spi.c b/src/soc/sifive/fu540/spi.c new file mode 100644 index 0000000..01a58dd --- /dev/null +++ b/src/soc/sifive/fu540/spi.c @@ -0,0 +1,58 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2018 SiFive, Inc + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <soc/spi.h> + +/** + * Wait until SPI is ready for transmission and transmit byte. + */ +void spi_tx(spi_ctrl* spictrl, uint8_t in) +{ +#if __riscv_atomic + int32_t r; + do { + asm volatile ( + "amoor.w %0, %2, %1\n" + : "=r" (r), "+A" (spictrl->txdata.raw_bits) + : "r" (in) + ); + } while (r < 0); +#else + while ((int32_t) spictrl->txdata.raw_bits < 0); + spictrl->txdata.data = in; +#endif +} + + +/** + * Wait until SPI receive queue has data and read byte. + */ +uint8_t spi_rx(spi_ctrl* spictrl) +{ + int32_t out; + while ((out = (int32_t) spictrl->rxdata.raw_bits) < 0); + return (uint8_t) out; +} + + +/** + * Transmit a byte and receive a byte. + */ +uint8_t spi_txrx(spi_ctrl* spictrl, uint8_t in) +{ + spi_tx(spictrl, in); + return spi_rx(spictrl); +} +
Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: initialize flash ......................................................................
Patch Set 7:
Have anything update?
Hello ron minnich, Shawn C, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30466
to look at the new patch set (#8).
Change subject: fu540: add code to initialize flash ......................................................................
fu540: add code to initialize flash
SiFive's ZSBL has initialized flash, but only 16MB of space is available. I fixed it to use all 32MB.
Change-Id: I8cd803369da5526eff90400c15b91bbf6b477c69 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/Makefile.inc A src/mainboard/sifive/hifive-unleashed/bootblock.c A src/mainboard/sifive/hifive-unleashed/flash.c M src/mainboard/sifive/hifive-unleashed/romstage.c M src/soc/sifive/fu540/Makefile.inc A src/soc/sifive/fu540/include/soc/spi.h A src/soc/sifive/fu540/include/soc/spi_flash.h A src/soc/sifive/fu540/spi.c 8 files changed, 481 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/30466/8
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: fu540: add code to initialize flash ......................................................................
Patch Set 8:
(4 comments)
https://review.coreboot.org/#/c/30466/8/src/soc/sifive/fu540/include/soc/spi... File src/soc/sifive/fu540/include/soc/spi.h:
https://review.coreboot.org/#/c/30466/8/src/soc/sifive/fu540/include/soc/spi... PS8, Line 20: #define _ASSERT_SIZEOF(type, size) _Static_assert(sizeof(type) == (size), #type " must be " #size " bytes wide") line over 80 characters
https://review.coreboot.org/#/c/30466/8/src/soc/sifive/fu540/include/soc/spi... PS8, Line 233: // This should not overflow as long as (f_in - 1 + 2*f_sck) does not exceed line over 80 characters
https://review.coreboot.org/#/c/30466/8/src/soc/sifive/fu540/spi.c File src/soc/sifive/fu540/spi.c:
https://review.coreboot.org/#/c/30466/8/src/soc/sifive/fu540/spi.c@33 PS8, Line 33: while ((int32_t) spictrl->txdata.raw_bits < 0); trailing statements should be on next line
https://review.coreboot.org/#/c/30466/8/src/soc/sifive/fu540/spi.c@45 PS8, Line 45: while ((out = (int32_t) spictrl->rxdata.raw_bits) < 0); trailing statements should be on next line
Hello ron minnich, Shawn C, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30466
to look at the new patch set (#9).
Change subject: fu540: add code to initialize flash ......................................................................
fu540: add code to initialize flash
SiFive's ZSBL has initialized flash, but only 16MB of space is available. I fixed it to use all 32MB.
Change-Id: I8cd803369da5526eff90400c15b91bbf6b477c69 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/Makefile.inc A src/mainboard/sifive/hifive-unleashed/bootblock.c A src/mainboard/sifive/hifive-unleashed/flash.c M src/mainboard/sifive/hifive-unleashed/romstage.c M src/soc/sifive/fu540/Makefile.inc A src/soc/sifive/fu540/include/soc/spi.h A src/soc/sifive/fu540/include/soc/spi_flash.h A src/soc/sifive/fu540/spi.c 8 files changed, 486 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/30466/9
Hello ron minnich, Shawn C, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30466
to look at the new patch set (#10).
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
soc/sifive/fu540: add code to initialize flash
SiFive's ZSBL has initialized flash, but only 16MB of space is available. I fixed it to use all 32MB.
Change-Id: I8cd803369da5526eff90400c15b91bbf6b477c69 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/Makefile.inc A src/mainboard/sifive/hifive-unleashed/bootblock.c A src/mainboard/sifive/hifive-unleashed/flash.c M src/mainboard/sifive/hifive-unleashed/romstage.c M src/soc/sifive/fu540/Makefile.inc A src/soc/sifive/fu540/include/soc/spi.h A src/soc/sifive/fu540/include/soc/spi_flash.h A src/soc/sifive/fu540/spi.c 8 files changed, 486 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/30466/10
Philipp Hug has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 10:
(2 comments)
Thanks! Has this been tested on hw?
https://review.coreboot.org/#/c/30466/10/src/mainboard/sifive/hifive-unleash... File src/mainboard/sifive/hifive-unleashed/flash.c:
https://review.coreboot.org/#/c/30466/10/src/mainboard/sifive/hifive-unleash... PS10, Line 30: .command_code = 0x6c please use a constant for 0x6c. Is this? #define SPINOR_OP_READ4_1_1_4 0x6c /* Read data bytes (Quad SPI) */
https://review.coreboot.org/#/c/30466/10/src/mainboard/sifive/hifive-unleash... PS10, Line 40: 0x66, same here. please use a constant or add a comment.
Hello ron minnich, Shawn C, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30466
to look at the new patch set (#11).
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
soc/sifive/fu540: add code to initialize flash
SiFive's ZSBL has initialized flash, but only 16MB of space is available. I fixed it to use all 32MB.
Change-Id: I8cd803369da5526eff90400c15b91bbf6b477c69 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/Makefile.inc A src/mainboard/sifive/hifive-unleashed/bootblock.c A src/mainboard/sifive/hifive-unleashed/flash.c M src/mainboard/sifive/hifive-unleashed/romstage.c M src/soc/sifive/fu540/Makefile.inc A src/soc/sifive/fu540/include/soc/spi.h A src/soc/sifive/fu540/include/soc/spi_flash.h A src/soc/sifive/fu540/spi.c 8 files changed, 486 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/30466/11
Hello ron minnich, Shawn C, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30466
to look at the new patch set (#13).
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
soc/sifive/fu540: add code to initialize flash
SiFive's ZSBL has initialized flash, but only 16MB of space is available. I fixed it to use all 32MB.
Change-Id: I8cd803369da5526eff90400c15b91bbf6b477c69 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/Makefile.inc A src/mainboard/sifive/hifive-unleashed/bootblock.c A src/mainboard/sifive/hifive-unleashed/flash.c M src/mainboard/sifive/hifive-unleashed/romstage.c M src/soc/sifive/fu540/Makefile.inc A src/soc/sifive/fu540/include/soc/spi.h A src/soc/sifive/fu540/include/soc/spi_flash.h A src/soc/sifive/fu540/spi.c 8 files changed, 493 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/30466/13
Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 13:
(2 comments)
https://review.coreboot.org/#/c/30466/10/src/mainboard/sifive/hifive-unleash... File src/mainboard/sifive/hifive-unleashed/flash.c:
https://review.coreboot.org/#/c/30466/10/src/mainboard/sifive/hifive-unleash... PS10, Line 30: .command_code = 0x6c
please use a constant for 0x6c. […]
Done
https://review.coreboot.org/#/c/30466/10/src/mainboard/sifive/hifive-unleash... PS10, Line 40: 0x66,
same here. please use a constant or add a comment.
Done
Philipp Hug has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 13: Code-Review+1
(3 comments)
Thanks! Do you need to call flash_init in both romstage and bootblock? And could you also mention in the commit message how you tested it. E.g. TEST=Loaded a 20MB image on HiFive unleashed
https://review.coreboot.org/#/c/30466/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/30466/6//COMMIT_MSG@10 PS6, Line 10: available. I fixed it to use all 32MB.
Done
Done
https://review.coreboot.org/#/c/30466/6/src/mainboard/sifive/hifive-unleashe... File src/mainboard/sifive/hifive-unleashed/flash.c:
https://review.coreboot.org/#/c/30466/6/src/mainboard/sifive/hifive-unleashe... PS6, Line 30: .command_code = 0x6c
Done
Done
https://review.coreboot.org/#/c/30466/6/src/mainboard/sifive/hifive-unleashe... PS6, Line 40: 0x66,
Done
Done
Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 13:
(3 comments)
Thanks! Do you need to call flash_init in both romstage and bootblock? And could you also mention in the commit message how you tested it. E.g. TEST=Loaded a 20MB image on HiFive unleashed
Thx review. Yes, it needs to be initialized multiple times, because romstage re-initializes the clock, so you need to reconfigure the spi frequency. The test is to write coreboot to the board and reboot.
Jonathan Neuschäfer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 13:
(3 comments)
https://review.coreboot.org/#/c/30466/13/src/soc/sifive/fu540/include/soc/sp... File src/soc/sifive/fu540/include/soc/spi.h:
https://review.coreboot.org/#/c/30466/13/src/soc/sifive/fu540/include/soc/sp... PS13, Line 20: #define _ASSERT_SIZEOF(type, size) _Static_assert( \ : sizeof(type) == (size), \ : #type " must be " #size " bytes wide") This macro should ideally be in a non RISC-V-specific directory
https://review.coreboot.org/#/c/30466/13/src/soc/sifive/fu540/include/soc/sp... PS13, Line 217: // Inlining header functions in C : // https://stackoverflow.com/a/23699777/7433423 This stackoverflow link seems unnecessary, because inlining is a common C feature.
https://review.coreboot.org/#/c/30466/13/src/soc/sifive/fu540/include/soc/sp... File src/soc/sifive/fu540/include/soc/spi_flash.h:
https://review.coreboot.org/#/c/30466/13/src/soc/sifive/fu540/include/soc/sp... PS13, Line 29: */ That's a lot of functions for a header. Do they have to be in a header? Ideally, I'd prefer to have them in a .c file
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 13:
(5 comments)
https://review.coreboot.org/#/c/30466/13/src/mainboard/sifive/hifive-unleash... File src/mainboard/sifive/hifive-unleashed/flash.c:
https://review.coreboot.org/#/c/30466/13/src/mainboard/sifive/hifive-unleash... PS13, Line 21: extern void flash_init(void); This is going to burn you should you ever change flash_init for any reason. You will have to find and change this proto everywhere. Worse, if you miss even a single place, the code will misfunction in strange ways.
Can you put it in the include file for the mainboard or something?
https://review.coreboot.org/#/c/30466/13/src/mainboard/sifive/hifive-unleash... File src/mainboard/sifive/hifive-unleashed/romstage.c:
https://review.coreboot.org/#/c/30466/13/src/mainboard/sifive/hifive-unleash... PS13, Line 24: extern void flash_init(void); even a .h file in this directory, the standard is to call it mainboard.h
https://review.coreboot.org/#/c/30466/13/src/soc/sifive/fu540/include/soc/sp... File src/soc/sifive/fu540/include/soc/spi.h:
https://review.coreboot.org/#/c/30466/13/src/soc/sifive/fu540/include/soc/sp... PS13, Line 217: // Inlining header functions in C : // https://stackoverflow.com/a/23699777/7433423
This stackoverflow link seems unnecessary, because inlining is a common C feature.
and we try to avoid links in coreboot anyway. at least 25% of the links in our tree are out of date.
Further, this case of inlining seems not needed -- are you sure it's worth it? Were you able to measure it?
https://review.coreboot.org/#/c/30466/13/src/soc/sifive/fu540/include/soc/sp... File src/soc/sifive/fu540/include/soc/spi_flash.h:
https://review.coreboot.org/#/c/30466/13/src/soc/sifive/fu540/include/soc/sp... PS13, Line 29: */
That's a lot of functions for a header. Do they have to be in a header? […]
This should probably be in a .c, I can not see the case for inlining, if I am wrong please tell me :-)
https://review.coreboot.org/#/c/30466/13/src/soc/sifive/fu540/spi.c File src/soc/sifive/fu540/spi.c:
https://review.coreboot.org/#/c/30466/13/src/soc/sifive/fu540/spi.c@23 PS13, Line 23: #if __riscv_atomic we're going to need to do this differently, because I'm too dumb to understand this asm code. Would be nice to have a little set of functions called atomic and then call them, and have them compile to the right thing.
I'm not sure I see the need for atomic ops here -- aren't we running with one hart? can we remove this if there is no need?
Hello ron minnich, Shawn C, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30466
to look at the new patch set (#14).
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
soc/sifive/fu540: add code to initialize flash
SiFive's ZSBL has initialized flash, but only 16MB of space is available. I fixed it to use all 32MB.
Change-Id: I8cd803369da5526eff90400c15b91bbf6b477c69 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/Makefile.inc A src/mainboard/sifive/hifive-unleashed/bootblock.c A src/mainboard/sifive/hifive-unleashed/flash.c A src/mainboard/sifive/hifive-unleashed/flash.h M src/mainboard/sifive/hifive-unleashed/romstage.c M src/soc/sifive/fu540/Makefile.inc A src/soc/sifive/fu540/include/soc/spi.h A src/soc/sifive/fu540/include/soc/spi_flash.h A src/soc/sifive/fu540/spi.c 9 files changed, 512 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/30466/14
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/#/c/30466/14/src/mainboard/sifive/hifive-unleash... File src/mainboard/sifive/hifive-unleashed/flash.h:
https://review.coreboot.org/#/c/30466/14/src/mainboard/sifive/hifive-unleash... PS14, Line 21: #endif /* __HIFIVE_UNLEASHED_FLASH_H__ */ adding a line without newline at end of file
Hello ron minnich, Shawn C, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30466
to look at the new patch set (#15).
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
soc/sifive/fu540: add code to initialize flash
SiFive's ZSBL has initialized flash, but only 16MB of space is available. I fixed it to use all 32MB.
Change-Id: I8cd803369da5526eff90400c15b91bbf6b477c69 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/Makefile.inc A src/mainboard/sifive/hifive-unleashed/bootblock.c A src/mainboard/sifive/hifive-unleashed/flash.c A src/mainboard/sifive/hifive-unleashed/flash.h M src/mainboard/sifive/hifive-unleashed/romstage.c M src/soc/sifive/fu540/Makefile.inc A src/soc/sifive/fu540/include/soc/spi.h A src/soc/sifive/fu540/include/soc/spi_flash.h A src/soc/sifive/fu540/spi.c 9 files changed, 512 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/30466/15
Hello ron minnich, Shawn C, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30466
to look at the new patch set (#16).
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
soc/sifive/fu540: add code to initialize flash
SiFive's ZSBL has initialized flash, but only 16MB of space is available. I fixed it to use all 32MB.
Change-Id: I8cd803369da5526eff90400c15b91bbf6b477c69 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/Makefile.inc A src/mainboard/sifive/hifive-unleashed/bootblock.c A src/mainboard/sifive/hifive-unleashed/flash.c A src/mainboard/sifive/hifive-unleashed/flash.h M src/mainboard/sifive/hifive-unleashed/romstage.c M src/soc/sifive/fu540/Makefile.inc A src/soc/sifive/fu540/include/soc/spi.h A src/soc/sifive/fu540/include/soc/spi_flash.h A src/soc/sifive/fu540/spi.c 9 files changed, 510 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/30466/16
Hello ron minnich, Shawn C, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30466
to look at the new patch set (#17).
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
soc/sifive/fu540: add code to initialize flash
SiFive's ZSBL has initialized flash, but only 16MB of space is available. I fixed it to use all 32MB.
Change-Id: I8cd803369da5526eff90400c15b91bbf6b477c69 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/Makefile.inc A src/mainboard/sifive/hifive-unleashed/bootblock.c A src/mainboard/sifive/hifive-unleashed/flash.c A src/mainboard/sifive/hifive-unleashed/flash.h M src/mainboard/sifive/hifive-unleashed/romstage.c M src/soc/sifive/fu540/Makefile.inc A src/soc/sifive/fu540/include/soc/spi.h A src/soc/sifive/fu540/include/soc/spi_flash.h A src/soc/sifive/fu540/spi.c A src/soc/sifive/fu540/spi_flash.c 10 files changed, 556 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/30466/17
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 17:
(1 comment)
https://review.coreboot.org/#/c/30466/17/src/soc/sifive/fu540/spi_flash.c File src/soc/sifive/fu540/spi_flash.c:
https://review.coreboot.org/#/c/30466/17/src/soc/sifive/fu540/spi_flash.c@90 PS17, Line 90: } adding a line without newline at end of file
Hello ron minnich, Shawn C, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30466
to look at the new patch set (#18).
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
soc/sifive/fu540: add code to initialize flash
SiFive's ZSBL has initialized flash, but only 16MB of space is available. I fixed it to use all 32MB.
Change-Id: I8cd803369da5526eff90400c15b91bbf6b477c69 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/Makefile.inc A src/mainboard/sifive/hifive-unleashed/bootblock.c A src/mainboard/sifive/hifive-unleashed/flash.c A src/mainboard/sifive/hifive-unleashed/flash.h M src/mainboard/sifive/hifive-unleashed/romstage.c M src/soc/sifive/fu540/Makefile.inc A src/soc/sifive/fu540/include/soc/spi.h A src/soc/sifive/fu540/include/soc/spi_flash.h A src/soc/sifive/fu540/spi.c A src/soc/sifive/fu540/spi_flash.c 10 files changed, 558 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/30466/18
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 18:
(1 comment)
https://review.coreboot.org/#/c/30466/18/src/soc/sifive/fu540/spi_flash.c File src/soc/sifive/fu540/spi_flash.c:
https://review.coreboot.org/#/c/30466/18/src/soc/sifive/fu540/spi_flash.c@92 PS18, Line 92: } adding a line without newline at end of file
Hello ron minnich, Shawn C, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30466
to look at the new patch set (#19).
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
soc/sifive/fu540: add code to initialize flash
SiFive's ZSBL has initialized flash, but only 16MB of space is available. I fixed it to use all 32MB.
Change-Id: I8cd803369da5526eff90400c15b91bbf6b477c69 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/Makefile.inc A src/mainboard/sifive/hifive-unleashed/bootblock.c A src/mainboard/sifive/hifive-unleashed/flash.c A src/mainboard/sifive/hifive-unleashed/flash.h M src/mainboard/sifive/hifive-unleashed/romstage.c M src/soc/sifive/fu540/Makefile.inc A src/soc/sifive/fu540/include/soc/spi.h A src/soc/sifive/fu540/include/soc/spi_flash.h A src/soc/sifive/fu540/spi.c A src/soc/sifive/fu540/spi_flash.c 10 files changed, 558 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/30466/19
Hello ron minnich, Shawn C, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30466
to look at the new patch set (#20).
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
soc/sifive/fu540: add code to initialize flash
SiFive's ZSBL has initialized flash, but only 16MB of space is available. I fixed it to use all 32MB.
Change-Id: I8cd803369da5526eff90400c15b91bbf6b477c69 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/Makefile.inc A src/mainboard/sifive/hifive-unleashed/bootblock.c A src/mainboard/sifive/hifive-unleashed/flash.c A src/mainboard/sifive/hifive-unleashed/flash.h M src/mainboard/sifive/hifive-unleashed/romstage.c M src/soc/sifive/fu540/Makefile.inc A src/soc/sifive/fu540/include/soc/spi.h A src/soc/sifive/fu540/include/soc/spi_flash.h A src/soc/sifive/fu540/spi.c A src/soc/sifive/fu540/spi_flash.c 10 files changed, 561 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/30466/20
Hello ron minnich, Shawn C, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30466
to look at the new patch set (#21).
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
soc/sifive/fu540: add code to initialize flash
SiFive's ZSBL has initialized flash, but only 16MB of space is available. I fixed it to use all 32MB.
Change-Id: I8cd803369da5526eff90400c15b91bbf6b477c69 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/Makefile.inc A src/mainboard/sifive/hifive-unleashed/flash.c C src/mainboard/sifive/hifive-unleashed/flash.h R src/mainboard/sifive/hifive-unleashed/media.c M src/mainboard/sifive/hifive-unleashed/romstage.c M src/soc/sifive/fu540/Makefile.inc A src/soc/sifive/fu540/include/soc/spi.h A src/soc/sifive/fu540/include/soc/spi_flash.h A src/soc/sifive/fu540/spi.c A src/soc/sifive/fu540/spi_flash.c 10 files changed, 529 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/30466/21
Hello ron minnich, Shawn C, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30466
to look at the new patch set (#22).
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
soc/sifive/fu540: add code to initialize flash
SiFive's ZSBL has initialized flash, but only 16MB of space is available. I fixed it to use all 32MB.
Change-Id: I8cd803369da5526eff90400c15b91bbf6b477c69 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/Makefile.inc A src/mainboard/sifive/hifive-unleashed/flash.c C src/mainboard/sifive/hifive-unleashed/flash.h R src/mainboard/sifive/hifive-unleashed/media.c M src/soc/sifive/fu540/Makefile.inc A src/soc/sifive/fu540/include/soc/spi.h A src/soc/sifive/fu540/include/soc/spi_flash.h A src/soc/sifive/fu540/spi.c A src/soc/sifive/fu540/spi_flash.c 9 files changed, 527 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/30466/22
Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 22:
What else needs to be updated?
Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 22:
Please review this !!!
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 22:
(3 comments)
https://review.coreboot.org/#/c/30466/22/src/soc/sifive/fu540/spi_flash.c File src/soc/sifive/fu540/spi_flash.c:
https://review.coreboot.org/#/c/30466/22/src/soc/sifive/fu540/spi_flash.c@29 PS22, Line 29: // Max desired SPI clock is 10MHz why? is that a soc limit?
https://review.coreboot.org/#/c/30466/22/src/soc/sifive/fu540/spi_flash.c@40 PS22, Line 40: int _initialize_spi_flash_mmap( what does this function do?
https://review.coreboot.org/#/c/30466/22/src/soc/sifive/fu540/spi_flash.c@63 PS22, Line 63: int initialize_spi_flash_mmap_single( where's the difference to initialize_spi_flash_mmap_quad ?
Hello ron minnich, Shawn C, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30466
to look at the new patch set (#23).
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
soc/sifive/fu540: add code to initialize flash
SiFive's ZSBL has initialized flash, but only 16MB of space is available. I fixed it to use all 32MB.
Change-Id: I8cd803369da5526eff90400c15b91bbf6b477c69 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/Makefile.inc A src/mainboard/sifive/hifive-unleashed/flash.c C src/mainboard/sifive/hifive-unleashed/flash.h R src/mainboard/sifive/hifive-unleashed/media.c M src/soc/sifive/fu540/Makefile.inc A src/soc/sifive/fu540/include/soc/spi.h A src/soc/sifive/fu540/include/soc/spi_flash.h A src/soc/sifive/fu540/spi.c A src/soc/sifive/fu540/spi_flash.c 9 files changed, 504 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/30466/23
Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 23:
(3 comments)
https://review.coreboot.org/#/c/30466/22/src/soc/sifive/fu540/spi_flash.c File src/soc/sifive/fu540/spi_flash.c:
https://review.coreboot.org/#/c/30466/22/src/soc/sifive/fu540/spi_flash.c@29 PS22, Line 29: // Max desired SPI clock is 10MHz
why? is that a soc limit?
Not a limit, this is a desired value, used to ensure that FLASH can work, this comment may not be good
https://review.coreboot.org/#/c/30466/22/src/soc/sifive/fu540/spi_flash.c@40 PS22, Line 40: int _initialize_spi_flash_mmap(
what does this function do?
This function is used to map flash to memory space and tells soc how to operate flash
https://review.coreboot.org/#/c/30466/22/src/soc/sifive/fu540/spi_flash.c@63 PS22, Line 63: int initialize_spi_flash_mmap_single(
where's the difference to initialize_spi_flash_mmap_quad ?
These two functions are used to set the SPI to use how many data lines to transfer data. This setting is now moved to ffmt_rawbits, so repeat this feature.
Hello ron minnich, Shawn C, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30466
to look at the new patch set (#24).
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
soc/sifive/fu540: add code to initialize flash
SiFive's ZSBL has initialized flash, but only 16MB of space is available. I fixed it to use all 32MB.
Change-Id: I8cd803369da5526eff90400c15b91bbf6b477c69 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/Makefile.inc A src/mainboard/sifive/hifive-unleashed/flash.c C src/mainboard/sifive/hifive-unleashed/flash.h R src/mainboard/sifive/hifive-unleashed/media.c M src/soc/sifive/fu540/Makefile.inc A src/soc/sifive/fu540/include/soc/spi.h A src/soc/sifive/fu540/include/soc/spi_flash.h A src/soc/sifive/fu540/spi.c A src/soc/sifive/fu540/spi_flash.c 9 files changed, 503 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/30466/24
Hello ron minnich, Shawn C, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30466
to look at the new patch set (#25).
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
soc/sifive/fu540: add code to initialize flash
SiFive's ZSBL has initialized flash, but only 16MB of space is available. I fixed it to use all 32MB.
Change-Id: I8cd803369da5526eff90400c15b91bbf6b477c69 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/Makefile.inc A src/mainboard/sifive/hifive-unleashed/flash.c C src/mainboard/sifive/hifive-unleashed/flash.h R src/mainboard/sifive/hifive-unleashed/media.c M src/soc/sifive/fu540/Makefile.inc A src/soc/sifive/fu540/include/soc/spi.h A src/soc/sifive/fu540/include/soc/spi_flash.h A src/soc/sifive/fu540/spi.c A src/soc/sifive/fu540/spi_flash.c 9 files changed, 496 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/30466/25
Hello ron minnich, Shawn C, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30466
to look at the new patch set (#26).
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
soc/sifive/fu540: add code to initialize flash
SiFive's ZSBL has initialized flash, but only 16MB of space is available. I fixed it to use all 32MB.
Change-Id: I8cd803369da5526eff90400c15b91bbf6b477c69 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/Makefile.inc A src/mainboard/sifive/hifive-unleashed/flash.c C src/mainboard/sifive/hifive-unleashed/flash.h R src/mainboard/sifive/hifive-unleashed/media.c M src/soc/sifive/fu540/Makefile.inc A src/soc/sifive/fu540/include/soc/spi.h A src/soc/sifive/fu540/include/soc/spi_flash.h A src/soc/sifive/fu540/spi.c A src/soc/sifive/fu540/spi_flash.c 9 files changed, 493 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/30466/26
Hello ron minnich, Shawn C, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30466
to look at the new patch set (#27).
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
soc/sifive/fu540: add code to initialize flash
SiFive's ZSBL has initialized flash, but only 16MB of space is available. I fixed it to use all 32MB.
Change-Id: I8cd803369da5526eff90400c15b91bbf6b477c69 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/Makefile.inc A src/mainboard/sifive/hifive-unleashed/flash.c C src/mainboard/sifive/hifive-unleashed/flash.h R src/mainboard/sifive/hifive-unleashed/media.c M src/soc/sifive/fu540/Makefile.inc A src/soc/sifive/fu540/include/soc/spi.h A src/soc/sifive/fu540/include/soc/spi_flash.h A src/soc/sifive/fu540/spi.c A src/soc/sifive/fu540/spi_flash.c 9 files changed, 493 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/30466/27
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 27: Code-Review+1
(5 comments)
https://review.coreboot.org/#/c/30466/27/src/mainboard/sifive/hifive-unleash... File src/mainboard/sifive/hifive-unleashed/Makefile.inc:
https://review.coreboot.org/#/c/30466/27/src/mainboard/sifive/hifive-unleash... PS27, Line 24: ramstage-y += flash.c isn't it sufficient to compile it in the stage that calls boot_device_init()? I guess that is bootblock?
https://review.coreboot.org/#/c/30466/27/src/soc/sifive/fu540/include/soc/sp... File src/soc/sifive/fu540/include/soc/spi.h:
https://review.coreboot.org/#/c/30466/27/src/soc/sifive/fu540/include/soc/sp... PS27, Line 172: typedef volatile struct { I'd prefer not to have a typedef
https://review.coreboot.org/#/c/30466/27/src/soc/sifive/fu540/spi_flash.c File src/soc/sifive/fu540/spi_flash.c:
https://review.coreboot.org/#/c/30466/27/src/soc/sifive/fu540/spi_flash.c@39 PS27, Line 39: static int _initialize_spi_flash_mmap( move everything in here to initialize_spi_flash_mmap, there's no need for a separate function.
https://review.coreboot.org/#/c/30466/27/src/soc/sifive/fu540/spi_flash.c@46 PS27, Line 46: // Max desired SPI clock is 10MHz same as above
https://review.coreboot.org/#/c/30466/27/src/soc/sifive/fu540/spi_flash.c@61 PS27, Line 61: add comment and explain what this function does, as done on initialize_spi_flash_direct
Hello ron minnich, Shawn C, Patrick Rudolph, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30466
to look at the new patch set (#28).
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
soc/sifive/fu540: add code to initialize flash
SiFive's ZSBL has initialized flash, but only 16MB of space is available. I fixed it to use all 32MB.
Change-Id: I8cd803369da5526eff90400c15b91bbf6b477c69 Signed-off-by: Xiang Wang wxjstz@126.com --- M src/mainboard/sifive/hifive-unleashed/Makefile.inc A src/mainboard/sifive/hifive-unleashed/flash.c C src/mainboard/sifive/hifive-unleashed/flash.h R src/mainboard/sifive/hifive-unleashed/media.c M src/soc/sifive/fu540/Makefile.inc A src/soc/sifive/fu540/include/soc/spi.h A src/soc/sifive/fu540/include/soc/spi_flash.h A src/soc/sifive/fu540/spi.c A src/soc/sifive/fu540/spi_flash.c 9 files changed, 480 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/30466/28
Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 28:
(5 comments)
https://review.coreboot.org/#/c/30466/27/src/mainboard/sifive/hifive-unleash... File src/mainboard/sifive/hifive-unleashed/Makefile.inc:
https://review.coreboot.org/#/c/30466/27/src/mainboard/sifive/hifive-unleash... PS27, Line 24: ramstage-y += flash.c
isn't it sufficient to compile it in the stage that calls boot_device_init()? I guess that is bootbl […]
boot_device_init will be called at each stage, the linker will need this file
https://review.coreboot.org/#/c/30466/27/src/soc/sifive/fu540/include/soc/sp... File src/soc/sifive/fu540/include/soc/spi.h:
https://review.coreboot.org/#/c/30466/27/src/soc/sifive/fu540/include/soc/sp... PS27, Line 172: typedef volatile struct {
I'd prefer not to have a typedef
Done
https://review.coreboot.org/#/c/30466/27/src/soc/sifive/fu540/spi_flash.c File src/soc/sifive/fu540/spi_flash.c:
https://review.coreboot.org/#/c/30466/27/src/soc/sifive/fu540/spi_flash.c@39 PS27, Line 39: static int _initialize_spi_flash_mmap(
move everything in here to initialize_spi_flash_mmap, […]
Done
https://review.coreboot.org/#/c/30466/27/src/soc/sifive/fu540/spi_flash.c@46 PS27, Line 46: // Max desired SPI clock is 10MHz
same as above
Done
https://review.coreboot.org/#/c/30466/27/src/soc/sifive/fu540/spi_flash.c@61 PS27, Line 61:
add comment and explain what this function does, as done on initialize_spi_flash_direct
Done
Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 28:
Have anything update?
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 28: Code-Review+1
Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 28:
Have anything update?
Philipp Hug has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 28:
Is it safe to call flash_init while running from flash in the bootblock? (MSEL=1)
Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 28:
Is it safe to call flash_init while running from flash in the bootblock? (MSEL=1)
The current coreboot depends on ZSBL. Our link start address is at L2LIM (0x08000000), not the start address of flash memory map 0x20000000.
Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 28:
Is it safe to call flash_init while running from flash in the bootblock? (MSEL=1)
SECTIONS { L2LIM_START(FU540_L2LIM) BOOTBLOCK(FU540_L2LIM, 64K) CAR_STACK(FU540_L2LIM + 64K, 20K) PRERAM_CBMEM_CONSOLE(FU540_L2LIM + 84K, 8K) ROMSTAGE(FU540_L2LIM + 128K, 128K) PRERAM_CBFS_CACHE(FU540_L2LIM + 256K, 128K) L2LIM_END(FU540_L2LIM + 2M)
DRAM_START(FU540_DRAM) RAMSTAGE(FU540_DRAM, 256K) MEM_STACK(FU540_DRAM + 256K, 20K) POSTRAM_CBFS_CACHE(FU540_DRAM + 512K, 32M - 512K) }
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 29:
(4 comments)
I've got a Unleashed to test your code and found it useful after working with it less than 5 minutes. Some more comments as I finally understand what's going on.
https://review.coreboot.org/#/c/30466/29/src/mainboard/sifive/hifive-unleash... File src/mainboard/sifive/hifive-unleashed/flash.c:
https://review.coreboot.org/#/c/30466/29/src/mainboard/sifive/hifive-unleash... PS29, Line 35: .data_proto = SPI_PROTO_Q, That should read MSEL and determine the boot mode. If running in single bit mode use SPI_PROTO_S.
https://review.coreboot.org/#/c/30466/29/src/mainboard/sifive/hifive-unleash... PS29, Line 42: initialize_spi_flash_mmap( That should read MSEL and determine the boot mode. User can select between FU540_QSPI0, FU540_QSPI1, FU540_QSPI2 ?
https://review.coreboot.org/#/c/30466/29/src/soc/sifive/fu540/spi.c File src/soc/sifive/fu540/spi.c:
https://review.coreboot.org/#/c/30466/29/src/soc/sifive/fu540/spi.c@59 PS29, Line 59: } That should implement "const struct spi_ctrlr_buses spi_ctrlr_bus_map" and "const size_t spi_ctrlr_bus_map_count" in order to use the generic src/drivers/spi/spi-generic.c and existing SPI drivers.
https://review.coreboot.org/#/c/30466/29/src/soc/sifive/fu540/spi_flash.c File src/soc/sifive/fu540/spi_flash.c:
https://review.coreboot.org/#/c/30466/29/src/soc/sifive/fu540/spi_flash.c@52 PS29, Line 52: spi_txrx(spictrl, command_enable); Should use generic spi_xfer()
Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 29:
(4 comments)
https://review.coreboot.org/#/c/30466/29/src/mainboard/sifive/hifive-unleash... File src/mainboard/sifive/hifive-unleashed/flash.c:
https://review.coreboot.org/#/c/30466/29/src/mainboard/sifive/hifive-unleash... PS29, Line 35: .data_proto = SPI_PROTO_Q,
That should read MSEL and determine the boot mode. If running in single bit mode use SPI_PROTO_S.
These codes are for HiFive Unleashed, the hardware link as QPI, and using SPI here only makes the performance worse, no need
https://review.coreboot.org/#/c/30466/29/src/mainboard/sifive/hifive-unleash... PS29, Line 42: initialize_spi_flash_mmap(
That should read MSEL and determine the boot mode. […]
These codes used for HiFive Unleashed. This board doesn't have that much choice.
https://review.coreboot.org/#/c/30466/29/src/soc/sifive/fu540/spi.c File src/soc/sifive/fu540/spi.c:
https://review.coreboot.org/#/c/30466/29/src/soc/sifive/fu540/spi.c@59 PS29, Line 59: }
That should implement […]
This code is too heavy. Here is just mapping flash to memory space
https://review.coreboot.org/#/c/30466/29/src/soc/sifive/fu540/spi_flash.c File src/soc/sifive/fu540/spi_flash.c:
https://review.coreboot.org/#/c/30466/29/src/soc/sifive/fu540/spi_flash.c@52 PS29, Line 52: spi_txrx(spictrl, command_enable);
Should use generic spi_xfer()
spi-generic.c is too heavy. Here is just mapping flash to memory space
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 29:
(3 comments)
Please let me know if you plan to update this commit. I've you don't got time I can work on the requested code changes.
https://review.coreboot.org/#/c/30466/29/src/mainboard/sifive/hifive-unleash... File src/mainboard/sifive/hifive-unleashed/flash.c:
https://review.coreboot.org/#/c/30466/29/src/mainboard/sifive/hifive-unleash... PS29, Line 35: .data_proto = SPI_PROTO_Q,
These codes are for HiFive Unleashed, the hardware link as QPI, and using SPI here only makes the pe […]
I don't see why ignoring the MSEL should be a good choice. If the user selects SPI mode instead of QSPI, for whatever reason, we should support it. And it's only a bit that needs to be toggled.
https://review.coreboot.org/#/c/30466/29/src/mainboard/sifive/hifive-unleash... PS29, Line 42: initialize_spi_flash_mmap(
These codes used for HiFive Unleashed. This board doesn't have that much choice.
As far as I understand you can connect an additional flash on SPI1 and there's an MSEL to support that booting mode. SPI2 seems to be SDcard only, so out of scope if this patch.
https://review.coreboot.org/#/c/30466/29/src/soc/sifive/fu540/spi_flash.c File src/soc/sifive/fu540/spi_flash.c:
https://review.coreboot.org/#/c/30466/29/src/soc/sifive/fu540/spi_flash.c@52 PS29, Line 52: spi_txrx(spictrl, command_enable);
spi-generic.c is too heavy. […]
There's no reason to increase code fragmentation. In order to write to flash we need src/drivers/spi anyways. Also you might want to attach additional hardware to the SPI header which uses an existing driver in coreboot.
Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 29:
(1 comment)
https://review.coreboot.org/#/c/30466/29/src/mainboard/sifive/hifive-unleash... File src/mainboard/sifive/hifive-unleashed/flash.c:
https://review.coreboot.org/#/c/30466/29/src/mainboard/sifive/hifive-unleash... PS29, Line 42: initialize_spi_flash_mmap(
As far as I understand you can connect an additional flash on SPI1 and there's an MSEL to support th […]
see https://review.coreboot.org/#/c/coreboot/+/30901/
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 29:
(3 comments)
https://review.coreboot.org/#/c/30466/29/src/mainboard/sifive/hifive-unleash... File src/mainboard/sifive/hifive-unleashed/flash.h:
https://review.coreboot.org/#/c/30466/29/src/mainboard/sifive/hifive-unleash... PS29, Line 4: * Copyright (C) 2018 HardenedLinux Per today's announcement about the change in how we do copyright notices, you can remove this copyright line.
https://review.coreboot.org/#/c/30466/29/src/mainboard/sifive/hifive-unleash... PS29, Line 16: #ifndef __HIFIVE_UNLEASHED_FLASH_H__ we've stopped guarding prototypes with #ifdef it seems so you can delete the preprocessor guards here. Once this is done, you will have a file with one prototype. Can it be put in some other file?
https://review.coreboot.org/#/c/30466/29/src/soc/sifive/fu540/spi_flash.c File src/soc/sifive/fu540/spi_flash.c:
https://review.coreboot.org/#/c/30466/29/src/soc/sifive/fu540/spi_flash.c@52 PS29, Line 52: spi_txrx(spictrl, command_enable);
There's no reason to increase code fragmentation. […]
Please follow Rudolph's suggestion. He is right. We've worked hard to avoid this kind of code fragmentation in coreboot, even at the cost of using "heavy" code. The cost is worth it.
Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Patch Set 29:
Please refer: https://review.coreboot.org/c/coreboot/+/33055
Xiang Wang has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/30466 )
Change subject: soc/sifive/fu540: add code to initialize flash ......................................................................
Abandoned