[PATCH v9 0/5] mfd: axp20x: add support for AXP202 and AXP209

[PATCH v9 0/5] mfd: axp20x: add support for AXP202 and AXP209

[Chen-Yu Tsai]

Hi everyone,

This is v9 of the axp209 series. v9 changes the DT bindings per Lee's
recommendations. This introduces 3 new patches to update the regulator
driver and dtsi to use the new bindings.

The regulator patches are based on the axp209 topic branch from the
regulator tree, which atm is just the 2 patches I did to use the
regulator core DT parsing functions:

    regulator: axp20x: Migrate to regulator core's simplified DT parsing code
    regulator: axp20x: Fill regulators_node and of_match descriptor fields

So far, "x-powers,dcdc-workmode" is not used anywhere. "x-powers,dcdc-freq"
is only used in axp209.dtsi which was just introduced. It should be fairly
safe to change the bindings.

Lee, Mark, could you take a look at the new changes? If they're OK, could
you merge them through your respective trees?

Maxime, could you merge the axp209.dtsi patch after the others are merged?

Thank you!


Regards,
ChenYu


Changes since v8:

    - Dropped patches that were merged
    - Changed "x-powers,dcdc-workmode" to boolean flag
      "x-powers,dcdc-workmode-pwm", and default to auto when not present.
    - Use standard "clock-frequency" instead of custom "x-powers,dcdc-freq"
      and use HZ as unit.
    - Added missing ldo5 regulator node in DT binding example.
    - Added 2 patches to update regulator driver to new bindings.
    - Added patch to update axp209.dtsi to new bindings.


Carlo Caione (2):
  dt-bindings: add vendor-prefix for X-Powers
  mfd: AXP20x: Add bindings documentation

Chen-Yu Tsai (3):
  regulators: axp20x: Use "clock-frequency" property for dcdc work
    frequency
  regulators: axp20x: Change dcdc-workmode DT property to boolean flag
  ARM: sunxi: dts: axp209: Use clock-frequency for DC-DC work frequency

 Documentation/devicetree/bindings/mfd/axp20x.txt   | 102 +++++++++++++++++++++
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 arch/arm/boot/dts/axp209.dtsi                      |   2 +-
 drivers/regulator/axp20x-regulator.c               |  33 +++----
 4 files changed, 121 insertions(+), 17 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/axp20x.txt

-- 
2.1.4

[PATCH v9 1/5] dt-bindings: add vendor-prefix for X-Powers

[Chen-Yu Tsai]

From: Carlo Caione <carlo@caione.org>

Signed-off-by: Carlo Caione <carlo@caione.org>
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 38c13dc19983..76e288a17996 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -173,6 +173,7 @@ voipac	Voipac Technologies s.r.o.
 winbond Winbond Electronics corp.
 wlf	Wolfson Microelectronics
 wm	Wondermedia Technologies, Inc.
+x-powers	X-Powers
 xes	Extreme Engineering Solutions (X-ES)
 xillybus	Xillybus Ltd.
 xlnx	Xilinx
-- 
2.1.4

[PATCH v9 2/5] mfd: AXP20x: Add bindings documentation

[Chen-Yu Tsai]

From: Carlo Caione <carlo@caione.org>

Bindings documentation for the AXP20x driver. In this file also
sub-nodes are documented.

Signed-off-by: Carlo Caione <carlo@caione.org>
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
[wens@csie.org: clarify interrupt source for the axp PMIC]
[wens@csie.org: explain dcdc-workmode in detail and trim lines to 80 chars]
[wens@csie.org: make regulator supplies optional if using unregulated input]
[wens@csie.org: use cubieboard2 regulator nodes as example]
[wens@csie.org: change dcdc-workmode to dcdc-workmode-pwm boolean property]
[wens@csie.org: use clock-frequency for dcdc regulator work frequency]
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 Documentation/devicetree/bindings/mfd/axp20x.txt | 102 +++++++++++++++++++++++
 1 file changed, 102 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/axp20x.txt

diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
new file mode 100644
index 000000000000..c3c80db242c7
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
@@ -0,0 +1,102 @@
+AXP202/AXP209 device tree bindings
+
+The axp20x family current members :
+axp202 (X-Powers)
+axp209 (X-Powers)
+
+Required properties:
+- compatible: "x-powers,axp202" or "x-powers,axp209"
+- reg: The I2C slave address for the AXP chip
+- interrupt-parent: The parent interrupt controller
+- interrupts: SoC NMI / GPIO interrupt connected to the PMIC's IRQ pin
+- interrupt-controller: axp20x has its own internal IRQs
+- #interrupt-cells: Should be set to 1
+- regulators: A node that houses a sub-node for each regulator. The regulators
+	      are bound using their name as listed here: dcdc2, dcdc3, ldo1,
+	      ldo2, ldo3, ldo4, ldo5.  The bindings details of individual
+	      regulator device can be found in:
+	      Documentation/devicetree/bindings/regulator/regulator.txt with
+	      the exception of x-powers,dcdc-freq. Regulators not used should
+	      still be listed for completeness and that the regulator subsystem
+	      properly registers them.
+
+Optional properties:
+- regulator supplies - may be omitted if inputs are unregulated, such as using
+		       the IPSOUT output from the PMIC
+  - acin-supply: The input supply for LDO1
+  - vin2-supply: The input supply for DCDC2
+  - vin3-supply: The input supply for DCDC3
+  - ldo24in-supply: The input supply for LDO2, LDO4
+  - ldo3in-supply: The input supply for LDO3
+  - ldo5in-supply: The input supply for LDO5
+
+Optional properties for regulators node:
+- clock-frequency: defines the work frequency of DC-DC regulators
+		   (range: 750kHz - 1875kHz). Default: 1.5MHz
+
+Optional properties for DCDC regulators:
+- x-powers,dcdc-workmode-pwm: Force the DCDC regulators to run in PWM mode
+			      The DCDC regulators rormally work in a mixed
+			      PWM/PFM mode, using PFM under light loads and
+			      switching to PWM for heavier loads. Forcing
+			      PWM mode trades efficiency under light loads
+			      for lower output noise. This probably makes
+			      sense for HiFi audio related applications that
+			      aren't battery constrained.
+
+Example:
+
+axp209: pmic@34 {
+	compatible = "x-powers,axp209";
+	reg = <0x34>;
+	interrupt-parent = <&nmi_intc>;
+	interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+	interrupt-controller;
+	#interrupt-cells = <1>;
+
+	regulators {
+		clock-frequency = <1500000>;
+
+		vdd_cpu: dcdc2 {
+			regulator-always-on;
+			regulator-min-microvolt = <1000000>;
+			regulator-max-microvolt = <1450000>;
+			regulator-name = "vdd-cpu";
+		};
+
+		vdd_int_dll: dcdc3 {
+			regulator-always-on;
+			regulator-min-microvolt = <1000000>;
+			regulator-max-microvolt = <1400000>;
+			regulator-name = "vdd-int-dll";
+		};
+
+		vdd_rtc: ldo1 {
+			regulator-always-on;
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1400000>;
+			regulator-name = "vdd-rtc";
+		};
+
+		avcc: ldo2 {
+			regulator-always-on;
+			regulator-min-microvolt = <2700000>;
+			regulator-max-microvolt = <3300000>;
+			regulator-name = "avcc";
+		};
+
+		ldo3 {
+			/* unused */
+		};
+
+		csi1_io_2v8: ldo4 {
+			/* output on extension headers */
+			regulator-name = "csi1-io-2v8";
+		};
+
+		ldo5 {
+			/* unused */
+		};
+	};
+};
+
-- 
2.1.4

[PATCH v9 3/5] regulators: axp20x: Use "clock-frequency" property for dcdc work frequency

[Chen-Yu Tsai]

Since the dcdc regulator work frequency is a property of the regulators
DT sub-tree, there is no issue of naming collisions, so we can use the
standard "clock-frequency" binding for this.

The driver sets a reasonable default value (same as the datasheet), and
only 1 dts has the old binding, with the default value.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/regulator/axp20x-regulator.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index e4331f5e5d7d..990ac453abc8 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -186,8 +186,10 @@ static int axp20x_regulator_parse_dt(struct platform_device *pdev)
 	if (!regulators) {
 		dev_warn(&pdev->dev, "regulators node not found\n");
 	} else {
-		dcdcfreq = 1500;
-		of_property_read_u32(regulators, "x-powers,dcdc-freq", &dcdcfreq);
+		dcdcfreq = 1500000;
+		of_property_read_u32(regulators, "clock-frequency", &dcdcfreq);
+		/* axp20x_set_dcdc_freq() takes kHZ */
+		dcdcfreq /= 1000;
 		ret = axp20x_set_dcdc_freq(pdev, dcdcfreq);
 		if (ret < 0) {
 			dev_err(&pdev->dev, "Error setting dcdc frequency: %d\n", ret);
-- 
2.1.4

[PATCH v9 4/5] regulators: axp20x: Change dcdc-workmode DT property to boolean flag

[Chen-Yu Tsai]

The "x-powers,dcdc-workmode" property only has 2 values. A request was
made to change this to a boolean flag named "x-powers,dcdc-workmode-pwm".
Given that this particular binding hasn't been used anywhere, it should
be safe to change.

Previously the absence of the property meant just keeping the PMIC in
whatever state was set by the bootloader. After this change, the absence
means using the "auto" (PWM/PFM hybrid) mode, which is also the hardware's
default.

This patch also makes axp20x_set_dcdc_workmode() ignore attempts to set
the workmode for non-DCDC regulators.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/regulator/axp20x-regulator.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 990ac453abc8..0b099e5ece3f 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -202,19 +202,21 @@ static int axp20x_regulator_parse_dt(struct platform_device *pdev)
 	return 0;
 }
 
-static int axp20x_set_dcdc_workmode(struct regulator_dev *rdev, int id, u32 workmode)
+static int axp20x_set_dcdc_workmode(struct regulator_dev *rdev, int id, bool workmode)
 {
 	unsigned int mask = AXP20X_WORKMODE_DCDC2_MASK;
+	u32 val;
 
+	/* ignore setting workmode for non-dcdc regulators */
 	if ((id != AXP20X_DCDC2) && (id != AXP20X_DCDC3))
-		return -EINVAL;
+		return 0;
 
 	if (id == AXP20X_DCDC3)
 		mask = AXP20X_WORKMODE_DCDC3_MASK;
 
-	workmode <<= ffs(mask) - 1;
+	val = workmode ? mask : 0;
 
-	return regmap_update_bits(rdev->regmap, AXP20X_DCDC_MODE, mask, workmode);
+	return regmap_update_bits(rdev->regmap, AXP20X_DCDC_MODE, mask, val);
 }
 
 static int axp20x_regulator_probe(struct platform_device *pdev)
@@ -225,8 +227,8 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 		.dev = pdev->dev.parent,
 		.regmap = axp20x->regmap,
 	};
