On 03/03/2025 11:31, Conor Dooley wrote:
> 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@xxxxxxxxxx> 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", },
> |+ {}
Yes, except that one of the devices should be the fallback, not generic
"fpga".
Best regards,
Krzysztof
