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