-	int ret, i;
-	u32 workmode;
+	int i;
+	bool workmode;
 
 	/* This only sets the dcdc freq. Ignore any errors */
 	axp20x_regulator_parse_dt(pdev);
@@ -241,14 +243,11 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 			return PTR_ERR(rdev);
 		}
 
-		ret = of_property_read_u32(rdev->dev.of_node,
-					   "x-powers,dcdc-workmode",
-					   &workmode);
-		if (!ret) {
-			if (axp20x_set_dcdc_workmode(rdev, i, workmode))
-				dev_err(&pdev->dev, "Failed to set workmode on %s\n",
-					axp20x_regulators[i].name);
-		}
+		workmode = of_property_read_bool(rdev->dev.of_node,
+						 "x-powers,dcdc-workmode-pwm");
+		if (axp20x_set_dcdc_workmode(rdev, i, workmode))
+			dev_err(&pdev->dev, "Failed to set workmode on %s\n",
+				axp20x_regulators[i].name);
 	}
 
 	return 0;
-- 
2.1.4

[PATCH v9 5/5] ARM: sunxi: dts: axp209: Use clock-frequency for DC-DC work frequency

[Chen-Yu Tsai]

Update the axp209 dtsi to use new standard bindings for DCDC regulator
work frequency.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm/boot/dts/axp209.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/axp209.dtsi b/arch/arm/boot/dts/axp209.dtsi
index c20cf537f5a5..ee9487583d31 100644
--- a/arch/arm/boot/dts/axp209.dtsi
+++ b/arch/arm/boot/dts/axp209.dtsi
@@ -60,7 +60,7 @@
 
 	regulators {
 		/* Default work frequency for buck regulators */
-		x-powers,dcdc-freq = <1500>;
+		clock-frequency = <1500000>;
 
 		reg_dcdc2: dcdc2 {
 			regulator-name = "dcdc2";
-- 
2.1.4

Re: [PATCH v9 2/5] mfd: AXP20x: Add bindings documentation

[Lee Jones]

On Thu, 22 Jan 2015, Chen-Yu Tsai wrote:

> From: Carlo Caione <carlo@caione.org>
> 
> Bindings documentation for the AXP20x driver. In this file also
> sub-nodes are documented.
> 
> Signed-off-by: Carlo Caione <carlo@caione.org>
> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> [wens@csie.org: clarify interrupt source for the axp PMIC]
> [wens@csie.org: explain dcdc-workmode in detail and trim lines to 80 chars]
> [wens@csie.org: make regulator supplies optional if using unregulated input]
> [wens@csie.org: use cubieboard2 regulator nodes as example]
> [wens@csie.org: change dcdc-workmode to dcdc-workmode-pwm boolean property]
> [wens@csie.org: use clock-frequency for dcdc regulator work frequency]
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  Documentation/devicetree/bindings/mfd/axp20x.txt | 102 +++++++++++++++++++++++
>  1 file changed, 102 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/axp20x.txt

This looks okay to me now.

If Mark is happy with the remainder, I'll merge this.

For my own reference:
Acked-by: Lee Jones <lee.jones@linaro.org>

> diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
> new file mode 100644
> index 000000000000..c3c80db242c7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
> @@ -0,0 +1,102 @@
> +AXP202/AXP209 device tree bindings
> +
> +The axp20x family current members :
> +axp202 (X-Powers)
> +axp209 (X-Powers)
> +
> +Required properties:
> +- compatible: "x-powers,axp202" or "x-powers,axp209"
> +- reg: The I2C slave address for the AXP chip
> +- interrupt-parent: The parent interrupt controller
> +- interrupts: SoC NMI / GPIO interrupt connected to the PMIC's IRQ pin
> +- interrupt-controller: axp20x has its own internal IRQs
> +- #interrupt-cells: Should be set to 1
> +- regulators: A node that houses a sub-node for each regulator. The regulators
> +	      are bound using their name as listed here: dcdc2, dcdc3, ldo1,
> +	      ldo2, ldo3, ldo4, ldo5.  The bindings details of individual
> +	      regulator device can be found in:
> +	      Documentation/devicetree/bindings/regulator/regulator.txt with
> +	      the exception of x-powers,dcdc-freq. Regulators not used should
> +	      still be listed for completeness and that the regulator subsystem
> +	      properly registers them.
> +
> +Optional properties:
> +- regulator supplies - may be omitted if inputs are unregulated, such as using
> +		       the IPSOUT output from the PMIC
> +  - acin-supply: The input supply for LDO1
> +  - vin2-supply: The input supply for DCDC2
> +  - vin3-supply: The input supply for DCDC3
> +  - ldo24in-supply: The input supply for LDO2, LDO4
> +  - ldo3in-supply: The input supply for LDO3
> +  - ldo5in-supply: The input supply for LDO5
> +
> +Optional properties for regulators node:
> +- clock-frequency: defines the work frequency of DC-DC regulators
> +		   (range: 750kHz - 1875kHz). Default: 1.5MHz
> +
> +Optional properties for DCDC regulators:
> +- x-powers,dcdc-workmode-pwm: Force the DCDC regulators to run in PWM mode
> +			      The DCDC regulators rormally work in a mixed
> +			      PWM/PFM mode, using PFM under light loads and
> +			      switching to PWM for heavier loads. Forcing
> +			      PWM mode trades efficiency under light loads
> +			      for lower output noise. This probably makes
> +			      sense for HiFi audio related applications that
> +			      aren't battery constrained.
> +
> +Example:
> +
> +axp209: pmic@34 {
> +	compatible = "x-powers,axp209";
> +	reg = <0x34>;
> +	interrupt-parent = <&nmi_intc>;
> +	interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> +	interrupt-controller;
> +	#interrupt-cells = <1>;
> +
> +	regulators {
> +		clock-frequency = <1500000>;
> +
> +		vdd_cpu: dcdc2 {
> +			regulator-always-on;
> +			regulator-min-microvolt = <1000000>;
> +			regulator-max-microvolt = <1450000>;
> +			regulator-name = "vdd-cpu";
> +		};
> +
> +		vdd_int_dll: dcdc3 {
> +			regulator-always-on;
> +			regulator-min-microvolt = <1000000>;
> +			regulator-max-microvolt = <1400000>;
> +			regulator-name = "vdd-int-dll";
> +		};
> +
> +		vdd_rtc: ldo1 {
> +			regulator-always-on;
> +			regulator-min-microvolt = <1200000>;
> +			regulator-max-microvolt = <1400000>;
> +			regulator-name = "vdd-rtc";
> +		};
> +
> +		avcc: ldo2 {
> +			regulator-always-on;
> +			regulator-min-microvolt = <2700000>;
> +			regulator-max-microvolt = <3300000>;
> +			regulator-name = "avcc";
> +		};
> +
> +		ldo3 {
> +			/* unused */
> +		};
> +
> +		csi1_io_2v8: ldo4 {
> +			/* output on extension headers */
> +			regulator-name = "csi1-io-2v8";
> +		};
> +
> +		ldo5 {
> +			/* unused */
> +		};
> +	};
> +};
> +

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v9 3/5] regulators: axp20x: Use "clock-frequency" property for dcdc work frequency

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 686 bytes --]

