RE: [PATCH v9 1/3] dt-binding: clock: ast2700: modify soc0/1 clock define

[Ryan Chen <ryan_chen@aspeedtech.com>]

> >> 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.
Since there is no in-tree usage of `SCU0_CLK_UART_DIV13`, its removal does not cause an ABI break.

> > #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`.

Let me know if you prefer this approach.


> 
> Best regards,
> Krzysztof