[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 --]

[PATCH v2 3/5] regulator: qcom: Don't enable DRMS in driver

[Bjorn Andersson]

The driver itself should not flag regulators as being DRMS compatible,
this should come from board or dt files.

Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---
 drivers/regulator/qcom_rpm-regulator.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/regulator/qcom_rpm-regulator.c b/drivers/regulator/qcom_rpm-regulator.c
index 15e07c2..ddca8cb 100644
--- a/drivers/regulator/qcom_rpm-regulator.c
+++ b/drivers/regulator/qcom_rpm-regulator.c
@@ -732,10 +732,6 @@ static int rpm_reg_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	/* Regulators with ia property suppports drms */
-	if (vreg->parts->ia.mask)
-		initdata->constraints.valid_ops_mask |= REGULATOR_CHANGE_DRMS;
-
 	key = "bias-pull-down";
 	if (of_property_read_bool(pdev->dev.of_node, key)) {
 		ret = rpm_reg_set(vreg, &vreg->parts->pd, 1);
-- 
1.8.2.2

Re: [PATCH v2 3/5] regulator: qcom: Don't enable DRMS in driver

[Stephen Boyd]

On 04/01/15 15:55, Bjorn Andersson wrote:
> The driver itself should not flag regulators as being DRMS compatible,
> this should come from board or dt files.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>

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 4/5] regulator: qcom: Refactor of-parsing code

[Bjorn Andersson]

Refactor out all custom property parsing code from the probe function
into a function suitable for regulator_desc->of_parse_cb usage.

Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---
 drivers/regulator/qcom_rpm-regulator.c | 141 +++++++++++++++++++--------------
 1 file changed, 81 insertions(+), 60 deletions(-)

diff --git a/drivers/regulator/qcom_rpm-regulator.c b/drivers/regulator/qcom_rpm-regulator.c
index ddca8cb..bd8360c 100644
--- a/drivers/regulator/qcom_rpm-regulator.c
+++ b/drivers/regulator/qcom_rpm-regulator.c
@@ -645,7 +645,9 @@ static int rpm_reg_set(struct qcom_rpm_reg *vreg,
 	return 0;
 }
 
