[U-Boot] [PATCH] mtd: do not include cfi_flash.h if no flash
Do not include cfi_flash.h if CONFIG_SYS_NO_FLASH. Because the flash_info is undefined in flash.h if CONFIG_SYS_NO_FLASH.
Signed-off-by: Thomas Chou thomas@wytron.com.tw --- include/mtd/cfi_flash.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/mtd/cfi_flash.h b/include/mtd/cfi_flash.h index 048b477..526898a 100644 --- a/include/mtd/cfi_flash.h +++ b/include/mtd/cfi_flash.h @@ -8,6 +8,8 @@ #ifndef __CFI_FLASH_H__ #define __CFI_FLASH_H__
+#ifndef CONFIG_SYS_NO_FLASH + #define FLASH_CMD_CFI 0x98 #define FLASH_CMD_READ_ID 0x90 #define FLASH_CMD_RESET 0xff @@ -182,4 +184,6 @@ u32 flash_read32(void *addr); u64 flash_read64(void *addr); #endif
+#endif /* !CONFIG_SYS_NO_FLASH */ + #endif /* __CFI_FLASH_H__ */
On Sunday, October 25, 2015 at 04:02:58 AM, Thomas Chou wrote:
Do not include cfi_flash.h if CONFIG_SYS_NO_FLASH. Because the flash_info is undefined in flash.h if CONFIG_SYS_NO_FLASH.
Signed-off-by: Thomas Chou thomas@wytron.com.tw
include/mtd/cfi_flash.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/mtd/cfi_flash.h b/include/mtd/cfi_flash.h index 048b477..526898a 100644 --- a/include/mtd/cfi_flash.h +++ b/include/mtd/cfi_flash.h @@ -8,6 +8,8 @@ #ifndef __CFI_FLASH_H__ #define __CFI_FLASH_H__
+#ifndef CONFIG_SYS_NO_FLASH
Hi!
Something tells me this is not the right way to fix things up, but I suspect this idea came from flash.h , right ? This legacy code is a terrible mess :-(
I'd kinda expect that it'd be always safe to include the cfi_flash.h because cfi_flash.h would include flash.h and take care of having all the types available (not the case now). I'd also expect to have all the CFI command macros available if I include cfi_flash.h, so this new ifdef would introduce a surprising behavior for me.
I wonder if we cannot find some better solution instead of adding some more ifdefs to the already horrible maze of ifdefs there. What do you think please ?
#define FLASH_CMD_CFI 0x98 #define FLASH_CMD_READ_ID 0x90 #define FLASH_CMD_RESET 0xff @@ -182,4 +184,6 @@ u32 flash_read32(void *addr); u64 flash_read64(void *addr); #endif
+#endif /* !CONFIG_SYS_NO_FLASH */
#endif /* __CFI_FLASH_H__ */
Best regards, Marek Vasut
Hi Marek,
On 10/25/2015 12:00 PM, Marek Vasut wrote:
On Sunday, October 25, 2015 at 04:02:58 AM, Thomas Chou wrote:
Do not include cfi_flash.h if CONFIG_SYS_NO_FLASH. Because the flash_info is undefined in flash.h if CONFIG_SYS_NO_FLASH.
Signed-off-by: Thomas Chou thomas@wytron.com.tw
include/mtd/cfi_flash.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/mtd/cfi_flash.h b/include/mtd/cfi_flash.h index 048b477..526898a 100644 --- a/include/mtd/cfi_flash.h +++ b/include/mtd/cfi_flash.h @@ -8,6 +8,8 @@ #ifndef __CFI_FLASH_H__ #define __CFI_FLASH_H__
+#ifndef CONFIG_SYS_NO_FLASH
Hi!
Something tells me this is not the right way to fix things up, but I suspect this idea came from flash.h , right ? This legacy code is a terrible mess :-(
I'd kinda expect that it'd be always safe to include the cfi_flash.h because cfi_flash.h would include flash.h and take care of having all the types available (not the case now). I'd also expect to have all the CFI command macros available if I include cfi_flash.h, so this new ifdef would introduce a surprising behavior for me.
I wonder if we cannot find some better solution instead of adding some more ifdefs to the already horrible maze of ifdefs there. What do you think please ?
Fully agreed. Please allow me to withdraw this patch and fix it in the other way. You are always welcome.
Best regards, Thomas
On Sunday, October 25, 2015 at 02:29:06 PM, Thomas Chou wrote:
Hi Marek,
On 10/25/2015 12:00 PM, Marek Vasut wrote:
On Sunday, October 25, 2015 at 04:02:58 AM, Thomas Chou wrote:
Do not include cfi_flash.h if CONFIG_SYS_NO_FLASH. Because the flash_info is undefined in flash.h if CONFIG_SYS_NO_FLASH.
Signed-off-by: Thomas Chou thomas@wytron.com.tw
include/mtd/cfi_flash.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/mtd/cfi_flash.h b/include/mtd/cfi_flash.h index 048b477..526898a 100644 --- a/include/mtd/cfi_flash.h +++ b/include/mtd/cfi_flash.h @@ -8,6 +8,8 @@
#ifndef __CFI_FLASH_H__ #define __CFI_FLASH_H__
+#ifndef CONFIG_SYS_NO_FLASH
Hi!
Something tells me this is not the right way to fix things up, but I suspect this idea came from flash.h , right ? This legacy code is a terrible mess :-(
I'd kinda expect that it'd be always safe to include the cfi_flash.h because cfi_flash.h would include flash.h and take care of having all the types available (not the case now). I'd also expect to have all the CFI command macros available if I include cfi_flash.h, so this new ifdef would introduce a surprising behavior for me.
I wonder if we cannot find some better solution instead of adding some more ifdefs to the already horrible maze of ifdefs there. What do you think please ?
Fully agreed. Please allow me to withdraw this patch and fix it in the other way. You are always welcome.
Wow, hey this is seriously cool response. Thanks a lot!
btw. do you plan to send a PR with all the nios2 goodies (patches) any soon ?
Best regards, Marek Vasut
Hi Marek,
On 10/25/2015 09:30 PM, Marek Vasut wrote:
btw. do you plan to send a PR with all the nios2 goodies (patches) any soon ?
I did send it. And Tom applied them already.
I am working on max10 devboard. I am cleaning up the flash code and will add an altera quadspi driver. The altera tse needs update, too.
Best regards, Thomas
On Monday, October 26, 2015 at 12:15:15 AM, Thomas Chou wrote:
Hi Marek,
Hi!
On 10/25/2015 09:30 PM, Marek Vasut wrote:
btw. do you plan to send a PR with all the nios2 goodies (patches) any soon ?
I did send it. And Tom applied them already.
Excellent , thanks!
I am working on max10 devboard. I am cleaning up the flash code and will add an altera quadspi driver. The altera tse needs update, too.
Yay :-)
Best regards, Marek Vasut
participants (2)
-
Marek Vasut -
Thomas Chou