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

Xu Yilun
2025-04-24T10:06:48Z

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:
> > 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

I don't think this should be a property for fpga_mgr device. It is for
FPGA reprogramming. You insmod the reprograming driver not for
reprogramming, just to enable the already programmed functionality.

My idea is, to load the fpga_region with an image tagged "external-fpga-config".

Thanks,
Yilun

> 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
>