* [U-Boot] [PATCH] mtd: do not include cfi_flash.h if no flash
@ 2015-10-25 3:02 Thomas Chou
2015-10-25 4:00 ` Marek Vasut
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Chou @ 2015-10-25 3:02 UTC (permalink / raw)
To: u-boot
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__ */
--
2.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] mtd: do not include cfi_flash.h if no flash
2015-10-25 3:02 [U-Boot] [PATCH] mtd: do not include cfi_flash.h if no flash Thomas Chou
@ 2015-10-25 4:00 ` Marek Vasut
2015-10-25 13:29 ` Thomas Chou
0 siblings, 1 reply; 6+ messages in thread
From: Marek Vasut @ 2015-10-25 4:00 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] mtd: do not include cfi_flash.h if no flash
2015-10-25 4:00 ` Marek Vasut
@ 2015-10-25 13:29 ` Thomas Chou
2015-10-25 13:30 ` Marek Vasut
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Chou @ 2015-10-25 13:29 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] mtd: do not include cfi_flash.h if no flash
2015-10-25 13:29 ` Thomas Chou
@ 2015-10-25 13:30 ` Marek Vasut
2015-10-25 23:15 ` Thomas Chou
0 siblings, 1 reply; 6+ messages in thread
From: Marek Vasut @ 2015-10-25 13:30 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] mtd: do not include cfi_flash.h if no flash
2015-10-25 13:30 ` Marek Vasut
@ 2015-10-25 23:15 ` Thomas Chou
2015-10-25 23:25 ` Marek Vasut
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Chou @ 2015-10-25 23:15 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] mtd: do not include cfi_flash.h if no flash
2015-10-25 23:15 ` Thomas Chou
@ 2015-10-25 23:25 ` Marek Vasut
0 siblings, 0 replies; 6+ messages in thread
From: Marek Vasut @ 2015-10-25 23:25 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-10-25 23:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-25 3:02 [U-Boot] [PATCH] mtd: do not include cfi_flash.h if no flash Thomas Chou
2015-10-25 4:00 ` Marek Vasut
2015-10-25 13:29 ` Thomas Chou
2015-10-25 13:30 ` Marek Vasut
2015-10-25 23:15 ` Thomas Chou
2015-10-25 23:25 ` Marek Vasut
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox