linux-kernel - Re: [PATCH v13 1/2] dt-bindings: clock: add loongson-2 boot clock index

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]
Date:   Thu, 9 Mar 2023 17:12:35 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     zhuyinbo <zhuyinbo@...ngson.cn>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org,
        devicetree@...r.kernel.org
Cc:     Jianmin Lv <lvjianmin@...ngson.cn>,
        Liu Peibao <liupeibao@...ngson.cn>, wanghongliang@...ngson.cn,
        loongson-kernel@...ts.loongnix.cn
Subject: Re: [PATCH v13 1/2] dt-bindings: clock: add loongson-2 boot clock
 index

On 09/03/2023 13:44, zhuyinbo wrote:
> 
> 在 2023/3/9 下午2:25, Krzysztof Kozlowski 写道:
>> On 09/03/2023 02:43, zhuyinbo wrote:
>>> 在 2023/3/8 下午6:38, Krzysztof Kozlowski 写道:
>>>> On 08/03/2023 10:24, zhuyinbo wrote:
>>>>>>>> That's an ABI break and commit msg does not explain it.
>>>>>>> you meaning is that need add a explanation in commit msg that why
>>>>>> You need good explanation to break the ABI. I don't understand the
>>>>>> commit msg, but anyway I could not find there justification for ABI
>>>>>> break. If you do not have good justification, don't break the ABI,
>>>>> The commit msg is the patch commit  log,  and I maybe not got it about
>>>>> break the ABI.  You said about "break the ABI"
>>>>>
>>>>> is whether is location issue about "LOONGSON2_BOOT_CLK"?   if yes,   the
>>>>> LOONGSON2_BOOT_CLK was placed
>>>>>
>>>>> after LOONGSON2_PIX1_PLL that is due to their clock parent is same.
>>>>> and I whether need add this explanation
>>>>>
>>>>> in patch commit log description?
>>>> Unfortunately I do not understand single thing from this.
>>>>
>>>> Best regards,
>>>> Krzysztof
>>> The patch commit log description is patch desription.  as follows:
>>>
>>>
>>> commit 592bc2b4106d787ea166ba16bfde6b3101ab1a8a
>>> Author: Yinbo Zhu <zhuyinbo@...ngson.cn>
>>> Date:   Tue Mar 7 17:18:32 2023 +0800
>>>
>>>       dt-bindings: clock: add loongson-2 boot clock index
>>>
>>>       The Loongson-2 boot clock was used to spi and lio peripheral and
>>>       this patch was to add boot clock index number.
>> I cannot understand this either.
> I will rework commit msg .
>>
>>>
>>> and your advice is "That's an ABI break and commit msg does not explain it."
>>>
>>> I got it  from your advice that was to add a explanation about
>>> LOONGSON2_BOOT_CLK's
>>>
>>> location issue in patch description, right?
>> ABI break needs justification, why do you think it is fine or who
>> is/isn't affected etc. Your commit msg does not explain why ABI break is
>> okay. It doesn't even explain to me why you need it.
>   #define LOONGSON2_DC_PLL                               3
>   #define LOONGSON2_PIX0_PLL                             4
>   #define LOONGSON2_PIX1_PLL                             5
> -#define LOONGSON2_NODE_CLK                             6
> -#define LOONGSON2_HDA_CLK                              7
> -#define LOONGSON2_GPU_CLK                              8
> -#define LOONGSON2_DDR_CLK                              9
> -#define LOONGSON2_GMAC_CLK                             10
> -#define LOONGSON2_DC_CLK                               11
> -#define LOONGSON2_APB_CLK                              12
> -#define LOONGSON2_USB_CLK                              13
> -#define LOONGSON2_SATA_CLK                             14
> -#define LOONGSON2_PIX0_CLK                             15
> -#define LOONGSON2_PIX1_CLK                             16
> -#define LOONGSON2_CLK_END                              17
> +#define LOONGSON2_BOOT_CLK                             6
> +#define LOONGSON2_NODE_CLK                             7
> 
> after add my patch, if dts still use above macro and not cause any 
> issue. but
> 
> if dts not use macro rather than use original clk number index that will 
> cause a uncorrect clk,
> 
> eg.
> 
> -#define LOONGSON2_NODE_CLK                             6
> 
> +#define LOONGSON2_NODE_CLK                             7
> 
>   this issue is that what you said about  "ABI break",  isn't it ?
> 
> 
> About your advice and question and I will use following description as 
> patch  commit msg,  what do you think?
> 
> 
> dt-bindings: clock: add loongson-2 boot clock index
> 
> The spi need to use boot clock and this patch is to add a boot clock 
> index about  LOONGSON2_BOOT_CLK
> 
> and the LOONGSON2_BOOT_CLK was placed in after LOONGSON2_PIX1_PLL that 
> due to
> 
> LOONGSON2_PIX1_PLL,  LOONGSON2_PIX0_PLL , LOONGSON2_DC_PLL and 
> LOONGSON2_BOOT_CLK
> 
> has same parent clock.  In addition, the Loongson  code of the community 
> is still in the development stage,
> 
> so this patch modification will  not cause uncorrect clk quote issue at 
> present.

So the reason is same parent clock...? That's not much. These are IDs
and parent clock do not matter. Drop the ID change.

Best regards,
Krzysztof