Re: [PATCH 1/2] dt-bindings: regulator: Add pf8x00 regulator
On Thu, Dec 10, 2020 at 11:24:17PM +0100, Adrien Grassein wrote: > Le lun. 7 déc. 2020 à 14:55, Mark Brown a écrit : > > On Sun, Dec 06, 2020 at 01:26:28AM +0100, Adrien Grassein wrote: > > > Add dt-bindings for the pf8x00 driver. > > Please submit patches using subject lines reflecting the style for the > > subsystem, this makes it easier for people to identify relevant patches. > > Look at what existing commits in the area you're changing are doing and > > make sure your subject lines visually resemble what they're doing. > > There's no need to resubmit to fix this alone. > For v2 I just copy-paste another commit message to be sure to be conform. For patches for a given subsystem you should use the prefix the subsystem uses, for regulator that's "regulator: ". > > > +$ref: /schemas/types.yaml#definitions/flag > > > +description: | > > > + Only available for ldo2. When specified, use the VSELECT > > > pin > > > + of the chip to control the output voltage of the ldo02 > > > regulator. > > Shouldn't there be a GPIO specified somewhere or something so that the > > VSELECT pin can be controlled? > I think I read better documentation for this point. Sorry, it was very > unclear. > VSELECT is in fact an input pin of the chip. The configuration just enabled > it. Then presumably you need some binding to specify how to control this input too? > > > + nxp,quad-phase: > > > +$ref: /schemas/types.yaml#definitions/flag > > > +description: | > > > + This allow regulators sw1 and sw2, or sw3 and sw4 or sw4 > > > and sw5 > > > + to work together to deliver a maximum 10A current. > > Presumably this must be set on both the regulators being grouped > > together? > Not. Only the sw1 configuration will be taken in account. That needs to be documented then. signature.asc Description: PGP signature
Re: [PATCH 1/2] dt-bindings: regulator: Add pf8x00 regulator
Hi, Thanks for reviewing my patches. Le lun. 7 déc. 2020 à 14:55, Mark Brown a écrit : > > On Sun, Dec 06, 2020 at 01:26:28AM +0100, Adrien Grassein wrote: > > Add dt-bindings for the pf8x00 driver. > > Please submit patches using subject lines reflecting the style for the > subsystem, this makes it easier for people to identify relevant patches. > Look at what existing commits in the area you're changing are doing and > make sure your subject lines visually resemble what they're doing. > There's no need to resubmit to fix this alone. > For v2 I just copy-paste another commit message to be sure to be conform. > > + compatible: > > +enum: > > + - nxp,pf8x00 > > Compatible strings should be for specific devices not wildcards. > > > + nxp,hw-en: > > +$ref: /schemas/types.yaml#definitions/flag > > +description: | > > + Only available for ldo2. Used to enable or disable ld02. > > I don't understand what this is documenting - what is "hw-en" and how is > it used to enable or disable LDO2? I think I read better documentation for this point. Sorry, it was very unclear. > > > + nxp,vselect-en: > > +$ref: /schemas/types.yaml#definitions/flag > > +description: | > > + Only available for ldo2. When specified, use the VSELECT pin > > + of the chip to control the output voltage of the ldo02 > > regulator. > > Shouldn't there be a GPIO specified somewhere or something so that the > VSELECT pin can be controlled? I think I read better documentation for this point. Sorry, it was very unclear. VSELECT is in fact an input pin of the chip. The configuration just enabled it. > > > + nxp,ilim-ma: > > +$ref: /schemas/types.yaml#definitions/uint32 > > +minimum: 2100 > > +maximum: 4500 > > +default: 2100 > > +enum: [ 2100, 2600, 3000, 4500 ] > > +description: | > > + Defines the maximum current delivered by the regulator (in > > mA). > > Is this not a fixed property of the regulator? It's not. It's configurable by software. > > > + nxp,quad-phase: > > +$ref: /schemas/types.yaml#definitions/flag > > +description: | > > + This allow regulators sw1 and sw2, or sw3 and sw4 or sw4 > > and sw5 > > + to work together to deliver a maximum 10A current. > > Presumably this must be set on both the regulators being grouped > together? Not. Only the sw1 configuration will be taken in account. Thanks again, Adrien
Re: [PATCH 1/2] dt-bindings: regulator: Add pf8x00 regulator
On Sun, Dec 06, 2020 at 01:26:28AM +0100, Adrien Grassein wrote:
> Add dt-bindings for the pf8x00 driver.
>
> Signed-off-by: Adrien Grassein
> ---
> .../regulator/nxp,pf8x00-regulator.yaml | 223 ++
> MAINTAINERS | 6 +
> 2 files changed, 229 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
>
> diff --git
> a/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
> b/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
> new file mode 100644
> index ..f4e4f545c5a3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
> @@ -0,0 +1,223 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/nxp,pf8x00-regulator.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP PF8x00 Power Management Integrated Circuit regulators
> +
> +maintainers:
> + - Adrien Grassein
> +
> +description: |
> + pf8x00 are a 12-chanel regulator PMIC family. Regulators nodes should
> + be named to ldo<>, sw<> and vnss. The definition for each of these nodes
> + is defined using the standard binding for regulators at
> + Documentation/devicetree/bindings/regulator/regulator.txt.
> + Datasheet is available at
> https://www.nxp.com/docs/en/data-sheet/PF8100_PF8200.pdf
> +
> +properties:
> + compatible:
> +enum:
> + - nxp,pf8x00
> +
> + reg:
> +maxItems: 1
> +
> + regulators:
> +type: object
> +description: |
Don't need '|' if no formatting.
> + list of regulators provided by this controller
> +
> +patternProperties:
> + "^ldo[1-4]$":
> +type: object
> +$ref: regulator.yaml#
> +description: |
> + Properties for single LDO regulator.
> +
> +properties:
> + regulator-name:
> +pattern: "^ldo[1-4]$"
> +description: |
> + should be ldo1", ..., "ldo4"
> +
> + nxp,hw-en:
> +$ref: /schemas/types.yaml#definitions/flag
> +description: |
> + Only available for ldo2. Used to enable or disable ld02.
> +
> + nxp,vselect-en:
> +$ref: /schemas/types.yaml#definitions/flag
> +description: |
> + Only available for ldo2. When specified, use the VSELECT pin
> + of the chip to control the output voltage of the ldo02
> regulator.
> +
> +unevaluatedProperties: false
> +
> + "^sw[1-7]$":
> +type: object
> +$ref: regulator.yaml#
> +description: |
> + Properties for single SW regulator.
> +
> +properties:
> + regulator-name:
> +pattern: "^sw[1-7]$"
> +description: |
> + should be sw1", ..., "sw7"
> +
> + nxp,ilim-ma:
Use unit suffix defined in property-units.txt.
> +$ref: /schemas/types.yaml#definitions/uint32
> +minimum: 2100
> +maximum: 4500
> +default: 2100
> +enum: [ 2100, 2600, 3000, 4500 ]
> +description: |
> + Defines the maximum current delivered by the regulator (in mA).
> +
> + nxp,phase:
> +$ref: /schemas/types.yaml#definitions/uint32
> +minimum: 0
> +maximum: 315
> +default: 0
> +enum: [ 0, 45, 90, 135, 180, 225, 270, 315 ]
> +description: |
> + This controls the phase shift of the switching frequency.
> +
> + nxp,fsl,fast-slew:
nxp or fsl?
> +$ref: /schemas/types.yaml#definitions/flag
> +description: |
> + Controls the DVS ramp of the regulator.
> +
> + nxp,quad-phase:
> +$ref: /schemas/types.yaml#definitions/flag
> +description: |
> + This allow regulators sw1 and sw2, or sw3 and sw4 or sw4 and
> sw5
> + to work together to deliver a maximum 10A current.
> +
> + nxp,dual-phase:
> +$ref: /schemas/types.yaml#definitions/flag
> +description: |
> + This allow regulators sw1, sw2, sw3 and sw4 to work together
> + to deliver a maximum 5A current.
> +
> +unevaluatedProperties: false
> +
> + "^vsnvs$":
Move to 'properties', not a pattern.
> +type: object
> +$ref: regulator.yaml#
> +description: |
> + Properties for vsnvs regulator.
> +
> +unevaluatedProperties: false
> +
> +additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - regulators
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + pmic: pf8100@8 {
pmic@8
> +compatible = "nxp,pf8x00";
> +reg = <0x08>;
> +
> +
Re: [PATCH 1/2] dt-bindings: regulator: Add pf8x00 regulator
On Sun, Dec 06, 2020 at 01:26:28AM +0100, Adrien Grassein wrote: > Add dt-bindings for the pf8x00 driver. Please submit patches using subject lines reflecting the style for the subsystem, this makes it easier for people to identify relevant patches. Look at what existing commits in the area you're changing are doing and make sure your subject lines visually resemble what they're doing. There's no need to resubmit to fix this alone. > + compatible: > +enum: > + - nxp,pf8x00 Compatible strings should be for specific devices not wildcards. > + nxp,hw-en: > +$ref: /schemas/types.yaml#definitions/flag > +description: | > + Only available for ldo2. Used to enable or disable ld02. I don't understand what this is documenting - what is "hw-en" and how is it used to enable or disable LDO2? > + nxp,vselect-en: > +$ref: /schemas/types.yaml#definitions/flag > +description: | > + Only available for ldo2. When specified, use the VSELECT pin > + of the chip to control the output voltage of the ldo02 > regulator. Shouldn't there be a GPIO specified somewhere or something so that the VSELECT pin can be controlled? > + nxp,ilim-ma: > +$ref: /schemas/types.yaml#definitions/uint32 > +minimum: 2100 > +maximum: 4500 > +default: 2100 > +enum: [ 2100, 2600, 3000, 4500 ] > +description: | > + Defines the maximum current delivered by the regulator (in mA). Is this not a fixed property of the regulator? > + nxp,quad-phase: > +$ref: /schemas/types.yaml#definitions/flag > +description: | > + This allow regulators sw1 and sw2, or sw3 and sw4 or sw4 and > sw5 > + to work together to deliver a maximum 10A current. Presumably this must be set on both the regulators being grouped together? signature.asc Description: PGP signature
[PATCH 1/2] dt-bindings: regulator: Add pf8x00 regulator
Add dt-bindings for the pf8x00 driver.
Signed-off-by: Adrien Grassein
---
.../regulator/nxp,pf8x00-regulator.yaml | 223 ++
MAINTAINERS | 6 +
2 files changed, 229 insertions(+)
create mode 100644
Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
diff --git
a/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
b/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
new file mode 100644
index ..f4e4f545c5a3
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
@@ -0,0 +1,223 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/nxp,pf8x00-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP PF8x00 Power Management Integrated Circuit regulators
+
+maintainers:
+ - Adrien Grassein
+
+description: |
+ pf8x00 are a 12-chanel regulator PMIC family. Regulators nodes should
+ be named to ldo<>, sw<> and vnss. The definition for each of these nodes
+ is defined using the standard binding for regulators at
+ Documentation/devicetree/bindings/regulator/regulator.txt.
+ Datasheet is available at
https://www.nxp.com/docs/en/data-sheet/PF8100_PF8200.pdf
+
+properties:
+ compatible:
+enum:
+ - nxp,pf8x00
+
+ reg:
+maxItems: 1
+
+ regulators:
+type: object
+description: |
+ list of regulators provided by this controller
+
+patternProperties:
+ "^ldo[1-4]$":
+type: object
+$ref: regulator.yaml#
+description: |
+ Properties for single LDO regulator.
+
+properties:
+ regulator-name:
+pattern: "^ldo[1-4]$"
+description: |
+ should be ldo1", ..., "ldo4"
+
+ nxp,hw-en:
+$ref: /schemas/types.yaml#definitions/flag
+description: |
+ Only available for ldo2. Used to enable or disable ld02.
+
+ nxp,vselect-en:
+$ref: /schemas/types.yaml#definitions/flag
+description: |
+ Only available for ldo2. When specified, use the VSELECT pin
+ of the chip to control the output voltage of the ldo02 regulator.
+
+unevaluatedProperties: false
+
+ "^sw[1-7]$":
+type: object
+$ref: regulator.yaml#
+description: |
+ Properties for single SW regulator.
+
+properties:
+ regulator-name:
+pattern: "^sw[1-7]$"
+description: |
+ should be sw1", ..., "sw7"
+
+ nxp,ilim-ma:
+$ref: /schemas/types.yaml#definitions/uint32
+minimum: 2100
+maximum: 4500
+default: 2100
+enum: [ 2100, 2600, 3000, 4500 ]
+description: |
+ Defines the maximum current delivered by the regulator (in mA).
+
+ nxp,phase:
+$ref: /schemas/types.yaml#definitions/uint32
+minimum: 0
+maximum: 315
+default: 0
+enum: [ 0, 45, 90, 135, 180, 225, 270, 315 ]
+description: |
+ This controls the phase shift of the switching frequency.
+
+ nxp,fsl,fast-slew:
+$ref: /schemas/types.yaml#definitions/flag
+description: |
+ Controls the DVS ramp of the regulator.
+
+ nxp,quad-phase:
+$ref: /schemas/types.yaml#definitions/flag
+description: |
+ This allow regulators sw1 and sw2, or sw3 and sw4 or sw4 and sw5
+ to work together to deliver a maximum 10A current.
+
+ nxp,dual-phase:
+$ref: /schemas/types.yaml#definitions/flag
+description: |
+ This allow regulators sw1, sw2, sw3 and sw4 to work together
+ to deliver a maximum 5A current.
+
+unevaluatedProperties: false
+
+ "^vsnvs$":
+type: object
+$ref: regulator.yaml#
+description: |
+ Properties for vsnvs regulator.
+
+unevaluatedProperties: false
+
+additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - regulators
+
+additionalProperties: false
+
+examples:
+ - |
+i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ pmic: pf8100@8 {
+compatible = "nxp,pf8x00";
+reg = <0x08>;
+
+regulators {
+reg_ldo1: ldo1 {
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-max-microvolt = <500>;
+ regulator-min-microvolt = <150>;
+};
+
+reg_ldo2: ldo2 {
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-max-microvolt = <500>;
+ regulator-min-microvolt = <150>;
+};
+
+reg_ldo3: ldo3 {
+ regulator-always-on;
+
Re: [PATCH 1/2] dt-bindings: regulator: Add pf8x00 regulator bindings
On Mon, 30 Nov 2020 16:53:28 +0530, Jagan Teki wrote: > Add NXP PF8100/PF8121A/PF8200 regulators bindings. Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next Thanks! [1/2] regulator: Add pf8x00 regulator bindings commit: 4b748fb3448b5d39a58cdea55d72e8dcd128f4c6 [2/2] regulator: Add NXP PF8X00 regulator driver commit: d3795d6321ecaa55d94dc24c3b1e3cce608aabd6 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
[PATCH 1/2] dt-bindings: regulator: Add pf8x00 regulator bindings
Add NXP PF8100/PF8121A/PF8200 regulators bindings.
Signed-off-by: Jagan Teki
---
.../regulator/nxp,pf8x00-regulator.yaml | 211 ++
1 file changed, 211 insertions(+)
create mode 100644
Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
diff --git
a/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
b/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
new file mode 100644
index ..a6c259ce9785
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/nxp,pf8x00-regulator.yaml
@@ -0,0 +1,211 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/nxp,pf8x00-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP PF8100/PF8121A/PF8200 PMIC regulators
+
+maintainers:
+ - Jagan Teki
+ - Troy Kisky
+
+description: |
+ PF8100/PF8121A/PF8200 is a PMIC designed for highperformance consumer
+ applications. It features seven high efficiency buck converters, four
+ linear and one vsnvs regulators. It has built-in one time programmable
+ fuse bank for device configurations.
+
+properties:
+ compatible:
+enum:
+ - nxp,pf8x00
+
+ reg:
+maxItems: 1
+
+ regulators:
+type: object
+description: |
+ list of regulators provided by this controller
+
+patternProperties:
+ "^ldo[1-4]$":
+type: object
+$ref: regulator.yaml#
+description:
+ Properties for single LDO regulator.
+
+properties:
+ regulator-name:
+pattern: "^ldo[1-4]$"
+description:
+ should be "ldo1", ..., "ldo4"
+
+unevaluatedProperties: false
+
+ "^buck[1-7]$":
+type: object
+$ref: regulator.yaml#
+description:
+ Properties for single BUCK regulator.
+
+properties:
+ regulator-name:
+pattern: "^buck[1-7]$"
+description:
+ should be "buck1", ..., "buck7"
+
+ nxp,ilim-ma:
+$ref: "/schemas/types.yaml#/definitions/uint32"
+minimum: 2100
+maximum: 4500
+description:
+ BUCK regulators current limit in mA.
+
+ Listed current limits in mA are,
+ 2100 (default)
+ 2600
+ 3000
+ 4500
+
+ nxp,phase-shift:
+$ref: "/schemas/types.yaml#/definitions/uint32"
+minimum: 45
+maximum: 0
+description:
+ BUCK regulators phase shift control in degrees.
+
+ Listed phase shift control values in degrees are,
+ 45
+ 90
+ 135
+ 180
+ 225
+ 270
+ 315
+ 0 (default)
+
+unevaluatedProperties: false
+
+ "^vsnvs$":
+type: object
+$ref: regulator.yaml#
+description:
+ Properties for single VSNVS regulator.
+
+properties:
+ regulator-name:
+pattern: "^vsnvs$"
+description:
+ should be "vsnvs"
+
+unevaluatedProperties: false
+
+additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - regulators
+
+additionalProperties: false
+
+examples:
+ - |
+i2c1 {
+#address-cells = <1>;
+#size-cells = <0>;
+
+pmic@8 {
+compatible = "nxp,pf8x00";
+reg = <0x08>;
+
+regulators {
+reg_ldo1: ldo1 {
+regulator-always-on;
+regulator-boot-on;
+regulator-max-microvolt = <500>;
+regulator-min-microvolt = <150>;
+};
+
+reg_ldo2: ldo2 {
+regulator-always-on;
+regulator-boot-on;
+regulator-max-microvolt = <500>;
+regulator-min-microvolt = <150>;
+};
+
+reg_ldo3: ldo3 {
+regulator-always-on;
+regulator-boot-on;
+regulator-max-microvolt = <500>;
+regulator-min-microvolt = <150>;
+};
+
+reg_ldo4: ldo4 {
+regulator-always-on;
+regulator-boot-on;
+regulator-max-microvolt = <500>;
+regulator-min-microvolt = <150>;
+};
+
+reg_buck1: buck1 {
+nxp,ilim-ma = <4500>;
+regulator-always-on;
+regulator-boot-on;
+regulator-max-microvolt = <180>;
+regulator-min-microvolt = <40>;
+};
+
+reg_buck2: buck2 {
+regulator-always-on;
+regulator-boot-on;
+
