| Message ID | 20220114162008.70468-4-apatel@ventanamicro.com |
|---|---|
| State | Superseded |
| Delegated to: | Andes |
| Headers | show |
| Series | QEMU spike machine support for U-Boot | expand |
> From: Anup Patel <apatel@ventanamicro.com> > Sent: Saturday, January 15, 2022 12:20 AM > To: Rick Jian-Zhi Chen(陳建志) <rick@andestech.com>; Bin Meng <bmeng.cn@gmail.com> > Cc: Atish Patra <atishp@atishpatra.org>; Alistair Francis <Alistair.Francis@wdc.com>; Anup Patel <anup@brainfault.org>; U-Boot Mailing List <u-boot@lists.denx.de>; Anup Patel <apatel@ventanamicro.com> > Subject: [PATCH 3/3] riscv: qemu: Implement is_flash_available() for MTD NOR > > Currently, if MTD NOR is enabled then U-Boot tries to issue flash commands even when CFI flash DT node is not present. This causes access fault on RISC-V emulators or ISS which do not emulate CFI flash. To handle this issue, we implement is_flash_available() for qemu-riscv board which will return 1 only if CFI flash DT node is present. > > Fixes: d248627f9d42 ("riscv: qemu: Enable MTD NOR flash support") > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > --- > board/emulation/qemu-riscv/qemu-riscv.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) Reviewed-by: Rick Chen <rick@andestech.com>
On Sat, Jan 15, 2022 at 12:20 AM Anup Patel <apatel@ventanamicro.com> wrote: > > Currently, if MTD NOR is enabled then U-Boot tries to issue flash > commands even when CFI flash DT node is not present. This causes > access fault on RISC-V emulators or ISS which do not emulate CFI > flash. To handle this issue, we implement is_flash_available() for > qemu-riscv board which will return 1 only if CFI flash DT node is > present. > > Fixes: d248627f9d42 ("riscv: qemu: Enable MTD NOR flash support") > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > --- > board/emulation/qemu-riscv/qemu-riscv.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/board/emulation/qemu-riscv/qemu-riscv.c b/board/emulation/qemu-riscv/qemu-riscv.c > index b0d9dd59b1..cd02dae1ab 100644 > --- a/board/emulation/qemu-riscv/qemu-riscv.c > +++ b/board/emulation/qemu-riscv/qemu-riscv.c > @@ -8,6 +8,7 @@ > #include <env.h> > #include <fdtdec.h> > #include <image.h> > +#include <linux/libfdt.h> > #include <log.h> > #include <spl.h> > #include <init.h> > @@ -16,6 +17,22 @@ > > DECLARE_GLOBAL_DATA_PTR; > > +#if IS_ENABLED(CONFIG_MTD_NOR_FLASH) > +int is_flash_available(void) > +{ > + if (IS_ENABLED(CONFIG_OF_SEPARATE) || IS_ENABLED(CONFIG_OF_BOARD)) { Why is this if statement needed? All QEMU riscv* defconfigs are using CONFIG_OF_BOARD, and there is no OF_SEPARATE use case since QEMU DTBs are always consumed by U-Boot. > + const void *fdt = > + (const void *)(uintptr_t)gd->arch.firmware_fdt_addr; > + int rc = fdt_node_offset_by_compatible(fdt, -1, "cfi-flash"); > + > + if (rc >= 0) > + return 1; > + } > + > + return 0; > +} > +#endif > + > int board_init(void) > { > /* Regards, Bin
On Tue, Jan 25, 2022 at 10:33 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > On Sat, Jan 15, 2022 at 12:20 AM Anup Patel <apatel@ventanamicro.com> wrote: > > > > Currently, if MTD NOR is enabled then U-Boot tries to issue flash > > commands even when CFI flash DT node is not present. This causes > > access fault on RISC-V emulators or ISS which do not emulate CFI > > flash. To handle this issue, we implement is_flash_available() for > > qemu-riscv board which will return 1 only if CFI flash DT node is > > present. > > > > Fixes: d248627f9d42 ("riscv: qemu: Enable MTD NOR flash support") > > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > > --- > > board/emulation/qemu-riscv/qemu-riscv.c | 17 +++++++++++++++++ > > 1 file changed, 17 insertions(+) > > > > diff --git a/board/emulation/qemu-riscv/qemu-riscv.c b/board/emulation/qemu-riscv/qemu-riscv.c > > index b0d9dd59b1..cd02dae1ab 100644 > > --- a/board/emulation/qemu-riscv/qemu-riscv.c > > +++ b/board/emulation/qemu-riscv/qemu-riscv.c > > @@ -8,6 +8,7 @@ > > #include <env.h> > > #include <fdtdec.h> > > #include <image.h> > > +#include <linux/libfdt.h> > > #include <log.h> > > #include <spl.h> > > #include <init.h> > > @@ -16,6 +17,22 @@ > > > > DECLARE_GLOBAL_DATA_PTR; > > > > +#if IS_ENABLED(CONFIG_MTD_NOR_FLASH) > > +int is_flash_available(void) > > +{ > > + if (IS_ENABLED(CONFIG_OF_SEPARATE) || IS_ENABLED(CONFIG_OF_BOARD)) { > > Why is this if statement needed? > > All QEMU riscv* defconfigs are using CONFIG_OF_BOARD, and there is no > OF_SEPARATE use case since QEMU DTBs are always consumed by U-Boot. I added the if statement for the case if someone disables CONFIG_OF_BOARD but I can remove it if you insist. > > > + const void *fdt = > > + (const void *)(uintptr_t)gd->arch.firmware_fdt_addr; > > + int rc = fdt_node_offset_by_compatible(fdt, -1, "cfi-flash"); > > + > > + if (rc >= 0) > > + return 1; > > + } > > + > > + return 0; > > +} > > +#endif > > + > > int board_init(void) > > { > > /* > > Regards, > Bin Regards, Anup
On Tue, Jan 25, 2022 at 2:02 PM Anup Patel <apatel@ventanamicro.com> wrote: > > On Tue, Jan 25, 2022 at 10:33 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > On Sat, Jan 15, 2022 at 12:20 AM Anup Patel <apatel@ventanamicro.com> wrote: > > > > > > Currently, if MTD NOR is enabled then U-Boot tries to issue flash > > > commands even when CFI flash DT node is not present. This causes > > > access fault on RISC-V emulators or ISS which do not emulate CFI > > > flash. To handle this issue, we implement is_flash_available() for > > > qemu-riscv board which will return 1 only if CFI flash DT node is > > > present. > > > > > > Fixes: d248627f9d42 ("riscv: qemu: Enable MTD NOR flash support") > > > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > > > --- > > > board/emulation/qemu-riscv/qemu-riscv.c | 17 +++++++++++++++++ > > > 1 file changed, 17 insertions(+) > > > > > > diff --git a/board/emulation/qemu-riscv/qemu-riscv.c b/board/emulation/qemu-riscv/qemu-riscv.c > > > index b0d9dd59b1..cd02dae1ab 100644 > > > --- a/board/emulation/qemu-riscv/qemu-riscv.c > > > +++ b/board/emulation/qemu-riscv/qemu-riscv.c > > > @@ -8,6 +8,7 @@ > > > #include <env.h> > > > #include <fdtdec.h> > > > #include <image.h> > > > +#include <linux/libfdt.h> > > > #include <log.h> > > > #include <spl.h> > > > #include <init.h> > > > @@ -16,6 +17,22 @@ > > > > > > DECLARE_GLOBAL_DATA_PTR; > > > > > > +#if IS_ENABLED(CONFIG_MTD_NOR_FLASH) > > > +int is_flash_available(void) > > > +{ > > > + if (IS_ENABLED(CONFIG_OF_SEPARATE) || IS_ENABLED(CONFIG_OF_BOARD)) { > > > > Why is this if statement needed? > > > > All QEMU riscv* defconfigs are using CONFIG_OF_BOARD, and there is no > > OF_SEPARATE use case since QEMU DTBs are always consumed by U-Boot. > > I added the if statement for the case if someone disables CONFIG_OF_BOARD > but I can remove it if you insist. > I see. But I if we give a dtb to QEMU without using CONFIG_OF_BOARD, the dtb may still contain a node for flash so the logic you added is still needed. So I think you can just remove the if statement. Also instead of using fdt_node_offset_by_compatible API please use ofnode_device_is_compatible. Regards, Bin
On Tue, Jan 25, 2022 at 3:16 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > On Tue, Jan 25, 2022 at 2:02 PM Anup Patel <apatel@ventanamicro.com> wrote: > > > > On Tue, Jan 25, 2022 at 10:33 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > On Sat, Jan 15, 2022 at 12:20 AM Anup Patel <apatel@ventanamicro.com> wrote: > > > > > > > > Currently, if MTD NOR is enabled then U-Boot tries to issue flash > > > > commands even when CFI flash DT node is not present. This causes > > > > access fault on RISC-V emulators or ISS which do not emulate CFI > > > > flash. To handle this issue, we implement is_flash_available() for > > > > qemu-riscv board which will return 1 only if CFI flash DT node is > > > > present. > > > > > > > > Fixes: d248627f9d42 ("riscv: qemu: Enable MTD NOR flash support") > > > > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > > > > --- > > > > board/emulation/qemu-riscv/qemu-riscv.c | 17 +++++++++++++++++ > > > > 1 file changed, 17 insertions(+) > > > > > > > > diff --git a/board/emulation/qemu-riscv/qemu-riscv.c b/board/emulation/qemu-riscv/qemu-riscv.c > > > > index b0d9dd59b1..cd02dae1ab 100644 > > > > --- a/board/emulation/qemu-riscv/qemu-riscv.c > > > > +++ b/board/emulation/qemu-riscv/qemu-riscv.c > > > > @@ -8,6 +8,7 @@ > > > > #include <env.h> > > > > #include <fdtdec.h> > > > > #include <image.h> > > > > +#include <linux/libfdt.h> > > > > #include <log.h> > > > > #include <spl.h> > > > > #include <init.h> > > > > @@ -16,6 +17,22 @@ > > > > > > > > DECLARE_GLOBAL_DATA_PTR; > > > > > > > > +#if IS_ENABLED(CONFIG_MTD_NOR_FLASH) > > > > +int is_flash_available(void) > > > > +{ > > > > + if (IS_ENABLED(CONFIG_OF_SEPARATE) || IS_ENABLED(CONFIG_OF_BOARD)) { > > > > > > Why is this if statement needed? > > > > > > All QEMU riscv* defconfigs are using CONFIG_OF_BOARD, and there is no > > > OF_SEPARATE use case since QEMU DTBs are always consumed by U-Boot. > > > > I added the if statement for the case if someone disables CONFIG_OF_BOARD > > but I can remove it if you insist. > > > > I see. But I if we give a dtb to QEMU without using CONFIG_OF_BOARD, > the dtb may still contain a node for flash so the logic you added is > still needed. > > So I think you can just remove the if statement. Okay, I will remove the "if ()" check. > > Also instead of using fdt_node_offset_by_compatible API please use > ofnode_device_is_compatible. Sure, I will try ofnode_device_is_compatible(). > > Regards, > Bin Regards, Anup
diff --git a/board/emulation/qemu-riscv/qemu-riscv.c b/board/emulation/qemu-riscv/qemu-riscv.c index b0d9dd59b1..cd02dae1ab 100644 --- a/board/emulation/qemu-riscv/qemu-riscv.c +++ b/board/emulation/qemu-riscv/qemu-riscv.c @@ -8,6 +8,7 @@ #include <env.h> #include <fdtdec.h> #include <image.h> +#include <linux/libfdt.h> #include <log.h> #include <spl.h> #include <init.h> @@ -16,6 +17,22 @@ DECLARE_GLOBAL_DATA_PTR; +#if IS_ENABLED(CONFIG_MTD_NOR_FLASH) +int is_flash_available(void) +{ + if (IS_ENABLED(CONFIG_OF_SEPARATE) || IS_ENABLED(CONFIG_OF_BOARD)) { + const void *fdt = + (const void *)(uintptr_t)gd->arch.firmware_fdt_addr; + int rc = fdt_node_offset_by_compatible(fdt, -1, "cfi-flash"); + + if (rc >= 0) + return 1; + } + + return 0; +} +#endif + int board_init(void) { /*
Currently, if MTD NOR is enabled then U-Boot tries to issue flash commands even when CFI flash DT node is not present. This causes access fault on RISC-V emulators or ISS which do not emulate CFI flash. To handle this issue, we implement is_flash_available() for qemu-riscv board which will return 1 only if CFI flash DT node is present. Fixes: d248627f9d42 ("riscv: qemu: Enable MTD NOR flash support") Signed-off-by: Anup Patel <apatel@ventanamicro.com> --- board/emulation/qemu-riscv/qemu-riscv.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)