Re: [PATCH 1/2] dt-bindings: fpga: zynq: Document ICAP on boot

[Krzysztof Kozlowski <krzk@kernel.org>]

On 31/03/2025 15:07, Sam Winchenbach wrote:
>>> Before writing the fabric to the FPGA the driver disables the ICAP, enabling
>>> the PCAP. Once writing is complete it unconditionally disables the PCAP,
>>> enabling the ICAP. This patch just makes it so, depending on the use case,
>>> the ICAP can be enabled at boot. This will not prevent the system from being
>>> able to load a fabric through the driver. I added in this boolean so existing
>>> behavior would be maintained.
>>>
>>> Do you recommend another approach such as writing to a sysfs attribute to
>>> switch from PCAP to ICAP?
>> Not sure yet. Can't you check the status of ICAP before programming and
>> then enable it only if was enabled before?
> 
> I am having a bit of difficulty understanding this so let's talk about cases
> where the ICAP is enabled/disabled -
> 
> 1. When writing the fabric from the driver
>    In this situation it might make sense to read the state of the ICAP
>    interface when preparing the fabric, before enabling PCAP. When the write
>    completes you could re-enable the ICAP if it was previously enabled.
> 
>    This might be outside the scope of this change - and I am not comfortable
>    enough with this use-case to understand potential side effects from doing
>    this. Logically it makes sense, but there may be a very specific reason that
>    the ICAP must be enabled after doing a fabric load or partial
>    reconfiguration.
> 
> 2. When the FPGA driver loads and is probed by the DTS
>    In this situation, which is covered by this patch, the FPGA is loaded by
>    BootROM/FSBL but contains functionality that requires the ICAP. Unless the
>    user has made modifications to the FSBL or 3rd stage bootloader there is no
>    clear way to enable the ICAP interface. Checking to see if it had been
>    enabled prior to loading this driver does not (in my opinion) make a lot of
>    sense here.
> 
>    Perhaps the name of the DTS is confusing? The suffix '-on-load' was meant to
>    indicate when the driver was loaded, not the fabric. Would the suffix
>    '-on-probe' be more clear?
None of these two, because you refer to software. Property is fine but
you need to describe the actual state of hardware or system or entire
stack, e.g. "fpga-with-sem".
Best regards,
Krzysztof