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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f57ca7c6-cb60-42cd-bba1-b48144bdef14@kernel.org>
Date: Sat, 28 Sep 2024 14:53:44 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Ian Dannapel <iansdannapel@...il.com>
Cc: mdf@...nel.org, hao.wu@...el.com, yilun.xu@...el.com, trix@...hat.com,
 robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 neil.armstrong@...aro.org, heiko.stuebner@...rry.de, rafal@...ecki.pl,
 linus.walleij@...aro.org, linux-fpga@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] dt-bindings: fpga: Add Efinix serial SPI programming
 bindings

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