| 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
| ||
|
Message-ID: <20250320-unbiased-cooperative-coyote-e7e255@krzk-bin>
Date: Thu, 20 Mar 2025 10:06:53 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Alexis Czezar Torreno <alexisczezar.torreno@...log.com>
Cc: Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v2 1/2] dt-bindings: regulator: add adi,adp5055-regulator
On Thu, Mar 20, 2025 at 02:53:54PM +0800, Alexis Czezar Torreno wrote:
> Add documentation for devicetree bindings for ADP5055. The device consists
> of 3 buck regulators able to connect to high input voltages of up to 18V
> with no preregulators.
>
Please use subject prefixes matching the subsystem. You can get them for
example with 'git log --oneline -- DIRECTORY_OR_FILE' on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@...log.com>
> ---
> .../bindings/regulator/adi,adp5055-regulator.yaml | 161 +++++++++++++++++++++
> MAINTAINERS | 6 +
> 2 files changed, 167 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/regulator/adi,adp5055-regulator.yaml b/Documentation/devicetree/bindings/regulator/adi,adp5055-regulator.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..fc8f1e61ba321f8b4c6f8c1e3d0e91d570fb4953
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/adi,adp5055-regulator.yaml
> @@ -0,0 +1,161 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/adi,adp5055-regulator.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices ADP5055 Triple Buck Regulator
> +
> +maintainers:
> + - Alexis Czezar Torreno <alexisczezar.torreno@...log.com>
> +
> +description: |
> + The ADP5055 combines three high performance buck regulators.
> + The device enables direct connection to high input voltages
> + up to 18 V with no preregulators.
> + https://www.analog.com/media/en/technical-documentation/data-sheets/adp5055.pdf
> +
> +properties:
> + compatible:
> + enum:
> + - adi,adp5055
> +
> + reg:
> + enum:
> + - 0x70
> + - 0x71
> +
> + adi,tset-us:
> + description:
> + Setting time used by the device. This is changed via soldering
> + specific resistor values on the CFG2 pin.
> + enum: [2600, 20800]
> + default: 2600
> +
> + adi,hw-en-array-gpios:
Drop prefix, drop "array" and this probably will be changed anyway.
> + description:
> + Asserted during driver probe. Each array entry acts as the
s/Asserted during driver probe.//
If driver moves this code to other place, does it mean bindings are
wrong?
> + hardware enable for channels 0-2. Should be marked 0 for active
What does it mean "0" for active low? No, active low has its own flag.
Use proper flags and implement it properly in the driver.
What is hardware enable and software enable? Is it enable-gpios per
regulator? Then why this isn't in the regulator node, just like we
expect for all regulator bindings?
> + low. Requires all three channels to be initialized. Not adding
> + the property turns the system to a software only enable mode.
> + minItems: 3
> + maxItems: 3
> +
> + adi,ocp-blanking:
> + description:
> + If present, the over current protection
> + blanking (OCP) for all channels is on.
Don't
wrap
at
random
places, plese.
> + type: boolean
> +
> + adi,delay-power-good:
> + description:
> + Configures delay timer of the power good (PWRGD) pin.
> + Delay is based on Tset which can be 2.6 ms or 20.8 ms.
> + type: boolean
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> +patternProperties:
> + "^channel@([0-2])$":
This is a mess... never tested and makes no sense. Either this is a
regulator or a channel. Looks like regulator, but you called it a
channel. If regulator, then missing ref to regulator schema.
> + type: object
> + additionalProperties: false
> +
> + properties:
> + reg:
> + description: The channel number representing each buck converter.
> + maximum: 2
> +
> + adi,dvs-limit-upper-microvolt:
> + description:
> + Configure the allowable upper side limit of the
> + voltage output of each channel in microvolt.
> + Voltages are in 12mV steps, value is autoadjusted.
> + Vout_high = Vout + DVS_upper_limit.
And how do you configure vout?
> + minimum: 12000
> + maximum: 192000
> + default: 192000
> +
> + adi,dvs-limit-lower-microvolt:
> + description:
> + Configure the allowable lower side limit of the
> + voltage output of each channel in microvolt.
> + Voltages are in 12mV steps, value is autoadjusted.
> + Vout_low = Vout + DVS_lower_limit.
> + minimum: -190500
> + maximum: -10500
> + default: -190500
> +
> + adi,fast-transient:
> + description:
> + Configures the fast transient sensitivity for each channel.
> + "none" - No fast transient.
> + "3G_1.5%" - 1.5% window with 3*350uA/V
> + "5G_1.5%" - 1.5% window with 5*350uA/V
> + "5G_2.5%" - 2.5% window with 5*350uA/V
> + enum: [none, 3G_1.5%, 5G_1.5%, 5G_2.5%]
> + default: 5G_2.5%
> +
> + adi,mask-power-good:
> + description:
> + If present, masks individual channels to the external
> + PWRGD hardware pin.
> + type: boolean
> +
> + required:
> + - regulator-name
> +
> +required:
> + - compatible
> + - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + regulator@70 {
> + compatible = "adi,adp5055";
> + reg = <0x70>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + adi,tset-us = <2600>;
> + adi,hw-en-array-gpios = <&gpio 17 0>,
> + <&gpio 18 0>,
> + <&gpio 19 0>;
No, use proper defines and proper flags.
> +
> + adi,ocp-blanking;
> + adi,delay-power-good;
> +
> + DCDC0 {
Your schema said something else. Test your patches before sending, not
through our systems.
Best regards,
Krzysztof
Powered by blists - more mailing lists