| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 31 Oct 2022 16:59:19 -0500
From: Rob Herring <robh@...nel.org>
To: Paul Cercueil <paul@...pouillou.net>
Cc: Liam Girdwood <lgirdwood@...il.com>,
Mark Brown <broonie@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH 1/1] dt-bindings: Convert active-semi PMIC docs to YAML
schemas
On Sun, Oct 30, 2022 at 11:37:15AM +0000, Paul Cercueil wrote:
> Create YAML bindings for the Active-semi PMICs and remove the old text
> files.
>
> The bindings aren't perfect, for instance I couldn't find good
> descriptions for the vendor properties in the "charger" node of the
> ACT8945A because I am not familiar with the hardware and these
> properties were not documented anywhere.
>
> The YAML schemas are a bit different than what is described in the old
> text files, because these were sometimes wrong or had missing
> information. This is the case for the ACT8600 documentation, which
> specified the valid node names for the regulators, while the driver was
> expecting different names. This led to the current situation where we
> have two different boards using different names for the regulators:
> - arch/mips/boot/dts/ingenic/ci20.dts uses the names documented in the
> text file,
> - arch/mips/boot/dts/ingenic/gcw0.dts uses the names that the driver
> expects.
> In theory, the driver should be fixed to follow the documentation, and
> accept both naming schemes. In practice though, when the PMIC node was
> added to the ci20.dts board file, the names were already wrong in
> regards to what the driver expected, so it never really worked
> correctly and wasn't tested properly. Furthermore, in that board the
> consumers of the regulators aren't working for various other reasons
> (invalid GPIOs, etc.).
>
> For that reason, for the ACT8600 bindings I decided to only use the node
> names that the driver expects (and that gcw0.dts uses), instead of
> accepting both old and new names. A follow-up patch will update the CI20
> board to use the new regulator names.
>
> Signed-off-by: Paul Cercueil <paul@...pouillou.net>
> ---
>
> Note:
> I set Liam Girdwood and Mark Brown as the maintainers by default, since
> it doesn't appear that anybody is managing the Active-semi drivers, but
> if anybody steps up I can update it.
>
> Cheers,
> -Paul
>
> .../bindings/regulator/act8865-regulator.txt | 117 --------
> .../bindings/regulator/act8945a-regulator.txt | 113 --------
> .../regulator/active-semi,act8600.yaml | 143 ++++++++++
> .../regulator/active-semi,act8846.yaml | 209 ++++++++++++++
> .../regulator/active-semi,act8865.yaml | 162 +++++++++++
> .../regulator/active-semi,act8945a.yaml | 263 ++++++++++++++++++
> 6 files changed, 777 insertions(+), 230 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/regulator/act8865-regulator.txt
> delete mode 100644 Documentation/devicetree/bindings/regulator/act8945a-regulator.txt
> create mode 100644 Documentation/devicetree/bindings/regulator/active-semi,act8600.yaml
> create mode 100644 Documentation/devicetree/bindings/regulator/active-semi,act8846.yaml
> create mode 100644 Documentation/devicetree/bindings/regulator/active-semi,act8865.yaml
> create mode 100644 Documentation/devicetree/bindings/regulator/active-semi,act8945a.yaml
[...]
> diff --git a/Documentation/devicetree/bindings/regulator/active-semi,act8600.yaml b/Documentation/devicetree/bindings/regulator/active-semi,act8600.yaml
> new file mode 100644
> index 000000000000..bf8c5145939e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/active-semi,act8600.yaml
> @@ -0,0 +1,143 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/active-semi,act8600.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Active-semi ACT8600 regulator
> +
> +maintainers:
> + - Liam Girdwood <lgirdwood@...il.com>
> + - Mark Brown <broonie@...nel.org>
> +
> +properties:
> + compatible:
> + const: active-semi,act8600
> +
> + reg:
> + description: I2C address
> + maxItems: 1
> +
> + system-power-controller:
> + description: |
Don't need '|'.
> + Indicates that the ACT8600 is responsible for powering OFF
> + the system.
> + type: boolean
> +
> + active-semi,vsel-high:
> + description: |
> + Indicates the VSEL pin is high. If this property is missing,
> + the VSEL pin is assumed to be low.
> + type: boolean
> +
> + regulators:
> + type: object
> + unevaluatedProperties: false
Don't need both this and additionalProperties. As additionalProperties
is sufficient here, use it, but move it here. It's easier to read that
way IMO.
> +
> + properties:
> + DCDC1:
> + type: object
> + $ref: /schemas/regulator/regulator.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + vp1-supply:
> + description: Handle to the VP1 input supply
> +
> + DCDC2:
> + type: object
> + $ref: /schemas/regulator/regulator.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + vp2-supply:
> + description: Handle to the VP2 input supply
> +
> + DCDC3:
> + type: object
> + $ref: /schemas/regulator/regulator.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + vp3-supply:
> + description: Handle to the VP3 input supply
> +
> + patternProperties:
> + "^((SUDCDC_REG4)|(LDO_REG9)|(LDO_REG10))$":
You can drop the inner parenthesis.
> + type: object
> + $ref: /schemas/regulator/regulator.yaml#
> + unevaluatedProperties: false
> +
> + "^LDO[5-8]$":
> + type: object
> + $ref: /schemas/regulator/regulator.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + inl-supply:
> + description: Handle to the INL input supply
> +
> + additionalProperties: false
> +
> +additionalProperties: false
> +
> +required:
> + - reg
> + - compatible
> + - regulators
> +
> +examples:
> + - |
> + i2c3 {
i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + act8600: pmic@5a {
Drop unused labels.
> + compatible = "active-semi,act8600";
> + reg = <0x5a>;
> +
> + regulators {
> + SUDCDC_REG4 {
> + regulator-min-microvolt = <5300000>;
> + regulator-max-microvolt = <5300000>;
> + inl-supply = <&vcc>;
> + };
> +
> + LDO5 {
> + regulator-min-microvolt = <2500000>;
> + regulator-max-microvolt = <2500000>;
> + inl-supply = <&vcc>;
> + };
> +
> + LDO6 {
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + inl-supply = <&vcc>;
> + };
> +
> + LDO7 {
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + inl-supply = <&vcc>;
> + };
> +
> + LDO8 {
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-always-on;
> + inl-supply = <&vcc>;
> + };
> +
> + LDO_REG9 {
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-always-on;
> + inl-supply = <&vcc>;
> + };
> +
> + LDO_REG10 {
> + inl-supply = <&vcc>;
> + };
> + };
> + };
> + };
Similar comments on the rest.
Rob
Powered by blists - more mailing lists