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: falseExample 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