Re: [PATCH] fpga: define dummy fpga_load function for debug build

[Michal Simek <michal.simek@amd.com>]

Hi,

On 8/28/23 12:00, Chanho Park wrote:
> Hi,
> 
>> -----Original Message-----
>> From: Eugen Hristev <eugen.hristev@collabora.com>
>> Sent: Monday, August 28, 2023 5:47 PM
>> To: Chanho Park <chanho61.park@samsung.com>; 'Michal Simek'
>> <michal.simek@amd.com>; u-boot@lists.denx.de
>> Cc: Simon Glass <sjg@chromium.org>
>> Subject: Re: [PATCH] fpga: define dummy fpga_load function for debug build
>>
>> On 8/28/23 03:21, Chanho Park wrote:
>>> Hi,
>>>
>>>> -----Original Message-----
>>>> From: Michal Simek <michal.simek@amd.com>
>>>> Sent: Friday, August 25, 2023 4:23 PM
>>>> To: Chanho Park <chanho61.park@samsung.com>; u-boot@lists.denx.de
>>>> Subject: Re: [PATCH] fpga: define dummy fpga_load function for debug
>>>> build
>>>>
>>>> Hi,
>>>>
>>>> On 8/16/23 08:54, Chanho Park wrote:
>>>>> This fixes below build error when CC_OPTIMIZE_FOR_DEBUG is enabled
>>>>> and CONFIG_SPL_FPGA is not enabled.
>>>>>
>>>>> ../common/spl/spl_fit.c:591: undefined reference to `fpga_load'
>>>>> collect2: error: ld returned 1 exit status
>>>>>
>>>>> Signed-off-by: Chanho Park <chanho61.park@samsung.com>
>>>>> ---
>>>>>     include/fpga.h | 8 ++++++++
>>>>>     1 file changed, 8 insertions(+)
>>>>>
>>>>> diff --git a/include/fpga.h b/include/fpga.h index
>>>>> ed688cc0fa3b..44f2755a3f10 100644
>>>>> --- a/include/fpga.h
>>>>> +++ b/include/fpga.h
>>>>> @@ -60,8 +60,16 @@ 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 CONFIG_IS_ENABLED(FPGA)
>>>>>     int fpga_load(int devnum, const void *buf, size_t bsize,
>>>>>     	      bitstream_type bstype, int flags);
>>>>> +#else
>>>>> +static inline int fpga_load(int devnum, const void *buf, size_t bsize,
>>>>> +	      bitstream_type bstype, int flags) {
>>>>> +	return FPGA_FAIL;
>>>>> +}
>>>>> +#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,
>>>>
>>>> There is another patch targeting the same code.
>>>> Please take a look at
>>>> https://lore.kernel.org/r/20230808102227.34233-1-
>>>> eugen.hristev@collabora.com
>>>
>>> I wasn't aware that there was an attempt to fix the issue. If I knew it,
>> I would reply the patch...
>>>
>>>>
>>>> and work together to come up with the patch which covers both cases.
>>>
>>> Yes. I also tried to make the patch with CONFIG_CC_OPTIMIZE_FOR_DEBUG
>> guard but I couldn't find any codes that uses the guard.
>>> And I was also worried about the CC_OPTIMIZE_FOR_DEBUG's behavior. It
>> could be related with the optimization level of the compiler.
>>> That's why I put the guard with #if CONFIG_IS_ENABLED(FPGA).
>>>
>>> Best Regards,
>>> Chanho Park
>>>
>>
>>
>> Hi Chanho,
>>
>> Simon suggested to use CONFIG_CC_OPTIMIZE_FOR_DEBUG in this case here:
>>
>> https://protect2.fireeye.com/v1/url?k=a911b2cb-c86a1843-a9103984-
>> 74fe4860018a-146c33e2b586dc8a&q=1&e=e288ab48-75e8-400b-91c9-
>> e8bd95520918&u=https%3A%2F%2Fpatchwork.ozlabs.org%2Fproject%2Fuboot%2Fpatc
>> h%2F20230619102839.277902-1-eugen.hristev%40collabora.com%2F
>>
>> As I told Michal, basically your patch and my patch do the same thing.
>> I suggested him to pick the one that he thinks it's best. (or even suggest
>> another way)
> 
> I agree. Either way, I'm fine with it.

ok. I prefer Eugen patch and Chanho please send your tag against that patch.
And let me comment Eugen one.

Thanks,
Michal