On 25/02/2025 10:49, Ryan Chen wrote: >>>> Subject: Re: [PATCH v9 1/3] dt-binding: clock: ast2700: modify soc0/1 >>>> clock define >>>> >>>> On 24/02/2025 10:55, Ryan Chen wrote: >>>>> -remove redundant SOC0_CLK_UART_DIV13: >>>>> SOC0_CLK_UART_DIV13 is not use at clk-ast2700.c, the clock source >>>>> tree is uart clk src -> uart_div_table -> uart clk. >>>>> >>>>> -Change SOC0_CLK_HPLL_DIV_AHB to SOC0_CLK_AHBMUX: >>>>> modify clock tree implement. >>>>> older CLK_AHB use mpll_div_ahb/hpll_div_ahb to be ahb clock source. >>>>> mpll->mpll_div_ahb >>>>> -> clk_ahb >>>>> hpll->hpll_div_ahb >>>> >>>> >>>> I can barely understand it and from the pieces I got, it does not >>>> explain need for ABI break. >>>> >>> >>> #1. SCU0_CLK_UART_DIV13 is redundant, it does not impact ABI break >> >> You did not explain how it does not impact. Clock was exported, there was a >> user and now there is no clock. User stops working. ABI break. >> > > Sorry, SCU0_CLK_UART_DIV13 was defined, but was never referenced in any upstream device trees. That's incomplete definition of ABI > Since there is no in-tree usage of `SCU0_CLK_UART_DIV13`, its removal does not cause an ABI break. You ignored out of tree users. Please read carefully ABI docs. > >>> #2. Change SOC0_CLK_HPLL_DIV_AHB to SOC0_CLK_AHBMUX Older >> implement >>> where `mpll_div_ahb` and `hpll_div_ahb` were **hardcoded dividers** for >> AHB. >>> In **the new approach (v8)**, I refactored the clock tree to clock tree. >> >> I still cannot parse sentences like "refactoring A to A". It's meaningless to me. >> >>> It should be ABI-safe change >> >> No, you do not understand the ABI. You removed a clock ID, that's the ABI >> change. >> >> Otherwise explain how this is not changing ABI. >> >> >>> >>> Or you want to keep original SOC0_CLK_HPLL_DIV_AHB define and then add >> SOC0_CLK_AHBMUX. >>> To be 1st patch, then 2n patch remove redundant >> SOC0_CLK_HPLL_DIV_AHB? >> >> If you break the ABI you need to clearly explain why. We have long >> conversations and you still did not say why. >> > Sorry, my point will be following steps to avoid potential ABI issues, > I can modify the patch series as follows: > 1. **Patch 1:** Add `SOC0_CLK_AHBMUX` without removing `SOC0_CLK_HPLL_DIV_AHB`. > 2. **Patch 2:** Finally remove `SOC0_CLK_HPLL_DIV_AHB`. I do not understand what changed here. You remove exported clock which is ABI, so how is this answering my question. You keep dodging my questions. Here I asked "why". I do not see any answer why. Best regards, Krzysztof
