On 23/11/2022 12:16, Yu Tu wrote:

Hi Krzysztof,
	Thank you for your reply.

On 2022/11/23 18:08, Krzysztof Kozlowski wrote:

quoted

[ EXTERNAL EMAIL ]

On 23/11/2022 03:13, Yu Tu wrote:

quoted

Add the S4 PLL clock controller found and bindings in the s4 SoC family.

Signed-off-by: Yu Tu <redacted>
---
  .../bindings/clock/amlogic,s4-pll-clkc.yaml   |  51 +

This is v5 and still bindings are here? Bindings are always separate
patches. Use subject prefixes matching the subsystem (git log --oneline
-- ...).

And this was split, wasn't it? What happened here?!?

Put bindings and clock driver patch together from Jerome. Maybe you can 
read this chat history.
https://lore.kernel.or/all/1jy1v6z14n.fsf@starbuckisacylon.baylibre.com/

Link does not explain me anything. It mentions series, which is totally
different than mixing it one patch!

Anyway you should have warnings from checkpatch.

Bindings are always separate patches.

(...)

quoted

quoted

+
+properties:
+  compatible:
+    const: amlogic,s4-pll-clkc
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: xtal
+
+  "#clock-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - "#clock-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    clkc_pll: clock-controller@fe008000 {
+      compatible = "amlogic,s4-pll-clkc";
+      reg = <0xfe008000 0x1e8>;
+      clocks = <&xtal>;
+      clock-names = "xtal";
+      #clock-cells = <1>;
+    };

quoted

+#endif /* __MESON_S4_PLL_H__ */

diff --git a/include/dt-bindings/clock/amlogic,s4-pll-clkc.h b/include/dt-bindings/clock/amlogic,s4-pll-clkc.h
new file mode 100644
index 000000000000..345f87023886
--- /dev/null
+++ b/include/dt-bindings/clock/amlogic,s4-pll-clkc.h

This belongs to bindings patch, not driver.

quoted

@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
+/*
+ * Copyright (c) 2021 Amlogic, Inc. All rights reserved.
+ * Author: Yu Tu <yu.tu@amlogic.com>
+ */
+
+#ifndef _DT_BINDINGS_CLOCK_AMLOGIC_S4_PLL_CLKC_H
+#define _DT_BINDINGS_CLOCK_AMLOGIC_S4_PLL_CLKC_H
+
+/*
+ * CLKID index values
+ */
+
+#define CLKID_FIXED_PLL			1
+#define CLKID_FCLK_DIV2			3

Indexes start from 0 and are incremented by 1. Not by 2.

NAK.

I remember Jerome discussing this with you.You can look at this 
submission history.
https://lore.kernel.org/all/c088e01c-0714-82be-8347-6140daf56640@linaro.org/ (local)

You pointed to my arguments, so what is this proving? That you ignored
feedback? Or was there some other mail?

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel