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

[Krzysztof Kozlowski]

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@xxxxxxxxxxx>
>>> 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