On Thu, Jan 22, 2015 at 11:39:05PM +0800, Chen-Yu Tsai wrote:
> Since the dcdc regulator work frequency is a property of the regulators
> DT sub-tree, there is no issue of naming collisions, so we can use the
> standard "clock-frequency" binding for this.
> 
> The driver sets a reasonable default value (same as the datasheet), and
> only 1 dts has the old binding, with the default value.

I don't see any benefit from this change - not only is this an
incompatible change (which may affect out of tree users as well as in
tree users) but it's not really doing anything with the clock API which
I'd expect to be happening for a clock API property.  What's the goal
here?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v9 4/5] regulators: axp20x: Change dcdc-workmode DT property to boolean flag

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1171 bytes --]

On Thu, Jan 22, 2015 at 11:39:06PM +0800, Chen-Yu Tsai wrote:
> The "x-powers,dcdc-workmode" property only has 2 values. A request was
> made to change this to a boolean flag named "x-powers,dcdc-workmode-pwm".
> Given that this particular binding hasn't been used anywhere, it should
> be safe to change.
> 
> Previously the absence of the property meant just keeping the PMIC in
> whatever state was set by the bootloader. After this change, the absence
> means using the "auto" (PWM/PFM hybrid) mode, which is also the hardware's
> default.
> 
> This patch also makes axp20x_set_dcdc_workmode() ignore attempts to set
> the workmode for non-DCDC regulators.

Again I don't understand what this is supposed to accomplish, this seems
to just make the binding less capable by forcing Linux to make a
decision on the configuration rather than allowing it to use the
hardware defaults untouched.  This runs contrary to our normal approach
to regulaors which is to try to avoid touching the hardware unless
explicit configuration is provided to say it's OK which in turn is there
because getting regulators wrong can be such a bad thing for the system.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v9 2/5] mfd: AXP20x: Add bindings documentation

[Chen-Yu Tsai]

Hi Lee,