-static int rpm_reg_of_parse_freq(struct device *dev, struct qcom_rpm_reg *vreg)
+static int rpm_reg_of_parse_freq(struct device *dev,
+				 struct device_node *node,
+				 struct qcom_rpm_reg *vreg)
 {
 	static const int freq_table[] = {
 		19200000, 9600000, 6400000, 4800000, 3840000, 3200000, 2740000,
@@ -659,7 +661,7 @@ static int rpm_reg_of_parse_freq(struct device *dev, struct qcom_rpm_reg *vreg)
 	int i;
 
 	key = "qcom,switch-mode-frequency";
-	ret = of_property_read_u32(dev->of_node, key, &freq);
+	ret = of_property_read_u32(node, key, &freq);
 	if (ret) {
 		dev_err(dev, "regulator requires %s property\n", key);
 		return -EINVAL;
@@ -676,84 +678,40 @@ static int rpm_reg_of_parse_freq(struct device *dev, struct qcom_rpm_reg *vreg)
 	return -EINVAL;
 }
 
-static int rpm_reg_probe(struct platform_device *pdev)
+static int rpm_reg_of_parse(struct device_node *node,
+			    const struct regulator_desc *desc,
+			    struct regulator_config *config)
 {
-	struct regulator_init_data *initdata;
-	const struct qcom_rpm_reg *template;
-	const struct of_device_id *match;
-	struct regulator_config config = { };
-	struct regulator_dev *rdev;
-	struct qcom_rpm_reg *vreg;
+	struct qcom_rpm_reg *vreg = config->driver_data;
+	struct device *dev = config->dev;
 	const char *key;
 	u32 force_mode;
 	bool pwm;
 	u32 val;
 	int ret;
 
-	match = of_match_device(rpm_of_match, &pdev->dev);
-	template = match->data;
-
-	vreg = devm_kmalloc(&pdev->dev, sizeof(*vreg), GFP_KERNEL);
-	if (!vreg) {
-		dev_err(&pdev->dev, "failed to allocate vreg\n");
-		return -ENOMEM;
-	}
-	memcpy(vreg, template, sizeof(*vreg));
-	mutex_init(&vreg->lock);
-	vreg->dev = &pdev->dev;
-	vreg->desc.id = -1;
-	vreg->desc.owner = THIS_MODULE;
-	vreg->desc.type = REGULATOR_VOLTAGE;
-	vreg->desc.name = pdev->dev.of_node->name;
-	vreg->desc.supply_name = "vin";
-
-	vreg->rpm = dev_get_drvdata(pdev->dev.parent);
-	if (!vreg->rpm) {
-		dev_err(&pdev->dev, "unable to retrieve handle to rpm\n");
-		return -ENODEV;
-	}
-
-	initdata = of_get_regulator_init_data(&pdev->dev, pdev->dev.of_node,
-					      &vreg->desc);
-	if (!initdata)
-		return -EINVAL;
-
-	key = "reg";
-	ret = of_property_read_u32(pdev->dev.of_node, key, &val);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to read %s\n", key);
-		return ret;
-	}
-	vreg->resource = val;
-
-	if ((vreg->parts->uV.mask || vreg->parts->mV.mask) &&
-	    (!initdata->constraints.min_uV || !initdata->constraints.max_uV)) {
-		dev_err(&pdev->dev, "no voltage specified for regulator\n");
-		return -EINVAL;
-	}
-
 	key = "bias-pull-down";
-	if (of_property_read_bool(pdev->dev.of_node, key)) {
+	if (of_property_read_bool(node, key)) {
 		ret = rpm_reg_set(vreg, &vreg->parts->pd, 1);
 		if (ret) {
-			dev_err(&pdev->dev, "%s is invalid", key);
+			dev_err(dev, "%s is invalid", key);
 			return ret;
 		}
 	}
 
 	if (vreg->parts->freq.mask) {
-		ret = rpm_reg_of_parse_freq(&pdev->dev, vreg);
+		ret = rpm_reg_of_parse_freq(dev, node, vreg);
 		if (ret < 0)
 			return ret;
 	}
 
 	if (vreg->parts->pm.mask) {
 		key = "qcom,power-mode-hysteretic";
-		pwm = !of_property_read_bool(pdev->dev.of_node, key);
+		pwm = !of_property_read_bool(node, key);
 
 		ret = rpm_reg_set(vreg, &vreg->parts->pm, pwm);
 		if (ret) {
-			dev_err(&pdev->dev, "failed to set power mode\n");
+			dev_err(dev, "failed to set power mode\n");
 			return ret;
 		}
 	}
@@ -762,11 +720,11 @@ static int rpm_reg_probe(struct platform_device *pdev)
 		force_mode = -1;
 
 		key = "qcom,force-mode";
-		ret = of_property_read_u32(pdev->dev.of_node, key, &val);
+		ret = of_property_read_u32(node, key, &val);
 		if (ret == -EINVAL) {
 			val = QCOM_RPM_FORCE_MODE_NONE;
 		} else if (ret < 0) {
-			dev_err(&pdev->dev, "failed to read %s\n", key);
+			dev_err(dev, "failed to read %s\n", key);
 			return ret;
 		}
 
@@ -801,21 +759,84 @@ static int rpm_reg_probe(struct platform_device *pdev)
 		}
 
 		if (force_mode == -1) {
-			dev_err(&pdev->dev, "invalid force mode\n");
+			dev_err(dev, "invalid force mode\n");
 			return -EINVAL;
 		}
 
 		ret = rpm_reg_set(vreg, &vreg->parts->fm, force_mode);
 		if (ret) {
-			dev_err(&pdev->dev, "failed to set force mode\n");
+			dev_err(dev, "failed to set force mode\n");
 			return ret;
 		}
 	}
 
+	return 0;
+}
+
+static int rpm_reg_probe(struct platform_device *pdev)
+{
+	struct regulator_init_data *initdata;
+	const struct qcom_rpm_reg *template;
+	const struct of_device_id *match;
+	struct regulator_config config = { };
+	struct regulator_dev *rdev;
+	struct qcom_rpm_reg *vreg;
+	const char *key;
+	u32 val;
+	int ret;
+
+	match = of_match_device(rpm_of_match, &pdev->dev);
+	template = match->data;
+
+	vreg = devm_kmalloc(&pdev->dev, sizeof(*vreg), GFP_KERNEL);
+	if (!vreg) {
+		dev_err(&pdev->dev, "failed to allocate vreg\n");
+		return -ENOMEM;
+	}
+	memcpy(vreg, template, sizeof(*vreg));
+	mutex_init(&vreg->lock);
+	vreg->dev = &pdev->dev;
+	vreg->desc.id = -1;
+	vreg->desc.owner = THIS_MODULE;
+	vreg->desc.type = REGULATOR_VOLTAGE;
+	vreg->desc.name = pdev->dev.of_node->name;
+	vreg->desc.supply_name = "vin";
+
+	vreg->rpm = dev_get_drvdata(pdev->dev.parent);
+	if (!vreg->rpm) {
+		dev_err(&pdev->dev, "unable to retrieve handle to rpm\n");
+		return -ENODEV;
+	}
+
+	initdata = of_get_regulator_init_data(&pdev->dev, pdev->dev.of_node,
+					      &vreg->desc);
+	if (!initdata)
+		return -EINVAL;
+
+	key = "reg";
+	ret = of_property_read_u32(pdev->dev.of_node, key, &val);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to read %s\n", key);
+		return ret;
+	}
+	vreg->resource = val;
+
+	if ((vreg->parts->uV.mask || vreg->parts->mV.mask) &&
+	    (!initdata->constraints.min_uV || !initdata->constraints.max_uV)) {
+		dev_err(&pdev->dev, "no voltage specified for regulator\n");
+		return -EINVAL;
+	}
+
+
 	config.dev = &pdev->dev;
 	config.init_data = initdata;
 	config.driver_data = vreg;
 	config.of_node = pdev->dev.of_node;
+
+	ret = rpm_reg_of_parse(pdev->dev.of_node, &vreg->desc, &config);
+	if (ret)
+		return ret;
+
 	rdev = devm_regulator_register(&pdev->dev, &vreg->desc, &config);
 	if (IS_ERR(rdev)) {
 		dev_err(&pdev->dev, "can't register regulator\n");
-- 
1.8.2.2

Re: [PATCH v2 4/5] regulator: qcom: Refactor of-parsing code

[Stephen Boyd]

On 04/01/15 15:55, Bjorn Andersson wrote:
> Refactor out all custom property parsing code from the probe function
> into a function suitable for regulator_desc->of_parse_cb usage.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
>


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 5/5] regulator: qcom: Rework to single platform device

[Bjorn Andersson]

Modeling the individual RPM resources as platform devices consumes at
least 12-15kb of RAM, just to hold the platform_device structs. Rework
this to instead have one device per pmic exposed by the RPM.

With this representation we can more accurately define the input pins on
the pmic and have the supply description match the data sheet.

Suggested-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---
 drivers/regulator/qcom_rpm-regulator.c | 244 ++++++++++++++++++++++-----------
 1 file changed, 161 insertions(+), 83 deletions(-)

diff --git a/drivers/regulator/qcom_rpm-regulator.c b/drivers/regulator/qcom_rpm-regulator.c
index bd8360c..40cf6ff 100644
--- a/drivers/regulator/qcom_rpm-regulator.c
+++ b/drivers/regulator/qcom_rpm-regulator.c
@@ -607,31 +607,6 @@ static const struct qcom_rpm_reg smb208_smps = {
 	.supports_force_mode_bypass = false,
 };
 
-static const struct of_device_id rpm_of_match[] = {
-	{ .compatible = "qcom,rpm-pm8058-pldo",     .data = &pm8058_pldo },
-	{ .compatible = "qcom,rpm-pm8058-nldo",     .data = &pm8058_nldo },
-	{ .compatible = "qcom,rpm-pm8058-smps",     .data = &pm8058_smps },
-	{ .compatible = "qcom,rpm-pm8058-ncp",      .data = &pm8058_ncp },
-	{ .compatible = "qcom,rpm-pm8058-switch",   .data = &pm8058_switch },
-
-	{ .compatible = "qcom,rpm-pm8901-pldo",     .data = &pm8901_pldo },
-	{ .compatible = "qcom,rpm-pm8901-nldo",     .data = &pm8901_nldo },
-	{ .compatible = "qcom,rpm-pm8901-ftsmps",   .data = &pm8901_ftsmps },
-	{ .compatible = "qcom,rpm-pm8901-switch",   .data = &pm8901_switch },
-
-	{ .compatible = "qcom,rpm-pm8921-pldo",     .data = &pm8921_pldo },
-	{ .compatible = "qcom,rpm-pm8921-nldo",     .data = &pm8921_nldo },
-	{ .compatible = "qcom,rpm-pm8921-nldo1200", .data = &pm8921_nldo1200 },
-	{ .compatible = "qcom,rpm-pm8921-smps",     .data = &pm8921_smps },
-	{ .compatible = "qcom,rpm-pm8921-ftsmps",   .data = &pm8921_ftsmps },
-	{ .compatible = "qcom,rpm-pm8921-ncp",      .data = &pm8921_ncp },
-	{ .compatible = "qcom,rpm-pm8921-switch",   .data = &pm8921_switch },
-
-	{ .compatible = "qcom,rpm-smb208", .data = &smb208_smps },
-	{ }
-};
-MODULE_DEVICE_TABLE(of, rpm_of_match);
-
 static int rpm_reg_set(struct qcom_rpm_reg *vreg,
 		       const struct request_member *req,
 		       const int value)
