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] [day] [month] [year] [list]
Message-ID: <1368312a-4f48-4363-b97c-5f0675f721f7@kernel.org>
Date: Thu, 3 Oct 2024 09:47:05 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: "Mahapatra, Amit Kumar" <amit.kumar-mahapatra@....com>,
 Conor Dooley <conor@...nel.org>
Cc: "broonie@...nel.org" <broonie@...nel.org>,
 "robh@...nel.org" <robh@...nel.org>, "krzk+dt@...nel.org"
 <krzk+dt@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
 "Simek, Michal" <michal.simek@....com>,
 "linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
 "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
 "linux-arm-kernel@...ts.infradead.org"
 <linux-arm-kernel@...ts.infradead.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "git (AMD-Xilinx)" <git@....com>,
 "amitrkcian2002@...il.com" <amitrkcian2002@...il.com>
Subject: Re: [PATCH] dt-bindings: spi: xilinx: Add clocks & clock-names
 properties

On 03/10/2024 09:42, Mahapatra, Amit Kumar wrote:
> Hello Krzysztof,
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzk@...nel.org>
>> Sent: Thursday, October 3, 2024 12:31 PM
>> To: Mahapatra, Amit Kumar <amit.kumar-mahapatra@....com>; Conor Dooley
>> <conor@...nel.org>
>> Cc: broonie@...nel.org; robh@...nel.org; krzk+dt@...nel.org;
>> conor+dt@...nel.org; Simek, Michal <michal.simek@....com>; linux-
>> spi@...r.kernel.org; devicetree@...r.kernel.org; linux-arm-
>> kernel@...ts.infradead.org; linux-kernel@...r.kernel.org; git (AMD-Xilinx)
>> <git@....com>; amitrkcian2002@...il.com
>> Subject: Re: [PATCH] dt-bindings: spi: xilinx: Add clocks & clock-names properties
>>
>> On 03/10/2024 08:23, Mahapatra, Amit Kumar wrote:
>>> Hello Conor,
>>>
>>>> -----Original Message-----
>>>> From: Conor Dooley <conor@...nel.org>
>>>> Sent: Monday, September 30, 2024 10:10 PM
>>>> To: Mahapatra, Amit Kumar <amit.kumar-mahapatra@....com>
>>>> Cc: Krzysztof Kozlowski <krzk@...nel.org>; broonie@...nel.org;
>>>> robh@...nel.org;
>>>> krzk+dt@...nel.org; conor+dt@...nel.org; Simek, Michal
>>>> <michal.simek@....com>; linux-spi@...r.kernel.org;
>>>> devicetree@...r.kernel.org; linux-arm-kernel@...ts.infradead.org;
>>>> linux-kernel@...r.kernel.org; git (AMD-Xilinx) <git@....com>;
>>>> amitrkcian2002@...il.com
>>>> Subject: Re: [PATCH] dt-bindings: spi: xilinx: Add clocks &
>>>> clock-names properties
>>>>
>>>> On Mon, Sep 30, 2024 at 03:44:47PM +0000, Mahapatra, Amit Kumar wrote:
>>>>> Hello Conor,
>>>>>
>>>>>>>>>>> Subject: Re: [PATCH] dt-bindings: spi: xilinx: Add clocks &
>>>>>>>>>>> clock-names properties
>>>>>>>>>>>
>>>>>>>>>>> On Mon, Sep 23, 2024 at 06:02:42PM +0530, Amit Kumar Mahapatra
>>>> wrote:
>>>>>>>>>>>> Include the 'clocks' and 'clock-names' properties in the AXI
>>>>>>>>>>>> Quad-SPI bindings. When the AXI4-Lite interface is enabled,
>>>>>>>>>>>> the core operates in legacy mode, maintaining backward
>>>>>>>>>>>> compatibility with version 1.00, and uses 's_axi_aclk' and
>>>>>>>>>>>> 'ext_spi_clk'. For the AXI interface, it uses 's_axi4_aclk'
>>>>>>>>>>>> and
>>>> 'ext_spi_clk'.
>>>>>>
>>>>>>>>>>>> +      properties:
>>>>>>>>>>>> +        clock-names:
>>>>>>>>>>>> +          items:
>>>>>>>>>>>> +            - const: s_axi_aclk
>>>>>>>>>>>> +            - const: ext_spi_clk
>>>>>>>>>>>
>>>>>>>>>>> These are all clocks, there should be no need to have "clk" in the
>> names.
>>>>>>>>>>
>>>>>>>>>> These are the names exported by the IP and used by the DTG.
>>>>>>>>>
>>>>>>>>> So? This is a binding, not a verilog file.
>>>>>>>>
>>>>>>>> Axi Quad SPI is an FPGA-based IP, and the clock names are derived
>>>>>>>> from the IP signal names as specified in the IP documentation [1].
>>>>>>>> We chose these names to ensure alignment with the I/O signal
>>>>>>>> names listed in Table 2-2 on page 19 of [1].
>>>>>>>>
>>>>>>>> [1]
>>>>>>>> chrome-extension://efaidnbmnnnibpcajpcglclefindmkaj/https://www.amd.
>>>>>>>> com/content/dam/xilinx/support/documents/ip_documentation/axi_qu
>>>>>>>> ad_s
>>>>>>>> pi/v3_2/pg153-axi-quad-spi.pdf
>>>>>>>
>>>>>>> So if hardware engineers call them "pink_pony_clk_aclk_really_clk"
>>>>>>> we should follow...
>>>>>>>
>>>>>>>  - bus or axi
>>>>>>>  - ext_spi or spi
>>>>>>>
>>>>>>> You have descriptions of each item to reference real signals.
>>>>>>> Conor's comment is valid - do no make it verilog file.
>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>>>> +
>>>>>>>>>>>> +    else:
>>>>>>>>>>>> +      properties:
>>>>>>>>>>>> +        clock-names:
>>>>>>>>>>>> +          items:
>>>>>>>>>>>> +            - const: s_axi4_aclk
>>>>>>>>>>>> +            - const: ext_spi_clk
>>>>>>>
>>>>>>> Nah, these are the same.
>>>>>>
>>>>>> They may be different, depending on whether or not the driver has
>>>>>> to handle "axi4- lite" versus "axi" differently. That said, I find
>>>>>> the commit message kinda odd in that it states that axi4-lite goes
>>>>>> with the s_axi_aclk
>>>> clock and axi goes with s_axi4_aclk.
>>>>>
>>>>> Apologies for the typo. When the AXI4 interface is enabled, it uses
>>>>> s_axi4_aclk, and when the AXI4-Lite interface is enabled, it uses s_axi_aclk.
>>>>>
>>>>> In my next series I will update my commit message & change the
>>>>> clock-names 's_axi4_aclk', 's_axi_aclk' & 'ext_spi_clk' to 'axi4',
>>>>> 'axi' & 'ref' respectively
>>>>
>>>> There's no driver here, so it is hard to know (why isn't there?) -
>>>> are you using the axi
>>>
>>> We are working on the driver. Once it is ready we will send it to upstream.
>>
>> Why would you send separate binding from driver? That's only making everything
>> more difficult...
> 
> Alright, I will send the next version along with the driver changes.
> 
>>
>>>
>>>> v axi4 to do some sort of differentiation in the driver?
>>> In the driver we don't do any different operations based on the clocks
>>> , we simply enable the available clocks in the driver.
>>
>> So it is the same clock?
> 
> These are two different clocks and depending on the mode—Legacy or 
> Enhanced—either clock will be enabled, but the purpose of both the 
> clocks are the same. We can have the name 'axi' for both the 
> clocks (axi & axi4). Please let me know if you are fine with this.

Please read my first comment in this thread. We are dragging this way
too much...

Best regards,
Krzysztof


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