On Thu, Jan 22, 2015 at 5:02 PM, Lee Jones <lee.jones@linaro.org> wrote:
> On Thu, 22 Jan 2015, Chen-Yu Tsai wrote:
>
>> From: Carlo Caione <carlo@caione.org>
>>
>> Bindings documentation for the AXP20x driver. In this file also
>> sub-nodes are documented.
>>
>> Signed-off-by: Carlo Caione <carlo@caione.org>
>> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> [wens@csie.org: clarify interrupt source for the axp PMIC]
>> [wens@csie.org: explain dcdc-workmode in detail and trim lines to 80 chars]
>> [wens@csie.org: make regulator supplies optional if using unregulated input]
>> [wens@csie.org: use cubieboard2 regulator nodes as example]
>> [wens@csie.org: change dcdc-workmode to dcdc-workmode-pwm boolean property]
>> [wens@csie.org: use clock-frequency for dcdc regulator work frequency]
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> ---
>>  Documentation/devicetree/bindings/mfd/axp20x.txt | 102 +++++++++++++++++++++++
>>  1 file changed, 102 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mfd/axp20x.txt
>
> This looks okay to me now.
>
> If Mark is happy with the remainder, I'll merge this.

So Mark is not happy with this actually, for a few reasons:

1. The driver was already merged a couple versions back, and
   we have been using the dcdc-freq binding.

2. Furthermore, the dcdc-freq binding has nothing to do with
   the common clock framework. Using the clock-frequencies
   binding implies it does.

3. The change from a integer value to a boolean flag for
   dcdc-mode makes it less capable, i.e. the kernel can't
   just accept the current state of the hardware, which
   should be a valid option for regulators to not screw
   up the system accidentally.

Any chance you could merge the previous version (v8)?


Regards,
ChenYu

> For my own reference:
> Acked-by: Lee Jones <lee.jones@linaro.org>
>
>> diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
>> new file mode 100644
>> index 000000000000..c3c80db242c7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
>> @@ -0,0 +1,102 @@
>> +AXP202/AXP209 device tree bindings
>> +
>> +The axp20x family current members :
>> +axp202 (X-Powers)
>> +axp209 (X-Powers)
>> +
>> +Required properties:
>> +- compatible: "x-powers,axp202" or "x-powers,axp209"
>> +- reg: The I2C slave address for the AXP chip
>> +- interrupt-parent: The parent interrupt controller
>> +- interrupts: SoC NMI / GPIO interrupt connected to the PMIC's IRQ pin
>> +- interrupt-controller: axp20x has its own internal IRQs
>> +- #interrupt-cells: Should be set to 1
>> +- regulators: A node that houses a sub-node for each regulator. The regulators
>> +           are bound using their name as listed here: dcdc2, dcdc3, ldo1,
>> +           ldo2, ldo3, ldo4, ldo5.  The bindings details of individual
>> +           regulator device can be found in:
>> +           Documentation/devicetree/bindings/regulator/regulator.txt with
>> +           the exception of x-powers,dcdc-freq. Regulators not used should
>> +           still be listed for completeness and that the regulator subsystem
>> +           properly registers them.
>> +
>> +Optional properties:
>> +- regulator supplies - may be omitted if inputs are unregulated, such as using
>> +                    the IPSOUT output from the PMIC
>> +  - acin-supply: The input supply for LDO1
>> +  - vin2-supply: The input supply for DCDC2
>> +  - vin3-supply: The input supply for DCDC3
>> +  - ldo24in-supply: The input supply for LDO2, LDO4
>> +  - ldo3in-supply: The input supply for LDO3
>> +  - ldo5in-supply: The input supply for LDO5
>> +
>> +Optional properties for regulators node:
>> +- clock-frequency: defines the work frequency of DC-DC regulators
>> +                (range: 750kHz - 1875kHz). Default: 1.5MHz
>> +
>> +Optional properties for DCDC regulators:
>> +- x-powers,dcdc-workmode-pwm: Force the DCDC regulators to run in PWM mode
>> +                           The DCDC regulators rormally work in a mixed
>> +                           PWM/PFM mode, using PFM under light loads and
>> +                           switching to PWM for heavier loads. Forcing
>> +                           PWM mode trades efficiency under light loads
>> +                           for lower output noise. This probably makes
>> +                           sense for HiFi audio related applications that
>> +                           aren't battery constrained.
>> +
>> +Example:
>> +
>> +axp209: pmic@34 {
>> +     compatible = "x-powers,axp209";
>> +     reg = <0x34>;
>> +     interrupt-parent = <&nmi_intc>;
>> +     interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
>> +     interrupt-controller;
>> +     #interrupt-cells = <1>;
>> +
>> +     regulators {
>> +             clock-frequency = <1500000>;
>> +
>> +             vdd_cpu: dcdc2 {
>> +                     regulator-always-on;
>> +                     regulator-min-microvolt = <1000000>;
>> +                     regulator-max-microvolt = <1450000>;
>> +                     regulator-name = "vdd-cpu";
>> +             };
>> +
>> +             vdd_int_dll: dcdc3 {
>> +                     regulator-always-on;
>> +                     regulator-min-microvolt = <1000000>;
>> +                     regulator-max-microvolt = <1400000>;
>> +                     regulator-name = "vdd-int-dll";
>> +             };
>> +
>> +             vdd_rtc: ldo1 {
>> +                     regulator-always-on;
>> +                     regulator-min-microvolt = <1200000>;
>> +                     regulator-max-microvolt = <1400000>;
>> +                     regulator-name = "vdd-rtc";
>> +             };
>> +
>> +             avcc: ldo2 {
>> +                     regulator-always-on;
>> +                     regulator-min-microvolt = <2700000>;
>> +                     regulator-max-microvolt = <3300000>;
>> +                     regulator-name = "avcc";
>> +             };
>> +
>> +             ldo3 {
>> +                     /* unused */
>> +             };
>> +
>> +             csi1_io_2v8: ldo4 {
>> +                     /* output on extension headers */
>> +                     regulator-name = "csi1-io-2v8";
>> +             };
>> +
>> +             ldo5 {
>> +                     /* unused */
>> +             };
>> +     };
>> +};
>> +
>
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v9 2/5] mfd: AXP20x: Add bindings documentation