@@ -773,74 +748,177 @@ static int rpm_reg_of_parse(struct device_node *node,
 	return 0;
 }
 
-static int rpm_reg_probe(struct platform_device *pdev)
-{
-	struct regulator_init_data *initdata;
+struct rpm_regulator_data {
+	const char *name;
+	int resource;
 	const struct qcom_rpm_reg *template;
-	const struct of_device_id *match;
-	struct regulator_config config = { };
-	struct regulator_dev *rdev;
-	struct qcom_rpm_reg *vreg;
-	const char *key;
-	u32 val;
-	int ret;
+	const char *supply;
+};
+
+static const struct rpm_regulator_data rpm_pm8058_regulators[] = {
+	{ "l0",   QCOM_RPM_PM8058_LDO0,   &pm8058_nldo, "vdd_l0_l1_lvs"	},
+	{ "l1",   QCOM_RPM_PM8058_LDO1,   &pm8058_nldo, "vdd_l0_l1_lvs" },
+	{ "l2",   QCOM_RPM_PM8058_LDO2,   &pm8058_pldo, "vdd_l2_l11_l12" },
+	{ "l3",   QCOM_RPM_PM8058_LDO3,   &pm8058_pldo, "vdd_l3_l4_l5" },
+	{ "l4",   QCOM_RPM_PM8058_LDO4,   &pm8058_pldo, "vdd_l3_l4_l5" },
+	{ "l5",   QCOM_RPM_PM8058_LDO5,   &pm8058_pldo, "vdd_l3_l4_l5" },
+	{ "l6",   QCOM_RPM_PM8058_LDO6,   &pm8058_pldo, "vdd_l6_l7" },
+	{ "l7",   QCOM_RPM_PM8058_LDO7,   &pm8058_pldo, "vdd_l6_l7" },
+	{ "l8",   QCOM_RPM_PM8058_LDO8,   &pm8058_pldo, "vdd_l8" },
+	{ "l9",   QCOM_RPM_PM8058_LDO9,   &pm8058_pldo, "vdd_l9" },
+	{ "l10",  QCOM_RPM_PM8058_LDO10,  &pm8058_pldo, "vdd_l10" },
+	{ "l11",  QCOM_RPM_PM8058_LDO11,  &pm8058_pldo, "vdd_l2_l11_l12" },
+	{ "l12",  QCOM_RPM_PM8058_LDO12,  &pm8058_pldo, "vdd_l2_l11_l12" },
+	{ "l13",  QCOM_RPM_PM8058_LDO13,  &pm8058_pldo, "vdd_l13_l16" },
+	{ "l14",  QCOM_RPM_PM8058_LDO14,  &pm8058_pldo, "vdd_l14_l15" },
+	{ "l15",  QCOM_RPM_PM8058_LDO15,  &pm8058_pldo, "vdd_l14_l15" },
+	{ "l16",  QCOM_RPM_PM8058_LDO16,  &pm8058_pldo, "vdd_l13_l16" },
+	{ "l17",  QCOM_RPM_PM8058_LDO17,  &pm8058_pldo, "vdd_l17_l18" },
+	{ "l18",  QCOM_RPM_PM8058_LDO18,  &pm8058_pldo, "vdd_l17_l18" },
+	{ "l19",  QCOM_RPM_PM8058_LDO19,  &pm8058_pldo, "vdd_l19_l20" },
+	{ "l20",  QCOM_RPM_PM8058_LDO20,  &pm8058_pldo, "vdd_l19_l20" },
+	{ "l21",  QCOM_RPM_PM8058_LDO21,  &pm8058_nldo, "vdd_l21" },
+	{ "l22",  QCOM_RPM_PM8058_LDO22,  &pm8058_nldo, "vdd_l22" },
+	{ "l23",  QCOM_RPM_PM8058_LDO23,  &pm8058_nldo, "vdd_l23_l24_l25" },
+	{ "l24",  QCOM_RPM_PM8058_LDO24,  &pm8058_nldo, "vdd_l23_l24_l25" },
+	{ "l25",  QCOM_RPM_PM8058_LDO25,  &pm8058_nldo, "vdd_l23_l24_l25" },
+
+	{ "s0",   QCOM_RPM_PM8058_SMPS0,  &pm8058_smps, "vdd_s0" },
+	{ "s1",   QCOM_RPM_PM8058_SMPS1,  &pm8058_smps, "vdd_s1" },
+	{ "s2",   QCOM_RPM_PM8058_SMPS2,  &pm8058_smps, "vdd_s2" },
+	{ "s3",   QCOM_RPM_PM8058_SMPS3,  &pm8058_smps, "vdd_s3" },
+	{ "s4",   QCOM_RPM_PM8058_SMPS4,  &pm8058_smps, "vdd_s4" },
+
+	{ "lvs0", QCOM_RPM_PM8058_LVS0, &pm8058_switch, "vdd_l0_l1_lvs" },
+	{ "lvs1", QCOM_RPM_PM8058_LVS1, &pm8058_switch, "vdd_l0_l1_lvs" },
+
+	{ "ncp",  QCOM_RPM_PM8058_NCP, &pm8058_ncp, "vdd_ncp" },
+	{ }
+};
 
-	match = of_match_device(rpm_of_match, &pdev->dev);
-	template = match->data;
+static const struct rpm_regulator_data rpm_pm8901_regulators[] = {
+	{ "l0",   QCOM_RPM_PM8901_LDO0, &pm8901_nldo, "vdd_l0" },
+	{ "l1",   QCOM_RPM_PM8901_LDO1, &pm8901_pldo, "vdd_l1" },
+	{ "l2",   QCOM_RPM_PM8901_LDO2, &pm8901_pldo, "vdd_l2" },
+	{ "l3",   QCOM_RPM_PM8901_LDO3, &pm8901_pldo, "vdd_l3" },
+	{ "l4",   QCOM_RPM_PM8901_LDO4, &pm8901_pldo, "vdd_l4" },
+	{ "l5",   QCOM_RPM_PM8901_LDO5, &pm8901_pldo, "vdd_l5" },
+	{ "l6",   QCOM_RPM_PM8901_LDO6, &pm8901_pldo, "vdd_l6" },
 
-	vreg = devm_kmalloc(&pdev->dev, sizeof(*vreg), GFP_KERNEL);
-	if (!vreg) {
-		dev_err(&pdev->dev, "failed to allocate vreg\n");
-		return -ENOMEM;
-	}
-	memcpy(vreg, template, sizeof(*vreg));
-	mutex_init(&vreg->lock);
-	vreg->dev = &pdev->dev;
-	vreg->desc.id = -1;
-	vreg->desc.owner = THIS_MODULE;
-	vreg->desc.type = REGULATOR_VOLTAGE;
-	vreg->desc.name = pdev->dev.of_node->name;
-	vreg->desc.supply_name = "vin";
-
-	vreg->rpm = dev_get_drvdata(pdev->dev.parent);
-	if (!vreg->rpm) {
-		dev_err(&pdev->dev, "unable to retrieve handle to rpm\n");
-		return -ENODEV;
-	}
+	{ "s0",   QCOM_RPM_PM8901_SMPS0, &pm8901_ftsmps, "vdd_s0" },
+	{ "s1",   QCOM_RPM_PM8901_SMPS1, &pm8901_ftsmps, "vdd_s1" },
+	{ "s2",   QCOM_RPM_PM8901_SMPS2, &pm8901_ftsmps, "vdd_s2" },
+	{ "s3",   QCOM_RPM_PM8901_SMPS3, &pm8901_ftsmps, "vdd_s3" },
+	{ "s4",   QCOM_RPM_PM8901_SMPS4, &pm8901_ftsmps, "vdd_s4" },
 
-	initdata = of_get_regulator_init_data(&pdev->dev, pdev->dev.of_node,
-					      &vreg->desc);
-	if (!initdata)
-		return -EINVAL;
+	{ "lvs0", QCOM_RPM_PM8901_LVS0, &pm8901_switch, "lvs0_in" },
+	{ "lvs1", QCOM_RPM_PM8901_LVS1, &pm8901_switch, "lvs1_in" },
+	{ "lvs2", QCOM_RPM_PM8901_LVS2, &pm8901_switch, "lvs2_in" },
+	{ "lvs3", QCOM_RPM_PM8901_LVS3, &pm8901_switch, "lvs3_in" },
 
-	key = "reg";
-	ret = of_property_read_u32(pdev->dev.of_node, key, &val);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to read %s\n", key);
-		return ret;
-	}
-	vreg->resource = val;
+	{ "mvs", QCOM_RPM_PM8901_MVS, &pm8901_switch, "mvs_in" },
+	{ }
+};
 
