Re: [PATCH v11 10/13] fpga: zynqmp: optimize zynqmppl_load() code

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

On 7/8/22 15:15, Oleksandr Suvorov wrote:
> Hi Michal,
> 
> On Fri, Jul 8, 2022 at 3:43 PM Michal Simek <michal.simek@amd.com> wrote:
>>
>>
>>
>> On 7/5/22 21:23, Oleksandr Suvorov wrote:
>>> Optimize function code preparing to add secure bitstream types
>>> support.
>>
>> Can you please extend this? I understand what you do below but better
>> description will be good.
> 
> Ok, if I'll realize how to do this :)
> 
>>>
>>> Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
>>> Tested-by: Ricardo Salveti <ricardo@foundries.io>
>>> Tested-by: Adrian Fiergolski <adrian.fiergolski@fastree3d.com>
>>> ---
>>>
>>> (no changes since v1)
>>>
>>>    drivers/fpga/zynqmppl.c | 27 +++++++++++++--------------
>>>    1 file changed, 13 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/fpga/zynqmppl.c b/drivers/fpga/zynqmppl.c
>>> index 239c498f7b5..6959b8ae97e 100644
>>> --- a/drivers/fpga/zynqmppl.c
>>> +++ b/drivers/fpga/zynqmppl.c
>>> @@ -199,46 +199,45 @@ static int zynqmp_validate_bitstream(xilinx_desc *desc, const void *buf,
>>>        return 0;
>>>    }
>>>
>>> -static int zynqmp_load(xilinx_desc *desc, const void *buf, size_t bsize,
>>> -                  bitstream_type bstype, int flags)
>>> +static int zynqmp_load(xilinx_desc *desc, const void *buf,
>>> +                    size_t bsize, bitstream_type bstype,
>>> +                    int flags)
>>
>> This is unrelated to commit. This is purely coding style change.
> 
> Ok, I'll separate to another commit.
> 
>>>    {
>>>        ALLOC_CACHE_ALIGN_BUFFER(u32, bsizeptr, 1);
>>>        u32 swap = 0;
>>>        ulong bin_buf;
>>>        int ret;
>>>        u32 buf_lo, buf_hi;
>>> +     u32 bsize_req = (u32)bsize;
>>>        u32 ret_payload[PAYLOAD_ARG_CNT];
>>> -     bool xilfpga_old = false;
>>> +
>>> +     debug("%s called!\n", __func__);
>>>
>>>        if (zynqmp_firmware_version() <= PMUFW_V1_0) {
>>>                puts("WARN: PMUFW v1.0 or less is detected\n");
>>>                puts("WARN: Not all bitstream formats are supported\n");
>>>                puts("WARN: Please upgrade PMUFW\n");
>>> -             xilfpga_old = true;
>>> -             if (zynqmp_validate_bitstream(desc, buf, bsize, bsize, &swap))
>>> +             if (zynqmp_validate_bitstream(desc, buf, bsize,
>>> +                                           bsize, &swap))
>>
>> This is also coding style change only.
> 
> Ok.
> 
>>>                        return FPGA_FAIL;
>>>                bsizeptr = (u32 *)&bsize;
>>>                flush_dcache_range((ulong)bsizeptr,
>>>                                   (ulong)bsizeptr + sizeof(size_t));
>>> +             bsize_req = (u32)(uintptr_t)bsizeptr;
>>>                bstype |= BIT(ZYNQMP_FPGA_BIT_NS);
>>> +     } else {
>>> +             bstype = 0;
>>>        }
>>>
>>>        bin_buf = zynqmp_align_dma_buffer((u32 *)buf, bsize, swap);
>>>
>>> -     debug("%s called!\n", __func__);
>>
>> nit: And this also has nothing to do with optimization. You just changed location.
> 
> Michal, what is this closer to? Refactor?
> It's not only about changing location. Now there is only one call of
> xilinx_pm_request().

I know but it s not described in commit message. Diff was really just passing 
different argument. You just change the logic about them where you use clear 
bstype for secure bitstreams and align bsize_req

But you are not changing logic around bsizeptr where old xilfpga expects size to 
be a pointer not value.

Thanks,
Michal

commit 31bcb3444cbd5002ca9d8f6a3a2644092748cdba
Author:     Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com>
AuthorDate: Thu Mar 15 00:17:24 2018 +0530
Commit:     Michal Simek <michal.simek@amd.com>
CommitDate: Mon Apr 9 12:14:50 2018 +0200

     fpga: zynqmp: Fix the nonsecure bitstream loading issue

     Xilfpga library expects the size of bitstream in a pointer
     but currenly we are passing the size as a value. This patch
     fixes this issue.

     Signed-off-by: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
     Signed-off-by: Nava kishore Manne <navam@xilinx.com>
     Signed-off-by: Michal Simek <michal.simek@xilinx.com>