[PATCH v2 0/5] Refactor Qualcomm RPM regulator to single platform_device

[PATCH v2 0/5] Refactor Qualcomm RPM regulator to single platform_device

[Bjorn Andersson]

Stephen Boyd pointed out that the current design of the Qualcomm RPM and
regulator driver consumes 12-20kB of ram just for the platform_device structs.

This series starts with a new revision of the dt binding documentation for the
rpm regulators, introduces the regulator-allow-drms property, remove the
flagging of DRMS support from the qcom-rpm regulator driver, refactor the
qcom_rpm-regulator driver to move all custom parse code to a function suitable
for usage as of_parse_cb. The final patch defines the tables of registers and
change the probe function to register the appropriate regulators based on pmic.

As Stephen pointed out in his PATCH/RFC/argument [1], this gives a more
accurate representation of input supplies, as they are now named as in the
specification.

Note that for platforms with multiple pmics (e.g. 8660 and 8974) will have
multiple regulator subnodes to the rpm node - something that will be clearer
with this binding than the previously suggested.

[1] https://lkml.org/lkml/2015/2/26/713

Changes since v1:
- Reworked DRMS handling to not have the driver specify the support

Bjorn Andersson (5):
  mfd: devicetree: bindings: Add Qualcomm RPM regulator subnodes
  regulator: Introduce property to flag drms
  regulator: qcom: Don't enable DRMS in driver
  regulator: qcom: Refactor of-parsing code
  regulator: qcom: Rework to single platform device

 Documentation/devicetree/bindings/mfd/qcom-rpm.txt | 217 +++++++++++++++-
 .../devicetree/bindings/regulator/regulator.txt    |   1 +
 drivers/regulator/of_regulator.c                   |   3 +
 drivers/regulator/qcom_rpm-regulator.c             | 289 ++++++++++++++-------
 4 files changed, 401 insertions(+), 109 deletions(-)

-- 
1.8.2.2

[PATCH v2 1/5] mfd: devicetree: bindings: Add Qualcomm RPM regulator subnodes

[Bjorn Andersson]

Add the regulator subnodes to the Qualcomm RPM MFD device tree bindings.

Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---
 Documentation/devicetree/bindings/mfd/qcom-rpm.txt | 217 +++++++++++++++++++--
 1 file changed, 205 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/qcom-rpm.txt b/Documentation/devicetree/bindings/mfd/qcom-rpm.txt
index 85e3198..8eb1ca9 100644
--- a/Documentation/devicetree/bindings/mfd/qcom-rpm.txt
+++ b/Documentation/devicetree/bindings/mfd/qcom-rpm.txt
@@ -31,16 +31,6 @@ frequencies.
 	Value type: <string-array>
 	Definition: must be the three strings "ack", "err" and "wakeup", in order
 
-- #address-cells:
-	Usage: required
-	Value type: <u32>
-	Definition: must be 1
-
-- #size-cells:
-	Usage: required
-	Value type: <u32>
-	Definition: must be 0
-
 - qcom,ipc:
 	Usage: required
 	Value type: <prop-encoded-array>
@@ -52,6 +42,188 @@ frequencies.
 		    - u32 representing the ipc bit within the register
 
 
