Re: [PATCH V2 1/4] drivers/fpga/amd: Add new driver amd versal-pci

[Yidong Zhang <yidong.zhang@amd.com>]

On 3/2/25 23:57, Xu Yilun wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On Sat, Mar 01, 2025 at 11:03:29AM -0800, Yidong Zhang wrote:
>>
>>
>> On 3/1/25 00:20, Xu Yilun wrote:
>>> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>>>
>>>
>>>> My last question for this topic:
>>>> If we decide to upstream both userPF and mgmtPF driver together, could them
>>>> be both within the drivers/fpga/amd as in-tree driver? This will help user
>>>
>>> I don't look into your full driver stack. Generally, if your drivers are
>>> all about reprogramming, then yes. If they are also about all kinds of
>>> accelaration functions you'd better split them out in different domains.
>>> I may not have enough knowledge to make them correct.
>>>
>>
>> The driver has more features than just re-programing. The re-programing is
>> already done in the embedded firmware that's why the mgmtPF driver is just a
>> utility driver.
>>
>> The userPF driver has features such as:
>>    xdma (already in drivers/xilinx/xdma as platform driver)
>>    qdma (already in drivers/amd/qdma as platform driver)
>>    mailbox and more which have not been upstreamed in linux kernel yet.
>>
>> The driver architecture is:
>>
>>    userPF driver (as pci_driver)
>>      qdma (as platform_driver)
>>      ..
>>      mailbox (as platform_driver)
>>         /\
>>         ||
>>         \/
>>      mailbox (as platform_driver)
>>    mgmtPF driver (as pci_driver)
>>         /\
>>         ||
>>         \/
>>      Embedded firmware (re-programing done here)
>>
>> Right now, I am working on upstreaming the mgmtPF driver as pci_driver.
>> In the future, I think the userPF driver should be fitting into the
>> "drivers/fpga", given that should manage all these platform_drivers and
> 
> No I think userPF driver should manage all these *platform_devices*.
Yes, userPF driver manage all these "platform_devices" by leveraging the 
fpga_region callback ops. That's why I am thinking the userPF should be 
in drivers/fpga.

> Platform_drivers could be independent and put into proper domain folders.
Yes. All these "platform_devices" - drivers - should be put into proper 
domain. Aka, not in the drivers/fpga.

> 
>> utilize the fpga_region callbacks to online/offline services due to hardware
> 
> fpga_region should online/offline platform devices. Not services, which is the
> job of each platform_driver.

Yes. I think we are thinking the same concept - online/offline platform 
devices. Not the services like probe/remove. The online/offline are 
separate ops just designed for the platform_devices to be online/offline.

Thanks,
David

> 
> Thanks,
> Yilun
> 
>> changes after re-programing.
>>
>> Thanks,
>> David
>>
>>> Thanks,
>>> Yilun
>>>
>>>> find source code easily.