RE: [PATCH RESEND v3] fpga: add inline stub for fpga_load
Hi Eugen,
> -Original Message-
> From: Eugen Hristev
> Sent: Monday, August 28, 2023 8:47 PM
> To: Michal Simek ; u-boot@lists.denx.de;
> s...@chromium.org; Chanho Park
> Subject: Re: [PATCH RESEND v3] fpga: add inline stub for fpga_load
>
> On 8/28/23 13:53, Michal Simek wrote:
> > Hi Eugen, + Chanho,
> >
> > On 8/8/23 12:22, Eugen Hristev wrote:
> >> In case CC_OPTIMIZE_FOR_DEBUG is set, unused code will not be
> >> optimized out, hence the reference to fpga_load will be compiled.
> >> if DM_FPGA and SPL_FPGA are not set, the build will fail with :
> >
> > this is not correct. It is not DM_FPGA but only FPGA.
> >
> >
> >>
> >> aarch64-none-linux-gnu-ld.bfd: common/spl/spl_fit.o: in function
> >> `spl_fit_upload_fpga':
> >> u-boot/common/spl/spl_fit.c:595: undefined reference to `fpga_load'
> >>
> >> To solve this, added an inline empty stub in the header if
> >> CC_OPTIMIZE_FOR_DEBUG is set such that the build will succeed.
> >>
> >> Signed-off-by: Eugen Hristev
> >> ---
> >> Changes in v3:
> >> - return -ENOSYS
> >> Changes in v2:
> >> - this is a rework as suggested by Simon of this previous patch :
> >> https://protect2.fireeye.com/v1/url?k=eed360f4-8f5875c2-eed2ebbb-74fe
> >> 485cbff1-6ddceb7720c8265c=1=f4039d69-e052-4de7-b3b0-f25e8f4581a3&
> >> u=https%3A%2F%2Fpatchwork.ozlabs.org%2Fproject%2Fuboot%2Fpatch%2F2023
> >> 0619102839.277902-1-eugen.hristev%40collabora.com%2F
> >>
> >> include/fpga.h | 9 +
> >> 1 file changed, 9 insertions(+)
> >>
> >> diff --git a/include/fpga.h b/include/fpga.h index
> >> ed688cc0fa3b..33d0dbbe2ba4 100644
> >> --- a/include/fpga.h
> >> +++ b/include/fpga.h
> >> @@ -60,8 +60,17 @@ int fpga_add(fpga_type devtype, void *desc);
> >> int fpga_count(void);
> >> const fpga_desc *const fpga_get_desc(int devnum);
> >> int fpga_is_partial_data(int devnum, size_t img_len);
> >> +#if defined(CONFIG_CC_OPTIMIZE_FOR_DEBUG)
> >
> > And when you enable CONFIG_CC_OPTIMIZE_FOR_DEBUG and FPGA you get
> > compilation warnings.
> >
> > drivers/fpga/fpga.c:265:5: error: redefinition of 'fpga_load'
> >265 | int fpga_load(int devnum, const void *buf, size_t bsize,
> > bitstream_type bstype,
> >| ^
> > In file included from include/xilinx.h:7,
> > from drivers/fpga/fpga.c:11:
> > include/fpga.h:64:19: note: previous definition of 'fpga_load' with
> > type 'int(int, const void *, size_t, bitstream_type, int)' {aka
> > 'int(int, const void *, long unsigned int, bitstream_type, int)'}
> > 64 | static inline int fpga_load(int devnum, const void *buf,
> > size_t bsize,
> >| ^
> > make[2]: *** [scripts/Makefile.build:257: drivers/fpga/fpga.o] Error 1
> >
> > I means please tune that condition properly not to create additional
> > compilation warnings for other combinations.
> >
> > Thanks,
> > Michal
> >
>
> Hello Michal,
>
> Thanks for reviewing .
>
> Chanho, I cannot try this at the moment, if you are in a hurry you can
> send a v4 or v2 to your patch addressing this (or maybe your patch does
> not have this problem)
Sure. Actually, my patch does not have the problem.
Michal, could review my patch?
Best Regards,
Chanho Park
Re: [PATCH RESEND v3] fpga: add inline stub for fpga_load
On 8/28/23 13:53, Michal Simek wrote:
Hi Eugen, + Chanho,
On 8/8/23 12:22, Eugen Hristev wrote:
In case CC_OPTIMIZE_FOR_DEBUG is set, unused code will not be
optimized out,
hence the reference to fpga_load will be compiled.
if DM_FPGA and SPL_FPGA are not set, the build will fail with :
this is not correct. It is not DM_FPGA but only FPGA.
aarch64-none-linux-gnu-ld.bfd: common/spl/spl_fit.o: in function
`spl_fit_upload_fpga':
u-boot/common/spl/spl_fit.c:595: undefined reference to `fpga_load'
To solve this, added an inline empty stub in the header if
CC_OPTIMIZE_FOR_DEBUG is set such that the build will succeed.
Signed-off-by: Eugen Hristev
---
Changes in v3:
- return -ENOSYS
Changes in v2:
- this is a rework as suggested by Simon of this previous patch :
https://patchwork.ozlabs.org/project/uboot/patch/20230619102839.277902-1-eugen.hris...@collabora.com/
include/fpga.h | 9 +
1 file changed, 9 insertions(+)
diff --git a/include/fpga.h b/include/fpga.h
index ed688cc0fa3b..33d0dbbe2ba4 100644
--- a/include/fpga.h
+++ b/include/fpga.h
@@ -60,8 +60,17 @@ int fpga_add(fpga_type devtype, void *desc);
int fpga_count(void);
const fpga_desc *const fpga_get_desc(int devnum);
int fpga_is_partial_data(int devnum, size_t img_len);
+#if defined(CONFIG_CC_OPTIMIZE_FOR_DEBUG)
And when you enable CONFIG_CC_OPTIMIZE_FOR_DEBUG and FPGA you get
compilation warnings.
drivers/fpga/fpga.c:265:5: error: redefinition of 'fpga_load'
265 | int fpga_load(int devnum, const void *buf, size_t bsize,
bitstream_type bstype,
| ^
In file included from include/xilinx.h:7,
from drivers/fpga/fpga.c:11:
include/fpga.h:64:19: note: previous definition of 'fpga_load' with type
'int(int, const void *, size_t, bitstream_type, int)' {aka 'int(int,
const void *, long unsigned int, bitstream_type, int)'}
64 | static inline int fpga_load(int devnum, const void *buf, size_t
bsize,
| ^
make[2]: *** [scripts/Makefile.build:257: drivers/fpga/fpga.o] Error 1
I means please tune that condition properly not to create additional
compilation warnings for other combinations.
Thanks,
Michal
Hello Michal,
Thanks for reviewing .
Chanho, I cannot try this at the moment, if you are in a hurry you can
send a v4 or v2 to your patch addressing this (or maybe your patch does
not have this problem)
Thanks,
Eugen
+static inline int fpga_load(int devnum, const void *buf, size_t bsize,
+ bitstream_type bstype, int flags)
+{
+ return -ENOSYS;
+}
+#else
int fpga_load(int devnum, const void *buf, size_t bsize,
bitstream_type bstype, int flags);
+#endif
+
int fpga_fsload(int devnum, const void *buf, size_t size,
fpga_fs_info *fpga_fsinfo);
int fpga_loads(int devnum, const void *buf, size_t size,
Re: [PATCH RESEND v3] fpga: add inline stub for fpga_load
Hi Eugen, + Chanho,
On 8/8/23 12:22, Eugen Hristev wrote:
In case CC_OPTIMIZE_FOR_DEBUG is set, unused code will not be optimized out,
hence the reference to fpga_load will be compiled.
if DM_FPGA and SPL_FPGA are not set, the build will fail with :
this is not correct. It is not DM_FPGA but only FPGA.
aarch64-none-linux-gnu-ld.bfd: common/spl/spl_fit.o: in function
`spl_fit_upload_fpga':
u-boot/common/spl/spl_fit.c:595: undefined reference to `fpga_load'
To solve this, added an inline empty stub in the header if
CC_OPTIMIZE_FOR_DEBUG is set such that the build will succeed.
Signed-off-by: Eugen Hristev
---
Changes in v3:
- return -ENOSYS
Changes in v2:
- this is a rework as suggested by Simon of this previous patch :
https://patchwork.ozlabs.org/project/uboot/patch/20230619102839.277902-1-eugen.hris...@collabora.com/
include/fpga.h | 9 +
1 file changed, 9 insertions(+)
diff --git a/include/fpga.h b/include/fpga.h
index ed688cc0fa3b..33d0dbbe2ba4 100644
--- a/include/fpga.h
+++ b/include/fpga.h
@@ -60,8 +60,17 @@ int fpga_add(fpga_type devtype, void *desc);
int fpga_count(void);
const fpga_desc *const fpga_get_desc(int devnum);
int fpga_is_partial_data(int devnum, size_t img_len);
+#if defined(CONFIG_CC_OPTIMIZE_FOR_DEBUG)
And when you enable CONFIG_CC_OPTIMIZE_FOR_DEBUG and FPGA you get compilation
warnings.
drivers/fpga/fpga.c:265:5: error: redefinition of 'fpga_load'
265 | int fpga_load(int devnum, const void *buf, size_t bsize, bitstream_type
bstype,
| ^
In file included from include/xilinx.h:7,
from drivers/fpga/fpga.c:11:
include/fpga.h:64:19: note: previous definition of 'fpga_load' with type
'int(int, const void *, size_t, bitstream_type, int)' {aka 'int(int, const
void *, long unsigned int, bitstream_type, int)'}
64 | static inline int fpga_load(int devnum, const void *buf, size_t bsize,
| ^
make[2]: *** [scripts/Makefile.build:257: drivers/fpga/fpga.o] Error 1
I means please tune that condition properly not to create additional compilation
warnings for other combinations.
Thanks,
Michal
+static inline int fpga_load(int devnum, const void *buf, size_t bsize,
+ bitstream_type bstype, int flags)
+{
+ return -ENOSYS;
+}
+#else
int fpga_load(int devnum, const void *buf, size_t bsize,
bitstream_type bstype, int flags);
+#endif
+
int fpga_fsload(int devnum, const void *buf, size_t size,
fpga_fs_info *fpga_fsinfo);
int fpga_loads(int devnum, const void *buf, size_t size,
Re: [PATCH RESEND v3] fpga: add inline stub for fpga_load
On 8/25/23 10:21, Michal Simek wrote:
Hi,
On 8/8/23 12:22, Eugen Hristev wrote:
In case CC_OPTIMIZE_FOR_DEBUG is set, unused code will not be
optimized out,
hence the reference to fpga_load will be compiled.
if DM_FPGA and SPL_FPGA are not set, the build will fail with :
aarch64-none-linux-gnu-ld.bfd: common/spl/spl_fit.o: in function
`spl_fit_upload_fpga':
u-boot/common/spl/spl_fit.c:595: undefined reference to `fpga_load'
To solve this, added an inline empty stub in the header if
CC_OPTIMIZE_FOR_DEBUG is set such that the build will succeed.
Signed-off-by: Eugen Hristev
---
Changes in v3:
- return -ENOSYS
Changes in v2:
- this is a rework as suggested by Simon of this previous patch :
https://patchwork.ozlabs.org/project/uboot/patch/20230619102839.277902-1-eugen.hris...@collabora.com/
include/fpga.h | 9 +
1 file changed, 9 insertions(+)
diff --git a/include/fpga.h b/include/fpga.h
index ed688cc0fa3b..33d0dbbe2ba4 100644
--- a/include/fpga.h
+++ b/include/fpga.h
@@ -60,8 +60,17 @@ int fpga_add(fpga_type devtype, void *desc);
int fpga_count(void);
const fpga_desc *const fpga_get_desc(int devnum);
int fpga_is_partial_data(int devnum, size_t img_len);
+#if defined(CONFIG_CC_OPTIMIZE_FOR_DEBUG)
+static inline int fpga_load(int devnum, const void *buf, size_t bsize,
+ bitstream_type bstype, int flags)
+{
+ return -ENOSYS;
+}
+#else
int fpga_load(int devnum, const void *buf, size_t bsize,
bitstream_type bstype, int flags);
+#endif
+
int fpga_fsload(int devnum, const void *buf, size_t size,
fpga_fs_info *fpga_fsinfo);
int fpga_loads(int devnum, const void *buf, size_t size,
Actually there is another patch targeting the same code.
Please take a look at
https://lore.kernel.org/r/20230816065437.836392-1-chanho61.p...@samsung.com
and get Chanho on board and create a patch to cover both cases.
Thanks,
Michal
Hello Michal,
Looking at Chanho's patch, it appears that he is solving the same issue.
These are not two separate cases.
He fixed it the other way around : if CONFIG_FPGA define the right
prototype, else define the stub, while I did if CONFIG_DEBUG define
stub, else define the right prototype.
I think you should select the patch that you deem more appropriate to
fix this, I don't mind which one.
Thanks,
Eugen
Re: [PATCH RESEND v3] fpga: add inline stub for fpga_load
Hi,
On 8/8/23 12:22, Eugen Hristev wrote:
In case CC_OPTIMIZE_FOR_DEBUG is set, unused code will not be optimized out,
hence the reference to fpga_load will be compiled.
if DM_FPGA and SPL_FPGA are not set, the build will fail with :
aarch64-none-linux-gnu-ld.bfd: common/spl/spl_fit.o: in function
`spl_fit_upload_fpga':
u-boot/common/spl/spl_fit.c:595: undefined reference to `fpga_load'
To solve this, added an inline empty stub in the header if
CC_OPTIMIZE_FOR_DEBUG is set such that the build will succeed.
Signed-off-by: Eugen Hristev
---
Changes in v3:
- return -ENOSYS
Changes in v2:
- this is a rework as suggested by Simon of this previous patch :
https://patchwork.ozlabs.org/project/uboot/patch/20230619102839.277902-1-eugen.hris...@collabora.com/
include/fpga.h | 9 +
1 file changed, 9 insertions(+)
diff --git a/include/fpga.h b/include/fpga.h
index ed688cc0fa3b..33d0dbbe2ba4 100644
--- a/include/fpga.h
+++ b/include/fpga.h
@@ -60,8 +60,17 @@ int fpga_add(fpga_type devtype, void *desc);
int fpga_count(void);
const fpga_desc *const fpga_get_desc(int devnum);
int fpga_is_partial_data(int devnum, size_t img_len);
+#if defined(CONFIG_CC_OPTIMIZE_FOR_DEBUG)
+static inline int fpga_load(int devnum, const void *buf, size_t bsize,
+ bitstream_type bstype, int flags)
+{
+ return -ENOSYS;
+}
+#else
int fpga_load(int devnum, const void *buf, size_t bsize,
bitstream_type bstype, int flags);
+#endif
+
int fpga_fsload(int devnum, const void *buf, size_t size,
fpga_fs_info *fpga_fsinfo);
int fpga_loads(int devnum, const void *buf, size_t size,
Actually there is another patch targeting the same code.
Please take a look at
https://lore.kernel.org/r/20230816065437.836392-1-chanho61.p...@samsung.com
and get Chanho on board and create a patch to cover both cases.
Thanks,
Michal
