Re: [v4 2/3] dt-bindings: fpga: Add Efinix SPI programming bindings

[Conor Dooley <conor.dooley@microchip.com>]

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

On Mon, Mar 03, 2025 at 11:10:53AM +0100, Ian Dannapel wrote:
> Hi Conor, thanks for the quick response.
> 
> On Fri, Feb 28, 2025 at 7:28 PM Conor Dooley <conor@kernel.org> wrote:
> > > +description: |
> > > +  Efinix FPGAs (Trion, Topaz, and Titanium families) support loading bitstreams
> > > +  through "SPI Passive Mode".
> > > +  Note 1: Only bus width 1x is supported.
> > > +  Note 2: Additional pins hogs for bus width configuration must be set
> > > +  elsewhere, if necessary.
> > > +  Note 3: Topaz and Titanium support is based on documentation but remains
> > > +  untested.
> >
> > Points 1 and 3 here seem to be driver limitations, and shouldn't really
> > be present in a document describing the hardware?
> >
> Yes, they are driver limitations and probably do not belong here.
> 
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - efinix,trion-spi
> > > +      - efinix,titanium-spi
> > > +      - efinix,topaz-spi
> >
> > > +      - efinix,fpga-spi
> >
> > What hardware does this device represent? Other ones are obvious matches
> > to the families you mention, but what is this one?

> The proposed compatible is a generic fallback for any Efinix FPGA Series.

If it is a fallback, your binding should look like:
compatible:
  items:
    - enum:
        - efinix,trion-spi
        - efinix,titanium-spi
        - efinix,topaz-spi
    - const: efinix,fpga-spi

|+static const struct of_device_id efinix_spi_of_match[] = {
|+       { .compatible = "efinix,trion-spi", },
|+       { .compatible = "efinix,titanium-spi", },
|+       { .compatible = "efinix,topaz-spi", },

And these three compatibles can/should be removed from the driver, since
the fallback is required.

|+       { .compatible = "efinix,fpga-spi", },
|+       {}
|+};


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