| Message ID | 20230808102227.34233-1-eugen.hristev@collabora.com |
|---|---|
| State | Superseded |
| Delegated to: | Michal Simek |
| Headers | show |
| Series | [RESEND,v3] fpga: add inline stub for fpga_load | expand |
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 <eugen.hristev@collabora.com> > --- > 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.hristev@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.park@samsung.com and get Chanho on board and create a patch to cover both cases. Thanks, Michal
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 <eugen.hristev@collabora.com> >> --- >> 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.hristev@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.park@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
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 <eugen.hristev@collabora.com> > --- > 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.hristev@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,
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 <eugen.hristev@collabora.com> >> --- >> 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.hristev@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,
Hi Eugen, > -----Original Message----- > From: Eugen Hristev <eugen.hristev@collabora.com> > Sent: Monday, August 28, 2023 8:47 PM > To: Michal Simek <michal.simek@amd.com>; u-boot@lists.denx.de; > sjg@chromium.org; Chanho Park <chanho61.park@samsung.com> > 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 <eugen.hristev@collabora.com> > >> --- > >> 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&q=1&e=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
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,
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 <eugen.hristev@collabora.com> --- 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.hristev@collabora.com/ include/fpga.h | 9 +++++++++ 1 file changed, 9 insertions(+)