+= SUBNODES
+
+The RPM exposes resources to its subnodes. The below bindings specify the set
+of valid subnodes that can operate on these resources.
+
+== Regulators
+
+Regulator notes are identified by their compatible:
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be one of:
+		    "qcom,rpm-pm8058-regulators"
+		    "qcom,rpm-pm8901-regulators"
+		    "qcom,rpm-pm8921-regulators"
+
+- vdd_l0_l1_lvs-supply:
+- vdd_l2_l11_l12-supply:
+- vdd_l3_l4_l5-supply:
+- vdd_l6_l7-supply:
+- vdd_l8-supply:
+- vdd_l9-supply:
+- vdd_l10-supply:
+- vdd_l13_l16-supply:
+- vdd_l14_l15-supply:
+- vdd_l17_l18-supply:
+- vdd_l19_l20-supply:
+- vdd_l21-supply:
+- vdd_l22-supply:
+- vdd_l23_l24_l25-supply:
+- vdd_ncp-supply:
+- vdd_s0-supply:
+- vdd_s1-supply:
+- vdd_s2-supply:
+- vdd_s3-supply:
+- vdd_s4-supply:
+	Usage: optional (pm8058 only)
+	Value type: <phandle>
+	Definition: reference to regulator supplying the input pin, as
+		    described in the data sheet
+
+- lvs0_in-supply:
+- lvs1_in-supply:
+- lvs2_in-supply:
+- lvs3_in-supply:
+- mvs_in-supply:
+- vdd_l0-supply:
+- vdd_l1-supply:
+- vdd_l2-supply:
+- vdd_l3-supply:
+- vdd_l4-supply:
+- vdd_l5-supply:
+- vdd_l6-supply:
+- vdd_s0-supply:
+- vdd_s1-supply:
+- vdd_s2-supply:
+- vdd_s3-supply:
+- vdd_s4-supply:
+	Usage: optional (pm8901 only)
+	Value type: <phandle>
+	Definition: reference to regulator supplying the input pin, as
+		    described in the data sheet
+
+- vdd_l1_l2_l12_l18-supply:
+- vdd_l3_l15_l17-supply:
+- vdd_l4_l14-supply:
+- vdd_l5_l8_l16-supply:
+- vdd_l6_l7-supply:
+- vdd_l9_l11-supply:
+- vdd_l10_l22-supply:
+- vdd_l21_l23_l29-supply:
+- vdd_l24-supply:
+- vdd_l25-supply:
+- vdd_l26-supply:
+- vdd_l27-supply:
+- vdd_l28-supply:
+- vdd_ncp-supply:
+- vdd_s1-supply:
+- vdd_s2-supply:
+- vdd_s4-supply:
+- vdd_s5-supply:
+- vdd_s6-supply:
+- vdd_s7-supply:
+- vdd_s8-supply:
+- vin_5vs-supply:
+- vin_lvs1_3_6-supply:
+- vin_lvs2-supply:
+- vin_lvs4_5_7-supply:
+	Usage: optional (pm8921 only)
+	Value type: <phandle>
+	Definition: reference to regulator supplying the input pin, as
+		    described in the data sheet
+
+The regulator node houses sub-nodes for each regulator within the device. Each
+sub-node is identified using the node's name, with valid values listed for each
+of the pmics below.
+
+pm8058:
+	l0, l1, l2, l3, l4, l5, l6, l7, l8, l9, l10, l11, l12, l13, l14, l15,
+	l16, l17, l18, l19, l20, l21, l22, l23, l24, l25, s0, s1, s2, s3, s4,
+	lvs0, lvs1, ncp
+
+pm8901:
+	l0, l1, l2, l3, l4, l5, l6, s0, s1, s2, s3, s4, lvs0, lvs1, lvs2, lvs3,
+	mvs
+
+pm8921:
+	s1, s2, s3, s4, s7, s8, l1, l2, l3, l4, l5, l6, l7, l8, l9, l10, l11,
+	l12, l14, l15, l16, l17, l18, l21, l22, l23, l24, l25, l26, l27, l28,
+	l29, lvs1, lvs2, lvs3, lvs4, lvs5, lvs6, lvs7, usb-switch, hdmi-switch,
+	ncp
+
+The content of each sub-node is defined by the standard binding for regulators -
+see regulator.txt - with additional custom properties described below:
+
+=== Switch-mode Power Supply regulator custom properties
+
+- bias-pull-down:
+	Usage: optional
+	Value type: <empty>
+	Definition: enable pull down of the regulator when inactive
+
+- qcom,switch-mode-frequency:
+	Usage: required
+	Value type: <u32>
+	Definition: Frequency (Hz) of the switch-mode power supply;
+		    must be one of:
+		    19200000, 9600000, 6400000, 4800000, 3840000, 3200000,
+		    2740000, 2400000, 2130000, 1920000, 1750000, 1600000,
+		    1480000, 1370000, 1280000, 1200000
+
+- qcom,force-mode:
+	Usage: optional (default if no other qcom,force-mode is specified)
+	Value type: <u32>
+	Defintion: indicates that the regulator should be forced to a
+		   particular mode, valid values are:
+		   QCOM_RPM_FORCE_MODE_NONE - do not force any mode
+		   QCOM_RPM_FORCE_MODE_LPM - force into low power mode
+		   QCOM_RPM_FORCE_MODE_HPM - force into high power mode
+		   QCOM_RPM_FORCE_MODE_AUTO - allow regulator to automatically
+					      select its own mode based on
+					      realtime current draw, only for:
+					      pm8921 smps and ftsmps
+
+- qcom,power-mode-hysteretic:
+	Usage: optional
+	Value type: <empty>
+	Definition: select that the power supply should operate in hysteretic
+		    mode, instead of the default pwm mode
+
+=== Low-dropout regulator custom properties
+
+- bias-pull-down:
+	Usage: optional
+	Value type: <empty>
+	Definition: enable pull down of the regulator when inactive
+
+- qcom,force-mode:
+	Usage: optional
+	Value type: <u32>
+	Defintion: indicates that the regulator should not be forced to any
+		   particular mode, valid values are:
+		   QCOM_RPM_FORCE_MODE_NONE - do not force any mode
+		   QCOM_RPM_FORCE_MODE_LPM - force into low power mode
+		   QCOM_RPM_FORCE_MODE_HPM - force into high power mode
+		   QCOM_RPM_FORCE_MODE_BYPASS - set regulator to use bypass
+						mode, i.e.  to act as a switch
+						and not regulate, only for:
+						pm8921 pldo, nldo and nldo1200
+
+=== Negative Charge Pump custom properties
+
+- qcom,switch-mode-frequency:
+	Usage: required
+	Value type: <u32>
+	Definition: Frequency (Hz) of the swith mode power supply;
+		    must be one of:
+		    19200000, 9600000, 6400000, 4800000, 3840000, 3200000,
+		    2740000, 2400000, 2130000, 1920000, 1750000, 1600000,
+		    1480000, 1370000, 1280000, 1200000
+
 = EXAMPLE
 
 	#include <dt-bindings/mfd/qcom-rpm.h>
