Re: [PATCH 1/3] dt-bindings: fpga: stratix10: add support for Agilex5

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2775 bytes --]

On Tue, Nov 11, 2025 at 05:03:56PM +0800, Xu Yilun wrote:
> On Tue, Nov 11, 2025 at 03:48:31PM +0800, Khairul Anuar Romli wrote:
> > The Agilex 5 SoC FPGA manager introduces updated hardware features and
> > register maps that require explicit binding support to enable correct
> > initialization and control through the FPGA manager subsystem.
> > 
> > It allows FPGA manager drivers detect and configure Agilex 5 FPGA managers
> > properly. This changes also keep device tree bindings up to date with
> > hardware platforms changes.
> > 
> > Signed-off-by: Khairul Anuar Romli <khairul.anuar.romli@altera.com>
> > ---
> >  .../devicetree/bindings/fpga/intel,stratix10-soc-fpga-mgr.yaml   | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/fpga/intel,stratix10-soc-fpga-mgr.yaml b/Documentation/devicetree/bindings/fpga/intel,stratix10-soc-fpga-mgr.yaml
> > index 6e536d6b28a9..b531522cca07 100644
> > --- a/Documentation/devicetree/bindings/fpga/intel,stratix10-soc-fpga-mgr.yaml
> > +++ b/Documentation/devicetree/bindings/fpga/intel,stratix10-soc-fpga-mgr.yaml
> > @@ -23,6 +23,7 @@ properties:
> >      enum:
> >        - intel,stratix10-soc-fpga-mgr
> >        - intel,agilex-soc-fpga-mgr
> > +      - intel,agilex5-soc-fpga-mgr
> 
> I'm not quite familar about dt rules, but since these are all software
> defined strings, do we really have to write down every version even if
> they behave exactly the same? Seems a waste of time.

Sort of. You "have" to add a new string to the binding, because there
may end up being bugs that only manifest in one SoC etc, and having the
string pre-emptively is helpful. I'm not familiar with these particular
fpga managers, but if it has some way of identifying it's own version,
then doing that dynamically instead of having specific compatibles is
often permitted.

What is a waste of time is adding it to the driver. When the driver diff
looks like:
|--- a/drivers/fpga/stratix10-soc.c
|+++ b/drivers/fpga/stratix10-soc.c
|@@ -448,6 +448,7 @@ static void s10_remove(struct platform_device *pdev)
| static const struct of_device_id s10_of_match[] = {
|        {.compatible = "intel,stratix10-soc-fpga-mgr"},
|        {.compatible = "intel,agilex-soc-fpga-mgr"},
|+       {.compatible = "intel,agilex5-soc-fpga-mgr"},
|        {},
| };

That's a solid indicator that there should be a fallback compatible used
here, as the devices behave identically. A devicetree node would then
contain
compatible = "intel,agilex5-soc-fpga-mgr", "intel,agilex-soc-fpga-mgr";
instead of
compatible = "intel,agilex5-soc-fpga-mgr";
and the driver would match on the second compatible in the list.

pw-bot: changes-requested

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]