On 10/5/21 4:03 PM, Jorge Ramirez-Ortiz, Foundries wrote:
> On 05/10/21, Michal Simek wrote:
>>
>>
>> On 10/5/21 1:13 PM, Jorge Ramirez-Ortiz wrote:
>>> Add new compatible string u-boot,zynqmp-fpga-ddrauth to handle this
>>> use case.
>>>
>>> Signed-off-by: Jorge Ramirez-Ortiz <jo...@foundries.io>
>>> ---
>>> drivers/fpga/xilinx.c | 29 +++++++++++++++++++++++++----
>>> drivers/fpga/zynqmppl.c | 4 ++--
>>> include/xilinx.h | 2 +-
>>> 3 files changed, 28 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/fpga/xilinx.c b/drivers/fpga/xilinx.c
>>> index cbebefb55f..c8035105e2 100644
>>> --- a/drivers/fpga/xilinx.c
>>> +++ b/drivers/fpga/xilinx.c
>>> @@ -135,23 +135,44 @@ int fpga_loadbitstream(int devnum, char *fpgadata,
>>> size_t size,
>>> dataptr += 4;
>>> printf(" bytes in bitstream = %d\n", swapsize);
>>>
>>> - return fpga_load(devnum, dataptr, swapsize, bstype);
>>> + return fpga_load(devnum, dataptr, swapsize, bstype, NULL);
>>> }
>>>
>>> int xilinx_load(xilinx_desc *desc, const void *buf, size_t bsize,
>>> - bitstream_type bstype)
>>> + bitstream_type bstype, const char *compatible)
>>> {
>>> + struct fpga_secure_info info = { 0 };
>>> +
>>> if (!xilinx_validate (desc, (char *)__FUNCTION__)) {
>>> printf ("%s: Invalid device descriptor\n", __FUNCTION__);
>>> return FPGA_FAIL;
>>> }
>>>
>>> - if (!desc->operations || !desc->operations->load) {
>>> + if (!desc->operations) {
>>> printf("%s: Missing load operation\n", __func__);
>>> return FPGA_FAIL;
>>> }
>>>
>>> - return desc->operations->load(desc, buf, bsize, bstype);
>>> + if (!compatible || !strcmp(compatible, "u-boot,fpga-legacy")) {
>>> + if (!desc->operations->load) {
>>> + printf("%s: Missing load operation\n", __func__);
>>> + return FPGA_FAIL;
>>> + }
>>> + return desc->operations->load(desc, buf, bsize, bstype);
>>> + }
>>> +
>>> + if (!strcmp(compatible, "u-boot,zynqmp-fpga-ddrauth")) {
>>> + if (!desc->operations->loads) {
>>> + printf("%s: Missing load operation\n", __func__);
>>> + return FPGA_FAIL;
>>> + }
>>> + /* DDR authentication */
>>> + info.authflag = 1;
>>> + return desc->operations->loads(desc, buf, bsize, &info);
>>> + }
>>
>> I am not sure about this solution. First of all it forces SPL to have
>> additional code which just extending size. It means there must be a way
>> to enable/disable it.
>
> that can be contained of course...so not really an issue (ie compile
> out either fpga_load or fpga_loads depending on needs).
>
>>
>> The next thing is that you have zynqmp string in generic xilinx file
>> which doesn't look right. It would be better to deal with image types
>> directly in driver which is capable to handle it.
>
> sure that is easy..but the idea is the same
We just need to find out how this should be properly propagated and stored.
>
>> It means record fit image compatible string in the driver with
>> hooks/flags and determined what should be called.
>
> what about the compatible names? will you define those? or should I do
> it?
Just define it and let's see if this fits.
>
> For our use case, we need DDR authentication so that must be
> defined. I can add others and someone else can add the support needed?
>
Sure. None is asking you to support everything. Just add support for
regular bitstreams and your case. And adding support for something else
will be just +1 if interface is right.
Thanks,
Michal
