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

Krzysztof Kozlowski
2025-04-01T06:17:59Z

On Mon, Mar 31, 2025 at 09:07:03AM -0400, Sam Winchenbach wrote:
> On Mon, Mar 31, 2025 at 02:43:59PM +0200, Krzysztof Kozlowski wrote:
> > 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?

Neither on-load nor on-probe, because again you instruct the OS what it
should do. You should instead describe the hardware (or other parts of
software stack). Describe the condition, the hardware feature, the
characteristic observed. With proper phrasing the property should be
fine, but I still do not see that its name and description match actual
hardware.

Best regards,
Krzysztof