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

Sam Winchenbach
2025-03-31T13:07:23Z

On Mon, Mar 31, 2025 at 02:43:59PM +0200, Krzysztof Kozlowski wrote:
> On 31/03/2025 14:30, Sam Winchenbach wrote:
> >>
> >>> + type: boolean
> >>> + description: If present, the ICAP controller will be enabled when
> >>> + the driver probes. This is useful if the fabric is loaded
> >>> + during the boot process and contains a core, such as the SEM,
> >>
> >> I don't get how this is suitable for DT. If you decide to load the
> >> fabric from driver, that's driver decision so not DT.
> >
> > 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?

Let me know your thoughts,
-Sam

>
> Best regards,
> Krzysztof