From: Rob Herring <robh@kernel.org>
To: Adam Ward <Adam.Ward.opensource@diasemi.com>
Cc: Mark Brown <broonie@kernel.org>,
Liam Girdwood <lgirdwood@gmail.com>,
Vincent Whitchurch <vincent.whitchurch@axis.com>,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
Support Opensource <support.opensource@diasemi.com>
Subject: Re: [PATCH V4 01/10] regulator: Update DA9121 dt-bindings
Date: Mon, 7 Dec 2020 11:58:09 -0600 [thread overview]
Message-ID: <20201207175809.GA503826@robh.at.kernel.org> (raw)
In-Reply-To: <0606d3ded5fef4c38760246146f197db4ce3a374.1606830377.git.Adam.Ward.opensource@diasemi.com>
On Tue, Dec 01, 2020 at 01:52:27PM +0000, Adam Ward wrote:
> Update bindings for the Dialog Semiconductor DA9121 voltage regulator to
> add device variants.
> Because several variants have multiple regulators, and to regard potential
> to add GPIO support in future, the 'regulators' sub-node is added,
> following the precedent set by other multi-regulator devices, including
> the DA9211 family. This breaks compatibility with the original submission
> by Vincent Whitchurch - but as this is still in for-next, the alignment
> could be made before upstreaming occurs.
>
> Signed-off-by: Adam Ward <Adam.Ward.opensource@diasemi.com>
> ---
> .../devicetree/bindings/regulator/dlg,da9121.yaml | 164 +++++++++++++++++++--
> MAINTAINERS | 2 +
> .../dt-bindings/regulator/dlg,da9121-regulator.h | 22 +++
> 3 files changed, 177 insertions(+), 11 deletions(-)
> create mode 100644 include/dt-bindings/regulator/dlg,da9121-regulator.h
>
> diff --git a/Documentation/devicetree/bindings/regulator/dlg,da9121.yaml b/Documentation/devicetree/bindings/regulator/dlg,da9121.yaml
> index 2ece46e..6f2164f 100644
> --- a/Documentation/devicetree/bindings/regulator/dlg,da9121.yaml
> +++ b/Documentation/devicetree/bindings/regulator/dlg,da9121.yaml
> @@ -7,41 +7,183 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
> title: Dialog Semiconductor DA9121 voltage regulator
>
> maintainers:
> - - Vincent Whitchurch <vincent.whitchurch@axis.com>
> + - Adam Ward <Adam.Ward.opensource@diasemi.com>
> +
> +description: |
> + Dialog Semiconductor DA9121 Single-channel 10A double-phase buck converter
> + Dialog Semiconductor DA9122 Double-channel 5A single-phase buck converter
> + Dialog Semiconductor DA9220 Double-channel 3A single-phase buck converter
> + Dialog Semiconductor DA9217 Single-channel 6A double-phase buck converter
> + Dialog Semiconductor DA9130 Single-channel 10A double-phase buck converter
> + Dialog Semiconductor DA9131 Double-channel 5A single-phase buck converter
> + Dialog Semiconductor DA9132 Double-channel 3A single-phase buck converter
> +
> + Current limits
> +
> + This is PER PHASE, and the current limit setting in the devices reflect
> + that with a maximum 10A limit. Allowing for transients at/near double
> + the rated current, this translates across the device range to per
> + channel figures as so...
> +
> + | DA9121 DA9122 DA9220 DA9217 DA9140
> + | /DA9130 /DA9131 /DA9132
> + -----------------------------------------------------------------------------
> + Output current / channel | 10000000 5000000 3000000 6000000 40000000
> + Output current / phase | 5000000 5000000 3000000 3000000 9500000
> + -----------------------------------------------------------------------------
> + Min regulator-min-microvolt| 300000 300000 300000 300000 500000
> + Max regulator-max-microvolt| 1900000 1900000 1900000 1900000 1000000
> + Device hardware default | 1000000 1000000 1000000 1000000 1000000
> + -----------------------------------------------------------------------------
> + Min regulator-min-microamp | 7000000 3500000 3500000 7000000 26000000
> + Max regulator-max-microamp | 20000000 10000000 6000000 12000000 78000000
> + Device hardware default | 15000000 7500000 5500000 11000000 58000000
>
> properties:
> + $nodename:
> + pattern: "pmic@[0-9a-f]{1,2}"
> compatible:
> - const: dlg,da9121
> + enum:
> + - dlg,da9121
> + - dlg,da9122
> + - dlg,da9220
> + - dlg,da9217
> + - dlg,da9130
> + - dlg,da9131
> + - dlg,da9132
> + - dlg,da9140
>
> reg:
> maxItems: 1
> + description: Specifies the I2C slave address.
> +
> + interrupts:
> + maxItems: 1
> + description: IRQ line information.
> +
> + dlg,irq-polling-delay-passive-ms:
> + $ref: "/schemas/types.yaml#/definitions/uint32"
Don't need a type with a standard unit suffix.
> + minimum: 1000
> + maximum: 10000
> + description: |
> + Specify the polling period, measured in milliseconds, between interrupt status
> + update checks. Range 1000-10000 ms.
>
> - buck1:
> - description:
> - Initial data for the Buck1 regulator.
> - $ref: "regulator.yaml#"
> + regulators:
> type: object
> + $ref: regulator.yaml#
'regulators' node is not a regulator, so this line should be dropped.
> + description: |
> + This node defines the settings for the BUCK. The content of the
> + sub-node is defined by the standard binding for regulators; see regulator.yaml.
> + The DA9121 regulator is bound using their names listed below
> + buck1 - BUCK1
> + buck2 - BUCK2 //DA9122, DA9220, DA9131, DA9132 only
>
> -additionalProperties: false
> + patternProperties:
> + "^buck([1-2])$":
> + type: object
> + $ref: regulator.yaml#
> +
> + properties:
> + regulator-mode:
> + maxItems: 1
> + description: Defined in include/dt-bindings/regulator/dlg,da9121-regulator.h
'regulator-mode' is defined as a property of a
'regulator-state-(standby|mem|disk)' child node. I don't see how you
would use this with 'regulator-initial-mode' either.
> +
> + regulator-initial-mode:
> + maxItems: 1
'maxItems' applies to arrays and this is not an array. What you should
have is constraints on the values:
enum: [ 0, 1, 2, 3 ]
> + description: Defined in include/dt-bindings/regulator/dlg,da9121-regulator.h
> +
> + enable-gpios:
> + maxItems: 1
> + description: Specify a valid GPIO for platform control of the regulator
> +
> + dlg,ripple-cancel:
> + $ref: "/schemas/types.yaml#/definitions/uint32"
> + description: |
> + Defined in include/dt-bindings/regulator/dlg,da9121-regulator.h
> + Only present on multi-channel devices (DA9122, DA9220, DA9131, DA9132)
enum: [ 0, 1, 2, 3 ]
> +
> + unevaluatedProperties: false
>
> required:
> - compatible
> - reg
> + - regulators
> +
> +additionalProperties: false
>
> examples:
> - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> + #include <dt-bindings/regulator/dlg,da9121-regulator.h>
> i2c {
> #address-cells = <1>;
> #size-cells = <0>;
> - regulator@68 {
> + pmic@68 {
> compatible = "dlg,da9121";
> reg = <0x68>;
>
> - buck1 {
> - regulator-min-microvolt = <680000>;
> - regulator-max-microvolt = <820000>;
> + interrupt-parent = <&gpio6>;
> + interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
> +
> + dlg,irq-polling-delay-passive-ms = <2000>;
> +
> + regulators {
> + DA9121_BUCK1: buck1 {
> + regulator-name = "BUCK1";
> + regulator-min-microvolt = <300000>;
> + regulator-max-microvolt = <1900000>;
> + regulator-min-microamp = <7000000>;
> + regulator-max-microamp = <20000000>;
> + regulator-boot-on;
> + regulator-initial-mode = <DA9121_BUCK_MODE_AUTO>;
> + enable-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
> + };
> };
> };
> };
>
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> + #include <dt-bindings/regulator/dlg,da9121-regulator.h>
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + pmic@68 {
> + compatible = "dlg,da9122";
> + reg = <0x68>;
> +
> + interrupt-parent = <&gpio6>;
> + interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
> +
> + dlg,irq-polling-delay-passive-ms = <2000>;
> +
> + regulators {
> + DA9122_BUCK1: buck1 {
> + regulator-name = "BUCK1";
> + regulator-min-microvolt = <300000>;
> + regulator-max-microvolt = <1900000>;
> + regulator-min-microamp = <3500000>;
> + regulator-max-microamp = <10000000>;
> + regulator-boot-on;
> + regulator-initial-mode = <DA9121_BUCK_MODE_AUTO>;
> + enable-gpios = <&gpio6 1 GPIO_ACTIVE_HIGH>;
> + dlg,ripple-cancel = <DA9121_BUCK_RIPPLE_CANCEL_NONE>;
> + };
> + DA9122_BUCK2: buck2 {
> + regulator-name = "BUCK2";
> + regulator-min-microvolt = <300000>;
> + regulator-max-microvolt = <1900000>;
> + regulator-min-microamp = <3500000>;
> + regulator-max-microamp = <10000000>;
> + regulator-boot-on;
> + regulator-initial-mode = <DA9121_BUCK_MODE_AUTO>;
> + enable-gpios = <&gpio6 2 GPIO_ACTIVE_HIGH>;
> + dlg,ripple-cancel = <DA9121_BUCK_RIPPLE_CANCEL_NONE>;
> + };
> + };
> + };
> + };
> ...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9bff945..1e5b756 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5118,6 +5118,7 @@ S: Supported
> W: http://www.dialog-semiconductor.com/products
> F: Documentation/devicetree/bindings/input/da90??-onkey.txt
> F: Documentation/devicetree/bindings/mfd/da90*.txt
> +F: Documentation/devicetree/bindings/regulator/dlg,da9*.yaml
> F: Documentation/devicetree/bindings/regulator/da92*.txt
> F: Documentation/devicetree/bindings/regulator/slg51000.txt
> F: Documentation/devicetree/bindings/sound/da[79]*.txt
> @@ -5142,6 +5143,7 @@ F: drivers/rtc/rtc-da90??.c
> F: drivers/thermal/da90??-thermal.c
> F: drivers/video/backlight/da90??_bl.c
> F: drivers/watchdog/da90??_wdt.c
> +F: include/dt-bindings/regulator/dlg,da9*-regulator.h
> F: include/linux/mfd/da903x.h
> F: include/linux/mfd/da9052/
> F: include/linux/mfd/da9055/
> diff --git a/include/dt-bindings/regulator/dlg,da9121-regulator.h b/include/dt-bindings/regulator/dlg,da9121-regulator.h
> new file mode 100644
> index 0000000..954edf6
> --- /dev/null
> +++ b/include/dt-bindings/regulator/dlg,da9121-regulator.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +
> +#ifndef _DT_BINDINGS_REGULATOR_DLG_DA9121_H
> +#define _DT_BINDINGS_REGULATOR_DLG_DA9121_H
> +
> +/*
> + * These buck mode constants may be used to specify values in device tree
> + * properties (e.g. regulator-initial-mode).
> + * A description of the following modes is in the manufacturers datasheet.
> + */
> +
> +#define DA9121_BUCK_MODE_FORCE_PFM 0
> +#define DA9121_BUCK_MODE_FORCE_PWM 1
> +#define DA9121_BUCK_MODE_FORCE_PWM_SHEDDING 2
> +#define DA9121_BUCK_MODE_AUTO 3
> +
> +#define DA9121_BUCK_RIPPLE_CANCEL_NONE 0
> +#define DA9121_BUCK_RIPPLE_CANCEL_SMALL 1
> +#define DA9121_BUCK_RIPPLE_CANCEL_MID 2
> +#define DA9121_BUCK_RIPPLE_CANCEL_LARGE 3
> +
> +#endif
> --
> 1.9.1
>
next prev parent reply other threads:[~2020-12-07 17:59 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-01 13:52 [PATCH V4 00/10] regulator: da9121: extend support to variants, add features Adam Ward
2020-12-01 13:52 ` [PATCH V4 01/10] regulator: Update DA9121 dt-bindings Adam Ward
2020-12-07 17:58 ` Rob Herring [this message]
2020-12-08 10:41 ` Adam Ward
2020-12-01 13:52 ` [PATCH V4 02/10] regulator: da9121: Add header file Adam Ward
2020-12-01 13:52 ` [PATCH V4 03/10] regulator: da9121: Add device variants Adam Ward
2020-12-01 13:52 ` [PATCH V4 04/10] regulator: da9121: Add device variant regmaps Adam Ward
2020-12-01 13:52 ` [PATCH V4 05/10] regulator: da9121: Add device variant descriptors Adam Ward
2020-12-01 13:52 ` [PATCH V4 06/10] regulator: da9121: Add support for device variants via devicetree Adam Ward
2020-12-01 13:52 ` [PATCH V4 07/10] regulator: da9121: Update registration to support multiple buck variants Adam Ward
2020-12-01 13:52 ` [PATCH V4 08/10] regulator: da9121: add current support Adam Ward
2020-12-01 13:52 ` [PATCH V4 09/10] regulator: da9121: add mode support Adam Ward
2020-12-01 13:52 ` [PATCH V4 10/10] regulator: da9121: add interrupt support Adam Ward
2020-12-01 14:01 ` [PATCH V4 00/10] regulator: da9121: extend support to variants, add features Mark Brown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201207175809.GA503826@robh.at.kernel.org \
--to=robh@kernel.org \
--cc=Adam.Ward.opensource@diasemi.com \
--cc=broonie@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=support.opensource@diasemi.com \
--cc=vincent.whitchurch@axis.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.