Re: [PATCH v2 2/3] dt-bindings: clock, reset: rockchip: Add support for rk3562

Hi Krzysztof,

On 2024/12/27 16:28, Krzysztof Kozlowski wrote:

On Tue, Dec 24, 2024 at 05:23:09PM +0800, Kever Yang wrote:

quoted

From: Finley Xiao <redacted>

Add the dt-bindings header for the rk3562, that gets shared between
the clock controller and the clock references in the dts.
Add softreset ID for rk3562.

Signed-off-by: Finley Xiao <redacted>
Signed-off-by: Liang Chen <redacted>
Signed-off-by: Kever Yang <redacted>
---

Changes in v2:
- rename the file to rockchip,rk3562-cru.h
- remove CLK_NR_CLKS
- add new file for reset ID
- update to use dual license

  .../dt-bindings/clock/rockchip,rk3562-cru.h   | 377 ++++++++++++++++++
  .../dt-bindings/reset/rockchip,rk3562-cru.h   | 360 +++++++++++++++++

No, that's not a separate patch. Headers *ALWAYS* go with the bindings
patch.

Will fix.

quoted

  2 files changed, 737 insertions(+)
  create mode 100644 include/dt-bindings/clock/rockchip,rk3562-cru.h
  create mode 100644 include/dt-bindings/reset/rockchip,rk3562-cru.h

diff --git a/include/dt-bindings/clock/rockchip,rk3562-cru.h b/include/dt-bindings/clock/rockchip,rk3562-cru.h
new file mode 100644
index 000000000000..ad07ad3a12ad
--- /dev/null
+++ b/include/dt-bindings/clock/rockchip,rk3562-cru.h

@@ -0,0 +1,377 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */

Why not using license requested by checkpatch?

The checkpatch does not report error/warning for this license, and this 
is the same as many other SoCs.

Which license is recommend in the header file?

quoted

+/*
+ * Copyright (c) 2022-2024 Rockchip Electronics Co., Ltd.
+ * Author: Finley Xiao [off-list ref]
+ */
+
+#ifndef _DT_BINDINGS_CLK_ROCKCHIP_RK3562_H
+#define _DT_BINDINGS_CLK_ROCKCHIP_RK3562_H
+
+/* cru-clocks indices */
+
+/* cru plls */
+#define PLL_APLL			1

Start with 0. Your other binding also starts with 0, so be consistent.

Will fix.

quoted

+#define PLL_GPLL			2
+#define PLL_VPLL			3
+#define PLL_HPLL			4
+#define PLL_CPLL			5
+#define PLL_DPLL			6
+
+/* cru clocks */

Missing clock for 7. You are not supposed to have any holes here.

Will fix.

Thanks,

- Kever

quoted

+#define ARMCLK				8
+#define CLK_GPU				9
+#define ACLK_RKNN			10

Best regards,
Krzysztof