-	if ((vreg->parts->uV.mask || vreg->parts->mV.mask) &&
-	    (!initdata->constraints.min_uV || !initdata->constraints.max_uV)) {
-		dev_err(&pdev->dev, "no voltage specified for regulator\n");
-		return -EINVAL;
-	}
+static const struct rpm_regulator_data rpm_pm8921_regulators[] = {
+	{ "s1",  QCOM_RPM_PM8921_SMPS1, &pm8921_smps, "vdd_s1" },
+	{ "s2",  QCOM_RPM_PM8921_SMPS2, &pm8921_smps, "vdd_s2" },
+	{ "s3",  QCOM_RPM_PM8921_SMPS3, &pm8921_smps },
+	{ "s4",  QCOM_RPM_PM8921_SMPS4, &pm8921_smps, "vdd_s4" },
+	{ "s7",  QCOM_RPM_PM8921_SMPS7, &pm8921_smps, "vdd_s7" },
+	{ "s8",  QCOM_RPM_PM8921_SMPS8, &pm8921_smps, "vdd_s8"  },
+
+	{ "l1",  QCOM_RPM_PM8921_LDO1, &pm8921_nldo, "vdd_l1_l2_l12_l18" },
+	{ "l2",  QCOM_RPM_PM8921_LDO2, &pm8921_nldo, "vdd_l1_l2_l12_l18" },
+	{ "l3",  QCOM_RPM_PM8921_LDO3, &pm8921_pldo, "vdd_l3_l15_l17" },
+	{ "l4",  QCOM_RPM_PM8921_LDO4, &pm8921_pldo, "vdd_l4_l14" },
+	{ "l5",  QCOM_RPM_PM8921_LDO5, &pm8921_pldo, "vdd_l5_l8_l16" },
+	{ "l6",  QCOM_RPM_PM8921_LDO6, &pm8921_pldo, "vdd_l6_l7" },
+	{ "l7",  QCOM_RPM_PM8921_LDO7, &pm8921_pldo, "vdd_l6_l7" },
+	{ "l8",  QCOM_RPM_PM8921_LDO8, &pm8921_pldo, "vdd_l5_l8_l16" },
+	{ "l9",  QCOM_RPM_PM8921_LDO9, &pm8921_pldo, "vdd_l9_l11" },
+	{ "l10", QCOM_RPM_PM8921_LDO10, &pm8921_pldo, "vdd_l10_l22" },
+	{ "l11", QCOM_RPM_PM8921_LDO11, &pm8921_pldo, "vdd_l9_l11" },
+	{ "l12", QCOM_RPM_PM8921_LDO12, &pm8921_nldo, "vdd_l1_l2_l12_l18" },
+	{ "l14", QCOM_RPM_PM8921_LDO14, &pm8921_pldo, "vdd_l4_l14" },
+	{ "l15", QCOM_RPM_PM8921_LDO15, &pm8921_pldo, "vdd_l3_l15_l17" },
+	{ "l16", QCOM_RPM_PM8921_LDO16, &pm8921_pldo, "vdd_l5_l8_l16" },
+	{ "l17", QCOM_RPM_PM8921_LDO17, &pm8921_pldo, "vdd_l3_l15_l17" },
+	{ "l18", QCOM_RPM_PM8921_LDO18, &pm8921_nldo, "vdd_l1_l2_l12_l18" },
+	{ "l21", QCOM_RPM_PM8921_LDO21, &pm8921_pldo, "vdd_l21_l23_l29" },
+	{ "l22", QCOM_RPM_PM8921_LDO22, &pm8921_pldo, "vdd_l10_l22" },
+	{ "l23", QCOM_RPM_PM8921_LDO23, &pm8921_pldo, "vdd_l21_l23_l29" },
+	{ "l24", QCOM_RPM_PM8921_LDO24, &pm8921_nldo1200, "vdd_l24" },
+	{ "l25", QCOM_RPM_PM8921_LDO25, &pm8921_nldo1200, "vdd_l25" },
+	{ "l26", QCOM_RPM_PM8921_LDO26, &pm8921_nldo1200, "vdd_l26" },
+	{ "l27", QCOM_RPM_PM8921_LDO27, &pm8921_nldo1200, "vdd_l27" },
+	{ "l28", QCOM_RPM_PM8921_LDO28, &pm8921_nldo1200, "vdd_l28" },
+	{ "l29", QCOM_RPM_PM8921_LDO29, &pm8921_pldo, "vdd_l21_l23_l29" },
+
+	{ "lvs1", QCOM_RPM_PM8921_LVS1, &pm8921_switch, "vin_lvs1_3_6" },
+	{ "lvs2", QCOM_RPM_PM8921_LVS2, &pm8921_switch, "vin_lvs2" },
+	{ "lvs3", QCOM_RPM_PM8921_LVS3, &pm8921_switch, "vin_lvs1_3_6" },
+	{ "lvs4", QCOM_RPM_PM8921_LVS4, &pm8921_switch, "vin_lvs4_5_7" },
+	{ "lvs5", QCOM_RPM_PM8921_LVS5, &pm8921_switch, "vin_lvs4_5_7" },
+	{ "lvs6", QCOM_RPM_PM8921_LVS6, &pm8921_switch, "vin_lvs1_3_6" },
+	{ "lvs7", QCOM_RPM_PM8921_LVS7, &pm8921_switch, "vin_lvs4_5_7" },
+
+	{ "usb-switch", QCOM_RPM_USB_OTG_SWITCH, &pm8921_switch, "vin_5vs" },
+	{ "hdmi-switch", QCOM_RPM_HDMI_SWITCH, &pm8921_switch, "vin_5vs" },
+	{ "ncp", QCOM_RPM_PM8921_NCP, &pm8921_ncp, "vdd_ncp" },
+	{ }
+};
 
