Message-ID: <38b0c1f2-fb0e-722b-19f5-c94796ce0185@linaro.org>
Date:   Thu, 9 Mar 2023 17:12:35 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH v13 1/2] dt-bindings: clock: add loongson-2 boot clock
 index
Content-Language: en-US
To:     zhuyinbo <zhuyinbo@loongson.cn>,
        Michael Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
        devicetree@vger.kernel.org
Cc:     Jianmin Lv <lvjianmin@loongson.cn>,
        Liu Peibao <liupeibao@loongson.cn>, wanghongliang@loongson.cn,
        loongson-kernel@lists.loongnix.cn
References: <20230307115022.12846-1-zhuyinbo@loongson.cn>
 <692a62da-a9a1-fa23-6e24-723d73c3a423@linaro.org>
 <5e9b3bd5-d885-6237-5e14-2becb3c956cc@loongson.cn>
 <31e2a67a-c046-9501-80de-e754ed450195@linaro.org>
 <ace5159b-ebbd-7805-518c-ed3d39e4793e@loongson.cn>
 <b2f9cd15-b303-882a-d4e0-36d1f6d155a8@linaro.org>
 <eca81d44-2513-8002-980d-755af32f36c4@loongson.cn>
 <882bd054-9c3e-1b26-72a5-89ad92e21a79@linaro.org>
 <817a65e9-b605-4bc0-2ff5-fc7f5db8585c@loongson.cn>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <817a65e9-b605-4bc0-2ff5-fc7f5db8585c@loongson.cn>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

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@loongson.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