Subject: Re: [PATCH 3/4] dt-bindings: clock: Add AST2700 clock bindings

On 12/08/2024 11:39, Ryan Chen wrote:

quoted

quoted

Subject: RE: [PATCH 3/4] dt-bindings: clock: Add AST2700 clock
bindings

quoted

Subject: Re: [PATCH 3/4] dt-bindings: clock: Add AST2700 clock
bindings

On 12/08/2024 10:22, Ryan Chen wrote:

quoted

quoted

Subject: Re: [PATCH 3/4] dt-bindings: clock: Add AST2700 clock
bindings

On 12/08/2024 09:26, Ryan Chen wrote:

quoted

quoted

Subject: RE: [PATCH 3/4] dt-bindings: clock: Add AST2700 clock
bindings

quoted

Subject: Re: [PATCH 3/4] dt-bindings: clock: Add AST2700 clock
bindings

On 09/08/2024 07:47, Ryan Chen wrote:

quoted

quoted

Subject: Re: [PATCH 3/4] dt-bindings: clock: Add AST2700
clock bindings

Le 08/08/2024 ? 09:59, Ryan Chen a ?crit :

quoted

Add dt bindings for AST2700 clock controller

Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
  .../dt-bindings/clock/aspeed,ast2700-clk.h    | 175

++++++++++++++++++

quoted

  1 file changed, 175 insertions(+)
  create mode 100644
include/dt-bindings/clock/aspeed,ast2700-clk.h

diff --git a/include/dt-bindings/clock/aspeed,ast2700-clk.h

b/include/dt-bindings/clock/aspeed,ast2700-clk.h
new file mode 100644
index 000000000000..facf72352c3e

--- /dev/null
+++ b/include/dt-bindings/clock/aspeed,ast2700-clk.h

@@ -0,0 +1,175 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+*/
+/*
+ * Device Tree binding constants for AST2700 clock controller.
+ *
+ * Copyright (c) 2024 Aspeed Technology Inc.
+ */
+
+#ifndef __DT_BINDINGS_CLOCK_AST2700_H #define
+__DT_BINDINGS_CLOCK_AST2700_H
+
+/* SOC0 clk-gate */
+#define SCU0_CLK_GATE_MCLK (0) #define

SCU0_CLK_GATE_ECLK (1)

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

+#define SCU0_CLK_GATE_2DCLK        (2)
+#define SCU0_CLK_GATE_VCLK (3) #define

SCU0_CLK_GATE_BCLK

quoted

(4)

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

+#define SCU0_CLK_GATE_VGA0CLK      (5)
+#define SCU0_CLK_GATE_REFCLK       (6)
+#define SCU0_CLK_GATE_PORTBUSB2CLK (7) #define

SCU0_CLK_GATE_RSV8

quoted

quoted

quoted

+(8)
+#define SCU0_CLK_GATE_UHCICLK      (9)
+#define SCU0_CLK_GATE_VGA1CLK      (10)
+#define SCU0_CLK_GATE_DDRPHYCLK    (11)
+#define SCU0_CLK_GATE_E2M0CLK      (12)
+#define SCU0_CLK_GATE_HACCLK       (13)
+#define SCU0_CLK_GATE_PORTAUSB2CLK (14)
+#define SCU0_CLK_GATE_UART4CLK     (15)
+#define SCU0_CLK_GATE_SLICLK       (16)
+#define SCU0_CLK_GATE_DACCLK       (17)
+#define SCU0_CLK_GATE_DP   (18)
+#define SCU0_CLK_GATE_E2M1CLK      (19)
+#define SCU0_CLK_GATE_CRT0CLK      (20)
+#define SCU0_CLK_GATE_CRT1CLK      (21)
+#define SCU0_CLK_GATE_VLCLK        (22)
+#define SCU0_CLK_GATE_ECDSACLK     (23)
+#define SCU0_CLK_GATE_RSACLK       (24)
+#define SCU0_CLK_GATE_RVAS0CLK     (25)
+#define SCU0_CLK_GATE_UFSCLK       (26)
+#define SCU0_CLK_GATE_EMMCCLK      (27)
+#define SCU0_CLK_GATE_RVAS1CLK     (28)
+/* reserved 29 ~ 31*/

No, you cannot reserve IDs. They are always continous.

I think for mis-understood.
I will remove the comment.
And keep it is continuous. Thanks.

quoted

quoted

quoted

quoted

+#define SCU0_CLK_GATE_NUM

(SCU0_CLK_GATE_RVAS1CLK +

quoted

quoted

quoted

1)

quoted

quoted

quoted

quoted

quoted

No, not a binding.

I will modify by following.

#define SCU0_CLK_GATE_RVAS1CLK  (28)
#define SCU0_CLK_GATE_NUM       (SCU0_CLK_GATE_RVAS1CLK

+

quoted

quoted

1)

quoted

quoted

quoted

Nothing changed. Still not a binding. Why do you send the same and
expect different result? Drop.

Address feedback sent to you from previous versions of the patchset.
There was never a reply.

Sorry, mis-understood.
Since you think "#define SCU0_CLK_GATE_NUM" not a binding.
Do you mean I should #define SCU0_CLK_GATE_NUM in clk driver, not
in

binding header, am I right?

What did I write in the first Aspeed 2700 patch? So you are not
going to respond there? Are you going to implement entire feedback
received in the first version of the patchset?

Apologize again, I do the internal discussion, it should not send
"Introduce ASPEED AST27XX BMC SoC" series patch. it should be separate

series patch.

quoted

quoted

It should be bite by bite, example clk driver patches, platform
patches, interrupt patches.
So I am not going to response there, prefer here.

So I still not understood your point "not a binding" is ~

I review your point on
https://patchwork.kernel.org/project/linux-clk/patch/20240726110355.21
81563-3-kevin_chen at aspeedtech.com/

Do you mean I should not be gate naming here, all should be clk.
Example +#define SCU0_CLK_GATE_RVAS1CLK -> +#define

SCU0_CLK_RVAS1 am I right?

Drop the define for number of clocks from the header, because it is not a
binding. You can put it in the driver or not, I don't care and do not provide
guidance on this because I don't know if it makes sense at all.
What I know is that number of clocks is not related to binding. It is not needed
in the binding, either.

Sorry, I am confused.
if you think that number of clocks is not related to binding.
How dtsi claim for clk?
For example in dtsi.
include <dt-bindings/clock/aspeed,ast2700-clk.h>
usb3bhp: usb3bhp {
....
clocks = <&syscon0 SCU0_CLK_GATE_PORTAUSB>;
...
}

It need for dtsi binding include for clock enable. 

If there is no binding clock include file, how device know the clock index?

Best regards,
Krzysztof