[Lee Jones]

On Sat, 31 Jan 2015, Chen-Yu Tsai wrote:

> Hi Lee,
> 
> On Thu, Jan 22, 2015 at 5:02 PM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Thu, 22 Jan 2015, Chen-Yu Tsai wrote:
> >
> >> From: Carlo Caione <carlo@caione.org>
> >>
> >> Bindings documentation for the AXP20x driver. In this file also
> >> sub-nodes are documented.
> >>
> >> Signed-off-by: Carlo Caione <carlo@caione.org>
> >> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >> [wens@csie.org: clarify interrupt source for the axp PMIC]
> >> [wens@csie.org: explain dcdc-workmode in detail and trim lines to 80 chars]
> >> [wens@csie.org: make regulator supplies optional if using unregulated input]
> >> [wens@csie.org: use cubieboard2 regulator nodes as example]
> >> [wens@csie.org: change dcdc-workmode to dcdc-workmode-pwm boolean property]
> >> [wens@csie.org: use clock-frequency for dcdc regulator work frequency]
> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> >> ---
> >>  Documentation/devicetree/bindings/mfd/axp20x.txt | 102 +++++++++++++++++++++++
> >>  1 file changed, 102 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/mfd/axp20x.txt
> >
> > This looks okay to me now.
> >
> > If Mark is happy with the remainder, I'll merge this.
> 
> So Mark is not happy with this actually, for a few reasons:
> 
> 1. The driver was already merged a couple versions back, and
>    we have been using the dcdc-freq binding.
> 
> 2. Furthermore, the dcdc-freq binding has nothing to do with
>    the common clock framework. Using the clock-frequencies
>    binding implies it does.
> 
> 3. The change from a integer value to a boolean flag for
>    dcdc-mode makes it less capable, i.e. the kernel can't
>    just accept the current state of the hardware, which
>    should be a valid option for regulators to not screw
>    up the system accidentally.
> 
> Any chance you could merge the previous version (v8)?

You'll have to re-send it as v10 and obtain Acks in the normal way.