+static const struct of_device_id rpm_of_match[] = {
+	{ .compatible = "qcom,rpm-pm8058-regulators", .data = &rpm_pm8058_regulators },
+	{ .compatible = "qcom,rpm-pm8901-regulators", .data = &rpm_pm8901_regulators },
+	{ .compatible = "qcom,rpm-pm8921-regulators", .data = &rpm_pm8921_regulators },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, rpm_of_match);
 
-	config.dev = &pdev->dev;
-	config.init_data = initdata;
-	config.driver_data = vreg;
-	config.of_node = pdev->dev.of_node;
+static int rpm_reg_probe(struct platform_device *pdev)
+{
+	const struct rpm_regulator_data *reg;
+	const struct of_device_id *match;
+	struct regulator_config config = { };
+	struct regulator_dev *rdev;
+	struct qcom_rpm_reg *vreg;
 
-	ret = rpm_reg_of_parse(pdev->dev.of_node, &vreg->desc, &config);
-	if (ret)
-		return ret;
+	match = of_match_device(rpm_of_match, &pdev->dev);
+	for (reg = match->data; reg->name; reg++) {
+		vreg = devm_kmalloc(&pdev->dev, sizeof(*vreg), GFP_KERNEL);
+		if (!vreg) {
+			dev_err(&pdev->dev, "failed to allocate vreg\n");
+			return -ENOMEM;
+		}
+		memcpy(vreg, reg->template, sizeof(*vreg));
+		mutex_init(&vreg->lock);
+
+		vreg->dev = &pdev->dev;
+		vreg->resource = reg->resource;
+
+		vreg->desc.id = -1;
+		vreg->desc.owner = THIS_MODULE;
+		vreg->desc.type = REGULATOR_VOLTAGE;
+		vreg->desc.name = reg->name;
+		vreg->desc.supply_name = reg->supply;
+		vreg->desc.of_match = reg->name;
+		vreg->desc.of_parse_cb = rpm_reg_of_parse;
+
+		vreg->rpm = dev_get_drvdata(pdev->dev.parent);
+		if (!vreg->rpm) {
+			dev_err(&pdev->dev, "unable to retrieve handle to rpm\n");
+			return -ENODEV;
+		}
 
-	rdev = devm_regulator_register(&pdev->dev, &vreg->desc, &config);
-	if (IS_ERR(rdev)) {
-		dev_err(&pdev->dev, "can't register regulator\n");
-		return PTR_ERR(rdev);
+		config.dev = &pdev->dev;
+		config.driver_data = vreg;
+		rdev = devm_regulator_register(&pdev->dev, &vreg->desc, &config);
+		if (IS_ERR(rdev)) {
+			dev_err(&pdev->dev, "can't register regulator\n");
+			return PTR_ERR(rdev);
+		}
 	}
 
 	return 0;
-- 
1.8.2.2

Re: [PATCH v2 5/5] regulator: qcom: Rework to single platform device

[Stephen Boyd]

On 04/01/15 15:55, Bjorn Andersson wrote:
> +static int rpm_reg_probe(struct platform_device *pdev)
> +{
> +	const struct rpm_regulator_data *reg;
> +	const struct of_device_id *match;
> +	struct regulator_config config = { };
> +	struct regulator_dev *rdev;
> +	struct qcom_rpm_reg *vreg;
>  
> -	ret = rpm_reg_of_parse(pdev->dev.of_node, &vreg->desc, &config);
> -	if (ret)
> -		return ret;
> +	match = of_match_device(rpm_of_match, &pdev->dev);
> +	for (reg = match->data; reg->name; reg++) {
> +		vreg = devm_kmalloc(&pdev->dev, sizeof(*vreg), GFP_KERNEL);
> +		if (!vreg) {
> +			dev_err(&pdev->dev, "failed to allocate vreg\n");

Please remove useless error message on allocation failures.

> +			return -ENOMEM;
> +		}
> +		memcpy(vreg, reg->template, sizeof(*vreg));
> +		mutex_init(&vreg->lock);
> +
> +		vreg->dev = &pdev->dev;
> +		vreg->resource = reg->resource;
> +
> +		vreg->desc.id = -1;
> +		vreg->desc.owner = THIS_MODULE;
> +		vreg->desc.type = REGULATOR_VOLTAGE;
> +		vreg->desc.name = reg->name;
> +		vreg->desc.supply_name = reg->supply;
> +		vreg->desc.of_match = reg->name;
> +		vreg->desc.of_parse_cb = rpm_reg_of_parse;
> +
> +		vreg->rpm = dev_get_drvdata(pdev->dev.parent);
> +		if (!vreg->rpm) {
> +			dev_err(&pdev->dev, "unable to retrieve handle to rpm\n");
> +			return -ENODEV;
> +		}

Was there going to be a patch to lift this out of the loop (although I
hope the compiler can hoist it out).

>  
> -	rdev = devm_regulator_register(&pdev->dev, &vreg->desc, &config);
> -	if (IS_ERR(rdev)) {
> -		dev_err(&pdev->dev, "can't register regulator\n");
> -		return PTR_ERR(rdev);
> +		config.dev = &pdev->dev;
> +		config.driver_data = vreg;
> +		rdev = devm_regulator_register(&pdev->dev, &vreg->desc, &config);
> +		if (IS_ERR(rdev)) {
> +			dev_err(&pdev->dev, "can't register regulator\n");

It'd be nice to know which regulator failed to register.

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

Re: [PATCH v2 5/5] regulator: qcom: Rework to single platform device

[Bjorn Andersson]

On Thu, Apr 2, 2015 at 4:02 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 04/01/15 15:55, Bjorn Andersson wrote:
>> +static int rpm_reg_probe(struct platform_device *pdev)
>> +{
>> +     const struct rpm_regulator_data *reg;
>> +     const struct of_device_id *match;
>> +     struct regulator_config config = { };
>> +     struct regulator_dev *rdev;
>> +     struct qcom_rpm_reg *vreg;
>>
>> -     ret = rpm_reg_of_parse(pdev->dev.of_node, &vreg->desc, &config);
>> -     if (ret)
>> -             return ret;
>> +     match = of_match_device(rpm_of_match, &pdev->dev);
>> +     for (reg = match->data; reg->name; reg++) {
>> +             vreg = devm_kmalloc(&pdev->dev, sizeof(*vreg), GFP_KERNEL);
>> +             if (!vreg) {
>> +                     dev_err(&pdev->dev, "failed to allocate vreg\n");
>
> Please remove useless error message on allocation failures.
>

I didn't want to touch this line, to keep the diff as clean as
possible. But I forgot about the promised patch to clean up this and
the rpm reference retrieval.

Sorry about that - will prepare the patch and send it out.

[..]

>>
>> -     rdev = devm_regulator_register(&pdev->dev, &vreg->desc, &config);
>> -     if (IS_ERR(rdev)) {
>> -             dev_err(&pdev->dev, "can't register regulator\n");
>> -             return PTR_ERR(rdev);
>> +             config.dev = &pdev->dev;
>> +             config.driver_data = vreg;
>> +             rdev = devm_regulator_register(&pdev->dev, &vreg->desc, &config);
>> +             if (IS_ERR(rdev)) {
>> +                     dev_err(&pdev->dev, "can't register regulator\n");
>
> It'd be nice to know which regulator failed to register.
>

I agree, will update this.

Regards,
Bjorn

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 --]