Thread (26 messages) 26 messages, 4 authors, 2022-08-17

Re: [PATCH v3 1/4] regulator: dt-bindings: Add Allwinner D1 LDOs

From: Krzysztof Kozlowski <hidden>
Date: 2022-08-17 08:27:42
Also in: linux-devicetree, linux-sunxi, lkml

On 17/08/2022 11:15, Samuel Holland wrote:
quoted
quoted
+
+properties:
+  compatible:
+    enum:
+      - allwinner,sun20i-d1-analog-ldos
+
+  reg:
+    maxItems: 1
+
+  nvmem-cells:
+    items:
+      - description: NVMEM cell for the calibrated bandgap reference trim value
+
+  nvmem-cell-names:
+    items:
+      - const: bg_trim
+
+patternProperties:
+  "^(a|hp)ldo$":
+    type: object
+    $ref: regulator.yaml#
+    unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - nvmem-cells
+  - nvmem-cell-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    audio-codec@2030000 {
+        compatible = "simple-mfd", "syscon";
This cannot be on its own. Both require device specific compatible.
Again, the device-specific compatible does not exist, because the binding for
the audio codec has not been written (and it will be quite nontrivial).

So I can:
  1) Leave the example as-is until the audio codec binding gets written,
     and fill in the specific compatible at that time.
  2) Remove the example, with the reasoning that the example really
     belongs with the MFD parent (like for the other regulator). Then
     there will be no example until the audio codec binding is written.
  3) Drop the analog LDOs from this series entirely, and some parts
     of the SoC (like thermal monitoring) cannot be added to the DTSI
     until the audio codec binding is written.

What do you think?
How about just removing the audio-codec node? The schema is about
regulators, not audio-codec.

OTOH, if you have parent device schema, you could put the example only
there. But as I understand, you don't have, right?
The same question applies for the D1 SoC DTSI, where I use this same construct.
This is not correct and should be fixed. Either you add the schema with
compatible or please drop the device node from the DTSI.
(And technically this does validate with the current schema.)
quoted
quoted
+        reg = <0x2030000 0x1000>;
+        ranges;
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        regulators@2030348 {
+            compatible = "allwinner,sun20i-d1-analog-ldos";
+            reg = <0x2030348 0x4>;
+            nvmem-cells = <&bg_trim>;
+            nvmem-cell-names = "bg_trim";
+
+            reg_aldo: aldo {
+                regulator-min-microvolt = <1800000>;
+                regulator-max-microvolt = <1800000>;
+            };
+
+            reg_hpldo: hpldo {
+                regulator-min-microvolt = <1800000>;
+                regulator-max-microvolt = <1800000>;
+            };
+        };
+    };
+
+...
diff --git a/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.yaml b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.yaml
new file mode 100644
index 000000000000..e3e2810fb3d7
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.yaml
@@ -0,0 +1,37 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/allwinner,sun20i-d1-system-ldos.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner D1 System LDOs
+
+description:
+  Allwinner D1 contains a pair of general-purpose LDOs which are designed to
+  supply power inside and outside the SoC. They are controlled by a register
+  within the system control MMIO space.
Fix order.

quoted
+
+maintainers:
+  - Samuel Holland [off-list ref]
+
+properties:
+  compatible:
+    enum:
+      - allwinner,sun20i-d1-system-ldos
+
+  reg:
+    maxItems: 1
+
+patternProperties:
+  "^ldo[ab]$":
+    type: object
+    $ref: regulator.yaml#
+    unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false

Example please.
Rob asked me to move the example to the parent binding, so I did. The example is
added in patch 3.
Yeah, I noticed it later. It's fine.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help