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

[Krzysztof Kozlowski <krzk@kernel.org>]

On 28/09/2024 14:33, Ian Dannapel wrote:
>>>>
>>>>> +
>>>>> +  spi-cpha: true
>>>>> +
>>>>> +  spi-cpol: true
>>>>> +
>>>>> +  spi-max-frequency:
>>>>> +    maximum: 25000000
>>>>> +
>>>>> +  reg:
>>>>> +    maxItems: 1
>>>>> +
>>>>> +  creset-gpios:
>>>>
>>>> reset-gpios
>>>>
>>>> Do not invent own properties.
>>>>
>>>>> +    description:
>>>>> +      reset and re-configuration trigger pin (low active)
>>>>> +    maxItems: 1
>>>>> +
>>>>> +  cs-gpios:
>>>>> +    description:
>>>>> +      chip-select pin (low active)
>>>>
>>>> Eee? That's a property of controller, not child. Aren't you duplicating
>>>> existing controller property?
>>> This device uses this pin in combination with the reset to enter the
>>> programming mode. Also, the driver must guarantee that the pin is
>>
>> Isn't this the same on every SPI device?
> Yes, but I was not very clear. In this case the pin must be hold
> active including entering the programming mode. And if the controller

Just like every CS, no?

The only difference is that you must send entire programming sequence
without releasing the CS.

> transfers the data in bursts, the pin is also not allowed to go
> inactive between transfer bursts.
>>
>>> active for the whole transfer process, including ending dummy bits.
>>> This is why I added a warning to NOT use this driver with other
>>> devices on the same bus.
>>
>> Not really related. None of this grants exception from duplicating
>> controller's property.
>>
>> How do you think it will even work in Linux, if same GPIO is requested
>> twice (imagine controller also has it)? Till now, this would be -EBUSY.
> I expected that the controller is not able request the same gpio. From
> the controller point of view, it is a device that does not have a chip
> select. Not sure if the controller would be able to get to this gpio
> if it is not explicitly given.

But it could be given. Don't think only about your case.

Your description earlier clearly suggests it is CS. Description here
suggests it is not a CS.

No clue then.

>>
>> But regardless of implementation, I still do not understand why do you
>> need duplicate same chip-select. Maybe just the naming is the confusion,
>> dunno.
> This could be an option to make the difference to a "real chip-select"
> clear, but it would drift away from the datasheet naming. Eg,
> prog-select?

Please go back to datasheet. Which pin is this? CS, yes or not? If not,
then which other pin is CS?

Best regards,
Krzysztof