@@ -64,7 +236,28 @@ frequencies.
 		interrupts = <0 19 0>, <0 21 0>, <0 22 0>;
 		interrupt-names = "ack", "err", "wakeup";
 
-		#address-cells = <1>;
-		#size-cells = <0>;
+		regulators {
+			compatible = "qcom,rpm-pm8921-regulators";
+			vdd_l1_l2_l12_l18-supply = <&pm8921_s4>;
+
+			s1 {
+				regulator-min-microvolt = <1225000>;
+				regulator-max-microvolt = <1225000>;
+
+				bias-pull-down;
+
+				qcom,switch-mode-frequency = <3200000>;
+			};
+
+			pm8921_s4: s4 {
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+
+				qcom,switch-mode-frequency = <1600000>;
+				bias-pull-down;
+
+				qcom,force-mode = <QCOM_RPM_FORCE_MODE_AUTO>;
+			};
+		};
 	};
 
-- 
1.8.2.2

Re: [PATCH v2 1/5] mfd: devicetree: bindings: Add Qualcomm RPM regulator subnodes

[Lee Jones]

DT Ack please.

On Wed, 01 Apr 2015, Bjorn Andersson wrote:

> Add the regulator subnodes to the Qualcomm RPM MFD device tree bindings.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> ---
>  Documentation/devicetree/bindings/mfd/qcom-rpm.txt | 217 +++++++++++++++++++--
>  1 file changed, 205 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/qcom-rpm.txt b/Documentation/devicetree/bindings/mfd/qcom-rpm.txt
> index 85e3198..8eb1ca9 100644
> --- a/Documentation/devicetree/bindings/mfd/qcom-rpm.txt
> +++ b/Documentation/devicetree/bindings/mfd/qcom-rpm.txt
> @@ -31,16 +31,6 @@ frequencies.
>  	Value type: <string-array>
>  	Definition: must be the three strings "ack", "err" and "wakeup", in order
>  
> -- #address-cells:
> -	Usage: required
> -	Value type: <u32>
> -	Definition: must be 1
> -
> -- #size-cells:
> -	Usage: required
> -	Value type: <u32>
> -	Definition: must be 0
> -
>  - qcom,ipc:
>  	Usage: required
>  	Value type: <prop-encoded-array>
> @@ -52,6 +42,188 @@ frequencies.
>  		    - u32 representing the ipc bit within the register
>  
>  
> += SUBNODES
> +
> +The RPM exposes resources to its subnodes. The below bindings specify the set
> +of valid subnodes that can operate on these resources.
> +
> +== Regulators
> +
> +Regulator notes are identified by their compatible:
> +
> +- compatible:
> +	Usage: required
> +	Value type: <string>
> +	Definition: must be one of:
> +		    "qcom,rpm-pm8058-regulators"
> +		    "qcom,rpm-pm8901-regulators"
> +		    "qcom,rpm-pm8921-regulators"
> +
> +- vdd_l0_l1_lvs-supply:
> +- vdd_l2_l11_l12-supply:
> +- vdd_l3_l4_l5-supply:
> +- vdd_l6_l7-supply:
> +- vdd_l8-supply:
> +- vdd_l9-supply:
> +- vdd_l10-supply:
> +- vdd_l13_l16-supply:
> +- vdd_l14_l15-supply:
> +- vdd_l17_l18-supply:
> +- vdd_l19_l20-supply:
> +- vdd_l21-supply:
> +- vdd_l22-supply:
> +- vdd_l23_l24_l25-supply:
> +- vdd_ncp-supply:
> +- vdd_s0-supply:
> +- vdd_s1-supply:
> +- vdd_s2-supply:
> +- vdd_s3-supply:
> +- vdd_s4-supply:
> +	Usage: optional (pm8058 only)
> +	Value type: <phandle>
> +	Definition: reference to regulator supplying the input pin, as
> +		    described in the data sheet
> +
> +- lvs0_in-supply:
> +- lvs1_in-supply:
> +- lvs2_in-supply:
> +- lvs3_in-supply:
> +- mvs_in-supply:
> +- vdd_l0-supply:
> +- vdd_l1-supply:
> +- vdd_l2-supply:
> +- vdd_l3-supply:
> +- vdd_l4-supply:
> +- vdd_l5-supply:
> +- vdd_l6-supply:
> +- vdd_s0-supply:
> +- vdd_s1-supply:
> +- vdd_s2-supply:
> +- vdd_s3-supply:
> +- vdd_s4-supply:
> +	Usage: optional (pm8901 only)
> +	Value type: <phandle>
> +	Definition: reference to regulator supplying the input pin, as
> +		    described in the data sheet
> +
> +- vdd_l1_l2_l12_l18-supply:
> +- vdd_l3_l15_l17-supply:
> +- vdd_l4_l14-supply:
> +- vdd_l5_l8_l16-supply:
> +- vdd_l6_l7-supply:
> +- vdd_l9_l11-supply:
> +- vdd_l10_l22-supply:
> +- vdd_l21_l23_l29-supply:
> +- vdd_l24-supply:
> +- vdd_l25-supply:
> +- vdd_l26-supply:
> +- vdd_l27-supply:
> +- vdd_l28-supply:
> +- vdd_ncp-supply:
> +- vdd_s1-supply:
> +- vdd_s2-supply:
> +- vdd_s4-supply:
> +- vdd_s5-supply:
> +- vdd_s6-supply:
> +- vdd_s7-supply:
> +- vdd_s8-supply:
> +- vin_5vs-supply:
> +- vin_lvs1_3_6-supply:
> +- vin_lvs2-supply:
> +- vin_lvs4_5_7-supply:
> +	Usage: optional (pm8921 only)
> +	Value type: <phandle>
> +	Definition: reference to regulator supplying the input pin, as
> +		    described in the data sheet
> +
> +The regulator node houses sub-nodes for each regulator within the device. Each
> +sub-node is identified using the node's name, with valid values listed for each
> +of the pmics below.
> +
> +pm8058:
> +	l0, l1, l2, l3, l4, l5, l6, l7, l8, l9, l10, l11, l12, l13, l14, l15,
> +	l16, l17, l18, l19, l20, l21, l22, l23, l24, l25, s0, s1, s2, s3, s4,
> +	lvs0, lvs1, ncp
> +
> +pm8901:
> +	l0, l1, l2, l3, l4, l5, l6, s0, s1, s2, s3, s4, lvs0, lvs1, lvs2, lvs3,
> +	mvs
> +
> +pm8921:
> +	s1, s2, s3, s4, s7, s8, l1, l2, l3, l4, l5, l6, l7, l8, l9, l10, l11,
> +	l12, l14, l15, l16, l17, l18, l21, l22, l23, l24, l25, l26, l27, l28,
> +	l29, lvs1, lvs2, lvs3, lvs4, lvs5, lvs6, lvs7, usb-switch, hdmi-switch,
> +	ncp
> +
> +The content of each sub-node is defined by the standard binding for regulators -
> +see regulator.txt - with additional custom properties described below:
> +
> +=== Switch-mode Power Supply regulator custom properties
> +
> +- bias-pull-down:
> +	Usage: optional
> +	Value type: <empty>
> +	Definition: enable pull down of the regulator when inactive
> +
> +- qcom,switch-mode-frequency:
> +	Usage: required
> +	Value type: <u32>
> +	Definition: Frequency (Hz) of the switch-mode power supply;
> +		    must be one of:
> +		    19200000, 9600000, 6400000, 4800000, 3840000, 3200000,
> +		    2740000, 2400000, 2130000, 1920000, 1750000, 1600000,
> +		    1480000, 1370000, 1280000, 1200000
> +
> +- qcom,force-mode:
> +	Usage: optional (default if no other qcom,force-mode is specified)
> +	Value type: <u32>
> +	Defintion: indicates that the regulator should be forced to a
> +		   particular mode, valid values are:
> +		   QCOM_RPM_FORCE_MODE_NONE - do not force any mode
> +		   QCOM_RPM_FORCE_MODE_LPM - force into low power mode
> +		   QCOM_RPM_FORCE_MODE_HPM - force into high power mode
> +		   QCOM_RPM_FORCE_MODE_AUTO - allow regulator to automatically
> +					      select its own mode based on
> +					      realtime current draw, only for:
> +					      pm8921 smps and ftsmps
> +
> +- qcom,power-mode-hysteretic:
> +	Usage: optional
> +	Value type: <empty>
> +	Definition: select that the power supply should operate in hysteretic
> +		    mode, instead of the default pwm mode
> +
> +=== Low-dropout regulator custom properties
> +
> +- bias-pull-down:
> +	Usage: optional
> +	Value type: <empty>
> +	Definition: enable pull down of the regulator when inactive
> +
> +- qcom,force-mode:
> +	Usage: optional
> +	Value type: <u32>
> +	Defintion: indicates that the regulator should not be forced to any
> +		   particular mode, valid values are:
> +		   QCOM_RPM_FORCE_MODE_NONE - do not force any mode
> +		   QCOM_RPM_FORCE_MODE_LPM - force into low power mode
> +		   QCOM_RPM_FORCE_MODE_HPM - force into high power mode
> +		   QCOM_RPM_FORCE_MODE_BYPASS - set regulator to use bypass
> +						mode, i.e.  to act as a switch
> +						and not regulate, only for:
> +						pm8921 pldo, nldo and nldo1200
> +
> +=== Negative Charge Pump custom properties
> +
> +- qcom,switch-mode-frequency:
> +	Usage: required
> +	Value type: <u32>
> +	Definition: Frequency (Hz) of the swith mode power supply;
> +		    must be one of:
> +		    19200000, 9600000, 6400000, 4800000, 3840000, 3200000,
> +		    2740000, 2400000, 2130000, 1920000, 1750000, 1600000,
> +		    1480000, 1370000, 1280000, 1200000
> +
>  = EXAMPLE
>  
>  	#include <dt-bindings/mfd/qcom-rpm.h>
> @@ -64,7 +236,28 @@ frequencies.
>  		interrupts = <0 19 0>, <0 21 0>, <0 22 0>;
>  		interrupt-names = "ack", "err", "wakeup";
>  
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> +		regulators {
> +			compatible = "qcom,rpm-pm8921-regulators";
> +			vdd_l1_l2_l12_l18-supply = <&pm8921_s4>;
> +
> +			s1 {
> +				regulator-min-microvolt = <1225000>;
> +				regulator-max-microvolt = <1225000>;
> +
> +				bias-pull-down;
> +
> +				qcom,switch-mode-frequency = <3200000>;
> +			};
> +
> +			pm8921_s4: s4 {
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +
> +				qcom,switch-mode-frequency = <1600000>;
> +				bias-pull-down;
> +
> +				qcom,force-mode = <QCOM_RPM_FORCE_MODE_AUTO>;
> +			};
> +		};
>  	};
>  

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

