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

[Krzysztof Kozlowski <krzk@kernel.org>]

On 15/02/2025 03:14, Ryan Chen wrote:
>> -----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.

Wrap your replies correctly.

So all this means you exported clocks which are not clocks?
How are ABI consumers behaving now?

Anyway, any ABI impact must be clearly justified in commit msg.



Best regards,
Krzysztof