Re: [PATCH 3/3] riscv: qemu: Implement is_flash_available() for MTD NOR

2022-01-25 Thread Anup Patel 
On Tue, Jan 25, 2022 at 3:16 PM Bin Meng  wrote:
>
> On Tue, Jan 25, 2022 at 2:02 PM Anup Patel  wrote:
> >
> > On Tue, Jan 25, 2022 at 10:33 AM Bin Meng  wrote:
> > >
> > > On Sat, Jan 15, 2022 at 12:20 AM Anup Patel  
> > > 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 
> > > > ---
> > > >  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 
> > > >  #include 
> > > >  #include 
> > > > +#include 
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > > @@ -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

Re: [PATCH 3/3] riscv: qemu: Implement is_flash_available() for MTD NOR

2022-01-25 Thread Bin Meng 
On Tue, Jan 25, 2022 at 2:02 PM Anup Patel  wrote:
>
> On Tue, Jan 25, 2022 at 10:33 AM Bin Meng  wrote:
> >
> > On Sat, Jan 15, 2022 at 12:20 AM Anup Patel  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 
> > > ---
> > >  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 
> > >  #include 
> > >  #include 
> > > +#include 
> > >  #include 
> > >  #include 
> > >  #include 
> > > @@ -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

Re: [PATCH 3/3] riscv: qemu: Implement is_flash_available() for MTD NOR

2022-01-24 Thread Anup Patel 
On Tue, Jan 25, 2022 at 10:33 AM Bin Meng  wrote:
>
> On Sat, Jan 15, 2022 at 12:20 AM Anup Patel  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 
> > ---
> >  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 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -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

Re: [PATCH 3/3] riscv: qemu: Implement is_flash_available() for MTD NOR

2022-01-24 Thread Bin Meng 
On Sat, Jan 15, 2022 at 12:20 AM Anup Patel  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 
> ---
>  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 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -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

Re: [PATCH 3/3] riscv: qemu: Implement is_flash_available() for MTD NOR

2022-01-24 Thread Rick Chen 
> From: Anup Patel 
> Sent: Saturday, January 15, 2022 12:20 AM
> To: Rick Jian-Zhi Chen(陳建志) ; Bin Meng 
> 
> Cc: Atish Patra ; Alistair Francis 
> ; Anup Patel ; U-Boot Mailing 
> List ; Anup Patel 
> 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 
> ---
>  board/emulation/qemu-riscv/qemu-riscv.c | 17 +
>  1 file changed, 17 insertions(+)

Reviewed-by: Rick Chen

[PATCH 3/3] riscv: qemu: Implement is_flash_available() for MTD NOR

2022-01-14 Thread Anup Patel 
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 
---
 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 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -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)
 {
/*
-- 
2.25.1