linux-kernel - Re: [PATCH 1/3] dt-bindings: clock: fix C3 PLL input parameter

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: <1jikwczrys.fsf@starbuckisacylon.baylibre.com>
Date: Wed, 07 Aug 2024 09:56:27 +0200
From: Jerome Brunet <jbrunet@...libre.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc: Xianwei Zhao <xianwei.zhao@...ogic.com>,  Krzysztof Kozlowski
 <krzk@...nel.org>,  Neil Armstrong <neil.armstrong@...aro.org>,  Michael
 Turquette <mturquette@...libre.com>,  Stephen Boyd <sboyd@...nel.org>,
  Rob Herring <robh@...nel.org>,  Krzysztof Kozlowski <krzk+dt@...nel.org>,
  Conor Dooley <conor+dt@...nel.org>,  Chuan Liu <chuan.liu@...ogic.com>,
  Kevin Hilman <khilman@...libre.com>,  Martin Blumenstingl
 <martin.blumenstingl@...glemail.com>,  linux-amlogic@...ts.infradead.org,
  linux-clk@...r.kernel.org,  devicetree@...r.kernel.org,
  linux-kernel@...r.kernel.org,  linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 1/3] dt-bindings: clock: fix C3 PLL input parameter

On Wed 07 Aug 2024 at 07:44, Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org> wrote:

> On 07/08/2024 03:55, Xianwei Zhao wrote:
>> Hi Krzysztof,
>>      Thanks for your review.
>> 
>> On 2024/8/6 21:10, Krzysztof Kozlowski wrote:
>>> [ EXTERNAL EMAIL ]
>>>
>>> On 06/08/2024 12:27, Xianwei Zhao via B4 Relay wrote:
>>>> From: Xianwei Zhao <xianwei.zhao@...ogic.com>
>>>>
>>>> Add C3 PLL controller input clock parameters "fix".
>>>
>>> What is "parameters" here? Why you are adding it? Is it missing?
>>> Something is not working?
>>>
>> Yes. The previous submission was lost.
>
> What submission is lost?
>

He means it was just forgotten in the submission and noboby picked up on
it at the time. He is not not trying to sneak anything new. That input
is used everywhere in the driver and nothing would work without it.

It is just an honest mistake that he is trying to correct

Indeed the description could certainly detail this a bit more

>> 
>>>>
>>>> Fixes: 0e6be855a96d ("dt-bindings: clock: add Amlogic C3 PLL clock controller")
>>>
>>> Why? What bug are you fixing?
>> 
>> The input clock of PLL clock controller need the clock whose fw_name is 
>> called "fix".
>
> Then explain this in commit msg.
>
>>>
>>>> Signed-off-by: Xianwei Zhao <xianwei.zhao@...ogic.com>
>>>> ---
>>>>   Documentation/devicetree/bindings/clock/amlogic,c3-pll-clkc.yaml | 7 +++++--
>>>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/clock/amlogic,c3-pll-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,c3-pll-clkc.yaml
>>>> index 43de3c6fc1cf..700865cc9792 100644
>>>> --- a/Documentation/devicetree/bindings/clock/amlogic,c3-pll-clkc.yaml
>>>> +++ b/Documentation/devicetree/bindings/clock/amlogic,c3-pll-clkc.yaml
>>>> @@ -24,11 +24,13 @@ properties:
>>>>       items:
>>>>         - description: input top pll
>>>>         - description: input mclk pll
>>>> +      - description: input fix pll
>>>>
>>>>     clock-names:
>>>>       items:
>>>>         - const: top
>>>>         - const: mclk
>>>> +      - const: fix
>>>
>>> and that's not an ABI break because?
>> This is "fixed" clock.
>> I will modify "fix" to "fixed",in next version.

No keep the original name. No reason to change it and make more of a mess.

>
> With "fixed" it is still ABI break, right?

It is an ABI break but on a new and immature platform.
Noboby could really use that platform at this stage, so nothing is going
to break on anyone really.

>
> Best regards,
> Krzysztof

-- 
Jerome