On Monday 27 September 2021 15:17:59 Rob Herring wrote:

On Wed, Sep 22, 2021 at 5:56 AM Pali Rohár [off-list ref] wrote:

quoted

This change adds DT bindings documentation for device nodes with compatible
string "marvell,armada-3700-uart-clock".

Please resend to the DT list so that checks run and this gets reviewed
in a timely manner.

OK

quoted

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 .../bindings/clock/armada3700-uart-clock.yaml | 57 +++++++++++++++++++
 1 file changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml

diff --git a/Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml b/Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml
new file mode 100644
index 000000000000..5bdb23e0ba3e
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml

@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: GPL-2.0

Dual license. checkpatch will tell you which ones.

OK

quoted

+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/marvell,armada-3700-uart-clock#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+title: Marvell Armada 3720 UART clocks
+
+properties:
+  compatible:
+    const: marvell,armada-3700-uart-clock
+
+  reg:
+    items:
+      - description: UART Clock Control Register
+      - description: UART 2 Baud Rate Divisor Register
+
+  clocks:
+    description: |
+      List of parent clocks suitable for UART from following set:
+        "TBG-A-P", "TBG-B-P", "TBG-A-S", "TBG-B-S", "xtal"
+      UART clock can use one from this set and when more are provided
+      then kernel would choose and configure the most suitable one.
+      It is suggest to specify at least one TBG clock to achieve
+      baudrates above 230400 and also to specify clock which bootloader
+      used for UART (most probably xtal) for smooth boot log on UART.
+
+  clock-names:
+    items:
+      - const: TBG-A-P
+      - const: TBG-B-P
+      - const: TBG-A-S
+      - const: TBG-B-S
+      - const: xtal
+    minItems: 1
+    maxItems: 5

Don't need maxItems equal to length of 'items'.

OK

quoted

+
+  '#clock-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - '#clock-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    uartclk: uartclk@12000 {

clock-controller@12010

quoted

+      compatible = "marvell,armada-3700-uart-clock";
+      reg = <0x12010 0x4>, <0x12210 0x4>;

However, looks like this is part of some other block.

Yes, it is part of UART block.

Explanation is in commit message of patch 2/6.

And also discussed here:
https://lore.kernel.org/linux-serial/20210812200804.i4kbcs6ut27mapd3@pali/ (local)

The whole block
needs a binding (or at least the parent and whatever sub-functions you
know about).

Whole UART block has already binding. Clock driver just needs access to
these clock bits of these two registers which are in UART block. HW
designers decided that clock which drives UART2 has configuration in
UART1 address space. As explained in commit message of patch 2/6 there
is no easy way how to deal with it in DTS backward compatible way. So
clock and UART driver shares mutex for accessing these two shared
registers, and these two registers are defined in all 3 DT nodes: UART1,
UART2 and UART-clock.

quoted

+      clocks = <&tbg 0>, <&tbg 1>, <&tbg 2>, <&tbg 3>, <&xtalclk>;
+      clock-names = "TBG-A-P", "TBG-B-P", "TBG-A-S", "TBG-B-S", "xtal";
+      #clock-cells = <1>;
+    };
--
2.20.1

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