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

[Ryan Chen <ryan_chen@aspeedtech.com>]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Tuesday, February 11, 2025 4:18 PM
> To: Ryan Chen <ryan_chen@aspeedtech.com>
> Cc: Michael Turquette <mturquette@baylibre.com>; Stephen Boyd
> <sboyd@kernel.org>; Philipp Zabel <p.zabel@pengutronix.de>; Joel Stanley
> <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>;
> linux-clk@vger.kernel.org; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>;
> linux-arm-kernel@lists.infradead.org; linux-aspeed@lists.ozlabs.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v8 1/3] dt-binding: clock: ast2700: modify soc0/1 clock
> define
> 
> On Mon, Feb 10, 2025 at 04:50:02PM +0800, Ryan Chen wrote:
> > remove soc0 clock:
> 
> Why? Your commit msg must explain why. What is obvious from the diff, isn't
> it?
Thank you for your feedback. I will add explanation in next commit patch.
> 
> >  SOC0_CLK_UART_DIV13
> >  SOC0_CLK_HPLL_DIV_AHB
> >  SOC0_CLK_MPLL_DIV_AHB
> > add soc0 clock:
> >  SOC0_CLK_AHBMUX
> >  SOC0_CLK_MPHYSRC
> >  SOC0_CLK_U2PHY_REFCLKSRC
> > add soc1 clock:
> >  SOC1_CLK_I3C
> >
> > Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> > ---
> >  include/dt-bindings/clock/aspeed,ast2700-scu.h | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/dt-bindings/clock/aspeed,ast2700-scu.h
> > b/include/dt-bindings/clock/aspeed,ast2700-scu.h
> > index 63021af3caf5..c7389530629d 100644
> > --- a/include/dt-bindings/clock/aspeed,ast2700-scu.h
> > +++ b/include/dt-bindings/clock/aspeed,ast2700-scu.h
> > @@ -13,18 +13,17 @@
> >  #define SCU0_CLK_24M		1
> >  #define SCU0_CLK_192M		2
> >  #define SCU0_CLK_UART		3
> > -#define SCU0_CLK_UART_DIV13	3
> 
> NAK, ABI break without any explanation.

The `SCU0_CLK_UART_DIV13` was originally defined as a separate clock identifier, reviewing the AST2700 clock driver implement, I realized it is no longer necessary.
The clk-ast2700.c driver I have **integrated the SOC0 UART clock (`soc0_uartclk`) with `ast2700_clk_uart_div_table`**. 
The UART clock source will get from ast2700_clk_uart_div_table, that will div from source 24M div13 or div1.
> 
> >  #define SCU0_CLK_PSP		4
> >  #define SCU0_CLK_HPLL		5
> >  #define SCU0_CLK_HPLL_DIV2	6
> >  #define SCU0_CLK_HPLL_DIV4	7
> > -#define SCU0_CLK_HPLL_DIV_AHB	8
> > +#define SCU0_CLK_AHBMUX		8
> 
> NAK, ABI break without any explanation.

V6 clock-ast2700.c CLK_AHB implement mpll_div_ahb / hpll_div_ahb to be ahb clock source.
mpll-> mpll_div_ahb 
				-> clk_ahb
hpll-> hpll_div_ahb

V8 clock-ast2700.c I implement by SCU0_CLK_AHBMUX for more understand clock source divide tree.
Add define SCU0_CLK_AHBMUX replace SCU0_CLK_HPLL_DIV_AHB
mpll->
	ahb_mux -> div_table -> clk_ahb
hpll->

new add 
SOC0_CLK_MPHYSRC: UFS MPHY clock source.
SOC0_CLK_U2PHY_REFCLKSRC : USB2.0 phy clock reference source
SOC1_CLK_I3C: I3C clock source.
> 
> Best regards,
> Krzysztof