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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID:
 <OS8PR06MB7541DF787A2A77D6D029F258F2F82@OS8PR06MB7541.apcprd06.prod.outlook.com>
Date: Sun, 16 Feb 2025 01:24:58 +0000
From: Ryan Chen <ryan_chen@...eedtech.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
CC: Michael Turquette <mturquette@...libre.com>, Stephen Boyd
	<sboyd@...nel.org>, Philipp Zabel <p.zabel@...gutronix.de>, Joel Stanley
	<joel@....id.au>, Andrew Jeffery <andrew@...id.au>,
	"linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>, Rob Herring
	<robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
	<conor+dt@...nel.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-aspeed@...ts.ozlabs.org"
	<linux-aspeed@...ts.ozlabs.org>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v8 1/3] dt-binding: clock: ast2700: modify soc0/1 clock
 define

> Subject: Re: [PATCH v8 1/3] dt-binding: clock: ast2700: modify soc0/1 clock
> define
> 
> On 15/02/2025 03:14, Ryan Chen wrote:
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzk@...nel.org>
> >> Sent: Tuesday, February 11, 2025 4:18 PM
> >> To: Ryan Chen <ryan_chen@...eedtech.com>
> >> Cc: Michael Turquette <mturquette@...libre.com>; Stephen Boyd
> >> <sboyd@...nel.org>; Philipp Zabel <p.zabel@...gutronix.de>; Joel
> >> Stanley <joel@....id.au>; Andrew Jeffery <andrew@...id.au>;
> >> linux-clk@...r.kernel.org; Rob Herring <robh@...nel.org>; Krzysztof
> >> Kozlowski <krzk+dt@...nel.org>; Conor Dooley <conor+dt@...nel.org>;
> >> linux-arm-kernel@...ts.infradead.org; linux-aspeed@...ts.ozlabs.org;
> >> devicetree@...r.kernel.org; linux-kernel@...r.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@...eedtech.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?
In my original through will be separate clock identifier one is uart clk, another is uart clk_div13.
uart_clk src
				-> uart clk
uart clk_src div 13
and I real implement in v6, not use in clk-ast2700.c.
But I keep the SCU0_CLK_UART_DIV13 in dt-binding define, not remove it when submit.
The SCU0_CLK_UART_DIV13 is redundant.>
uart clk src -> uart_div_table -> uart clk


> Anyway, any ABI impact must be clearly justified in commit msg.
> 
Thanks for your feedback
I will add the commit message by following.

-Remove redundant SCU0_CLK_UART_DIV13.
SCU0_CLK_UART_DIV13 is not use at clk-ast2700.c
The clock source tree is uart clk src -> uart_div_table -> uart clk

-Change SCU0_CLK_HPLL_DIV_AHB to SCU0_CLK_AHBMUX
Modify clock tree implement in clk-ast2700.c.
old 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
new clock-ast2700.c 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 clock 
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