Dne četrtek, 04. avgust 2022 ob 05:03:07 CEST je Samuel Holland napisal(a):

Hi Rob,

Thanks again for your review.

On 8/2/22 10:04 AM, Rob Herring wrote:

quoted

On Tue, Aug 02, 2022 at 12:32:12AM -0500, Samuel Holland wrote:

quoted

The Allwinner D1 SoC contains two pairs of in-package LDOs. One pair is
for general purpose use. LDOA generally powers the board's 1.8 V rail.
LDOB generally powers the in-package DRAM, where applicable.

The other pair of LDOs powers the analog power domains inside the SoC,
including the audio codec, thermal sensor, and ADCs. These LDOs require
a 0.9 V bandgap voltage reference. The calibration value for the voltage
reference is stored in an eFuse, accessed via an NVMEM cell.

Neither LDO control register is in its own MMIO range; instead, each
regulator device relies on a regmap/syscon exported by its parent.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

Changes in v2:
 - Remove syscon property from bindings
 - Update binding examples to fix warnings and provide context
 
 .../allwinner,sun20i-d1-analog-ldos.yaml      | 65 +++++++++++++++++++
 .../allwinner,sun20i-d1-system-ldos.yaml      | 57 ++++++++++++++++
 2 files changed, 122 insertions(+)
 create mode 100644
 Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog-
 ldos.yaml create mode 100644
 Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-
 ldos.yaml>> 
diff --git
a/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog
-ldos.yaml
b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog
-ldos.yaml new file mode 100644
index 000000000000..19c984ef4e53

--- /dev/null
+++

b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog
-ldos.yaml @@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id:
http://devicetree.org/schemas/regulator/allwinner,sun20i-d1-analog-ldos.
yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner D1 Analog LDOs
+
+description:
+  Allwinner D1 contains a set of LDOs which are designed to supply
analog power +  inside and outside the SoC. They are controlled by a
register within the audio +  codec MMIO space, but which is not part of
the audio codec clock/reset domain. +
+maintainers:
+  - Samuel Holland [off-list ref]
+
+properties:
+  compatible:
+    enum:
+      - allwinner,sun20i-d1-analog-ldos
+
+  nvmem-cells:
+    items:
+      - description: NVMEM cell for the calibrated bandgap reference
trim value +
+  nvmem-cell-names:
+    items:
+      - const: bg_trim
+
+patternProperties:

quoted

+  "^(aldo|hpldo)$":

'^(a|hp)ldo$'

quoted

+    type: object
+    $ref: regulator.yaml#

       unevaluatedProperties: false

quoted

+
+required:
+  - compatible
+  - nvmem-cells
+  - nvmem-cell-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    audio-codec@2030000 {
+        compatible = "simple-mfd", "syscon";

Needs a specific compatible. Obviously there's some other functionality
here if this is an 'audio-codec'.

I am not yet ready to submit the binding or driver for the audio codec, as I
still need to work out the details for jack detection (and some other
issues). If I added the specific audio codec compatible now, without the
properties required for the sound driver, that would create backward
compatibility issues, right?

My intention is to add the specific compatible along with the ASoC support.

quoted

'simple-mfd' also means the child nodes have zero dependence on the
parent node and its resources.

That is correct. The regulators have zero dependencies on the audio codec
resources (clocks/resets/etc.). The _only_ relationship is the overlapping
address of the MMIO register.

LDO registers are at the end of analogue codec MMIO space, right? We can 
easily split that to separate device, like it's been done for A20 timer & 
watchdog.

Best regards,
Jernej

quoted

quoted

+        reg = <0x2030000 0x1000>;
+
+        regulators {
+            compatible = "allwinner,sun20i-d1-analog-ldos";

Is there a defined set of registers for these regulators? If so, put
them into 'reg'.

What do you suggest for 'ranges' in the parent device? I ask because the
SRAM/system controller binding requires an empty 'ranges' property.

With empty 'ranges', the child has to compute the relative address for use
with the parent's regmap, but maybe that is okay?

quoted

quoted

+            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..c95030a827d6

--- /dev/null
+++

b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system
-ldos.yaml @@ -0,0 +1,57 @@
+# 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.
+
+maintainers:
+  - Samuel Holland [off-list ref]
+
+properties:
+  compatible:
+    enum:
+      - allwinner,sun20i-d1-system-ldos
+
+patternProperties:

quoted

+  "^(ldoa|ldob)$":

'^ldo[ab]$'

Any reason the naming is not consistent with 'ldo' as the prefix or
suffix?

All four names match the pin names from the SoC datasheet.

quoted

quoted

+    type: object
+    $ref: regulator.yaml#

       unevaluatedProperties: false

I will fix these for v3.

Regards,
Samuel

quoted

quoted

+
+required:
+  - compatible
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    syscon@3000000 {
+        compatible = "allwinner,sun20i-d1-system-control";
+        reg = <0x3000000 0x1000>;
+        ranges;
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        regulators {
+            compatible = "allwinner,sun20i-d1-system-ldos";
+
+            reg_ldoa: ldoa {
+                regulator-min-microvolt = <1800000>;
+                regulator-max-microvolt = <1800000>;
+            };
+
+            reg_ldob: ldob {
+                regulator-name = "vcc-dram";
+                regulator-min-microvolt = <1500000>;
+                regulator-max-microvolt = <1500000>;
+            };
+        };
+    };
+
+...

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