> > For my own reference:
> > Acked-by: Lee Jones <lee.jones@linaro.org>
> >
> >> diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
> >> new file mode 100644
> >> index 000000000000..c3c80db242c7
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
> >> @@ -0,0 +1,102 @@
> >> +AXP202/AXP209 device tree bindings
> >> +
> >> +The axp20x family current members :
> >> +axp202 (X-Powers)
> >> +axp209 (X-Powers)
> >> +
> >> +Required properties:
> >> +- compatible: "x-powers,axp202" or "x-powers,axp209"
> >> +- reg: The I2C slave address for the AXP chip
> >> +- interrupt-parent: The parent interrupt controller
> >> +- interrupts: SoC NMI / GPIO interrupt connected to the PMIC's IRQ pin
> >> +- interrupt-controller: axp20x has its own internal IRQs
> >> +- #interrupt-cells: Should be set to 1
> >> +- regulators: A node that houses a sub-node for each regulator. The regulators
> >> +           are bound using their name as listed here: dcdc2, dcdc3, ldo1,
> >> +           ldo2, ldo3, ldo4, ldo5.  The bindings details of individual
> >> +           regulator device can be found in:
> >> +           Documentation/devicetree/bindings/regulator/regulator.txt with
> >> +           the exception of x-powers,dcdc-freq. Regulators not used should
> >> +           still be listed for completeness and that the regulator subsystem
> >> +           properly registers them.
> >> +
> >> +Optional properties:
> >> +- regulator supplies - may be omitted if inputs are unregulated, such as using
> >> +                    the IPSOUT output from the PMIC
> >> +  - acin-supply: The input supply for LDO1
> >> +  - vin2-supply: The input supply for DCDC2
> >> +  - vin3-supply: The input supply for DCDC3
> >> +  - ldo24in-supply: The input supply for LDO2, LDO4
> >> +  - ldo3in-supply: The input supply for LDO3
> >> +  - ldo5in-supply: The input supply for LDO5
> >> +
> >> +Optional properties for regulators node:
> >> +- clock-frequency: defines the work frequency of DC-DC regulators
> >> +                (range: 750kHz - 1875kHz). Default: 1.5MHz
> >> +
> >> +Optional properties for DCDC regulators:
> >> +- x-powers,dcdc-workmode-pwm: Force the DCDC regulators to run in PWM mode
> >> +                           The DCDC regulators rormally work in a mixed
> >> +                           PWM/PFM mode, using PFM under light loads and
> >> +                           switching to PWM for heavier loads. Forcing
> >> +                           PWM mode trades efficiency under light loads
> >> +                           for lower output noise. This probably makes
> >> +                           sense for HiFi audio related applications that
> >> +                           aren't battery constrained.
> >> +
> >> +Example:
> >> +
> >> +axp209: pmic@34 {
> >> +     compatible = "x-powers,axp209";
> >> +     reg = <0x34>;
> >> +     interrupt-parent = <&nmi_intc>;
> >> +     interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> >> +     interrupt-controller;
> >> +     #interrupt-cells = <1>;
> >> +
> >> +     regulators {
> >> +             clock-frequency = <1500000>;
> >> +
> >> +             vdd_cpu: dcdc2 {
> >> +                     regulator-always-on;
> >> +                     regulator-min-microvolt = <1000000>;
> >> +                     regulator-max-microvolt = <1450000>;
> >> +                     regulator-name = "vdd-cpu";
> >> +             };
> >> +
> >> +             vdd_int_dll: dcdc3 {
> >> +                     regulator-always-on;
> >> +                     regulator-min-microvolt = <1000000>;
> >> +                     regulator-max-microvolt = <1400000>;
> >> +                     regulator-name = "vdd-int-dll";
> >> +             };
> >> +
> >> +             vdd_rtc: ldo1 {
> >> +                     regulator-always-on;
> >> +                     regulator-min-microvolt = <1200000>;
> >> +                     regulator-max-microvolt = <1400000>;
> >> +                     regulator-name = "vdd-rtc";
> >> +             };
> >> +
> >> +             avcc: ldo2 {
> >> +                     regulator-always-on;
> >> +                     regulator-min-microvolt = <2700000>;
> >> +                     regulator-max-microvolt = <3300000>;
> >> +                     regulator-name = "avcc";
> >> +             };
> >> +
> >> +             ldo3 {
> >> +                     /* unused */
> >> +             };
> >> +
> >> +             csi1_io_2v8: ldo4 {
> >> +                     /* output on extension headers */
> >> +                     regulator-name = "csi1-io-2v8";
> >> +             };
> >> +
> >> +             ldo5 {
> >> +                     /* unused */
> >> +             };
> >> +     };
> >> +};
> >> +
> >

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v9 2/5] mfd: AXP20x: Add bindings documentation

[Chen-Yu Tsai]

On Mon, Feb 2, 2015 at 4:53 PM, Lee Jones <lee.jones@linaro.org> wrote:
> On Sat, 31 Jan 2015, Chen-Yu Tsai wrote:
>
>> Hi Lee,
>>
>> On Thu, Jan 22, 2015 at 5:02 PM, Lee Jones <lee.jones@linaro.org> wrote:
>> > On Thu, 22 Jan 2015, Chen-Yu Tsai wrote:
>> >
>> >> From: Carlo Caione <carlo@caione.org>
>> >>
>> >> Bindings documentation for the AXP20x driver. In this file also
>> >> sub-nodes are documented.
>> >>
>> >> Signed-off-by: Carlo Caione <carlo@caione.org>
>> >> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> >> [wens@csie.org: clarify interrupt source for the axp PMIC]
>> >> [wens@csie.org: explain dcdc-workmode in detail and trim lines to 80 chars]
>> >> [wens@csie.org: make regulator supplies optional if using unregulated input]
>> >> [wens@csie.org: use cubieboard2 regulator nodes as example]
>> >> [wens@csie.org: change dcdc-workmode to dcdc-workmode-pwm boolean property]
>> >> [wens@csie.org: use clock-frequency for dcdc regulator work frequency]
>> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> >> ---
>> >>  Documentation/devicetree/bindings/mfd/axp20x.txt | 102 +++++++++++++++++++++++
>> >>  1 file changed, 102 insertions(+)
>> >>  create mode 100644 Documentation/devicetree/bindings/mfd/axp20x.txt
>> >
>> > This looks okay to me now.
>> >
>> > If Mark is happy with the remainder, I'll merge this.
>>
>> So Mark is not happy with this actually, for a few reasons:
>>
>> 1. The driver was already merged a couple versions back, and
>>    we have been using the dcdc-freq binding.
>>
>> 2. Furthermore, the dcdc-freq binding has nothing to do with
>>    the common clock framework. Using the clock-frequencies
>>    binding implies it does.
>>
>> 3. The change from a integer value to a boolean flag for
>>    dcdc-mode makes it less capable, i.e. the kernel can't
>>    just accept the current state of the hardware, which
>>    should be a valid option for regulators to not screw
>>    up the system accidentally.
>>
>> Any chance you could merge the previous version (v8)?
>
> You'll have to re-send it as v10 and obtain Acks in the normal way.

OK. Before I do, would it be easier on you and Mark to have
separate binding files for the mfd and regulator bits?