Re: [PATCH v2 1/5] mfd: devicetree: bindings: Add Qualcomm RPM regulator subnodes

[Stephen Boyd]

On 04/01/15 15:55, Bjorn Andersson wrote:
> @@ -52,6 +42,188 @@ frequencies.
>  		    - u32 representing the ipc bit within the register
>  
>  
> += SUBNODES
> +
> +The RPM exposes resources to its subnodes. The below bindings specify the set
> +of valid subnodes that can operate on these resources.
> +
> +== Regulators
> +
> +Regulator notes are identified by their compatible:

s/notes/nodes/

Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 2/5] regulator: Introduce property to flag drms

[Bjorn Andersson]

Introduce "regulator-allow-drms" to make it possible for board
configuration to enable drms for regulators.

Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---
 Documentation/devicetree/bindings/regulator/regulator.txt | 1 +
 drivers/regulator/of_regulator.c                          | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
index abb26b5..a2377cb 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -10,6 +10,7 @@ Optional properties:
 - regulator-always-on: boolean, regulator should never be disabled
 - regulator-boot-on: bootloader/firmware enabled regulator
 - regulator-allow-bypass: allow the regulator to go into bypass mode
+- regulator-allow-drms: allow dynamic regulator mode switching
 - <name>-supply: phandle to the parent supply/regulator node
 - regulator-ramp-delay: ramp delay for regulator(in uV/uS)
   For hardware which supports disabling ramp rate, it should be explicitly
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 24e812c..bb52532 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -70,6 +70,9 @@ static void of_get_regulation_constraints(struct device_node *np,
 	if (of_property_read_bool(np, "regulator-allow-bypass"))
 		constraints->valid_ops_mask |= REGULATOR_CHANGE_BYPASS;
 
+	if (of_property_read_bool(np, "regulator-allow-drms"))
+		constraints->valid_ops_mask |= REGULATOR_CHANGE_DRMS;
+
 	ret = of_property_read_u32(np, "regulator-ramp-delay", &pval);
 	if (!ret) {
 		if (pval)
-- 
1.8.2.2

Re: [PATCH v2 2/5] regulator: Introduce property to flag drms

[Mark Brown]

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

On Wed, Apr 01, 2015 at 03:55:43PM -0700, Bjorn Andersson wrote:
> Introduce "regulator-allow-drms" to make it possible for board
> configuration to enable drms for regulators.

I don't think this is a good name, nobody unfamiliar with a fairly
obscure feature in the Linux regulator API is going to know what DRMS
means.  Something like change-load might be clearer.

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

Re: [PATCH v2 2/5] regulator: Introduce property to flag drms

[Bjorn Andersson]

On Thu, Apr 2, 2015 at 1:54 AM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Apr 01, 2015 at 03:55:43PM -0700, Bjorn Andersson wrote:
>> Introduce "regulator-allow-drms" to make it possible for board
>> configuration to enable drms for regulators.
>
> I don't think this is a good name, nobody unfamiliar with a fairly
> obscure feature in the Linux regulator API is going to know what DRMS
> means.  Something like change-load might be clearer.

Right. And with the cleanup of the drms handling (drms_uA_update())
that we discussed during ELC it makes more sense for the Linux case as
well. I will rework this patch and include it when sending out the
cleanup.

Would you mind picking the other patches from this series or do you
want me to resend them? They are unrelated to this issue and will give
us working regulators (without set_load support) on the affected
platforms.

Regards,
Bjorn

Re: [PATCH v2 2/5] regulator: Introduce property to flag drms

[Mark Brown]

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

On Thu, Apr 02, 2015 at 02:35:07PM -0700, Bjorn Andersson wrote:

> Would you mind picking the other patches from this series or do you
> want me to resend them? They are unrelated to this issue and will give
> us working regulators (without set_load support) on the affected
> platforms.

There seem to be some dependencies (at least for patch 3) and Stephen
had some review comments on patch 5 so it might be safer to leave them -
but in theory yes.

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

Re: [PATCH v2 0/5] Refactor Qualcomm RPM regulator to single platform_device

[Stephen Boyd]

On 04/01/15 15:55, Bjorn Andersson wrote:
> Stephen Boyd pointed out that the current design of the Qualcomm RPM and
> regulator driver consumes 12-20kB of ram just for the platform_device structs.
>
> This series starts with a new revision of the dt binding documentation for the
> rpm regulators, introduces the regulator-allow-drms property, remove the
> flagging of DRMS support from the qcom-rpm regulator driver, refactor the
> qcom_rpm-regulator driver to move all custom parse code to a function suitable
> for usage as of_parse_cb. The final patch defines the tables of registers and
> change the probe function to register the appropriate regulators based on pmic.
>
> As Stephen pointed out in his PATCH/RFC/argument [1], this gives a more
> accurate representation of input supplies, as they are now named as in the
> specification.
>
> Note that for platforms with multiple pmics (e.g. 8660 and 8974) will have
> multiple regulator subnodes to the rpm node - something that will be clearer
> with this binding than the previously suggested.

What happens with debugfs when you have multiple pmics with the same
named regulator? I thought that in this case we needed to make the names
unique somehow or we would end up with the same directory for two
different regulators.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 0/5] Refactor Qualcomm RPM regulator to single platform_device

[Mark Brown]

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

On Thu, Apr 02, 2015 at 03:00:37PM -0700, Stephen Boyd wrote:

> What happens with debugfs when you have multiple pmics with the same
> named regulator? I thought that in this case we needed to make the names
> unique somehow or we would end up with the same directory for two
> different regulators.

Guenther sent a patch fixing that a while back.

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

Re: [PATCH v2 0/5] Refactor Qualcomm RPM regulator to single platform_device

[Stephen Boyd]

On 04/02/15 15:26, Mark Brown wrote:
> On Thu, Apr 02, 2015 at 03:00:37PM -0700, Stephen Boyd wrote:
>
>> What happens with debugfs when you have multiple pmics with the same
>> named regulator? I thought that in this case we needed to make the names
>> unique somehow or we would end up with the same directory for two
>> different regulators.
> Guenther sent a patch fixing that a while back.

This one?

commit a9eaa8130707d4013fb109b80323489c0d0111ac
Author:     Guenter Roeck <linux@roeck-us.net>
AuthorDate: Fri Oct 17 08:17:03 2014 -0700
Commit:     Mark Brown <broonie@kernel.org>
CommitDate: Fri Mar 27 16:14:18 2015 -0700

    regulator: Ensure unique regulator debugfs directory names

Ok. Seems to be only in next. I'm not sure it will work though because
in this case the parent device is the same for both regulators on
different PMICs that the RPM is controlling. I could be wrong though
because I haven't tested it.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 0/5] Refactor Qualcomm RPM regulator to single platform_device

[Bjorn Andersson]

On Thu, Apr 2, 2015 at 3:57 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 04/02/15 15:26, Mark Brown wrote:
>> On Thu, Apr 02, 2015 at 03:00:37PM -0700, Stephen Boyd wrote:
>>
>>> What happens with debugfs when you have multiple pmics with the same
>>> named regulator? I thought that in this case we needed to make the names
>>> unique somehow or we would end up with the same directory for two
>>> different regulators.
>> Guenther sent a patch fixing that a while back.
>
> This one?
>
> commit a9eaa8130707d4013fb109b80323489c0d0111ac
> Author:     Guenter Roeck <linux@roeck-us.net>
> AuthorDate: Fri Oct 17 08:17:03 2014 -0700
> Commit:     Mark Brown <broonie@kernel.org>
> CommitDate: Fri Mar 27 16:14:18 2015 -0700
>
>     regulator: Ensure unique regulator debugfs directory names
>
> Ok. Seems to be only in next. I'm not sure it will work though because
> in this case the parent device is the same for both regulators on
> different PMICs that the RPM is controlling. I could be wrong though
> because I haven't tested it.
>

You're right Stephen; for the platforms with multiple pmics this will
spit out a bunch of warnings and Guenter's fix does not change that
fact.

Either we make the regulator names more specific (like pm8058_l1) or
we build desc.name out of pdev->dev.of_node->name and the regulator
name. I like the latter, so unless someone object I'll respin it that
way.

Regards,
Bjorn

Re: [PATCH v2 0/5] Refactor Qualcomm RPM regulator to single platform_device

[Mark Brown]

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

On Thu, Apr 02, 2015 at 03:57:16PM -0700, Stephen Boyd wrote:
> On 04/02/15 15:26, Mark Brown wrote:

> > Guenther sent a patch fixing that a while back.

> This one?

Yes.

>     regulator: Ensure unique regulator debugfs directory names

> Ok. Seems to be only in next. I'm not sure it will work though because
> in this case the parent device is the same for both regulators on
> different PMICs that the RPM is controlling. I could be wrong though
> because I haven't tested it.

I'd say that if the driver is doing this then the driver is buggy - the
user should be able to distinguish between two regulators appearing for
the same device.  Either the RPM should create dummy devices for the two
PMICs or something should insert the PMIC name into the regulator name
somewhere along the line (perhaps in the static data).

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

Re: [PATCH v2 0/5] Refactor Qualcomm RPM regulator to single platform_device

[Bjorn Andersson]

On Mon, Apr 6, 2015 at 9:51 AM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Apr 02, 2015 at 03:57:16PM -0700, Stephen Boyd wrote:
>> On 04/02/15 15:26, Mark Brown wrote:
>
>> > Guenther sent a patch fixing that a while back.
>
>> This one?
>
> Yes.
>
>>     regulator: Ensure unique regulator debugfs directory names
>
>> Ok. Seems to be only in next. I'm not sure it will work though because
>> in this case the parent device is the same for both regulators on
>> different PMICs that the RPM is controlling. I could be wrong though
>> because I haven't tested it.
>
> I'd say that if the driver is doing this then the driver is buggy - the
> user should be able to distinguish between two regulators appearing for
> the same device.  Either the RPM should create dummy devices for the two
> PMICs or something should insert the PMIC name into the regulator name
> somewhere along the line (perhaps in the static data).

Sorry, I spoke before I had coffee...

The RPM will instantiate one regulator device per pmic and parent will
hence differ between the various regulator instances.
So with Guenter's patch this does indeed work.

Regards,
Bjorn

Re: [PATCH v2 0/5] Refactor Qualcomm RPM regulator to single platform_device

[Mark Brown]

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

On Mon, Apr 06, 2015 at 10:11:02AM -0700, Bjorn Andersson wrote:

> The RPM will instantiate one regulator device per pmic and parent will
> hence differ between the various regulator instances.
> So with Guenter's patch this does indeed work.

Ah, excellent!  That's easy then...

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