I noticed a few PMICs have separate files.

Thanks

ChenYu

>> > For my own reference:
>> > Acked-by: Lee Jones <lee.jones@linaro.org>
>> >
>> >> diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
>> >> new file mode 100644
>> >> index 000000000000..c3c80db242c7
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
>> >> @@ -0,0 +1,102 @@
>> >> +AXP202/AXP209 device tree bindings
>> >> +
>> >> +The axp20x family current members :
>> >> +axp202 (X-Powers)
>> >> +axp209 (X-Powers)
>> >> +
>> >> +Required properties:
>> >> +- compatible: "x-powers,axp202" or "x-powers,axp209"
>> >> +- reg: The I2C slave address for the AXP chip
>> >> +- interrupt-parent: The parent interrupt controller
>> >> +- interrupts: SoC NMI / GPIO interrupt connected to the PMIC's IRQ pin
>> >> +- interrupt-controller: axp20x has its own internal IRQs
>> >> +- #interrupt-cells: Should be set to 1
>> >> +- regulators: A node that houses a sub-node for each regulator. The regulators
>> >> +           are bound using their name as listed here: dcdc2, dcdc3, ldo1,
>> >> +           ldo2, ldo3, ldo4, ldo5.  The bindings details of individual
>> >> +           regulator device can be found in:
>> >> +           Documentation/devicetree/bindings/regulator/regulator.txt with
>> >> +           the exception of x-powers,dcdc-freq. Regulators not used should
>> >> +           still be listed for completeness and that the regulator subsystem
>> >> +           properly registers them.
>> >> +
>> >> +Optional properties:
>> >> +- regulator supplies - may be omitted if inputs are unregulated, such as using
>> >> +                    the IPSOUT output from the PMIC
>> >> +  - acin-supply: The input supply for LDO1
>> >> +  - vin2-supply: The input supply for DCDC2
>> >> +  - vin3-supply: The input supply for DCDC3
>> >> +  - ldo24in-supply: The input supply for LDO2, LDO4
>> >> +  - ldo3in-supply: The input supply for LDO3
>> >> +  - ldo5in-supply: The input supply for LDO5
>> >> +
>> >> +Optional properties for regulators node:
>> >> +- clock-frequency: defines the work frequency of DC-DC regulators
>> >> +                (range: 750kHz - 1875kHz). Default: 1.5MHz
>> >> +
>> >> +Optional properties for DCDC regulators:
>> >> +- x-powers,dcdc-workmode-pwm: Force the DCDC regulators to run in PWM mode
>> >> +                           The DCDC regulators rormally work in a mixed
>> >> +                           PWM/PFM mode, using PFM under light loads and
>> >> +                           switching to PWM for heavier loads. Forcing
>> >> +                           PWM mode trades efficiency under light loads
>> >> +                           for lower output noise. This probably makes
>> >> +                           sense for HiFi audio related applications that
>> >> +                           aren't battery constrained.
>> >> +
>> >> +Example:
>> >> +
>> >> +axp209: pmic@34 {
>> >> +     compatible = "x-powers,axp209";
>> >> +     reg = <0x34>;
>> >> +     interrupt-parent = <&nmi_intc>;
>> >> +     interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
>> >> +     interrupt-controller;
>> >> +     #interrupt-cells = <1>;
>> >> +
>> >> +     regulators {
>> >> +             clock-frequency = <1500000>;
>> >> +
>> >> +             vdd_cpu: dcdc2 {
>> >> +                     regulator-always-on;
>> >> +                     regulator-min-microvolt = <1000000>;
>> >> +                     regulator-max-microvolt = <1450000>;
>> >> +                     regulator-name = "vdd-cpu";
>> >> +             };
>> >> +
>> >> +             vdd_int_dll: dcdc3 {
>> >> +                     regulator-always-on;
>> >> +                     regulator-min-microvolt = <1000000>;
>> >> +                     regulator-max-microvolt = <1400000>;
>> >> +                     regulator-name = "vdd-int-dll";
>> >> +             };
>> >> +
>> >> +             vdd_rtc: ldo1 {
>> >> +                     regulator-always-on;
>> >> +                     regulator-min-microvolt = <1200000>;
>> >> +                     regulator-max-microvolt = <1400000>;
>> >> +                     regulator-name = "vdd-rtc";
>> >> +             };
>> >> +
>> >> +             avcc: ldo2 {
>> >> +                     regulator-always-on;
>> >> +                     regulator-min-microvolt = <2700000>;
>> >> +                     regulator-max-microvolt = <3300000>;
>> >> +                     regulator-name = "avcc";
>> >> +             };
>> >> +
>> >> +             ldo3 {
>> >> +                     /* unused */
>> >> +             };
>> >> +
>> >> +             csi1_io_2v8: ldo4 {
>> >> +                     /* output on extension headers */
>> >> +                     regulator-name = "csi1-io-2v8";
>> >> +             };
>> >> +
>> >> +             ldo5 {
>> >> +                     /* unused */
>> >> +             };
>> >> +     };
>> >> +};
>> >> +
>> >
>
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog