[PATCH 0/3] Qualcomm Resource Power Manager driver

[PATCH 0/3] Qualcomm Resource Power Manager driver

[Bjorn Andersson]

This series adds a regulator driver for the Resource Power Manager found in
Qualcomm 8660, 8960 and 8064 based devices.

The RPM driver exposes resources to its child devices, that can be accessed to
implement drivers for the regulators, clocks and bus frequency control that's
owned by the RPM in these devices.

Bjorn Andersson (3):
  mfd: devicetree: bindings: Add Qualcomm RPM DT binding
  mfd: qcom-rpm: Driver for the Qualcomm RPM
  regulator: qcom-rpm: Regulator driver for the Qualcomm RPM

 Documentation/devicetree/bindings/mfd/qcom,rpm.txt | 283 +++++++
 drivers/mfd/Kconfig                                |  15 +
 drivers/mfd/Makefile                               |   1 +
 drivers/mfd/qcom_rpm.c                             | 554 ++++++++++++++
 drivers/regulator/Kconfig                          |  12 +
 drivers/regulator/Makefile                         |   1 +
 drivers/regulator/qcom_rpm-regulator.c             | 852 +++++++++++++++++++++
 include/dt-bindings/mfd/qcom_rpm.h                 | 148 ++++
 include/linux/mfd/qcom_rpm.h                       |  13 +
 9 files changed, 1879 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/qcom,rpm.txt
 create mode 100644 drivers/mfd/qcom_rpm.c
 create mode 100644 drivers/regulator/qcom_rpm-regulator.c
 create mode 100644 include/dt-bindings/mfd/qcom_rpm.h
 create mode 100644 include/linux/mfd/qcom_rpm.h

-- 
1.8.2.2

[PATCH 1/3] mfd: devicetree: bindings: Add Qualcomm RPM DT binding

[Bjorn Andersson]

Add binding for the Qualcomm Resource Power Manager (RPM) found in 8660, 8960
and 8064 based devices. The binding currently describes the rpm itself and the
regulator subnodes.

Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---
 Documentation/devicetree/bindings/mfd/qcom,rpm.txt | 284 +++++++++++++++++++++
 include/dt-bindings/mfd/qcom_rpm.h                 | 142 +++++++++++
 2 files changed, 426 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/qcom,rpm.txt
 create mode 100644 include/dt-bindings/mfd/qcom_rpm.h

diff --git a/Documentation/devicetree/bindings/mfd/qcom,rpm.txt b/Documentation/devicetree/bindings/mfd/qcom,rpm.txt
new file mode 100644
index 0000000..3908a5d
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/qcom,rpm.txt
@@ -0,0 +1,284 @@
+Qualcomm Resource Power Manager (RPM)
+
+This driver is used to interface with the Resource Power Manager (RPM) found in
+various Qualcomm platforms. The RPM allows each component in the system to vote
+for state of the system resources, such as clocks, regulators and bus
+frequencies.
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be one of:
+		    "qcom,rpm-apq8064"
+		    "qcom,rpm-msm8660"
+		    "qcom,rpm-msm8960"
+
+- reg:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: two entries specifying the RPM's message ram and ipc register
+
+- reg-names:
+	Usage: required
+	Value type: <string-array>
+	Definition: must contain the following, in order:
+		    "msg_ram"
+		    "ipc"
+
+- interrupts:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: three entries specifying the RPM's:
+		    1. acknowledgement interrupt
+		    2. error interrupt
+		    3. wakeup interrupt
+
+- interrupt-names:
+	Usage: required
+	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
+
+
+= SUBDEVICES
+
+The RPM exposes resources to its subnodes. The below bindings specify the set
+of valid subnodes that can operate on these resources.
+
+== Switch-mode Power Supply regulator
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be one of:
+		    "qcom,rpm-pm8058-smps"
+		    "qcom,rpm-pm8901-ftsmps"
+		    "qcom,rpm-pm8921-smps"
+		    "qcom,rpm-pm8921-ftsmps"
+
+- reg:
+	Usage: required
+	Value type: <u32>
+	Definition: resource as defined in <dt-bindings/mfd/qcom_rpm.h>
+
+- 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,hpm-threshold:
+	Usage: optional
+	Value type: <u32>
+	Definition: indicates the breaking point at which the regulator should
+	            switch to high power mode
+
+- qcom,load-bias:
+	Usage: optional
+	Value type: <u32>
+	Definition: specifies a base load on the specific regulator
+
+- qcom,boot-load:
+	Usage: optional
+	Value type: <u32>
+	Definition: specifies the configured load on boot for the specific
+	            regulator
+
+- qcom,force-mode-none:
+	Usage: optional (default if no other qcom,force-mode is specified)
+	Value type: <empty>
+	Defintion: indicates that the regulator should not be forced to any
+	           particular mode
+
+- qcom,force-mode-lpm:
+	Usage: optional
+	Value type: <empty>
+	Definition: indicates that the regulator should be forced to operate in
+	            low-power-mode
+
+- qcom,force-mode-auto:
+	Usage: optional (only available for 8960/8064)
+	Value type: <empty>
+	Definition: indicates that the regulator should be automatically pick
+	            operating mode
+
+- qcom,force-mode-hpm:
+	Usage: optional (only available for 8960/8064)
+	Value type: <empty>
+	Definition: indicates that the regulator should be forced to operate in
+	            high-power-mode
+
+- qcom,force-mode-bypass: (only for 8960/8064)
+	Usage: optional (only available for 8960/8064)
+	Value type: <empty>
+	Definition: indicates that the regulator should be forced to operate in
+	            bypass mode
+
+- qcom,power-mode-hysteretic:
+	Usage: optional
+	Value type: <empty>
+	Definition: indicates that the power supply should operate in hysteretic
+		    mode (defaults to qcom,power-mode-pwm if not specified)
+
+- qcom,power-mode-pwm:
+	Usage: optional
+	Value type: <empty>
+	Definition: indicates that the power supply should operate in pwm mode
+
+Standard regulator bindings are used inside switch mode power supply subnodes.
+Check Documentation/devicetree/bindings/regulator/regulator.txt for more
+details.
+
+== Low-dropout regulator
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be one of:
+		    "qcom,rpm-pm8058-pldo"
+		    "qcom,rpm-pm8058-nldo"
+		    "qcom,rpm-pm8901-pldo"
+		    "qcom,rpm-pm8901-nldo"
+		    "qcom,rpm-pm8921-pldo"
+		    "qcom,rpm-pm8921-nldo"
+		    "qcom,rpm-pm8921-nldo1200"
+
+- reg:
+	Usage: required
+	Value type: <u32>
+	Definition: resource as defined in <dt-bindings/mfd/qcom_rpm.h>
+
+- qcom,hpm-threshold:
+	Usage: optional
+	Value type: <u32>
+	Definition: indicates the breaking point at which the regulator should
+	            switch to high power mode
+
+- qcom,load-bias:
+	Usage: optional
+	Value type: <u32>
+	Definition: specifies a base load on the specific regulator
+
+- qcom,boot-load:
+	Usage: optional
+	Value type: <u32>
+	Definition: specifies the configured load on boot for the specific
+	            regulator
+
+- qcom,force-mode-none:
+	Usage: optional (default if no other qcom,force-mode is specified)
+	Value type: <empty>
+	Defintion: indicates that the regulator should not be forced to any
+	           particular mode
+
+- qcom,force-mode-lpm:
+	Usage: optional
+	Value type: <empty>
+	Definition: indicates that the regulator should be forced to operate in
+	            low-power-mode
+
+- qcom,force-mode-auto:
+	Usage: optional (only available for 8960/8064)
+	Value type: <empty>
+	Definition: indicates that the regulator should be automatically pick
+	            operating mode
+
+- qcom,force-mode-hpm:
+	Usage: optional (only available for 8960/8064)
+	Value type: <empty>
+	Definition: indicates that the regulator should be forced to operate in
+	            high-power-mode
+
+- qcom,force-mode-bypass: (only for 8960/8064)
+	Usage: optional (only available for 8960/8064)
+	Value type: <empty>
+	Definition: indicates that the regulator should be forced to operate in
+	            bypass mode
+
+Standard regulator bindings are used inside switch low-dropout regulator
+subnodes.  Check Documentation/devicetree/bindings/regulator/regulator.txt for
+more details.
+
+== Negative Charge Pump
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be one of:
+		    "qcom,rpm-pm8058-ncp"
+		    "qcom,rpm-pm8921-ncp"
+		    "qcom,rpm-pm8921-nldo1200"
+
+- reg:
+	Usage: required
+	Value type: <u32>
+	Definition: resource as defined in <dt-bindings/mfd/qcom_rpm.h>
+
+- 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
+
+Standard regulator bindings are used inside negative charge pump regulator
+subnodes.  Check Documentation/devicetree/bindings/regulator/regulator.txt for
+more details.
+
+== Switch
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be one of:
+		    "qcom,rpm-pm8058-switch"
+		    "qcom,rpm-pm8901-switch"
+		    "qcom,rpm-pm8921-switch"
+
+- reg:
+	Usage: required
+	Value type: <u32>
+	Definition: resource as defined in <dt-bindings/mfd/qcom_rpm.h>
+
+
+= EXAMPLE
+
+	#include <dt-bindings/mfd/qcom_rpm.h>
+
+	rpm@108000 {
+		compatible = "qcom,rpm-msm8960";
+		reg = <0x108000 0x1000 0x2011008 0x4>;
+
+		interrupts = <0 19 0>, <0 21 0>, <0 22 0>;
+		interrupt-names = "ack", "err", "wakeup";
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		pm8921_s1: pm8921-s1 {
+			compatible = "qcom,rpm-pm8921-smps";
+			reg = <QCOM_RPM_PM8921_S1>;
+
+			regulator-min-microvolt = <1225000>;
+			regulator-max-microvolt = <1225000>;
+			regulator-always-on;
+
+			qcom,switch-mode-frequency = <3200000>;
+			qcom,hpm-threshold = <100000>;
+		};
+	};
+
diff --git a/include/dt-bindings/mfd/qcom_rpm.h b/include/dt-bindings/mfd/qcom_rpm.h
new file mode 100644
index 0000000..277e789
--- /dev/null
+++ b/include/dt-bindings/mfd/qcom_rpm.h
@@ -0,0 +1,142 @@
+/*
+ * This header provides constants for the Qualcomm RPM bindings.
+ */
+
+#ifndef _DT_BINDINGS_MFD_QCOM_RPM_H
+#define _DT_BINDINGS_MFD_QCOM_RPM_H
+
+#define QCOM_RPM_APPS_FABRIC_ARB		1
+#define QCOM_RPM_APPS_FABRIC_CLK		2
+#define QCOM_RPM_APPS_FABRIC_HALT		3
+#define QCOM_RPM_APPS_FABRIC_IOCTL		4
+#define QCOM_RPM_APPS_FABRIC_MODE		5
+#define QCOM_RPM_APPS_L2_CACHE_CTL		6
+#define QCOM_RPM_CFPB_CLK			7
+#define QCOM_RPM_CXO_BUFFERS			8
+#define QCOM_RPM_CXO_CLK			9
+#define QCOM_RPM_DAYTONA_FABRIC_CLK		10
+#define QCOM_RPM_DDR_DMM			11
+#define QCOM_RPM_EBI1_CLK			12
+#define QCOM_RPM_HDMI_SWITCH			13
+#define QCOM_RPM_MMFPB_CLK			14
+#define QCOM_RPM_MM_FABRIC_ARB			15
+#define QCOM_RPM_MM_FABRIC_CLK			16
+#define QCOM_RPM_MM_FABRIC_HALT			17
+#define QCOM_RPM_MM_FABRIC_IOCTL		18
+#define QCOM_RPM_MM_FABRIC_MODE			19
+#define QCOM_RPM_PLL_4				20
+#define QCOM_RPM_PM8058_LDO0			21
+#define QCOM_RPM_PM8058_LDO1			22
+#define QCOM_RPM_PM8058_LDO2			23
+#define QCOM_RPM_PM8058_LDO3			24
+#define QCOM_RPM_PM8058_LDO4			25
+#define QCOM_RPM_PM8058_LDO5			26
+#define QCOM_RPM_PM8058_LDO6			27
+#define QCOM_RPM_PM8058_LDO7			28
+#define QCOM_RPM_PM8058_LDO8			29
+#define QCOM_RPM_PM8058_LDO9			30
+#define QCOM_RPM_PM8058_LDO10			31
+#define QCOM_RPM_PM8058_LDO11			32
+#define QCOM_RPM_PM8058_LDO12			33
+#define QCOM_RPM_PM8058_LDO13			34
+#define QCOM_RPM_PM8058_LDO14			35
+#define QCOM_RPM_PM8058_LDO15			36
+#define QCOM_RPM_PM8058_LDO16			37
+#define QCOM_RPM_PM8058_LDO17			38
+#define QCOM_RPM_PM8058_LDO18			39
+#define QCOM_RPM_PM8058_LDO19			40
+#define QCOM_RPM_PM8058_LDO20			41
+#define QCOM_RPM_PM8058_LDO21			42
+#define QCOM_RPM_PM8058_LDO22			43
+#define QCOM_RPM_PM8058_LDO23			44
+#define QCOM_RPM_PM8058_LDO24			45
+#define QCOM_RPM_PM8058_LDO25			46
+#define QCOM_RPM_PM8058_LVS0			47
+#define QCOM_RPM_PM8058_LVS1			48
+#define QCOM_RPM_PM8058_NCP			49
+#define QCOM_RPM_PM8058_SMPS0			50
+#define QCOM_RPM_PM8058_SMPS1			51
+#define QCOM_RPM_PM8058_SMPS2			52
+#define QCOM_RPM_PM8058_SMPS3			53
+#define QCOM_RPM_PM8058_SMPS4			54
+#define QCOM_RPM_PM8821_L1			55
+#define QCOM_RPM_PM8821_S1			56
+#define QCOM_RPM_PM8821_S2			57
+#define QCOM_RPM_PM8901_LDO0			58
+#define QCOM_RPM_PM8901_LDO1			59
+#define QCOM_RPM_PM8901_LDO2			60
+#define QCOM_RPM_PM8901_LDO3			61
+#define QCOM_RPM_PM8901_LDO4			62
+#define QCOM_RPM_PM8901_LDO5			63
+#define QCOM_RPM_PM8901_LDO6			64
+#define QCOM_RPM_PM8901_LVS0			65
+#define QCOM_RPM_PM8901_LVS1			66
+#define QCOM_RPM_PM8901_LVS2			67
+#define QCOM_RPM_PM8901_LVS3			68
+#define QCOM_RPM_PM8901_MVS			69
+#define QCOM_RPM_PM8901_SMPS0			70
+#define QCOM_RPM_PM8901_SMPS1			71
+#define QCOM_RPM_PM8901_SMPS2			72
+#define QCOM_RPM_PM8901_SMPS3			73
+#define QCOM_RPM_PM8901_SMPS4			74
+#define QCOM_RPM_PM8921_CLK1			75
+#define QCOM_RPM_PM8921_CLK2			76
+#define QCOM_RPM_PM8921_L1			77
+#define QCOM_RPM_PM8921_L2			78
+#define QCOM_RPM_PM8921_L3			79
+#define QCOM_RPM_PM8921_L4			80
+#define QCOM_RPM_PM8921_L5			81
+#define QCOM_RPM_PM8921_L6			82
+#define QCOM_RPM_PM8921_L7			83
+#define QCOM_RPM_PM8921_L8			84
+#define QCOM_RPM_PM8921_L9			85
+#define QCOM_RPM_PM8921_L10			86
+#define QCOM_RPM_PM8921_L11			87
+#define QCOM_RPM_PM8921_L12			88
+#define QCOM_RPM_PM8921_L13			89
+#define QCOM_RPM_PM8921_L14			90
+#define QCOM_RPM_PM8921_L15			91
+#define QCOM_RPM_PM8921_L16			92
+#define QCOM_RPM_PM8921_L17			93
+#define QCOM_RPM_PM8921_L18			94
+#define QCOM_RPM_PM8921_L19			95
+#define QCOM_RPM_PM8921_L20			96
+#define QCOM_RPM_PM8921_L21			97
+#define QCOM_RPM_PM8921_L22			98
+#define QCOM_RPM_PM8921_L23			99
+#define QCOM_RPM_PM8921_L24			100
+#define QCOM_RPM_PM8921_L25			101
+#define QCOM_RPM_PM8921_L26			102
+#define QCOM_RPM_PM8921_L27			103
+#define QCOM_RPM_PM8921_L28			104
+#define QCOM_RPM_PM8921_L29			105
+#define QCOM_RPM_PM8921_LVS1			106
+#define QCOM_RPM_PM8921_LVS2			107
+#define QCOM_RPM_PM8921_LVS3			108
+#define QCOM_RPM_PM8921_LVS4			109
+#define QCOM_RPM_PM8921_LVS5			110
+#define QCOM_RPM_PM8921_LVS6			111
+#define QCOM_RPM_PM8921_LVS7			112
+#define QCOM_RPM_PM8921_MVS			113
+#define QCOM_RPM_PM8921_NCP			114
+#define QCOM_RPM_PM8921_S1			115
+#define QCOM_RPM_PM8921_S2			116
+#define QCOM_RPM_PM8921_S3			117
+#define QCOM_RPM_PM8921_S4			118
+#define QCOM_RPM_PM8921_S5			119
+#define QCOM_RPM_PM8921_S6			120
+#define QCOM_RPM_PM8921_S7			121
+#define QCOM_RPM_PM8921_S8			122
+#define QCOM_RPM_PXO_CLK			123
+#define QCOM_RPM_QDSS_CLK			124
+#define QCOM_RPM_SFPB_CLK			125
+#define QCOM_RPM_SMI_CLK			126
+#define QCOM_RPM_SYS_FABRIC_ARB			127
+#define QCOM_RPM_SYS_FABRIC_CLK			128
+#define QCOM_RPM_SYS_FABRIC_HALT		129
+#define QCOM_RPM_SYS_FABRIC_IOCTL		130
+#define QCOM_RPM_SYS_FABRIC_MODE		131
+#define QCOM_RPM_USB_OTG_SWITCH			132
+#define QCOM_RPM_VDDMIN_GPIO			133
+
+#endif
-- 
1.8.2.2

Re: [PATCH 1/3] mfd: devicetree: bindings: Add Qualcomm RPM DT binding

[Kumar Gala]

On May 27, 2014, at 12:28 PM, Bjorn Andersson <bjorn.andersson@sonymobile.com> wrote:

> Add binding for the Qualcomm Resource Power Manager (RPM) found in 8660, 8960
> and 8064 based devices. The binding currently describes the rpm itself and the
> regulator subnodes.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> ---
> Documentation/devicetree/bindings/mfd/qcom,rpm.txt | 284 +++++++++++++++++++++
> include/dt-bindings/mfd/qcom_rpm.h                 | 142 +++++++++++
> 2 files changed, 426 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/qcom,rpm.txt
> create mode 100644 include/dt-bindings/mfd/qcom_rpm.h
> 
> diff --git a/Documentation/devicetree/bindings/mfd/qcom,rpm.txt b/Documentation/devicetree/bindings/mfd/qcom,rpm.txt
> new file mode 100644
> index 0000000..3908a5d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/qcom,rpm.txt
> @@ -0,0 +1,284 @@
> +Qualcomm Resource Power Manager (RPM)
> +
> +This driver is used to interface with the Resource Power Manager (RPM) found in
> +various Qualcomm platforms. The RPM allows each component in the system to vote
> +for state of the system resources, such as clocks, regulators and bus
> +frequencies.
> +
> +- compatible:
> +	Usage: required
> +	Value type: <string>
> +	Definition: must be one of:
> +		    "qcom,rpm-apq8064"
> +		    "qcom,rpm-msm8660"
> +		    "qcom,rpm-msm8960"
> +
> +- reg:
> +	Usage: required
> +	Value type: <prop-encoded-array>
> +	Definition: two entries specifying the RPM's message ram and ipc register
> +
> +- reg-names:
> +	Usage: required
> +	Value type: <string-array>
> +	Definition: must contain the following, in order:
> +		    "msg_ram"
> +		    “ipc"

If order maters, it should be on reg not reg-names.  If order doesn’t mater than this should say the names should match the reg

> +
> +- interrupts:
> +	Usage: required
> +	Value type: <prop-encoded-array>
> +	Definition: three entries specifying the RPM's:
> +		    1. acknowledgement interrupt
> +		    2. error interrupt
> +		    3. wakeup interrupt
> +
> +- interrupt-names:
> +	Usage: required
> +	Value type: <string-array>
> +	Definition: must be the three strings "ack", "err" and "wakeup", in order

again, if order maters it should be with the interrupts prop, not the name.

> +
> +- #address-cells:
> +	Usage: required
> +	Value type: <u32>
> +	Definition: must be 1
> +
> +- #size-cells:
> +	Usage: required
> +	Value type: <u32>
> +	Definition: must be 0
> +
> +
> += SUBDEVICES
> +
> +The RPM exposes resources to its subnodes. The below bindings specify the set
> +of valid subnodes that can operate on these resources.
> +
> +== Switch-mode Power Supply regulator
> +
> +- compatible:
> +	Usage: required
> +	Value type: <string>
> +	Definition: must be one of:
> +		    "qcom,rpm-pm8058-smps"
> +		    "qcom,rpm-pm8901-ftsmps"
> +		    "qcom,rpm-pm8921-smps"
> +		    "qcom,rpm-pm8921-ftsmps"
> +
> +- reg:
> +	Usage: required
> +	Value type: <u32>
> +	Definition: resource as defined in <dt-bindings/mfd/qcom_rpm.h>

Can we provide a bit more description about what “namespace” this reg is work in.

> +
> +- 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,hpm-threshold:
> +	Usage: optional
> +	Value type: <u32>
> +	Definition: indicates the breaking point at which the regulator should
> +	            switch to high power mode

in what units?

> +
> +- qcom,load-bias:
> +	Usage: optional
> +	Value type: <u32>
> +	Definition: specifies a base load on the specific regulator

in what units?

> +
> +- qcom,boot-load:
> +	Usage: optional
> +	Value type: <u32>
> +	Definition: specifies the configured load on boot for the specific
> +	            regulator

in what units?

> +
> +- qcom,force-mode-none:
> +	Usage: optional (default if no other qcom,force-mode is specified)
> +	Value type: <empty>
> +	Defintion: indicates that the regulator should not be forced to any
> +	           particular mode
> +
> +- qcom,force-mode-lpm:
> +	Usage: optional
> +	Value type: <empty>
> +	Definition: indicates that the regulator should be forced to operate in
> +	            low-power-mode
> +
> +- qcom,force-mode-auto:
> +	Usage: optional (only available for 8960/8064)
> +	Value type: <empty>
> +	Definition: indicates that the regulator should be automatically pick
> +	            operating mode
> +
> +- qcom,force-mode-hpm:
> +	Usage: optional (only available for 8960/8064)
> +	Value type: <empty>
> +	Definition: indicates that the regulator should be forced to operate in
> +	            high-power-mode
> +
> +- qcom,force-mode-bypass: (only for 8960/8064)
> +	Usage: optional (only available for 8960/8064)
> +	Value type: <empty>
> +	Definition: indicates that the regulator should be forced to operate in
> +	            bypass mode
> +

Is force-mode really an enum?  Or can we really have multiple force-mode’s set?

> +- qcom,power-mode-hysteretic:
> +	Usage: optional
> +	Value type: <empty>
> +	Definition: indicates that the power supply should operate in hysteretic
> +		    mode (defaults to qcom,power-mode-pwm if not specified)
> +
> +- qcom,power-mode-pwm:
> +	Usage: optional
> +	Value type: <empty>
> +	Definition: indicates that the power supply should operate in pwm mode
> +

Same question here is power-mode either pwm or hysteretic or can we set both?

> +Standard regulator bindings are used inside switch mode power supply subnodes.
> +Check Documentation/devicetree/bindings/regulator/regulator.txt for more
> +details.
> +
> +== Low-dropout regulator
> +
> +- compatible:
> +	Usage: required
> +	Value type: <string>
> +	Definition: must be one of:
> +		    "qcom,rpm-pm8058-pldo"
> +		    "qcom,rpm-pm8058-nldo"
> +		    "qcom,rpm-pm8901-pldo"
> +		    "qcom,rpm-pm8901-nldo"
> +		    "qcom,rpm-pm8921-pldo"
> +		    "qcom,rpm-pm8921-nldo"
> +		    "qcom,rpm-pm8921-nldo1200"
> +
> +- reg:
> +	Usage: required
> +	Value type: <u32>
> +	Definition: resource as defined in <dt-bindings/mfd/qcom_rpm.h>
> +
> +- qcom,hpm-threshold:
> +	Usage: optional
> +	Value type: <u32>
> +	Definition: indicates the breaking point at which the regulator should
> +	            switch to high power mode
> +
> +- qcom,load-bias:
> +	Usage: optional
> +	Value type: <u32>
> +	Definition: specifies a base load on the specific regulator
> +
> +- qcom,boot-load:
> +	Usage: optional
> +	Value type: <u32>
> +	Definition: specifies the configured load on boot for the specific
> +	            regulator
> +
> +- qcom,force-mode-none:
> +	Usage: optional (default if no other qcom,force-mode is specified)
> +	Value type: <empty>
> +	Defintion: indicates that the regulator should not be forced to any
> +	           particular mode
> +
> +- qcom,force-mode-lpm:
> +	Usage: optional
> +	Value type: <empty>
> +	Definition: indicates that the regulator should be forced to operate in
> +	            low-power-mode
> +
> +- qcom,force-mode-auto:
> +	Usage: optional (only available for 8960/8064)
> +	Value type: <empty>
> +	Definition: indicates that the regulator should be automatically pick
> +	            operating mode
> +
> +- qcom,force-mode-hpm:
> +	Usage: optional (only available for 8960/8064)
> +	Value type: <empty>
> +	Definition: indicates that the regulator should be forced to operate in
> +	            high-power-mode
> +
> +- qcom,force-mode-bypass: (only for 8960/8064)
> +	Usage: optional (only available for 8960/8064)
> +	Value type: <empty>
> +	Definition: indicates that the regulator should be forced to operate in
> +	            bypass mode
> +
> +Standard regulator bindings are used inside switch low-dropout regulator
> +subnodes.  Check Documentation/devicetree/bindings/regulator/regulator.txt for
> +more details.
> +
> +== Negative Charge Pump
> +
> +- compatible:
> +	Usage: required
> +	Value type: <string>
> +	Definition: must be one of:
> +		    "qcom,rpm-pm8058-ncp"
> +		    "qcom,rpm-pm8921-ncp"
> +		    "qcom,rpm-pm8921-nldo1200"
> +
> +- reg:
> +	Usage: required
> +	Value type: <u32>
> +	Definition: resource as defined in <dt-bindings/mfd/qcom_rpm.h>
> +
> +- 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
> +
> +Standard regulator bindings are used inside negative charge pump regulator
> +subnodes.  Check Documentation/devicetree/bindings/regulator/regulator.txt for
> +more details.
> +
> +== Switch
> +
> +- compatible:
> +	Usage: required
> +	Value type: <string>
> +	Definition: must be one of:
> +		    "qcom,rpm-pm8058-switch"
> +		    "qcom,rpm-pm8901-switch"
> +		    "qcom,rpm-pm8921-switch"
> +
> +- reg:
> +	Usage: required
> +	Value type: <u32>
> +	Definition: resource as defined in <dt-bindings/mfd/qcom_rpm.h>
> +
> +
> += EXAMPLE
> +
> +	#include <dt-bindings/mfd/qcom_rpm.h>
> +
> +	rpm@108000 {
> +		compatible = "qcom,rpm-msm8960";
> +		reg = <0x108000 0x1000 0x2011008 0x4>;
> +
> +		interrupts = <0 19 0>, <0 21 0>, <0 22 0>;
> +		interrupt-names = "ack", "err", "wakeup";
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		pm8921_s1: pm8921-s1 {
> +			compatible = "qcom,rpm-pm8921-smps";
> +			reg = <QCOM_RPM_PM8921_S1>;
> +
> +			regulator-min-microvolt = <1225000>;
> +			regulator-max-microvolt = <1225000>;
> +			regulator-always-on;
> +
> +			qcom,switch-mode-frequency = <3200000>;
> +			qcom,hpm-threshold = <100000>;
> +		};
> +	};
> +
> diff --git a/include/dt-bindings/mfd/qcom_rpm.h b/include/dt-bindings/mfd/qcom_rpm.h
> new file mode 100644
> index 0000000..277e789
> --- /dev/null
> +++ b/include/dt-bindings/mfd/qcom_rpm.h
> @@ -0,0 +1,142 @@
> +/*
> + * This header provides constants for the Qualcomm RPM bindings.
> + */
> +
> +#ifndef _DT_BINDINGS_MFD_QCOM_RPM_H
> +#define _DT_BINDINGS_MFD_QCOM_RPM_H
> +
> +#define QCOM_RPM_APPS_FABRIC_ARB		1
> +#define QCOM_RPM_APPS_FABRIC_CLK		2
> +#define QCOM_RPM_APPS_FABRIC_HALT		3
> +#define QCOM_RPM_APPS_FABRIC_IOCTL		4
> +#define QCOM_RPM_APPS_FABRIC_MODE		5
> +#define QCOM_RPM_APPS_L2_CACHE_CTL		6
> +#define QCOM_RPM_CFPB_CLK			7
> +#define QCOM_RPM_CXO_BUFFERS			8
> +#define QCOM_RPM_CXO_CLK			9
> +#define QCOM_RPM_DAYTONA_FABRIC_CLK		10
> +#define QCOM_RPM_DDR_DMM			11
> +#define QCOM_RPM_EBI1_CLK			12
> +#define QCOM_RPM_HDMI_SWITCH			13
> +#define QCOM_RPM_MMFPB_CLK			14
> +#define QCOM_RPM_MM_FABRIC_ARB			15
> +#define QCOM_RPM_MM_FABRIC_CLK			16
> +#define QCOM_RPM_MM_FABRIC_HALT			17
> +#define QCOM_RPM_MM_FABRIC_IOCTL		18
> +#define QCOM_RPM_MM_FABRIC_MODE			19
> +#define QCOM_RPM_PLL_4				20
> +#define QCOM_RPM_PM8058_LDO0			21
> +#define QCOM_RPM_PM8058_LDO1			22
> +#define QCOM_RPM_PM8058_LDO2			23
> +#define QCOM_RPM_PM8058_LDO3			24
> +#define QCOM_RPM_PM8058_LDO4			25
> +#define QCOM_RPM_PM8058_LDO5			26
> +#define QCOM_RPM_PM8058_LDO6			27
> +#define QCOM_RPM_PM8058_LDO7			28
> +#define QCOM_RPM_PM8058_LDO8			29
> +#define QCOM_RPM_PM8058_LDO9			30
> +#define QCOM_RPM_PM8058_LDO10			31
> +#define QCOM_RPM_PM8058_LDO11			32
> +#define QCOM_RPM_PM8058_LDO12			33
> +#define QCOM_RPM_PM8058_LDO13			34
> +#define QCOM_RPM_PM8058_LDO14			35
> +#define QCOM_RPM_PM8058_LDO15			36
> +#define QCOM_RPM_PM8058_LDO16			37
> +#define QCOM_RPM_PM8058_LDO17			38
> +#define QCOM_RPM_PM8058_LDO18			39
> +#define QCOM_RPM_PM8058_LDO19			40
> +#define QCOM_RPM_PM8058_LDO20			41
> +#define QCOM_RPM_PM8058_LDO21			42
> +#define QCOM_RPM_PM8058_LDO22			43
> +#define QCOM_RPM_PM8058_LDO23			44
> +#define QCOM_RPM_PM8058_LDO24			45
> +#define QCOM_RPM_PM8058_LDO25			46
> +#define QCOM_RPM_PM8058_LVS0			47
> +#define QCOM_RPM_PM8058_LVS1			48
> +#define QCOM_RPM_PM8058_NCP			49
> +#define QCOM_RPM_PM8058_SMPS0			50
> +#define QCOM_RPM_PM8058_SMPS1			51
> +#define QCOM_RPM_PM8058_SMPS2			52
> +#define QCOM_RPM_PM8058_SMPS3			53
> +#define QCOM_RPM_PM8058_SMPS4			54
> +#define QCOM_RPM_PM8821_L1			55
> +#define QCOM_RPM_PM8821_S1			56
> +#define QCOM_RPM_PM8821_S2			57
> +#define QCOM_RPM_PM8901_LDO0			58
> +#define QCOM_RPM_PM8901_LDO1			59
> +#define QCOM_RPM_PM8901_LDO2			60
> +#define QCOM_RPM_PM8901_LDO3			61
> +#define QCOM_RPM_PM8901_LDO4			62
> +#define QCOM_RPM_PM8901_LDO5			63
> +#define QCOM_RPM_PM8901_LDO6			64
> +#define QCOM_RPM_PM8901_LVS0			65
> +#define QCOM_RPM_PM8901_LVS1			66
> +#define QCOM_RPM_PM8901_LVS2			67
> +#define QCOM_RPM_PM8901_LVS3			68
> +#define QCOM_RPM_PM8901_MVS			69
> +#define QCOM_RPM_PM8901_SMPS0			70
> +#define QCOM_RPM_PM8901_SMPS1			71
> +#define QCOM_RPM_PM8901_SMPS2			72
> +#define QCOM_RPM_PM8901_SMPS3			73
> +#define QCOM_RPM_PM8901_SMPS4			74
> +#define QCOM_RPM_PM8921_CLK1			75
> +#define QCOM_RPM_PM8921_CLK2			76
> +#define QCOM_RPM_PM8921_L1			77
> +#define QCOM_RPM_PM8921_L2			78
> +#define QCOM_RPM_PM8921_L3			79
> +#define QCOM_RPM_PM8921_L4			80
> +#define QCOM_RPM_PM8921_L5			81
> +#define QCOM_RPM_PM8921_L6			82
> +#define QCOM_RPM_PM8921_L7			83
> +#define QCOM_RPM_PM8921_L8			84
> +#define QCOM_RPM_PM8921_L9			85
> +#define QCOM_RPM_PM8921_L10			86
> +#define QCOM_RPM_PM8921_L11			87
> +#define QCOM_RPM_PM8921_L12			88
> +#define QCOM_RPM_PM8921_L13			89
> +#define QCOM_RPM_PM8921_L14			90
> +#define QCOM_RPM_PM8921_L15			91
> +#define QCOM_RPM_PM8921_L16			92
> +#define QCOM_RPM_PM8921_L17			93
> +#define QCOM_RPM_PM8921_L18			94
> +#define QCOM_RPM_PM8921_L19			95
> +#define QCOM_RPM_PM8921_L20			96
> +#define QCOM_RPM_PM8921_L21			97
> +#define QCOM_RPM_PM8921_L22			98
> +#define QCOM_RPM_PM8921_L23			99
> +#define QCOM_RPM_PM8921_L24			100
> +#define QCOM_RPM_PM8921_L25			101
> +#define QCOM_RPM_PM8921_L26			102
> +#define QCOM_RPM_PM8921_L27			103
> +#define QCOM_RPM_PM8921_L28			104
> +#define QCOM_RPM_PM8921_L29			105
> +#define QCOM_RPM_PM8921_LVS1			106
> +#define QCOM_RPM_PM8921_LVS2			107
> +#define QCOM_RPM_PM8921_LVS3			108
> +#define QCOM_RPM_PM8921_LVS4			109
> +#define QCOM_RPM_PM8921_LVS5			110
> +#define QCOM_RPM_PM8921_LVS6			111
> +#define QCOM_RPM_PM8921_LVS7			112
> +#define QCOM_RPM_PM8921_MVS			113
> +#define QCOM_RPM_PM8921_NCP			114
> +#define QCOM_RPM_PM8921_S1			115
> +#define QCOM_RPM_PM8921_S2			116
> +#define QCOM_RPM_PM8921_S3			117
> +#define QCOM_RPM_PM8921_S4			118
> +#define QCOM_RPM_PM8921_S5			119
> +#define QCOM_RPM_PM8921_S6			120
> +#define QCOM_RPM_PM8921_S7			121
> +#define QCOM_RPM_PM8921_S8			122
> +#define QCOM_RPM_PXO_CLK			123
> +#define QCOM_RPM_QDSS_CLK			124
> +#define QCOM_RPM_SFPB_CLK			125
> +#define QCOM_RPM_SMI_CLK			126
> +#define QCOM_RPM_SYS_FABRIC_ARB			127
> +#define QCOM_RPM_SYS_FABRIC_CLK			128
> +#define QCOM_RPM_SYS_FABRIC_HALT		129
> +#define QCOM_RPM_SYS_FABRIC_IOCTL		130
> +#define QCOM_RPM_SYS_FABRIC_MODE		131
> +#define QCOM_RPM_USB_OTG_SWITCH			132
> +#define QCOM_RPM_VDDMIN_GPIO			133

Are these values arbitrary or do they mean something to hw?

> +
> +#endif
> -- 
> 1.8.2.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 1/3] mfd: devicetree: bindings: Add Qualcomm RPM DT binding

[Bjorn Andersson]

On Wed, May 28, 2014 at 9:34 AM, Kumar Gala <galak@codeaurora.org> wrote:
>
> On May 27, 2014, at 12:28 PM, Bjorn Andersson <bjorn.andersson@sonymobile.com> wrote:
>
>> Add binding for the Qualcomm Resource Power Manager (RPM) found in 8660, 8960
>> and 8064 based devices. The binding currently describes the rpm itself and the
>> regulator subnodes.
>>
>> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
>> ---
>> Documentation/devicetree/bindings/mfd/qcom,rpm.txt | 284 +++++++++++++++++++++
>> include/dt-bindings/mfd/qcom_rpm.h                 | 142 +++++++++++
>> 2 files changed, 426 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mfd/qcom,rpm.txt
>> create mode 100644 include/dt-bindings/mfd/qcom_rpm.h
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/qcom,rpm.txt b/Documentation/devicetree/bindings/mfd/qcom,rpm.txt
>> new file mode 100644
>> index 0000000..3908a5d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/qcom,rpm.txt
>> @@ -0,0 +1,284 @@
>> +Qualcomm Resource Power Manager (RPM)
>> +
>> +This driver is used to interface with the Resource Power Manager (RPM) found in
>> +various Qualcomm platforms. The RPM allows each component in the system to vote
>> +for state of the system resources, such as clocks, regulators and bus
>> +frequencies.
>> +
>> +- compatible:
>> +     Usage: required
>> +     Value type: <string>
>> +     Definition: must be one of:
>> +                 "qcom,rpm-apq8064"
>> +                 "qcom,rpm-msm8660"
>> +                 "qcom,rpm-msm8960"
>> +
>> +- reg:
>> +     Usage: required
>> +     Value type: <prop-encoded-array>
>> +     Definition: two entries specifying the RPM's message ram and ipc register
>> +
>> +- reg-names:
>> +     Usage: required
>> +     Value type: <string-array>
>> +     Definition: must contain the following, in order:
>> +                 "msg_ram"
>> +                 “ipc"
>
> If order maters, it should be on reg not reg-names.  If order doesn’t mater than this should say the names should match the reg

The DT guys have been clear on that the order of the reg property must
be fixed, no matter if you have reg-names or not. But I will make sure
to clarify this by being specific in the definition of "reg" as well.

>
>> +
>> +- interrupts:
>> +     Usage: required
>> +     Value type: <prop-encoded-array>
>> +     Definition: three entries specifying the RPM's:
>> +                 1. acknowledgement interrupt
>> +                 2. error interrupt
>> +                 3. wakeup interrupt
>> +
>> +- interrupt-names:
>> +     Usage: required
>> +     Value type: <string-array>
>> +     Definition: must be the three strings "ack", "err" and "wakeup", in order
>
> again, if order maters it should be with the interrupts prop, not the name.

I'll add something to the definition of "interrupts" to clarify this fact.

>
>> +
>> +- #address-cells:
>> +     Usage: required
>> +     Value type: <u32>
>> +     Definition: must be 1
>> +
>> +- #size-cells:
>> +     Usage: required
>> +     Value type: <u32>
>> +     Definition: must be 0
>> +
>> +
>> += SUBDEVICES
>> +
>> +The RPM exposes resources to its subnodes. The below bindings specify the set
>> +of valid subnodes that can operate on these resources.
>> +
>> +== Switch-mode Power Supply regulator
>> +
>> +- compatible:
>> +     Usage: required
>> +     Value type: <string>
>> +     Definition: must be one of:
>> +                 "qcom,rpm-pm8058-smps"
>> +                 "qcom,rpm-pm8901-ftsmps"
>> +                 "qcom,rpm-pm8921-smps"
>> +                 "qcom,rpm-pm8921-ftsmps"
>> +
>> +- reg:
>> +     Usage: required
>> +     Value type: <u32>
>> +     Definition: resource as defined in <dt-bindings/mfd/qcom_rpm.h>
>
> Can we provide a bit more description about what “namespace” this reg is work in.

Based on that I split this up in the different regulator types I
should be able to glob the possible resources here now; I'll give it a
try.

>
>> +
>> +- 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,hpm-threshold:
>> +     Usage: optional
>> +     Value type: <u32>
>> +     Definition: indicates the breaking point at which the regulator should
>> +                 switch to high power mode
>
> in what units?

Thanks

>
>> +
>> +- qcom,load-bias:
>> +     Usage: optional
>> +     Value type: <u32>
>> +     Definition: specifies a base load on the specific regulator
>
> in what units?

Thanks

>
>> +
>> +- qcom,boot-load:
>> +     Usage: optional
>> +     Value type: <u32>
>> +     Definition: specifies the configured load on boot for the specific
>> +                 regulator
>
> in what units?

Thanks

>
>> +
>> +- qcom,force-mode-none:
>> +     Usage: optional (default if no other qcom,force-mode is specified)
>> +     Value type: <empty>
>> +     Defintion: indicates that the regulator should not be forced to any
>> +                particular mode
>> +
>> +- qcom,force-mode-lpm:
>> +     Usage: optional
>> +     Value type: <empty>
>> +     Definition: indicates that the regulator should be forced to operate in
>> +                 low-power-mode
>> +
>> +- qcom,force-mode-auto:
>> +     Usage: optional (only available for 8960/8064)
>> +     Value type: <empty>
>> +     Definition: indicates that the regulator should be automatically pick
>> +                 operating mode
>> +
>> +- qcom,force-mode-hpm:
>> +     Usage: optional (only available for 8960/8064)
>> +     Value type: <empty>
>> +     Definition: indicates that the regulator should be forced to operate in
>> +                 high-power-mode
>> +
>> +- qcom,force-mode-bypass: (only for 8960/8064)
>> +     Usage: optional (only available for 8960/8064)
>> +     Value type: <empty>
>> +     Definition: indicates that the regulator should be forced to operate in
>> +                 bypass mode
>> +
>
> Is force-mode really an enum?  Or can we really have multiple force-mode’s set?
>

Force mode is an enum, you can only set one of these.

Looking around you can find three ways of doing this:
1) Make it an int and provide defines in a dt-bindings header file
2) Make it a string and list the possible values
3) Make it a set of boolean properties

I just spent some time in the pinctrl struff that implements 3), I
like the feel of it and I like how the dts ended up looking like.

But if there's a strong opinion regarding this I could change it.

>> +- qcom,power-mode-hysteretic:
>> +     Usage: optional
>> +     Value type: <empty>
>> +     Definition: indicates that the power supply should operate in hysteretic
>> +                 mode (defaults to qcom,power-mode-pwm if not specified)
>> +
>> +- qcom,power-mode-pwm:
>> +     Usage: optional
>> +     Value type: <empty>
>> +     Definition: indicates that the power supply should operate in pwm mode
>> +
>
> Same question here is power-mode either pwm or hysteretic or can we set both?
>

Same as for force-mode

>> +Standard regulator bindings are used inside switch mode power supply subnodes.
>> +Check Documentation/devicetree/bindings/regulator/regulator.txt for more
>> +details.
>> +

[...]

>> diff --git a/include/dt-bindings/mfd/qcom_rpm.h b/include/dt-bindings/mfd/qcom_rpm.h
>> new file mode 100644
>> index 0000000..277e789
>> --- /dev/null
>> +++ b/include/dt-bindings/mfd/qcom_rpm.h
[...]
>> +#define QCOM_RPM_SYS_FABRIC_CLK                      128
>> +#define QCOM_RPM_SYS_FABRIC_HALT             129
>> +#define QCOM_RPM_SYS_FABRIC_IOCTL            130
>> +#define QCOM_RPM_SYS_FABRIC_MODE             131
>> +#define QCOM_RPM_USB_OTG_SWITCH                      132
>> +#define QCOM_RPM_VDDMIN_GPIO                 133
>
> Are these values arbitrary or do they mean something to hw?

They are just arbitrary values, similar to what you find in e.g. the
clock definitions.

Thanks for the review.

Regards,
Bjorn

Re: [PATCH 1/3] mfd: devicetree: bindings: Add Qualcomm RPM DT binding

[Frank Rowand]

On 5/29/2014 11:24 AM, Bjorn Andersson wrote:
> On Wed, May 28, 2014 at 9:34 AM, Kumar Gala <galak@codeaurora.org> wrote:
>>
>> On May 27, 2014, at 12:28 PM, Bjorn Andersson <bjorn.andersson@sonymobile.com> wrote:
>>
>>> Add binding for the Qualcomm Resource Power Manager (RPM) found in 8660, 8960
>>> and 8064 based devices. The binding currently describes the rpm itself and the
>>> regulator subnodes.
>>>
>>> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
>>> ---
>>> Documentation/devicetree/bindings/mfd/qcom,rpm.txt | 284 +++++++++++++++++++++
>>> include/dt-bindings/mfd/qcom_rpm.h                 | 142 +++++++++++
>>> 2 files changed, 426 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/mfd/qcom,rpm.txt
>>> create mode 100644 include/dt-bindings/mfd/qcom_rpm.h
>>>
>>> diff --git a/Documentation/devicetree/bindings/mfd/qcom,rpm.txt b/Documentation/devicetree/bindings/mfd/qcom,rpm.txt
>>> new file mode 100644
>>> index 0000000..3908a5d
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mfd/qcom,rpm.txt

< snip >

>>> +
>>> +- qcom,force-mode-none:
>>> +     Usage: optional (default if no other qcom,force-mode is specified)
>>> +     Value type: <empty>
>>> +     Defintion: indicates that the regulator should not be forced to any
>>> +                particular mode
>>> +
>>> +- qcom,force-mode-lpm:
>>> +     Usage: optional
>>> +     Value type: <empty>
>>> +     Definition: indicates that the regulator should be forced to operate in
>>> +                 low-power-mode
>>> +
>>> +- qcom,force-mode-auto:
>>> +     Usage: optional (only available for 8960/8064)
>>> +     Value type: <empty>
>>> +     Definition: indicates that the regulator should be automatically pick
>>> +                 operating mode
>>> +
>>> +- qcom,force-mode-hpm:
>>> +     Usage: optional (only available for 8960/8064)
>>> +     Value type: <empty>
>>> +     Definition: indicates that the regulator should be forced to operate in
>>> +                 high-power-mode
>>> +
>>> +- qcom,force-mode-bypass: (only for 8960/8064)
>>> +     Usage: optional (only available for 8960/8064)
>>> +     Value type: <empty>
>>> +     Definition: indicates that the regulator should be forced to operate in
>>> +                 bypass mode
>>> +
>>
>> Is force-mode really an enum?  Or can we really have multiple force-mode’s set?
>>
> 
> Force mode is an enum, you can only set one of these.
> 
> Looking around you can find three ways of doing this:
> 1) Make it an int and provide defines in a dt-bindings header file
> 2) Make it a string and list the possible values
> 3) Make it a set of boolean properties
> 
> I just spent some time in the pinctrl struff that implements 3), I
> like the feel of it and I like how the dts ended up looking like.
> 
> But if there's a strong opinion regarding this I could change it.

The problem with separate properties is that there is not a way to
detect the error of specifying multiple conflicting force-modes
at compile time.

It also means you can not override the default force-mode from one
.dts/.dtsi in another .dts/.dtsi.

< snip >

-Frank

Re: [PATCH 1/3] mfd: devicetree: bindings: Add Qualcomm RPM DT binding

[Srinivas Kandagatla]

Hi Bjorn,

On 27/05/14 18:28, Bjorn Andersson wrote:
> Add binding for the Qualcomm Resource Power Manager (RPM) found in 8660, 8960
> and 8064 based devices. The binding currently describes the rpm itself and the
> regulator subnodes.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> ---
>   Documentation/devicetree/bindings/mfd/qcom,rpm.txt | 284 +++++++++++++++++++++
>   include/dt-bindings/mfd/qcom_rpm.h                 | 142 +++++++++++
>   2 files changed, 426 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/mfd/qcom,rpm.txt
>   create mode 100644 include/dt-bindings/mfd/qcom_rpm.h
>
> diff --git a/Documentation/devicetree/bindings/mfd/qcom,rpm.txt b/Documentation/devicetree/bindings/mfd/qcom,rpm.txt
> new file mode 100644
> index 0000000..3908a5d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/qcom,rpm.txt
> @@ -0,0 +1,284 @@
> +Qualcomm Resource Power Manager (RPM)
> +
> +This driver is used to interface with the Resource Power Manager (RPM) found in
> +various Qualcomm platforms. The RPM allows each component in the system to vote
> +for state of the system resources, such as clocks, regulators and bus
> +frequencies.
> +
> +- compatible:
> +	Usage: required
> +	Value type: <string>
> +	Definition: must be one of:
> +		    "qcom,rpm-apq8064"
> +		    "qcom,rpm-msm8660"
> +		    "qcom,rpm-msm8960"
> +
> +- reg:
> +	Usage: required
> +	Value type: <prop-encoded-array>
> +	Definition: two entries specifying the RPM's message ram and ipc register
> +
> +- reg-names:
> +	Usage: required
> +	Value type: <string-array>
> +	Definition: must contain the following, in order:
> +		    "msg_ram"
> +		    "ipc"

+1 for kumar's comment.

cant enforce the order here. should fix it in the driver.

> +
> +- interrupts:
> +	Usage: required
> +	Value type: <prop-encoded-array>
> +	Definition: three entries specifying the RPM's:
> +		    1. acknowledgement interrupt
> +		    2. error interrupt
> +		    3. wakeup interrupt
> +
> +- interrupt-names:
> +	Usage: required
> +	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
> +
> +
> += SUBDEVICES
> +
> +The RPM exposes resources to its subnodes. The below bindings specify the set
> +of valid subnodes that can operate on these resources.

Why should these devices be on sub nodes?

Any reason not to implement it like this,

rpm: rpm@108000 {
	compatible = "qcom,rpm-msm8960";
	reg = <0x108000 0x1000 0x2011008 0x4>;

	interrupts = <0 19 0>, <0 21 0>, <0 22 0>;
	interrupt-names = "ack", "err", "wakeup";
};

pm8921_s1: pm8921-s1 {
	compatible = "qcom,rpm-pm8921-smps";
	
	regulator-min-microvolt = <1225000>;
	regulator-max-microvolt = <1225000>;
	regulator-always-on;

	qcom,rpm = <&rpm QCOM_RPM_PM8921_S1>;
	qcom,switch-mode-frequency = <3200000>;
	qcom,hpm-threshold = <100000>;
};

This would simplify the driver code too and handle the interface neatly 
then depending on device hierarchy.
rpm would be a interface library to the clients. Makes the drivers more 
independent, and re-usable if we do this way.

??


> +
> +== Switch-mode Power Supply regulator
> +
> +- compatible:
> +	Usage: required
> +	Value type: <string>
> +	Definition: must be one of:
> +		    "qcom,rpm-pm8058-smps"
> +		    "qcom,rpm-pm8901-ftsmps"
> +		    "qcom,rpm-pm8921-smps"
> +		    "qcom,rpm-pm8921-ftsmps"
> +
> +- reg:
> +	Usage: required
> +	Value type: <u32>
> +	Definition: resource as defined in <dt-bindings/mfd/qcom_rpm.h>
> +
> +- 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,hpm-threshold:
> +	Usage: optional
> +	Value type: <u32>
> +	Definition: indicates the breaking point at which the regulator should
> +	            switch to high power mode
> +
> +- qcom,load-bias:
> +	Usage: optional
> +	Value type: <u32>
> +	Definition: specifies a base load on the specific regulator
> +
> +- qcom,boot-load:
> +	Usage: optional
> +	Value type: <u32>
> +	Definition: specifies the configured load on boot for the specific
> +	            regulator
> +

[...
> +- qcom,force-mode-none:
> +	Usage: optional (default if no other qcom,force-mode is specified)
> +	Value type: <empty>
> +	Defintion: indicates that the regulator should not be forced to any
> +	           particular mode
> +
> +- qcom,force-mode-lpm:
> +	Usage: optional
> +	Value type: <empty>
> +	Definition: indicates that the regulator should be forced to operate in
> +	            low-power-mode
> +
> +- qcom,force-mode-auto:
> +	Usage: optional (only available for 8960/8064)
> +	Value type: <empty>
> +	Definition: indicates that the regulator should be automatically pick
> +	            operating mode
> +
> +- qcom,force-mode-hpm:
> +	Usage: optional (only available for 8960/8064)
> +	Value type: <empty>
> +	Definition: indicates that the regulator should be forced to operate in
> +	            high-power-mode
> +
> +- qcom,force-mode-bypass: (only for 8960/8064)
> +	Usage: optional (only available for 8960/8064)
> +	Value type: <empty>
> +	Definition: indicates that the regulator should be forced to operate in
> +	            bypass mode
> +
...]

Probably qcom,force-mode:
	Usage: optional.
	Value type: <string>
	Definition: must be one of:
	"none"
	"lpm"
	"auto"
	"hpm"
	"bypass"

Makes it much simpler, as they seems to be mutually exclusive. simillar 
comments apply to other bindings too.


[...
> +- qcom,power-mode-hysteretic:
> +	Usage: optional
> +	Value type: <empty>
> +	Definition: indicates that the power supply should operate in hysteretic
> +		    mode (defaults to qcom,power-mode-pwm if not specified)
> +
> +- qcom,power-mode-pwm:
> +	Usage: optional
> +	Value type: <empty>
> +	Definition: indicates that the power supply should operate in pwm mode
> +
...]

Probably qcom,power-mode:
	Usage: optional.
	Value type: <string>
	Definition: must be one of:
	"pwm"
	"hysteretic"


Makes it much simpler, as they seems to be mutually exclusive. simillar 
comments apply to other bindings too.




Thanks,
srini
> +Standard regulator bindings are used inside switch mode power supply subnodes.
> +Check Documentation/devicetree/bindings/regulator/regulator.txt for more
> +details.
> +
> +== Low-dropout regulator
> +
> +- compatible:
> +	Usage: required
> +	Value type: <string>
> +	Definition: must be one of:
> +		    "qcom,rpm-pm8058-pldo"
> +		    "qcom,rpm-pm8058-nldo"
> +		    "qcom,rpm-pm8901-pldo"
> +		    "qcom,rpm-pm8901-nldo"
> +		    "qcom,rpm-pm8921-pldo"
> +		    "qcom,rpm-pm8921-nldo"
> +		    "qcom,rpm-pm8921-nldo1200"
> +
> +- reg:
> +	Usage: required
> +	Value type: <u32>
> +	Definition: resource as defined in <dt-bindings/mfd/qcom_rpm.h>
> +
> +- qcom,hpm-threshold:
> +	Usage: optional
> +	Value type: <u32>
> +	Definition: indicates the breaking point at which the regulator should
> +	            switch to high power mode
> +
> +- qcom,load-bias:
> +	Usage: optional
> +	Value type: <u32>
> +	Definition: specifies a base load on the specific regulator
> +
> +- qcom,boot-load:
> +	Usage: optional
> +	Value type: <u32>
> +	Definition: specifies the configured load on boot for the specific
> +	            regulator
> +
> +- qcom,force-mode-none:
> +	Usage: optional (default if no other qcom,force-mode is specified)
> +	Value type: <empty>
> +	Defintion: indicates that the regulator should not be forced to any
> +	           particular mode
> +
> +- qcom,force-mode-lpm:
> +	Usage: optional
> +	Value type: <empty>
> +	Definition: indicates that the regulator should be forced to operate in
> +	            low-power-mode
> +
> +- qcom,force-mode-auto:
> +	Usage: optional (only available for 8960/8064)
> +	Value type: <empty>
> +	Definition: indicates that the regulator should be automatically pick
> +	            operating mode
> +
> +- qcom,force-mode-hpm:
> +	Usage: optional (only available for 8960/8064)
> +	Value type: <empty>
> +	Definition: indicates that the regulator should be forced to operate in
> +	            high-power-mode
> +
> +- qcom,force-mode-bypass: (only for 8960/8064)
> +	Usage: optional (only available for 8960/8064)
> +	Value type: <empty>
> +	Definition: indicates that the regulator should be forced to operate in
> +	            bypass mode
> +
> +Standard regulator bindings are used inside switch low-dropout regulator
> +subnodes.  Check Documentation/devicetree/bindings/regulator/regulator.txt for
> +more details.
> +
> +== Negative Charge Pump
> +
> +- compatible:
> +	Usage: required
> +	Value type: <string>
> +	Definition: must be one of:
> +		    "qcom,rpm-pm8058-ncp"
> +		    "qcom,rpm-pm8921-ncp"
> +		    "qcom,rpm-pm8921-nldo1200"
> +
> +- reg:
> +	Usage: required
> +	Value type: <u32>
> +	Definition: resource as defined in <dt-bindings/mfd/qcom_rpm.h>
> +
> +- 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
> +
> +Standard regulator bindings are used inside negative charge pump regulator
> +subnodes.  Check Documentation/devicetree/bindings/regulator/regulator.txt for
> +more details.
> +
> +== Switch
> +
> +- compatible:
> +	Usage: required
> +	Value type: <string>
> +	Definition: must be one of:
> +		    "qcom,rpm-pm8058-switch"
> +		    "qcom,rpm-pm8901-switch"
> +		    "qcom,rpm-pm8921-switch"
> +
> +- reg:
> +	Usage: required
> +	Value type: <u32>
> +	Definition: resource as defined in <dt-bindings/mfd/qcom_rpm.h>
> +
> +
> += EXAMPLE
> +
> +	#include <dt-bindings/mfd/qcom_rpm.h>
> +
> +	rpm@108000 {
> +		compatible = "qcom,rpm-msm8960";
> +		reg = <0x108000 0x1000 0x2011008 0x4>;
> +
> +		interrupts = <0 19 0>, <0 21 0>, <0 22 0>;
> +		interrupt-names = "ack", "err", "wakeup";
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		pm8921_s1: pm8921-s1 {
> +			compatible = "qcom,rpm-pm8921-smps";
> +			reg = <QCOM_RPM_PM8921_S1>;
> +
> +			regulator-min-microvolt = <1225000>;
> +			regulator-max-microvolt = <1225000>;
> +			regulator-always-on;
> +
> +			qcom,switch-mode-frequency = <3200000>;
> +			qcom,hpm-threshold = <100000>;
> +		};
> +	};

...

Re: [PATCH 1/3] mfd: devicetree: bindings: Add Qualcomm RPM DT binding

[Kumar Gala]

On May 29, 2014, at 11:19 AM, Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:

>> += SUBDEVICES
>> +
>> +The RPM exposes resources to its subnodes. The below bindings specify the set
>> +of valid subnodes that can operate on these resources.
> 
> Why should these devices be on sub nodes?
> 
> Any reason not to implement it like this,
> 
> rpm: rpm@108000 {
> 	compatible = "qcom,rpm-msm8960";
> 	reg = <0x108000 0x1000 0x2011008 0x4>;
> 
> 	interrupts = <0 19 0>, <0 21 0>, <0 22 0>;
> 	interrupt-names = "ack", "err", "wakeup";
> };
> 
> pm8921_s1: pm8921-s1 {
> 	compatible = "qcom,rpm-pm8921-smps";
> 	
> 	regulator-min-microvolt = <1225000>;
> 	regulator-max-microvolt = <1225000>;
> 	regulator-always-on;
> 
> 	qcom,rpm = <&rpm QCOM_RPM_PM8921_S1>;
> 	qcom,switch-mode-frequency = <3200000>;
> 	qcom,hpm-threshold = <100000>;
> };
> 
> This would simplify the driver code too and handle the interface neatly then depending on device hierarchy.
> rpm would be a interface library to the clients. Makes the drivers more independent, and re-usable if we do this way.
> 
> ??

One reason to go with sub nodes is it creates a proper driver ordering dependency as I assume rpm driver will end up calling of_platform_populate for the sub nodes at the point that the RPM driver is ready.  We could do this with deferred probe but doing it explicitly is better in my opinion as it limits the amount of time between when RPM is ready vs when the children can start doing things

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 1/3] mfd: devicetree: bindings: Add Qualcomm RPM DT binding

[Srinivas Kandagatla]

On 29/05/14 17:30, Kumar Gala wrote:
>
> On May 29, 2014, at 11:19 AM, Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:
>
>>> += SUBDEVICES
>>> +
>>> +The RPM exposes resources to its subnodes. The below bindings specify the set
>>> +of valid subnodes that can operate on these resources.
>>
>> Why should these devices be on sub nodes?
>>
>> Any reason not to implement it like this,
>>
>> rpm: rpm@108000 {
>> 	compatible = "qcom,rpm-msm8960";
>> 	reg = <0x108000 0x1000 0x2011008 0x4>;
>>
>> 	interrupts = <0 19 0>, <0 21 0>, <0 22 0>;
>> 	interrupt-names = "ack", "err", "wakeup";
>> };
>>
>> pm8921_s1: pm8921-s1 {
>> 	compatible = "qcom,rpm-pm8921-smps";
>> 	
>> 	regulator-min-microvolt = <1225000>;
>> 	regulator-max-microvolt = <1225000>;
>> 	regulator-always-on;
>>
>> 	qcom,rpm = <&rpm QCOM_RPM_PM8921_S1>;
>> 	qcom,switch-mode-frequency = <3200000>;
>> 	qcom,hpm-threshold = <100000>;
>> };
>>
>> This would simplify the driver code too and handle the interface neatly then depending on device hierarchy.
>> rpm would be a interface library to the clients. Makes the drivers more independent, and re-usable if we do this way.
>>
>> ??
>
> One reason to go with sub nodes is it creates a proper driver ordering dependency as I assume rpm driver will end up calling of_platform_populate for the sub nodes at the point that the RPM driver is ready.  We could do this with deferred probe but doing it explicitly is better in my opinion as it limits the amount of time between when RPM is ready vs when the children can start doing things
>

I agree, there might be a win. But Am not sure to what extent this win 
is a win to rpm driver, as a side effect this brings other 
responsibilities to the rpm driver like adding sub-device power 
management awareness, device life cycle management to the rpm driver.

Only thing which made me think of this approach is the number of sub 
nodes it would end up with and passing ID using reg property.

Thanks,
srini

> - k
>

Re: [PATCH 1/3] mfd: devicetree: bindings: Add Qualcomm RPM DT binding

[Bjorn Andersson]

On Thu, May 29, 2014 at 9:19 AM, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
>> +- reg:
>> +       Usage: required
>> +       Value type: <prop-encoded-array>
>> +       Definition: two entries specifying the RPM's message ram and ipc
>> register
>> +
>> +- reg-names:
>> +       Usage: required
>> +       Value type: <string-array>
>> +       Definition: must contain the following, in order:
>> +                   "msg_ram"
>> +                   "ipc"
>
>
> +1 for kumar's comment.
>
> cant enforce the order here. should fix it in the driver.
>

Yes I can, this is as decided by the devicetree maintainers. The order
of e.g. reg and interrupts must be defined.

>> += SUBDEVICES
>> +
>> +The RPM exposes resources to its subnodes. The below bindings specify the
>> set
>> +of valid subnodes that can operate on these resources.
>
>
> Why should these devices be on sub nodes?
>
> Any reason not to implement it like this,
>
> rpm: rpm@108000 {
>         compatible = "qcom,rpm-msm8960";
>
>         reg = <0x108000 0x1000 0x2011008 0x4>;
>
>         interrupts = <0 19 0>, <0 21 0>, <0 22 0>;
>         interrupt-names = "ack", "err", "wakeup";
> };
>
> pm8921_s1: pm8921-s1 {
>         compatible = "qcom,rpm-pm8921-smps";
>
>         regulator-min-microvolt = <1225000>;
>         regulator-max-microvolt = <1225000>;
>         regulator-always-on;
>
>         qcom,rpm = <&rpm QCOM_RPM_PM8921_S1>;
>         qcom,switch-mode-frequency = <3200000>;
>         qcom,hpm-threshold = <100000>;
> };
>
> This would simplify the driver code too and handle the interface neatly then
> depending on device hierarchy.
> rpm would be a interface library to the clients. Makes the drivers more
> independent, and re-usable if we do this way.

The subnodes doesn't describe separate pieces of hardware but rather
pieces of the rpm, that's why they should live inside the rpm. There
will not be any re-use of these drivers outside having a rpm as
parent.

I do have some patches for family b, where I'm moving things around a
little bit in the implementation to be able to re-use child-devices
where that makes sense (clock implementation is the same for the two).
But that is implementation specific and does not affect the dt.


Implementation wise, having the individual subnodes as children in the
device model makes a lot of sense, as the children should be probed
when the rpm appears and when the rpm goes away it should bring down
all subnodes. If there was any power management it would be the same
thing.

So I think this makes for a cleaner implementation; all I need to do
is to call of_platform_populate at the end of the probe and in remove
I need to tell the children that they should go away. I do not need to
support any phandle based lookups and separate life cycle management.

>
> [...
>
>> +- qcom,force-mode-none:
>> +       Usage: optional (default if no other qcom,force-mode is specified)
>> +       Value type: <empty>
>> +       Defintion: indicates that the regulator should not be forced to
>> any
>> +                  particular mode
>> +
>> +- qcom,force-mode-lpm:
>> +       Usage: optional
>> +       Value type: <empty>
>> +       Definition: indicates that the regulator should be forced to
>> operate in
>> +                   low-power-mode
>> +
>> +- qcom,force-mode-auto:
>> +       Usage: optional (only available for 8960/8064)
>> +       Value type: <empty>
>> +       Definition: indicates that the regulator should be automatically
>> pick
>> +                   operating mode
>> +
>> +- qcom,force-mode-hpm:
>> +       Usage: optional (only available for 8960/8064)
>> +       Value type: <empty>
>> +       Definition: indicates that the regulator should be forced to
>> operate in
>> +                   high-power-mode
>> +
>> +- qcom,force-mode-bypass: (only for 8960/8064)
>> +       Usage: optional (only available for 8960/8064)
>> +       Value type: <empty>
>> +       Definition: indicates that the regulator should be forced to
>> operate in
>> +                   bypass mode
>> +
>
> ...]
>
> Probably qcom,force-mode:
>         Usage: optional.
>         Value type: <string>
>
>         Definition: must be one of:
>         "none"
>         "lpm"
>         "auto"
>         "hpm"
>         "bypass"
>
> Makes it much simpler, as they seems to be mutually exclusive. simillar
> comments apply to other bindings too.

Please see my answer to Kumar.


Thanks for the comments!

Regards,
Bjorn

Re: [PATCH 1/3] mfd: devicetree: bindings: Add Qualcomm RPM DT binding

[Srinivas Kandagatla]

On 29/05/14 19:38, Bjorn Andersson wrote:
> On Thu, May 29, 2014 at 9:19 AM, Srinivas Kandagatla
> <srinivas.kandagatla@linaro.org> wrote:
>>> +- reg:
>>> +       Usage: required
>>> +       Value type: <prop-encoded-array>
>>> +       Definition: two entries specifying the RPM's message ram and ipc
>>> register
>>> +
>>> +- reg-names:
>>> +       Usage: required
>>> +       Value type: <string-array>
>>> +       Definition: must contain the following, in order:
>>> +                   "msg_ram"
>>> +                   "ipc"
>>
>>
>> +1 for kumar's comment.
>>
>> cant enforce the order here. should fix it in the driver.
>>
>
> Yes I can, this is as decided by the devicetree maintainers. The order
> of e.g. reg and interrupts must be defined.
>
Does not make sense. Unless Am missing something obvious.
Having reg-names/interrupt-names would give driver flexibility to get 
the resources by name, as long as the order of reg and reg-names match.

So the order of reg is really not really necessary. Unless the driver is 
coded to access it via index.

Hardly 1/2 bindings documents enforce this.


>>> += SUBDEVICES
>>> +
>>> +The RPM exposes resources to its subnodes. The below bindings specify the
>>> set
>>> +of valid subnodes that can operate on these resources.
>>
>>
>> Why should these devices be on sub nodes?
>>
>> Any reason not to implement it like this,
>>
>> rpm: rpm@108000 {
>>          compatible = "qcom,rpm-msm8960";
>>
>>          reg = <0x108000 0x1000 0x2011008 0x4>;
>>
>>          interrupts = <0 19 0>, <0 21 0>, <0 22 0>;
>>          interrupt-names = "ack", "err", "wakeup";
>> };
>>
>> pm8921_s1: pm8921-s1 {
>>          compatible = "qcom,rpm-pm8921-smps";
>>
>>          regulator-min-microvolt = <1225000>;
>>          regulator-max-microvolt = <1225000>;
>>          regulator-always-on;
>>
>>          qcom,rpm = <&rpm QCOM_RPM_PM8921_S1>;
>>          qcom,switch-mode-frequency = <3200000>;
>>          qcom,hpm-threshold = <100000>;
>> };
>>
>> This would simplify the driver code too and handle the interface neatly then
>> depending on device hierarchy.
>> rpm would be a interface library to the clients. Makes the drivers more
>> independent, and re-usable if we do this way.
>
> The subnodes doesn't describe separate pieces of hardware but rather
> pieces of the rpm, that's why they should live inside the rpm. There
> will not be any re-use of these drivers outside having a rpm as
> parent.
>
> I do have some patches for family b, where I'm moving things around a
> little bit in the implementation to be able to re-use child-devices
> where that makes sense (clock implementation is the same for the two).
> But that is implementation specific and does not affect the dt.
>
Good point, Am more of thinking of some other SOCs might have similar pmic.

>
> Implementation wise, having the individual subnodes as children in the
> device model makes a lot of sense, as the children should be probed
> when the rpm appears and when the rpm goes away it should bring down
> all subnodes. If there was any power management it would be the same
> thing.
Thats great, you have already thought about it.
>
> So I think this makes for a cleaner implementation; all I need to do
> is to call of_platform_populate at the end of the probe and in remove
> I need to tell the children that they should go away. I do not need to
> support any phandle based lookups and separate life cycle management.
>
Am ok with either approaches.

>>
>> [...
>>
>>> +- qcom,force-mode-none:
>>> +       Usage: optional (default if no other qcom,force-mode is specified)
>>> +       Value type: <empty>
>>> +       Defintion: indicates that the regulator should not be forced to
>>> any
>>> +                  particular mode
>>> +
>>> +- qcom,force-mode-lpm:
>>> +       Usage: optional
>>> +       Value type: <empty>
>>> +       Definition: indicates that the regulator should be forced to
>>> operate in
>>> +                   low-power-mode
>>> +
>>> +- qcom,force-mode-auto:
>>> +       Usage: optional (only available for 8960/8064)
>>> +       Value type: <empty>
>>> +       Definition: indicates that the regulator should be automatically
>>> pick
>>> +                   operating mode
>>> +
>>> +- qcom,force-mode-hpm:
>>> +       Usage: optional (only available for 8960/8064)
>>> +       Value type: <empty>
>>> +       Definition: indicates that the regulator should be forced to
>>> operate in
>>> +                   high-power-mode
>>> +
>>> +- qcom,force-mode-bypass: (only for 8960/8064)
>>> +       Usage: optional (only available for 8960/8064)
>>> +       Value type: <empty>
>>> +       Definition: indicates that the regulator should be forced to
>>> operate in
>>> +                   bypass mode
>>> +
>>
>> ...]
>>
>> Probably qcom,force-mode:
>>          Usage: optional.
>>          Value type: <string>
>>
>>          Definition: must be one of:
>>          "none"
>>          "lpm"
>>          "auto"
>>          "hpm"
>>          "bypass"
>>
>> Makes it much simpler, as they seems to be mutually exclusive. simillar
>> comments apply to other bindings too.
>
> Please see my answer to Kumar.
>
Ok. I don’t have a strong feeling on any of those 3 approaches.

Thanks,
srini
>
> Thanks for the comments!
>
> Regards,
> Bjorn
>

Re: [PATCH 1/3] mfd: devicetree: bindings: Add Qualcomm RPM DT binding

[Lee Jones]

> Add binding for the Qualcomm Resource Power Manager (RPM) found in 8660, 8960
> and 8064 based devices. The binding currently describes the rpm itself and the
> regulator subnodes.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> ---
>  Documentation/devicetree/bindings/mfd/qcom,rpm.txt | 284 +++++++++++++++++++++
>  include/dt-bindings/mfd/qcom_rpm.h                 | 142 +++++++++++
>  2 files changed, 426 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/qcom,rpm.txt
>  create mode 100644 include/dt-bindings/mfd/qcom_rpm.h

[...]

> diff --git a/include/dt-bindings/mfd/qcom_rpm.h b/include/dt-bindings/mfd/qcom_rpm.h
> new file mode 100644
> index 0000000..277e789
> --- /dev/null
> +++ b/include/dt-bindings/mfd/qcom_rpm.h
> @@ -0,0 +1,142 @@
> +/*
> + * This header provides constants for the Qualcomm RPM bindings.
> + */

Proper header please.

> +#ifndef _DT_BINDINGS_MFD_QCOM_RPM_H
> +#define _DT_BINDINGS_MFD_QCOM_RPM_H
> +
> +#define QCOM_RPM_APPS_FABRIC_ARB		1
> +#define QCOM_RPM_APPS_FABRIC_CLK		2
> +#define QCOM_RPM_APPS_FABRIC_HALT		3

[...]

> +#define QCOM_RPM_SYS_FABRIC_MODE		131
> +#define QCOM_RPM_USB_OTG_SWITCH		132
> +#define QCOM_RPM_VDDMIN_GPIO			133

Are you sure you don't want these in 0xXY format?

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

Re: [PATCH 1/3] mfd: devicetree: bindings: Add Qualcomm RPM DT binding

[Bjorn Andersson]

On Thu, May 29, 2014 at 9:54 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> diff --git a/include/dt-bindings/mfd/qcom_rpm.h b/include/dt-bindings/mfd/qcom_rpm.h
>> new file mode 100644
>> index 0000000..277e789
>> --- /dev/null
>> +++ b/include/dt-bindings/mfd/qcom_rpm.h
>> @@ -0,0 +1,142 @@
>> +/*
>> + * This header provides constants for the Qualcomm RPM bindings.
>> + */
>
> Proper header please.

Will do.

>
>> +#ifndef _DT_BINDINGS_MFD_QCOM_RPM_H
>> +#define _DT_BINDINGS_MFD_QCOM_RPM_H
>> +
>> +#define QCOM_RPM_APPS_FABRIC_ARB             1
>> +#define QCOM_RPM_APPS_FABRIC_CLK             2
>> +#define QCOM_RPM_APPS_FABRIC_HALT            3
>
> [...]
>
>> +#define QCOM_RPM_SYS_FABRIC_MODE             131
>> +#define QCOM_RPM_USB_OTG_SWITCH              132
>> +#define QCOM_RPM_VDDMIN_GPIO                 133
>
> Are you sure you don't want these in 0xXY format?

This is just a list of virtual identifiers, so unless you object to
much I'll leave them in base 10.

Thanks,
Bjorn

[PATCH 2/3] mfd: qcom-rpm: Driver for the Qualcomm RPM

[Bjorn Andersson]

Driver for the Resource Power Manager (RPM) found in Qualcomm 8660, 8960 and
8064 based devices. The driver exposes resources that child drivers can operate
on; to implementing regulator, clock and bus frequency drivers.

Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---
 drivers/mfd/Kconfig          |  15 ++
 drivers/mfd/Makefile         |   1 +
 drivers/mfd/qcom_rpm.c       | 575 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/qcom_rpm.h |  12 +
 4 files changed, 603 insertions(+)
 create mode 100644 drivers/mfd/qcom_rpm.c
 create mode 100644 include/linux/mfd/qcom_rpm.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 3383412..e5122a7 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -482,6 +482,21 @@ config UCB1400_CORE
 	  To compile this driver as a module, choose M here: the
 	  module will be called ucb1400_core.
 
+config MFD_QCOM_RPM
+	tristate "Qualcomm Resource Power Manager (RPM)"
+	depends on ARCH_QCOM && OF
+	select MFD_CORE
+	help
+	  If you say yes to this option, support will be included for the
+	  Resource Power Manager system found in the Qualcomm 8660, 8960 and
+	  8064 based devices.
+
+	  This is required to access many regulators, clocks and bus
+	  frequencies controlled by the RPM on these devices.
+
+	  Say M here if you want to include support for the Qualcomm RPM as a
+	  module. This will build a module called "qcom_rpm".
+
 config MFD_PM8XXX
 	tristate
 
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 2851275..bbe4209 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -150,6 +150,7 @@ obj-$(CONFIG_MFD_SI476X_CORE)	+= si476x-core.o
 
 obj-$(CONFIG_MFD_CS5535)	+= cs5535-mfd.o
 obj-$(CONFIG_MFD_OMAP_USB_HOST)	+= omap-usb-host.o omap-usb-tll.o
+obj-$(CONFIG_MFD_QCOM_RPM)	+= qcom_rpm.o
 obj-$(CONFIG_MFD_PM8921_CORE) 	+= pm8921-core.o ssbi.o
 obj-$(CONFIG_TPS65911_COMPARATOR)	+= tps65911-comparator.o
 obj-$(CONFIG_MFD_TPS65090)	+= tps65090.o
diff --git a/drivers/mfd/qcom_rpm.c b/drivers/mfd/qcom_rpm.c
new file mode 100644
index 0000000..96135ab
--- /dev/null
+++ b/drivers/mfd/qcom_rpm.c
@@ -0,0 +1,575 @@
+/*
+ * Copyright (c) 2014, Sony Mobile Communications AB.
+ * Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of_platform.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+
+#include <linux/mfd/qcom_rpm.h>
+
+#include <dt-bindings/mfd/qcom_rpm.h>
+
+struct qcom_rpm_resource {
+	unsigned target_id;
+	unsigned status_id;
+	unsigned select_id;
+	unsigned size;
+};
+
+struct qcom_rpm {
+	struct device *dev;
+	struct completion ack;
+	struct mutex lock;
+
+	void __iomem *status_regs;
+	void __iomem *ctrl_regs;
+	void __iomem *req_regs;
+
+	void __iomem *ipc_rpm_reg;
+
+	u32 ack_status;
+
+	u32 version;
+
+	const struct qcom_rpm_resource *resource_table;
+	unsigned n_resources;
+};
+
+#define RPM_STATUS_REG(rpm, i)	((rpm)->status_regs + (i) * 4)
+#define RPM_CTRL_REG(rpm, i)	((rpm)->ctrl_regs + (i) * 4)
+#define RPM_REQ_REG(rpm, i)	((rpm)->req_regs + (i) * 4)
+
+#define RPM_REQUEST_TIMEOUT	(5 * HZ)
+
+#define RPM_REQUEST_CONTEXT	3
+#define RPM_REQ_SELECT		11
+#define RPM_ACK_CONTEXT		15
+#define RPM_ACK_SELECTOR	23
+#define RPM_SELECT_SIZE		7
+
+#define RPM_ACTIVE_STATE	BIT(0)
+#define RPM_NOTIFICATION	BIT(30)
+#define RPM_REJECTED		BIT(31)
+
+#define RPM_SIGNAL		BIT(2)
+
+static const struct qcom_rpm_resource apq8064_rpm_resource_table[] = {
+	[QCOM_RPM_CXO_CLK] =			{ 25, 9, 5, 1 },
+	[QCOM_RPM_PXO_CLK] =			{ 26, 10, 6, 1 },
+	[QCOM_RPM_APPS_FABRIC_CLK] =		{ 27, 11, 8, 1 },
+	[QCOM_RPM_SYS_FABRIC_CLK] =		{ 28, 12, 9, 1 },
+	[QCOM_RPM_MM_FABRIC_CLK] =		{ 29, 13, 10, 1 },
+	[QCOM_RPM_DAYTONA_FABRIC_CLK] =		{ 30, 14, 11, 1 },
+	[QCOM_RPM_SFPB_CLK] =			{ 31, 15, 12, 1 },
+	[QCOM_RPM_CFPB_CLK] =			{ 32, 16, 13, 1 },
+	[QCOM_RPM_MMFPB_CLK] =			{ 33, 17, 14, 1 },
+	[QCOM_RPM_EBI1_CLK] =			{ 34, 18, 16, 1 },
+	[QCOM_RPM_APPS_FABRIC_HALT] =		{ 35, 19, 18, 1 },
+	[QCOM_RPM_APPS_FABRIC_MODE] =		{ 37, 20, 19, 1 },
+	[QCOM_RPM_APPS_FABRIC_IOCTL] =		{ 40, 21, 20, 1 },
+	[QCOM_RPM_APPS_FABRIC_ARB] =		{ 41, 22, 21, 12 },
+	[QCOM_RPM_SYS_FABRIC_HALT] =		{ 53, 23, 22, 1 },
+	[QCOM_RPM_SYS_FABRIC_MODE] =		{ 55, 24, 23, 1 },
+	[QCOM_RPM_SYS_FABRIC_IOCTL] =		{ 58, 25, 24, 1 },
+	[QCOM_RPM_SYS_FABRIC_ARB] =		{ 59, 26, 25, 30 },
+	[QCOM_RPM_MM_FABRIC_HALT] =		{ 89, 27, 26, 1 },
+	[QCOM_RPM_MM_FABRIC_MODE] =		{ 91, 28, 27, 1 },
+	[QCOM_RPM_MM_FABRIC_IOCTL] =		{ 94, 29, 28, 1 },
+	[QCOM_RPM_MM_FABRIC_ARB] =		{ 95, 30, 29, 21 },
+	[QCOM_RPM_PM8921_S1] =			{ 116, 31, 30, 2 },
+	[QCOM_RPM_PM8921_S2] =			{ 118, 33, 31, 2 },
+	[QCOM_RPM_PM8921_S3] =			{ 120, 35, 32, 2 },
+	[QCOM_RPM_PM8921_S4] =			{ 122, 37, 33, 2 },
+	[QCOM_RPM_PM8921_S5] =			{ 124, 39, 34, 2 },
+	[QCOM_RPM_PM8921_S6] =			{ 126, 41, 35, 2 },
+	[QCOM_RPM_PM8921_S7] =			{ 128, 43, 36, 2 },
+	[QCOM_RPM_PM8921_S8] =			{ 130, 45, 37, 2 },
+	[QCOM_RPM_PM8921_L1] =			{ 132, 47, 38, 2 },
+	[QCOM_RPM_PM8921_L2] =			{ 134, 49, 39, 2 },
+	[QCOM_RPM_PM8921_L3] =			{ 136, 51, 40, 2 },
+	[QCOM_RPM_PM8921_L4] =			{ 138, 53, 41, 2 },
+	[QCOM_RPM_PM8921_L5] =			{ 140, 55, 42, 2 },
+	[QCOM_RPM_PM8921_L6] =			{ 142, 57, 43, 2 },
+	[QCOM_RPM_PM8921_L7] =			{ 144, 59, 44, 2 },
+	[QCOM_RPM_PM8921_L8] =			{ 146, 61, 45, 2 },
+	[QCOM_RPM_PM8921_L9] =			{ 148, 63, 46, 2 },
+	[QCOM_RPM_PM8921_L10] =			{ 150, 65, 47, 2 },
+	[QCOM_RPM_PM8921_L11] =			{ 152, 67, 48, 2 },
+	[QCOM_RPM_PM8921_L12] =			{ 154, 69, 49, 2 },
+	[QCOM_RPM_PM8921_L13] =			{ 156, 71, 50, 2 },
+	[QCOM_RPM_PM8921_L14] =			{ 158, 73, 51, 2 },
+	[QCOM_RPM_PM8921_L15] =			{ 160, 75, 52, 2 },
+	[QCOM_RPM_PM8921_L16] =			{ 162, 77, 53, 2 },
+	[QCOM_RPM_PM8921_L17] =			{ 164, 79, 54, 2 },
+	[QCOM_RPM_PM8921_L18] =			{ 166, 81, 55, 2 },
+	[QCOM_RPM_PM8921_L19] =			{ 168, 83, 56, 2 },
+	[QCOM_RPM_PM8921_L20] =			{ 170, 85, 57, 2 },
+	[QCOM_RPM_PM8921_L21] =			{ 172, 87, 58, 2 },
+	[QCOM_RPM_PM8921_L22] =			{ 174, 89, 59, 2 },
+	[QCOM_RPM_PM8921_L23] =			{ 176, 91, 60, 2 },
+	[QCOM_RPM_PM8921_L24] =			{ 178, 93, 61, 2 },
+	[QCOM_RPM_PM8921_L25] =			{ 180, 95, 62, 2 },
+	[QCOM_RPM_PM8921_L26] =			{ 182, 97, 63, 2 },
+	[QCOM_RPM_PM8921_L27] =			{ 184, 99, 64, 2 },
+	[QCOM_RPM_PM8921_L28] =			{ 186, 101, 65, 2 },
+	[QCOM_RPM_PM8921_L29] =			{ 188, 103, 66, 2 },
+	[QCOM_RPM_PM8921_CLK1] =		{ 190, 105, 67, 2 },
+	[QCOM_RPM_PM8921_CLK2] =		{ 192, 107, 68, 2 },
+	[QCOM_RPM_PM8921_LVS1] =		{ 194, 109, 69, 1 },
+	[QCOM_RPM_PM8921_LVS2] =		{ 195, 110, 70, 1 },
+	[QCOM_RPM_PM8921_LVS3] =		{ 196, 111, 71, 1 },
+	[QCOM_RPM_PM8921_LVS4] =		{ 197, 112, 72, 1 },
+	[QCOM_RPM_PM8921_LVS5] =		{ 198, 113, 73, 1 },
+	[QCOM_RPM_PM8921_LVS6] =		{ 199, 114, 74, 1 },
+	[QCOM_RPM_PM8921_LVS7] =		{ 200, 115, 75, 1 },
+	[QCOM_RPM_PM8821_S1] =			{ 201, 116, 76, 2 },
+	[QCOM_RPM_PM8821_S2] =			{ 203, 118, 77, 2 },
+	[QCOM_RPM_PM8821_L1] =			{ 205, 120, 78, 2 },
+	[QCOM_RPM_PM8921_NCP] =			{ 207, 122, 80, 2 },
+	[QCOM_RPM_CXO_BUFFERS] =		{ 209, 124, 81, 1 },
+	[QCOM_RPM_USB_OTG_SWITCH] =		{ 210, 125, 82, 1 },
+	[QCOM_RPM_HDMI_SWITCH] =		{ 211, 126, 83, 1 },
+	[QCOM_RPM_DDR_DMM] =			{ 212, 127, 84, 2 },
+	[QCOM_RPM_VDDMIN_GPIO] =		{ 215, 131, 89, 1 },
+};
+
+static const struct qcom_rpm apq8064_template = {
+	.version = 3,
+	.resource_table = apq8064_rpm_resource_table,
+	.n_resources = ARRAY_SIZE(apq8064_rpm_resource_table),
+};
+
+static const struct qcom_rpm_resource msm8660_rpm_resource_table[] = {
+	[QCOM_RPM_CXO_CLK] =			{ 32, 12, 5, 1 },
+	[QCOM_RPM_PXO_CLK] =			{ 33, 13, 6, 1 },
+	[QCOM_RPM_PLL_4] =			{ 34, 14, 7, 1 },
+	[QCOM_RPM_APPS_FABRIC_CLK] =		{ 35, 15, 8, 1 },
+	[QCOM_RPM_SYS_FABRIC_CLK] =		{ 36, 16, 9, 1 },
+	[QCOM_RPM_MM_FABRIC_CLK] =		{ 37, 17, 10, 1 },
+	[QCOM_RPM_DAYTONA_FABRIC_CLK] =		{ 38, 18, 11, 1 },
+	[QCOM_RPM_SFPB_CLK] =			{ 39, 19, 12, 1 },
+	[QCOM_RPM_CFPB_CLK] =			{ 40, 20, 13, 1 },
+	[QCOM_RPM_MMFPB_CLK] =			{ 41, 21, 14, 1 },
+	[QCOM_RPM_SMI_CLK] =			{ 42, 22, 15, 1 },
+	[QCOM_RPM_EBI1_CLK] =			{ 43, 23, 16, 1 },
+	[QCOM_RPM_APPS_L2_CACHE_CTL] =		{ 44, 24, 17, 1 },
+	[QCOM_RPM_APPS_FABRIC_HALT] =		{ 45, 25, 18, 2 },
+	[QCOM_RPM_APPS_FABRIC_MODE] =		{ 47, 26, 19, 3 },
+	[QCOM_RPM_APPS_FABRIC_ARB] =		{ 51, 28, 21, 6 },
+	[QCOM_RPM_SYS_FABRIC_HALT] =		{ 63, 29, 22, 2 },
+	[QCOM_RPM_SYS_FABRIC_MODE] =		{ 65, 30, 23, 3 },
+	[QCOM_RPM_SYS_FABRIC_ARB] =		{ 69, 32, 25, 22 },
+	[QCOM_RPM_MM_FABRIC_HALT] =		{ 105, 33, 26, 2 },
+	[QCOM_RPM_MM_FABRIC_MODE] =		{ 107, 34, 27, 3 },
+	[QCOM_RPM_MM_FABRIC_ARB] =		{ 111, 36, 29, 23 },
+	[QCOM_RPM_PM8901_SMPS0] =		{ 134, 37, 30, 2 },
+	[QCOM_RPM_PM8901_SMPS1] =		{ 136, 39, 31, 2 },
+	[QCOM_RPM_PM8901_SMPS2] =		{ 138, 41, 32, 2 },
+	[QCOM_RPM_PM8901_SMPS3] =		{ 140, 43, 33, 2 },
+	[QCOM_RPM_PM8901_SMPS4] =		{ 142, 45, 34, 2 },
+	[QCOM_RPM_PM8901_LDO0] =		{ 144, 47, 35, 2 },
+	[QCOM_RPM_PM8901_LDO1] =		{ 146, 49, 36, 2 },
+	[QCOM_RPM_PM8901_LDO2] =		{ 148, 51, 37, 2 },
+	[QCOM_RPM_PM8901_LDO3] =		{ 150, 53, 38, 2 },
+	[QCOM_RPM_PM8901_LDO4] =		{ 152, 55, 39, 2 },
+	[QCOM_RPM_PM8901_LDO5] =		{ 154, 57, 40, 2 },
+	[QCOM_RPM_PM8901_LDO6] =		{ 156, 59, 41, 2 },
+	[QCOM_RPM_PM8901_LVS0] =		{ 158, 61, 42, 1 },
+	[QCOM_RPM_PM8901_LVS1] =		{ 159, 62, 43, 1 },
+	[QCOM_RPM_PM8901_LVS2] =		{ 160, 63, 44, 1 },
+	[QCOM_RPM_PM8901_LVS3] =		{ 161, 64, 45, 1 },
+	[QCOM_RPM_PM8901_MVS] =			{ 162, 65, 46, 1 },
+	[QCOM_RPM_PM8058_SMPS0] =		{ 163, 66, 47, 2 },
+	[QCOM_RPM_PM8058_SMPS1] =		{ 165, 68, 48, 2 },
+	[QCOM_RPM_PM8058_SMPS2] =		{ 167, 70, 49, 2 },
+	[QCOM_RPM_PM8058_SMPS3] =		{ 169, 72, 50, 2 },
+	[QCOM_RPM_PM8058_SMPS4] =		{ 171, 74, 51, 2 },
+	[QCOM_RPM_PM8058_LDO0] =		{ 173, 76, 52, 2 },
+	[QCOM_RPM_PM8058_LDO1] =		{ 175, 78, 53, 2 },
+	[QCOM_RPM_PM8058_LDO2] =		{ 177, 80, 54, 2 },
+	[QCOM_RPM_PM8058_LDO3] =		{ 179, 82, 55, 2 },
+	[QCOM_RPM_PM8058_LDO4] =		{ 181, 84, 56, 2 },
+	[QCOM_RPM_PM8058_LDO5] =		{ 183, 86, 57, 2 },
+	[QCOM_RPM_PM8058_LDO6] =		{ 185, 88, 58, 2 },
+	[QCOM_RPM_PM8058_LDO7] =		{ 187, 90, 59, 2 },
+	[QCOM_RPM_PM8058_LDO8] =		{ 189, 92, 60, 2 },
+	[QCOM_RPM_PM8058_LDO9] =		{ 191, 94, 61, 2 },
+	[QCOM_RPM_PM8058_LDO10] =		{ 193, 96, 62, 2 },
+	[QCOM_RPM_PM8058_LDO11] =		{ 195, 98, 63, 2 },
+	[QCOM_RPM_PM8058_LDO12] =		{ 197, 100, 64, 2 },
+	[QCOM_RPM_PM8058_LDO13] =		{ 199, 102, 65, 2 },
+	[QCOM_RPM_PM8058_LDO14] =		{ 201, 104, 66, 2 },
+	[QCOM_RPM_PM8058_LDO15] =		{ 203, 106, 67, 2 },
+	[QCOM_RPM_PM8058_LDO16] =		{ 205, 108, 68, 2 },
+	[QCOM_RPM_PM8058_LDO17] =		{ 207, 110, 69, 2 },
+	[QCOM_RPM_PM8058_LDO18] =		{ 209, 112, 70, 2 },
+	[QCOM_RPM_PM8058_LDO19] =		{ 211, 114, 71, 2 },
+	[QCOM_RPM_PM8058_LDO20] =		{ 213, 116, 72, 2 },
+	[QCOM_RPM_PM8058_LDO21] =		{ 215, 118, 73, 2 },
+	[QCOM_RPM_PM8058_LDO22] =		{ 217, 120, 74, 2 },
+	[QCOM_RPM_PM8058_LDO23] =		{ 219, 122, 75, 2 },
+	[QCOM_RPM_PM8058_LDO24] =		{ 221, 124, 76, 2 },
+	[QCOM_RPM_PM8058_LDO25] =		{ 223, 126, 77, 2 },
+	[QCOM_RPM_PM8058_LVS0] =		{ 225, 128, 78, 1 },
+	[QCOM_RPM_PM8058_LVS1] =		{ 226, 129, 79, 1 },
+	[QCOM_RPM_PM8058_NCP] =			{ 227, 130, 80, 2 },
+	[QCOM_RPM_CXO_BUFFERS] =		{ 229, 132, 81, 1 },
+};
+
+static const struct qcom_rpm msm8660_template = {
+	.version = -1,
+	.resource_table = msm8660_rpm_resource_table,
+	.n_resources = ARRAY_SIZE(msm8660_rpm_resource_table),
+};
+
+static const struct qcom_rpm_resource msm8960_rpm_resource_table[] = {
+	[QCOM_RPM_CXO_CLK] =			{ 25, 9, 5, 1 },
+	[QCOM_RPM_PXO_CLK] =			{ 26, 10, 6, 1 },
+	[QCOM_RPM_APPS_FABRIC_CLK] =		{ 27, 11, 8, 1 },
+	[QCOM_RPM_SYS_FABRIC_CLK] =		{ 28, 12, 9, 1 },
+	[QCOM_RPM_MM_FABRIC_CLK] =		{ 29, 13, 10, 1 },
+	[QCOM_RPM_DAYTONA_FABRIC_CLK] =		{ 30, 14, 11, 1 },
+	[QCOM_RPM_SFPB_CLK] =			{ 31, 15, 12, 1 },
+	[QCOM_RPM_CFPB_CLK] =			{ 32, 16, 13, 1 },
+	[QCOM_RPM_MMFPB_CLK] =			{ 33, 17, 14, 1 },
+	[QCOM_RPM_EBI1_CLK] =			{ 34, 18, 16, 1 },
+	[QCOM_RPM_APPS_FABRIC_HALT] =		{ 35, 19, 18, 1 },
+	[QCOM_RPM_APPS_FABRIC_MODE] =		{ 37, 20, 19, 1 },
+	[QCOM_RPM_APPS_FABRIC_IOCTL] =		{ 40, 21, 20, 1 },
+	[QCOM_RPM_APPS_FABRIC_ARB] =		{ 41, 22, 21, 12 },
+	[QCOM_RPM_SYS_FABRIC_HALT] =		{ 53, 23, 22, 1 },
+	[QCOM_RPM_SYS_FABRIC_MODE] =		{ 55, 24, 23, 1 },
+	[QCOM_RPM_SYS_FABRIC_IOCTL] =		{ 58, 25, 24, 1 },
+	[QCOM_RPM_SYS_FABRIC_ARB] =		{ 59, 26, 25, 29 },
+	[QCOM_RPM_MM_FABRIC_HALT] =		{ 88, 27, 26, 1 },
+	[QCOM_RPM_MM_FABRIC_MODE] =		{ 90, 28, 27, 1 },
+	[QCOM_RPM_MM_FABRIC_IOCTL] =		{ 93, 29, 28, 1 },
+	[QCOM_RPM_MM_FABRIC_ARB] =		{ 94, 30, 29, 23 },
+	[QCOM_RPM_PM8921_S1] =			{ 117, 31, 30, 2 },
+	[QCOM_RPM_PM8921_S2] =			{ 119, 33, 31, 2 },
+	[QCOM_RPM_PM8921_S3] =			{ 121, 35, 32, 2 },
+	[QCOM_RPM_PM8921_S4] =			{ 123, 37, 33, 2 },
+	[QCOM_RPM_PM8921_S5] =			{ 125, 39, 34, 2 },
+	[QCOM_RPM_PM8921_S6] =			{ 127, 41, 35, 2 },
+	[QCOM_RPM_PM8921_S7] =			{ 129, 43, 36, 2 },
+	[QCOM_RPM_PM8921_S8] =			{ 131, 45, 37, 2 },
+	[QCOM_RPM_PM8921_L1] =			{ 133, 47, 38, 2 },
+	[QCOM_RPM_PM8921_L2] =			{ 135, 49, 39, 2 },
+	[QCOM_RPM_PM8921_L3] =			{ 137, 51, 40, 2 },
+	[QCOM_RPM_PM8921_L4] =			{ 139, 53, 41, 2 },
+	[QCOM_RPM_PM8921_L5] =			{ 141, 55, 42, 2 },
+	[QCOM_RPM_PM8921_L6] =			{ 143, 57, 43, 2 },
+	[QCOM_RPM_PM8921_L7] =			{ 145, 59, 44, 2 },
+	[QCOM_RPM_PM8921_L8] =			{ 147, 61, 45, 2 },
+	[QCOM_RPM_PM8921_L9] =			{ 149, 63, 46, 2 },
+	[QCOM_RPM_PM8921_L10] =			{ 151, 65, 47, 2 },
+	[QCOM_RPM_PM8921_L11] =			{ 153, 67, 48, 2 },
+	[QCOM_RPM_PM8921_L12] =			{ 155, 69, 49, 2 },
+	[QCOM_RPM_PM8921_L13] =			{ 157, 71, 50, 2 },
+	[QCOM_RPM_PM8921_L14] =			{ 159, 73, 51, 2 },
+	[QCOM_RPM_PM8921_L15] =			{ 161, 75, 52, 2 },
+	[QCOM_RPM_PM8921_L16] =			{ 163, 77, 53, 2 },
+	[QCOM_RPM_PM8921_L17] =			{ 165, 79, 54, 2 },
+	[QCOM_RPM_PM8921_L18] =			{ 167, 81, 55, 2 },
+	[QCOM_RPM_PM8921_L19] =			{ 169, 83, 56, 2 },
+	[QCOM_RPM_PM8921_L20] =			{ 171, 85, 57, 2 },
+	[QCOM_RPM_PM8921_L21] =			{ 173, 87, 58, 2 },
+	[QCOM_RPM_PM8921_L22] =			{ 175, 89, 59, 2 },
+	[QCOM_RPM_PM8921_L23] =			{ 177, 91, 60, 2 },
+	[QCOM_RPM_PM8921_L24] =			{ 179, 93, 61, 2 },
+	[QCOM_RPM_PM8921_L25] =			{ 181, 95, 62, 2 },
+	[QCOM_RPM_PM8921_L26] =			{ 183, 97, 63, 2 },
+	[QCOM_RPM_PM8921_L27] =			{ 185, 99, 64, 2 },
+	[QCOM_RPM_PM8921_L28] =			{ 187, 101, 65, 2 },
+	[QCOM_RPM_PM8921_L29] =			{ 189, 103, 66, 2 },
+	[QCOM_RPM_PM8921_CLK1] =		{ 191, 105, 67, 2 },
+	[QCOM_RPM_PM8921_CLK2] =		{ 193, 107, 68, 2 },
+	[QCOM_RPM_PM8921_LVS1] =		{ 195, 109, 69, 1 },
+	[QCOM_RPM_PM8921_LVS2] =		{ 196, 110, 70, 1 },
+	[QCOM_RPM_PM8921_LVS3] =		{ 197, 111, 71, 1 },
+	[QCOM_RPM_PM8921_LVS4] =		{ 198, 112, 72, 1 },
+	[QCOM_RPM_PM8921_LVS5] =		{ 199, 113, 73, 1 },
+	[QCOM_RPM_PM8921_LVS6] =		{ 200, 114, 74, 1 },
+	[QCOM_RPM_PM8921_LVS7] =		{ 201, 115, 75, 1 },
+	[QCOM_RPM_PM8921_NCP] =			{ 202, 116, 80, 2 },
+	[QCOM_RPM_CXO_BUFFERS] =		{ 204, 118, 81, 1 },
+	[QCOM_RPM_USB_OTG_SWITCH] =		{ 205, 119, 82, 1 },
+	[QCOM_RPM_HDMI_SWITCH] =		{ 206, 120, 83, 1 },
+	[QCOM_RPM_DDR_DMM] =			{ 207, 121, 84, 2 },
+};
+
+static const struct qcom_rpm msm8960_template = {
+	.version = 3,
+	.resource_table = msm8960_rpm_resource_table,
+	.n_resources = ARRAY_SIZE(msm8960_rpm_resource_table),
+};
+
+static const struct of_device_id qcom_rpm_of_match[] = {
+	{ .compatible = "qcom,rpm-apq8064", .data = &apq8064_template },
+	{ .compatible = "qcom,rpm-msm8660", .data = &msm8660_template },
+	{ .compatible = "qcom,rpm-msm8960", .data = &msm8960_template },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, qcom_rpm_of_match);
+
+struct qcom_rpm *dev_get_qcom_rpm(struct device *dev)
+{
+	return dev_get_drvdata(dev);
+}
+EXPORT_SYMBOL(dev_get_qcom_rpm);
+
+int qcom_rpm_write(struct qcom_rpm *rpm, int resource, u32 *buf, size_t count)
+{
+	const struct qcom_rpm_resource *res;
+	u32 sel_mask[RPM_SELECT_SIZE] = { 0 };
+	int left;
+	int ret = 0;
+	int i;
+
+	if (WARN_ON(resource < 0 || resource >= rpm->n_resources))
+		return -EINVAL;
+
+	res = &rpm->resource_table[resource];
+	if (WARN_ON(res->size != count))
+		return -EINVAL;
+
+	mutex_lock(&rpm->lock);
+
+	for (i = 0; i < res->size; i++)
+		writel_relaxed(buf[i], RPM_REQ_REG(rpm, res->target_id + i));
+
+	bitmap_set((unsigned long *)sel_mask, res->select_id, 1);
+	for (i = 0; i < ARRAY_SIZE(sel_mask); i++) {
+		writel_relaxed(sel_mask[i],
+			       RPM_CTRL_REG(rpm, RPM_REQ_SELECT + i));
+	}
+
+	writel_relaxed(RPM_ACTIVE_STATE,
+		       RPM_CTRL_REG(rpm, RPM_REQUEST_CONTEXT));
+
+	reinit_completion(&rpm->ack);
+	writel(RPM_SIGNAL, rpm->ipc_rpm_reg);
+
+	left = wait_for_completion_timeout(&rpm->ack, RPM_REQUEST_TIMEOUT);
+	if (!left)
+		ret = -ETIMEDOUT;
+	else if (rpm->ack_status & RPM_REJECTED)
+		ret = -EIO;
+
+	mutex_unlock(&rpm->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL(qcom_rpm_write);
+
+static irqreturn_t qcom_rpm_ack_interrupt(int irq, void *dev)
+{
+	struct qcom_rpm *rpm = dev;
+	u32 ack;
+	int i;
+
+	ack = readl_relaxed(RPM_CTRL_REG(rpm, RPM_ACK_CONTEXT));
+	for (i = 0; i < RPM_SELECT_SIZE; i++)
+		writel_relaxed(0, RPM_CTRL_REG(rpm, RPM_ACK_SELECTOR + i));
+	writel(0, RPM_CTRL_REG(rpm, RPM_ACK_CONTEXT));
+
+	if (ack & RPM_NOTIFICATION) {
+		dev_warn(rpm->dev, "ignoring notification!\n");
+	} else {
+		rpm->ack_status = ack;
+		complete(&rpm->ack);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t qcom_rpm_err_interrupt(int irq, void *dev)
+{
+	struct qcom_rpm *rpm = dev;
+
+	writel(0x1, rpm->ipc_rpm_reg);
+
+	dev_err(rpm->dev, "RPM triggered fatal error\n");
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t qcom_rpm_wakeup_interrupt(int irq, void *dev)
+{
+	return IRQ_HANDLED;
+}
+
+static int qcom_rpm_probe(struct platform_device *pdev)
+{
+	const struct of_device_id *match;
+	const struct qcom_rpm *template;
+	struct resource *res;
+	struct qcom_rpm *rpm;
+	u32 fw_version[3];
+	int irq_wakeup;
+	int irq_ack;
+	int irq_err;
+	int ret;
+
+	rpm = devm_kzalloc(&pdev->dev, sizeof(*rpm), GFP_KERNEL);
+	if (!rpm) {
+		dev_err(&pdev->dev, "Can't allocate qcom_rpm\n");
+		return -ENOMEM;
+	}
+	rpm->dev = &pdev->dev;
+	mutex_init(&rpm->lock);
+	init_completion(&rpm->ack);
+
+	irq_ack = platform_get_irq_byname(pdev, "ack");
+	if (irq_ack < 0) {
+		dev_err(&pdev->dev, "required ack interrupt missing\n");
+		return irq_ack;
+	}
+
+	irq_err = platform_get_irq_byname(pdev, "err");
+	if (irq_err < 0) {
+		dev_err(&pdev->dev, "required err interrupt missing\n");
+		return irq_err;
+	}
+
+	irq_wakeup = platform_get_irq_byname(pdev, "wakeup");
+	if (irq_wakeup < 0) {
+		dev_err(&pdev->dev, "required wakeup interrupt missing\n");
+		return irq_wakeup;
+	}
+
+	match = of_match_device(qcom_rpm_of_match, &pdev->dev);
+	template = match->data;
+
+	rpm->version = template->version;
+	rpm->resource_table = template->resource_table;
+	rpm->n_resources = template->n_resources;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	rpm->status_regs = devm_ioremap_resource(&pdev->dev, res);
+	rpm->ctrl_regs = rpm->status_regs + 0x400;
+	rpm->req_regs = rpm->status_regs + 0x600;
+	if (IS_ERR(rpm->status_regs))
+		return PTR_ERR(rpm->status_regs);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	rpm->ipc_rpm_reg = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(rpm->ipc_rpm_reg))
+		return PTR_ERR(rpm->ipc_rpm_reg);
+
+	dev_set_drvdata(&pdev->dev, rpm);
+
+	fw_version[0] = readl(RPM_STATUS_REG(rpm, 0));
+	fw_version[1] = readl(RPM_STATUS_REG(rpm, 1));
+	fw_version[2] = readl(RPM_STATUS_REG(rpm, 2));
+	if (fw_version[0] != rpm->version) {
+		dev_err(&pdev->dev,
+			"RPM version %u.%u.%u incompatible with driver version %u",
+			fw_version[0],
+			fw_version[1],
+			fw_version[2],
+			rpm->version);
+		return -EFAULT;
+	}
+
+	dev_info(&pdev->dev, "RPM firmware %u.%u.%u\n", fw_version[0],
+							fw_version[1],
+							fw_version[2]);
+
+	writel(fw_version[0], RPM_CTRL_REG(rpm, 0));
+	writel(fw_version[1], RPM_CTRL_REG(rpm, 1));
+	writel(fw_version[2], RPM_CTRL_REG(rpm, 2));
+
+	ret = devm_request_irq(&pdev->dev,
+			       irq_ack,
+			       qcom_rpm_ack_interrupt,
+			       IRQF_TRIGGER_RISING | IRQF_NO_SUSPEND,
+			       "qcom_rpm_ack",
+			       rpm);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to request ack interrupt\n");
+		return ret;
+	}
+
+	ret = irq_set_irq_wake(irq_ack, 1);
+	if (ret)
+		dev_warn(&pdev->dev, "failed to mark ack irq as wakeup\n");
+
+	ret = devm_request_irq(&pdev->dev,
+			       irq_err,
+			       qcom_rpm_err_interrupt,
+			       IRQF_TRIGGER_RISING,
+			       "qcom_rpm_err",
+			       rpm);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to request err interrupt\n");
+		return ret;
+	}
+
+	ret = devm_request_irq(&pdev->dev,
+			       irq_wakeup,
+			       qcom_rpm_wakeup_interrupt,
+			       IRQF_TRIGGER_RISING,
+			       "qcom_rpm_wakeup",
+			       rpm);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to request wakeup interrupt\n");
+		return ret;
+	}
+
+	ret = irq_set_irq_wake(irq_wakeup, 1);
+	if (ret)
+		dev_warn(&pdev->dev, "failed to mark wakeup irq as wakeup\n");
+
+	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
+}
+
+static int qcom_rpm_remove_child(struct device *dev, void *unused)
+{
+	platform_device_unregister(to_platform_device(dev));
+	return 0;
+}
+
+static int qcom_rpm_remove(struct platform_device *pdev)
+{
+	device_for_each_child(&pdev->dev, NULL, qcom_rpm_remove_child);
+	return 0;
+}
+
+static struct platform_driver qcom_rpm_driver = {
+	.probe = qcom_rpm_probe,
+	.remove = qcom_rpm_remove,
+	.driver  = {
+		.name  = "qcom_rpm",
+		.owner = THIS_MODULE,
+		.of_match_table = qcom_rpm_of_match,
+	},
+};
+
+static int __init qcom_rpm_init(void)
+{
+	return platform_driver_register(&qcom_rpm_driver);
+}
+arch_initcall(qcom_rpm_init);
+
+static void __exit qcom_rpm_exit(void)
+{
+	platform_driver_unregister(&qcom_rpm_driver);
+}
+module_exit(qcom_rpm_exit)
+
+MODULE_DESCRIPTION("Qualcomm Resource Power Manager driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/qcom_rpm.h b/include/linux/mfd/qcom_rpm.h
new file mode 100644
index 0000000..a52bc37
--- /dev/null
+++ b/include/linux/mfd/qcom_rpm.h
@@ -0,0 +1,12 @@
+#ifndef __QCOM_RPM_H__
+#define __QCOM_RPM_H__
+
+#include <linux/types.h>
+
+struct device;
+struct qcom_rpm;
+
+struct qcom_rpm *dev_get_qcom_rpm(struct device *dev);
+int qcom_rpm_write(struct qcom_rpm *rpm, int resource, u32 *buf, size_t count);
+
+#endif
-- 
1.8.2.2

Re: [PATCH 2/3] mfd: qcom-rpm: Driver for the Qualcomm RPM

[Srinivas Kandagatla]

Hi Bjorn,

On 27/05/14 18:28, Bjorn Andersson wrote:
> Driver for the Resource Power Manager (RPM) found in Qualcomm 8660, 8960 and
> 8064 based devices. The driver exposes resources that child drivers can operate
> on; to implementing regulator, clock and bus frequency drivers.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> ---
>   drivers/mfd/Kconfig          |  15 ++
>   drivers/mfd/Makefile         |   1 +
>   drivers/mfd/qcom_rpm.c       | 575 +++++++++++++++++++++++++++++++++++++++++++
>   include/linux/mfd/qcom_rpm.h |  12 +
>   4 files changed, 603 insertions(+)
>   create mode 100644 drivers/mfd/qcom_rpm.c
>   create mode 100644 include/linux/mfd/qcom_rpm.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 3383412..e5122a7 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -482,6 +482,21 @@ config UCB1400_CORE
>   	  To compile this driver as a module, choose M here: the
>   	  module will be called ucb1400_core.
>
> +config MFD_QCOM_RPM
> +	tristate "Qualcomm Resource Power Manager (RPM)
> +	depends on ARCH_QCOM && OF
> +	select MFD_CORE
> +	help
> +	  If you say yes to this option, support will be included for the
> +	  Resource Power Manager system found in the Qualcomm 8660, 8960 and
> +	  8064 based devices.
> +
> +	  This is required to access many regulators, clocks and bus
> +	  frequencies controlled by the RPM on these devices.
> +
> +	  Say M here if you want to include support for the Qualcomm RPM as a
> +	  module. This will build a module called "qcom_rpm".
> +
>   config MFD_PM8XXX
>   	tristate
>
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 2851275..bbe4209 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -150,6 +150,7 @@ obj-$(CONFIG_MFD_SI476X_CORE)	+= si476x-core.o
>
>   obj-$(CONFIG_MFD_CS5535)	+= cs5535-mfd.o
>   obj-$(CONFIG_MFD_OMAP_USB_HOST)	+= omap-usb-host.o omap-usb-tll.o
> +obj-$(CONFIG_MFD_QCOM_RPM)	+= qcom_rpm.o
>   obj-$(CONFIG_MFD_PM8921_CORE) 	+= pm8921-core.o ssbi.o
>   obj-$(CONFIG_TPS65911_COMPARATOR)	+= tps65911-comparator.o
>   obj-$(CONFIG_MFD_TPS65090)	+= tps65090.o
> diff --git a/drivers/mfd/qcom_rpm.c b/drivers/mfd/qcom_rpm.c
> new file mode 100644
> index 0000000..96135ab
> --- /dev/null
> +++ b/drivers/mfd/qcom_rpm.c
> @@ -0,0 +1,575 @@
> +/*
> + * Copyright (c) 2014, Sony Mobile Communications AB.
> + * Copyright (c) 2013, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/io.h>
> +#include <linux/interrupt.h>
> +
> +#include <linux/mfd/qcom_rpm.h>
> +
> +#include <dt-bindings/mfd/qcom_rpm.h>
> +
> +struct qcom_rpm_resource {
> +	unsigned target_id;
> +	unsigned status_id;
> +	unsigned select_id;
> +	unsigned size;
> +};
> +
> +struct qcom_rpm {
> +	struct device *dev;
> +	struct completion ack;
> +	struct mutex lock;
> +
> +	void __iomem *status_regs;
> +	void __iomem *ctrl_regs;
> +	void __iomem *req_regs;
> +
> +	void __iomem *ipc_rpm_reg;
> +
> +	u32 ack_status;
> +
> +	u32 version;
> +
> +	const struct qcom_rpm_resource *resource_table;
> +	unsigned n_resources;

the line spacing looks bit odd.
> +};
> +
> +#define RPM_STATUS_REG(rpm, i)	((rpm)->status_regs + (i) * 4)
> +#define RPM_CTRL_REG(rpm, i)	((rpm)->ctrl_regs + (i) * 4)
> +#define RPM_REQ_REG(rpm, i)	((rpm)->req_regs + (i) * 4)

Probably you could make these macros bit more generic by removing the 
rpm and let the calling code dereference it.

> +
> +#define RPM_REQUEST_TIMEOUT	(5 * HZ)
> +
> +#define RPM_REQUEST_CONTEXT	3
> +#define RPM_REQ_SELECT		11
> +#define RPM_ACK_CONTEXT		15
> +#define RPM_ACK_SELECTOR	23
> +#define RPM_SELECT_SIZE		7
> +
> +#define RPM_ACTIVE_STATE	BIT(0)
> +#define RPM_NOTIFICATION	BIT(30)
> +#define RPM_REJECTED		BIT(31)
> +
> +#define RPM_SIGNAL		BIT(2)
> +
> +static const struct qcom_rpm_resource apq8064_rpm_resource_table[] = {
> +	[QCOM_RPM_CXO_CLK] =			{ 25, 9, 5, 1 },
> +	[QCOM_RPM_PXO_CLK] =			{ 26, 10, 6, 1 },
> +	[QCOM_RPM_APPS_FABRIC_CLK] =		{ 27, 11, 8, 1 },
> +	[QCOM_RPM_SYS_FABRIC_CLK] =		{ 28, 12, 9, 1 },
> +	[QCOM_RPM_MM_FABRIC_CLK] =		{ 29, 13, 10, 1 },
> +	[QCOM_RPM_DAYTONA_FABRIC_CLK] =		{ 30, 14, 11, 1 },
> +	[QCOM_RPM_SFPB_CLK] =			{ 31, 15, 12, 1 },
> +	[QCOM_RPM_CFPB_CLK] =			{ 32, 16, 13, 1 },
> +	[QCOM_RPM_MMFPB_CLK] =			{ 33, 17, 14, 1 },
> +	[QCOM_RPM_EBI1_CLK] =			{ 34, 18, 16, 1 },
> +	[QCOM_RPM_APPS_FABRIC_HALT] =		{ 35, 19, 18, 1 },
> +	[QCOM_RPM_APPS_FABRIC_MODE] =		{ 37, 20, 19, 1 },
> +	[QCOM_RPM_APPS_FABRIC_IOCTL] =		{ 40, 21, 20, 1 },
> +	[QCOM_RPM_APPS_FABRIC_ARB] =		{ 41, 22, 21, 12 },
> +	[QCOM_RPM_SYS_FABRIC_HALT] =		{ 53, 23, 22, 1 },
> +	[QCOM_RPM_SYS_FABRIC_MODE] =		{ 55, 24, 23, 1 },
> +	[QCOM_RPM_SYS_FABRIC_IOCTL] =		{ 58, 25, 24, 1 },
> +	[QCOM_RPM_SYS_FABRIC_ARB] =		{ 59, 26, 25, 30 },
> +	[QCOM_RPM_MM_FABRIC_HALT] =		{ 89, 27, 26, 1 },
> +	[QCOM_RPM_MM_FABRIC_MODE] =		{ 91, 28, 27, 1 },
> +	[QCOM_RPM_MM_FABRIC_IOCTL] =		{ 94, 29, 28, 1 },
> +	[QCOM_RPM_MM_FABRIC_ARB] =		{ 95, 30, 29, 21 },
> +	[QCOM_RPM_PM8921_S1] =			{ 116, 31, 30, 2 },
> +	[QCOM_RPM_PM8921_S2] =			{ 118, 33, 31, 2 },
> +	[QCOM_RPM_PM8921_S3] =			{ 120, 35, 32, 2 },
> +	[QCOM_RPM_PM8921_S4] =			{ 122, 37, 33, 2 },
> +	[QCOM_RPM_PM8921_S5] =			{ 124, 39, 34, 2 },
> +	[QCOM_RPM_PM8921_S6] =			{ 126, 41, 35, 2 },
> +	[QCOM_RPM_PM8921_S7] =			{ 128, 43, 36, 2 },
> +	[QCOM_RPM_PM8921_S8] =			{ 130, 45, 37, 2 },
> +	[QCOM_RPM_PM8921_L1] =			{ 132, 47, 38, 2 },
> +	[QCOM_RPM_PM8921_L2] =			{ 134, 49, 39, 2 },
> +	[QCOM_RPM_PM8921_L3] =			{ 136, 51, 40, 2 },
> +	[QCOM_RPM_PM8921_L4] =			{ 138, 53, 41, 2 },
> +	[QCOM_RPM_PM8921_L5] =			{ 140, 55, 42, 2 },
> +	[QCOM_RPM_PM8921_L6] =			{ 142, 57, 43, 2 },
> +	[QCOM_RPM_PM8921_L7] =			{ 144, 59, 44, 2 },
> +	[QCOM_RPM_PM8921_L8] =			{ 146, 61, 45, 2 },
> +	[QCOM_RPM_PM8921_L9] =			{ 148, 63, 46, 2 },
> +	[QCOM_RPM_PM8921_L10] =			{ 150, 65, 47, 2 },
> +	[QCOM_RPM_PM8921_L11] =			{ 152, 67, 48, 2 },
> +	[QCOM_RPM_PM8921_L12] =			{ 154, 69, 49, 2 },
> +	[QCOM_RPM_PM8921_L13] =			{ 156, 71, 50, 2 },
> +	[QCOM_RPM_PM8921_L14] =			{ 158, 73, 51, 2 },
> +	[QCOM_RPM_PM8921_L15] =			{ 160, 75, 52, 2 },
> +	[QCOM_RPM_PM8921_L16] =			{ 162, 77, 53, 2 },
> +	[QCOM_RPM_PM8921_L17] =			{ 164, 79, 54, 2 },
> +	[QCOM_RPM_PM8921_L18] =			{ 166, 81, 55, 2 },
> +	[QCOM_RPM_PM8921_L19] =			{ 168, 83, 56, 2 },
> +	[QCOM_RPM_PM8921_L20] =			{ 170, 85, 57, 2 },
> +	[QCOM_RPM_PM8921_L21] =			{ 172, 87, 58, 2 },
> +	[QCOM_RPM_PM8921_L22] =			{ 174, 89, 59, 2 },
> +	[QCOM_RPM_PM8921_L23] =			{ 176, 91, 60, 2 },
> +	[QCOM_RPM_PM8921_L24] =			{ 178, 93, 61, 2 },
> +	[QCOM_RPM_PM8921_L25] =			{ 180, 95, 62, 2 },
> +	[QCOM_RPM_PM8921_L26] =			{ 182, 97, 63, 2 },
> +	[QCOM_RPM_PM8921_L27] =			{ 184, 99, 64, 2 },
> +	[QCOM_RPM_PM8921_L28] =			{ 186, 101, 65, 2 },
> +	[QCOM_RPM_PM8921_L29] =			{ 188, 103, 66, 2 },
> +	[QCOM_RPM_PM8921_CLK1] =		{ 190, 105, 67, 2 },
> +	[QCOM_RPM_PM8921_CLK2] =		{ 192, 107, 68, 2 },
> +	[QCOM_RPM_PM8921_LVS1] =		{ 194, 109, 69, 1 },
> +	[QCOM_RPM_PM8921_LVS2] =		{ 195, 110, 70, 1 },
> +	[QCOM_RPM_PM8921_LVS3] =		{ 196, 111, 71, 1 },
> +	[QCOM_RPM_PM8921_LVS4] =		{ 197, 112, 72, 1 },
> +	[QCOM_RPM_PM8921_LVS5] =		{ 198, 113, 73, 1 },
> +	[QCOM_RPM_PM8921_LVS6] =		{ 199, 114, 74, 1 },
> +	[QCOM_RPM_PM8921_LVS7] =		{ 200, 115, 75, 1 },
> +	[QCOM_RPM_PM8821_S1] =			{ 201, 116, 76, 2 },
> +	[QCOM_RPM_PM8821_S2] =			{ 203, 118, 77, 2 },
> +	[QCOM_RPM_PM8821_L1] =			{ 205, 120, 78, 2 },
> +	[QCOM_RPM_PM8921_NCP] =			{ 207, 122, 80, 2 },
> +	[QCOM_RPM_CXO_BUFFERS] =		{ 209, 124, 81, 1 },
> +	[QCOM_RPM_USB_OTG_SWITCH] =		{ 210, 125, 82, 1 },
> +	[QCOM_RPM_HDMI_SWITCH] =		{ 211, 126, 83, 1 },
> +	[QCOM_RPM_DDR_DMM] =			{ 212, 127, 84, 2 },
> +	[QCOM_RPM_VDDMIN_GPIO] =		{ 215, 131, 89, 1 },
> +};
> +
> +static const struct qcom_rpm apq8064_template = {
> +	.version = 3,
> +	.resource_table = apq8064_rpm_resource_table,
> +	.n_resources = ARRAY_SIZE(apq8064_rpm_resource_table),
> +};
> +
> +static const struct qcom_rpm_resource msm8660_rpm_resource_table[] = {
> +	[QCOM_RPM_CXO_CLK] =			{ 32, 12, 5, 1 },
> +	[QCOM_RPM_PXO_CLK] =			{ 33, 13, 6, 1 },
> +	[QCOM_RPM_PLL_4] =			{ 34, 14, 7, 1 },
> +	[QCOM_RPM_APPS_FABRIC_CLK] =		{ 35, 15, 8, 1 },
> +	[QCOM_RPM_SYS_FABRIC_CLK] =		{ 36, 16, 9, 1 },
> +	[QCOM_RPM_MM_FABRIC_CLK] =		{ 37, 17, 10, 1 },
> +	[QCOM_RPM_DAYTONA_FABRIC_CLK] =		{ 38, 18, 11, 1 },
> +	[QCOM_RPM_SFPB_CLK] =			{ 39, 19, 12, 1 },
> +	[QCOM_RPM_CFPB_CLK] =			{ 40, 20, 13, 1 },
> +	[QCOM_RPM_MMFPB_CLK] =			{ 41, 21, 14, 1 },
> +	[QCOM_RPM_SMI_CLK] =			{ 42, 22, 15, 1 },
> +	[QCOM_RPM_EBI1_CLK] =			{ 43, 23, 16, 1 },
> +	[QCOM_RPM_APPS_L2_CACHE_CTL] =		{ 44, 24, 17, 1 },
> +	[QCOM_RPM_APPS_FABRIC_HALT] =		{ 45, 25, 18, 2 },
> +	[QCOM_RPM_APPS_FABRIC_MODE] =		{ 47, 26, 19, 3 },
> +	[QCOM_RPM_APPS_FABRIC_ARB] =		{ 51, 28, 21, 6 },
> +	[QCOM_RPM_SYS_FABRIC_HALT] =		{ 63, 29, 22, 2 },
> +	[QCOM_RPM_SYS_FABRIC_MODE] =		{ 65, 30, 23, 3 },
> +	[QCOM_RPM_SYS_FABRIC_ARB] =		{ 69, 32, 25, 22 },
> +	[QCOM_RPM_MM_FABRIC_HALT] =		{ 105, 33, 26, 2 },
> +	[QCOM_RPM_MM_FABRIC_MODE] =		{ 107, 34, 27, 3 },
> +	[QCOM_RPM_MM_FABRIC_ARB] =		{ 111, 36, 29, 23 },
> +	[QCOM_RPM_PM8901_SMPS0] =		{ 134, 37, 30, 2 },
> +	[QCOM_RPM_PM8901_SMPS1] =		{ 136, 39, 31, 2 },
> +	[QCOM_RPM_PM8901_SMPS2] =		{ 138, 41, 32, 2 },
> +	[QCOM_RPM_PM8901_SMPS3] =		{ 140, 43, 33, 2 },
> +	[QCOM_RPM_PM8901_SMPS4] =		{ 142, 45, 34, 2 },
> +	[QCOM_RPM_PM8901_LDO0] =		{ 144, 47, 35, 2 },
> +	[QCOM_RPM_PM8901_LDO1] =		{ 146, 49, 36, 2 },
> +	[QCOM_RPM_PM8901_LDO2] =		{ 148, 51, 37, 2 },
> +	[QCOM_RPM_PM8901_LDO3] =		{ 150, 53, 38, 2 },
> +	[QCOM_RPM_PM8901_LDO4] =		{ 152, 55, 39, 2 },
> +	[QCOM_RPM_PM8901_LDO5] =		{ 154, 57, 40, 2 },
> +	[QCOM_RPM_PM8901_LDO6] =		{ 156, 59, 41, 2 },
> +	[QCOM_RPM_PM8901_LVS0] =		{ 158, 61, 42, 1 },
> +	[QCOM_RPM_PM8901_LVS1] =		{ 159, 62, 43, 1 },
> +	[QCOM_RPM_PM8901_LVS2] =		{ 160, 63, 44, 1 },
> +	[QCOM_RPM_PM8901_LVS3] =		{ 161, 64, 45, 1 },
> +	[QCOM_RPM_PM8901_MVS] =			{ 162, 65, 46, 1 },
> +	[QCOM_RPM_PM8058_SMPS0] =		{ 163, 66, 47, 2 },
> +	[QCOM_RPM_PM8058_SMPS1] =		{ 165, 68, 48, 2 },
> +	[QCOM_RPM_PM8058_SMPS2] =		{ 167, 70, 49, 2 },
> +	[QCOM_RPM_PM8058_SMPS3] =		{ 169, 72, 50, 2 },
> +	[QCOM_RPM_PM8058_SMPS4] =		{ 171, 74, 51, 2 },
> +	[QCOM_RPM_PM8058_LDO0] =		{ 173, 76, 52, 2 },
> +	[QCOM_RPM_PM8058_LDO1] =		{ 175, 78, 53, 2 },
> +	[QCOM_RPM_PM8058_LDO2] =		{ 177, 80, 54, 2 },
> +	[QCOM_RPM_PM8058_LDO3] =		{ 179, 82, 55, 2 },
> +	[QCOM_RPM_PM8058_LDO4] =		{ 181, 84, 56, 2 },
> +	[QCOM_RPM_PM8058_LDO5] =		{ 183, 86, 57, 2 },
> +	[QCOM_RPM_PM8058_LDO6] =		{ 185, 88, 58, 2 },
> +	[QCOM_RPM_PM8058_LDO7] =		{ 187, 90, 59, 2 },
> +	[QCOM_RPM_PM8058_LDO8] =		{ 189, 92, 60, 2 },
> +	[QCOM_RPM_PM8058_LDO9] =		{ 191, 94, 61, 2 },
> +	[QCOM_RPM_PM8058_LDO10] =		{ 193, 96, 62, 2 },
> +	[QCOM_RPM_PM8058_LDO11] =		{ 195, 98, 63, 2 },
> +	[QCOM_RPM_PM8058_LDO12] =		{ 197, 100, 64, 2 },
> +	[QCOM_RPM_PM8058_LDO13] =		{ 199, 102, 65, 2 },
> +	[QCOM_RPM_PM8058_LDO14] =		{ 201, 104, 66, 2 },
> +	[QCOM_RPM_PM8058_LDO15] =		{ 203, 106, 67, 2 },
> +	[QCOM_RPM_PM8058_LDO16] =		{ 205, 108, 68, 2 },
> +	[QCOM_RPM_PM8058_LDO17] =		{ 207, 110, 69, 2 },
> +	[QCOM_RPM_PM8058_LDO18] =		{ 209, 112, 70, 2 },
> +	[QCOM_RPM_PM8058_LDO19] =		{ 211, 114, 71, 2 },
> +	[QCOM_RPM_PM8058_LDO20] =		{ 213, 116, 72, 2 },
> +	[QCOM_RPM_PM8058_LDO21] =		{ 215, 118, 73, 2 },
> +	[QCOM_RPM_PM8058_LDO22] =		{ 217, 120, 74, 2 },
> +	[QCOM_RPM_PM8058_LDO23] =		{ 219, 122, 75, 2 },
> +	[QCOM_RPM_PM8058_LDO24] =		{ 221, 124, 76, 2 },
> +	[QCOM_RPM_PM8058_LDO25] =		{ 223, 126, 77, 2 },
> +	[QCOM_RPM_PM8058_LVS0] =		{ 225, 128, 78, 1 },
> +	[QCOM_RPM_PM8058_LVS1] =		{ 226, 129, 79, 1 },
> +	[QCOM_RPM_PM8058_NCP] =			{ 227, 130, 80, 2 },
> +	[QCOM_RPM_CXO_BUFFERS] =		{ 229, 132, 81, 1 },
> +};
> +
> +static const struct qcom_rpm msm8660_template = {
> +	.version = -1,
> +	.resource_table = msm8660_rpm_resource_table,
> +	.n_resources = ARRAY_SIZE(msm8660_rpm_resource_table),
> +};
> +
> +static const struct qcom_rpm_resource msm8960_rpm_resource_table[] = {
> +	[QCOM_RPM_CXO_CLK] =			{ 25, 9, 5, 1 },
> +	[QCOM_RPM_PXO_CLK] =			{ 26, 10, 6, 1 },
> +	[QCOM_RPM_APPS_FABRIC_CLK] =		{ 27, 11, 8, 1 },
> +	[QCOM_RPM_SYS_FABRIC_CLK] =		{ 28, 12, 9, 1 },
> +	[QCOM_RPM_MM_FABRIC_CLK] =		{ 29, 13, 10, 1 },
> +	[QCOM_RPM_DAYTONA_FABRIC_CLK] =		{ 30, 14, 11, 1 },
> +	[QCOM_RPM_SFPB_CLK] =			{ 31, 15, 12, 1 },
> +	[QCOM_RPM_CFPB_CLK] =			{ 32, 16, 13, 1 },
> +	[QCOM_RPM_MMFPB_CLK] =			{ 33, 17, 14, 1 },
> +	[QCOM_RPM_EBI1_CLK] =			{ 34, 18, 16, 1 },
> +	[QCOM_RPM_APPS_FABRIC_HALT] =		{ 35, 19, 18, 1 },
> +	[QCOM_RPM_APPS_FABRIC_MODE] =		{ 37, 20, 19, 1 },
> +	[QCOM_RPM_APPS_FABRIC_IOCTL] =		{ 40, 21, 20, 1 },
> +	[QCOM_RPM_APPS_FABRIC_ARB] =		{ 41, 22, 21, 12 },
> +	[QCOM_RPM_SYS_FABRIC_HALT] =		{ 53, 23, 22, 1 },
> +	[QCOM_RPM_SYS_FABRIC_MODE] =		{ 55, 24, 23, 1 },
> +	[QCOM_RPM_SYS_FABRIC_IOCTL] =		{ 58, 25, 24, 1 },
> +	[QCOM_RPM_SYS_FABRIC_ARB] =		{ 59, 26, 25, 29 },
> +	[QCOM_RPM_MM_FABRIC_HALT] =		{ 88, 27, 26, 1 },
> +	[QCOM_RPM_MM_FABRIC_MODE] =		{ 90, 28, 27, 1 },
> +	[QCOM_RPM_MM_FABRIC_IOCTL] =		{ 93, 29, 28, 1 },
> +	[QCOM_RPM_MM_FABRIC_ARB] =		{ 94, 30, 29, 23 },
> +	[QCOM_RPM_PM8921_S1] =			{ 117, 31, 30, 2 },
> +	[QCOM_RPM_PM8921_S2] =			{ 119, 33, 31, 2 },
> +	[QCOM_RPM_PM8921_S3] =			{ 121, 35, 32, 2 },
> +	[QCOM_RPM_PM8921_S4] =			{ 123, 37, 33, 2 },
> +	[QCOM_RPM_PM8921_S5] =			{ 125, 39, 34, 2 },
> +	[QCOM_RPM_PM8921_S6] =			{ 127, 41, 35, 2 },
> +	[QCOM_RPM_PM8921_S7] =			{ 129, 43, 36, 2 },
> +	[QCOM_RPM_PM8921_S8] =			{ 131, 45, 37, 2 },
> +	[QCOM_RPM_PM8921_L1] =			{ 133, 47, 38, 2 },
> +	[QCOM_RPM_PM8921_L2] =			{ 135, 49, 39, 2 },
> +	[QCOM_RPM_PM8921_L3] =			{ 137, 51, 40, 2 },
> +	[QCOM_RPM_PM8921_L4] =			{ 139, 53, 41, 2 },
> +	[QCOM_RPM_PM8921_L5] =			{ 141, 55, 42, 2 },
> +	[QCOM_RPM_PM8921_L6] =			{ 143, 57, 43, 2 },
> +	[QCOM_RPM_PM8921_L7] =			{ 145, 59, 44, 2 },
> +	[QCOM_RPM_PM8921_L8] =			{ 147, 61, 45, 2 },
> +	[QCOM_RPM_PM8921_L9] =			{ 149, 63, 46, 2 },
> +	[QCOM_RPM_PM8921_L10] =			{ 151, 65, 47, 2 },
> +	[QCOM_RPM_PM8921_L11] =			{ 153, 67, 48, 2 },
> +	[QCOM_RPM_PM8921_L12] =			{ 155, 69, 49, 2 },
> +	[QCOM_RPM_PM8921_L13] =			{ 157, 71, 50, 2 },
> +	[QCOM_RPM_PM8921_L14] =			{ 159, 73, 51, 2 },
> +	[QCOM_RPM_PM8921_L15] =			{ 161, 75, 52, 2 },
> +	[QCOM_RPM_PM8921_L16] =			{ 163, 77, 53, 2 },
> +	[QCOM_RPM_PM8921_L17] =			{ 165, 79, 54, 2 },
> +	[QCOM_RPM_PM8921_L18] =			{ 167, 81, 55, 2 },
> +	[QCOM_RPM_PM8921_L19] =			{ 169, 83, 56, 2 },
> +	[QCOM_RPM_PM8921_L20] =			{ 171, 85, 57, 2 },
> +	[QCOM_RPM_PM8921_L21] =			{ 173, 87, 58, 2 },
> +	[QCOM_RPM_PM8921_L22] =			{ 175, 89, 59, 2 },
> +	[QCOM_RPM_PM8921_L23] =			{ 177, 91, 60, 2 },
> +	[QCOM_RPM_PM8921_L24] =			{ 179, 93, 61, 2 },
> +	[QCOM_RPM_PM8921_L25] =			{ 181, 95, 62, 2 },
> +	[QCOM_RPM_PM8921_L26] =			{ 183, 97, 63, 2 },
> +	[QCOM_RPM_PM8921_L27] =			{ 185, 99, 64, 2 },
> +	[QCOM_RPM_PM8921_L28] =			{ 187, 101, 65, 2 },
> +	[QCOM_RPM_PM8921_L29] =			{ 189, 103, 66, 2 },
> +	[QCOM_RPM_PM8921_CLK1] =		{ 191, 105, 67, 2 },
> +	[QCOM_RPM_PM8921_CLK2] =		{ 193, 107, 68, 2 },
> +	[QCOM_RPM_PM8921_LVS1] =		{ 195, 109, 69, 1 },
> +	[QCOM_RPM_PM8921_LVS2] =		{ 196, 110, 70, 1 },
> +	[QCOM_RPM_PM8921_LVS3] =		{ 197, 111, 71, 1 },
> +	[QCOM_RPM_PM8921_LVS4] =		{ 198, 112, 72, 1 },
> +	[QCOM_RPM_PM8921_LVS5] =		{ 199, 113, 73, 1 },
> +	[QCOM_RPM_PM8921_LVS6] =		{ 200, 114, 74, 1 },
> +	[QCOM_RPM_PM8921_LVS7] =		{ 201, 115, 75, 1 },
> +	[QCOM_RPM_PM8921_NCP] =			{ 202, 116, 80, 2 },
> +	[QCOM_RPM_CXO_BUFFERS] =		{ 204, 118, 81, 1 },
> +	[QCOM_RPM_USB_OTG_SWITCH] =		{ 205, 119, 82, 1 },
> +	[QCOM_RPM_HDMI_SWITCH] =		{ 206, 120, 83, 1 },
> +	[QCOM_RPM_DDR_DMM] =			{ 207, 121, 84, 2 },
> +};
> +
> +static const struct qcom_rpm msm8960_template = {
> +	.version = 3,
> +	.resource_table = msm8960_rpm_resource_table,
> +	.n_resources = ARRAY_SIZE(msm8960_rpm_resource_table),
> +};
> +
> +static const struct of_device_id qcom_rpm_of_match[] = {
> +	{ .compatible = "qcom,rpm-apq8064", .data = &apq8064_template },
> +	{ .compatible = "qcom,rpm-msm8660", .data = &msm8660_template },
> +	{ .compatible = "qcom,rpm-msm8960", .data = &msm8960_template },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, qcom_rpm_of_match);
> +
> +struct qcom_rpm *dev_get_qcom_rpm(struct device *dev)
> +{
> +	return dev_get_drvdata(dev);
> +}
> +EXPORT_SYMBOL(dev_get_qcom_rpm);
> +
> +int qcom_rpm_write(struct qcom_rpm *rpm, int resource, u32 *buf, size_t count)
> +{
> +	const struct qcom_rpm_resource *res;
> +	u32 sel_mask[RPM_SELECT_SIZE] = { 0 };
> +	int left;
> +	int ret = 0;
> +	int i;
> +
> +	if (WARN_ON(resource < 0 || resource >= rpm->n_resources))
> +		return -EINVAL;
> +
> +	res = &rpm->resource_table[resource];
> +	if (WARN_ON(res->size != count))
> +		return -EINVAL;
> +
> +	mutex_lock(&rpm->lock);
> +
> +	for (i = 0; i < res->size; i++)
> +		writel_relaxed(buf[i], RPM_REQ_REG(rpm, res->target_id + i));
> +
> +	bitmap_set((unsigned long *)sel_mask, res->select_id, 1);
> +	for (i = 0; i < ARRAY_SIZE(sel_mask); i++) {
> +		writel_relaxed(sel_mask[i],
> +			       RPM_CTRL_REG(rpm, RPM_REQ_SELECT + i));
> +	}
> +
> +	writel_relaxed(RPM_ACTIVE_STATE,
> +		       RPM_CTRL_REG(rpm, RPM_REQUEST_CONTEXT));
> +
> +	reinit_completion(&rpm->ack);
> +	writel(RPM_SIGNAL, rpm->ipc_rpm_reg);
> +
> +	left = wait_for_completion_timeout(&rpm->ack, RPM_REQUEST_TIMEOUT);
> +	if (!left)
> +		ret = -ETIMEDOUT;
> +	else if (rpm->ack_status & RPM_REJECTED)
> +		ret = -EIO;
> +
> +	mutex_unlock(&rpm->lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(qcom_rpm_write);
> +
> +static irqreturn_t qcom_rpm_ack_interrupt(int irq, void *dev)
> +{
> +	struct qcom_rpm *rpm = dev;
> +	u32 ack;
> +	int i;
> +
> +	ack = readl_relaxed(RPM_CTRL_REG(rpm, RPM_ACK_CONTEXT));
/n
> +	for (i = 0; i < RPM_SELECT_SIZE; i++)
> +		writel_relaxed(0, RPM_CTRL_REG(rpm, RPM_ACK_SELECTOR + i));

/n
> +	writel(0, RPM_CTRL_REG(rpm, RPM_ACK_CONTEXT));
> +
> +	if (ack & RPM_NOTIFICATION) {
> +		dev_warn(rpm->dev, "ignoring notification!\n");
> +	} else {
> +		rpm->ack_status = ack;
> +		complete(&rpm->ack);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t qcom_rpm_err_interrupt(int irq, void *dev)
> +{
> +	struct qcom_rpm *rpm = dev;
> +
> +	writel(0x1, rpm->ipc_rpm_reg);
> +
> +	dev_err(rpm->dev, "RPM triggered fatal error\n");
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t qcom_rpm_wakeup_interrupt(int irq, void *dev)
> +{
> +	return IRQ_HANDLED;
> +}
> +
> +static int qcom_rpm_probe(struct platform_device *pdev)
> +{
> +	const struct of_device_id *match;
> +	const struct qcom_rpm *template;
> +	struct resource *res;
> +	struct qcom_rpm *rpm;
> +	u32 fw_version[3];
> +	int irq_wakeup;
> +	int irq_ack;
> +	int irq_err;
> +	int ret;
> +
> +	rpm = devm_kzalloc(&pdev->dev, sizeof(*rpm), GFP_KERNEL);

Sorry If I missed somthing obvious, But why not just use the structure 
from of_data. Is the global structure going to be used for something else?

Or make a seperate structure for of_data and not use struct qcom_rpm?


> +	if (!rpm) {
> +		dev_err(&pdev->dev, "Can't allocate qcom_rpm\n");
message not necessary as kernel will print the alocation failures.
> +		return -ENOMEM;
> +	}
> +	rpm->dev = &pdev->dev;
> +	mutex_init(&rpm->lock);
> +	init_completion(&rpm->ack);
> +
> +	irq_ack = platform_get_irq_byname(pdev, "ack");
> +	if (irq_ack < 0) {
> +		dev_err(&pdev->dev, "required ack interrupt missing\n");
> +		return irq_ack;
> +	}
> +
> +	irq_err = platform_get_irq_byname(pdev, "err");
> +	if (irq_err < 0) {
> +		dev_err(&pdev->dev, "required err interrupt missing\n");
> +		return irq_err;
> +	}
> +
> +	irq_wakeup = platform_get_irq_byname(pdev, "wakeup");
> +	if (irq_wakeup < 0) {
> +		dev_err(&pdev->dev, "required wakeup interrupt missing\n");
> +		return irq_wakeup;
> +	}
> +
> +	match = of_match_device(qcom_rpm_of_match, &pdev->dev);
> +	template = match->data;
> +
> +	rpm->version = template->version;
> +	rpm->resource_table = template->resource_table;
> +	rpm->n_resources = template->n_resources;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
Should't you use the platform_get_resource_byname here?

missed error case checks too.
> +	rpm->status_regs = devm_ioremap_resource(&pdev->dev, res);
> +	rpm->ctrl_regs = rpm->status_regs + 0x400;
> +	rpm->req_regs = rpm->status_regs + 0x600;
> +	if (IS_ERR(rpm->status_regs))
> +		return PTR_ERR(rpm->status_regs);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
Dito.
> +	rpm->ipc_rpm_reg = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(rpm->ipc_rpm_reg))
> +		return PTR_ERR(rpm->ipc_rpm_reg);
> +
> +	dev_set_drvdata(&pdev->dev, rpm);
> +
> +	fw_version[0] = readl(RPM_STATUS_REG(rpm, 0));
> +	fw_version[1] = readl(RPM_STATUS_REG(rpm, 1));
> +	fw_version[2] = readl(RPM_STATUS_REG(rpm, 2));
> +	if (fw_version[0] != rpm->version) {
> +		dev_err(&pdev->dev,
> +			"RPM version %u.%u.%u incompatible with driver version %u",
> +			fw_version[0],
> +			fw_version[1],
> +			fw_version[2],
> +			rpm->version);
> +		return -EFAULT;
> +	}
> +
> +	dev_info(&pdev->dev, "RPM firmware %u.%u.%u\n", fw_version[0],
> +							fw_version[1],
> +							fw_version[2]);
> +
> +	writel(fw_version[0], RPM_CTRL_REG(rpm, 0));
> +	writel(fw_version[1], RPM_CTRL_REG(rpm, 1));
> +	writel(fw_version[2], RPM_CTRL_REG(rpm, 2));
> +
> +	ret = devm_request_irq(&pdev->dev,
> +			       irq_ack,
> +			       qcom_rpm_ack_interrupt,
> +			       IRQF_TRIGGER_RISING | IRQF_NO_SUSPEND,
> +			       "qcom_rpm_ack",
> +			       rpm);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to request ack interrupt\n");
> +		return ret;
> +	}
> +

[ ..
> +	ret = irq_set_irq_wake(irq_ack, 1);
> +	if (ret)
> +		dev_warn(&pdev->dev, "failed to mark ack irq as wakeup\n");
> +
..]

Shouln't these be set as part of the pm suspend call, if the device is 
wakeup capable?

> +	ret = devm_request_irq(&pdev->dev,
> +			       irq_err,
> +			       qcom_rpm_err_interrupt,
> +			       IRQF_TRIGGER_RISING,
> +			       "qcom_rpm_err",
> +			       rpm);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to request err interrupt\n");
> +		return ret;
> +	}
> +
> +	ret = devm_request_irq(&pdev->dev,
> +			       irq_wakeup,
> +			       qcom_rpm_wakeup_interrupt,
> +			       IRQF_TRIGGER_RISING,
> +			       "qcom_rpm_wakeup",
> +			       rpm);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to request wakeup interrupt\n");
> +		return ret;
> +	}
> +
> +	ret = irq_set_irq_wake(irq_wakeup, 1);
> +	if (ret)
> +		dev_warn(&pdev->dev, "failed to mark wakeup irq as wakeup\n");
> +
> +	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
> +}
> +
> +static int qcom_rpm_remove_child(struct device *dev, void *unused)
> +{
> +	platform_device_unregister(to_platform_device(dev));
> +	return 0;
> +}
> +
> +static int qcom_rpm_remove(struct platform_device *pdev)
> +{
> +	device_for_each_child(&pdev->dev, NULL, qcom_rpm_remove_child);
> +	return 0;
> +}
> +
> +static struct platform_driver qcom_rpm_driver = {
> +	.probe = qcom_rpm_probe,
> +	.remove = qcom_rpm_remove,
> +	.driver  = {
> +		.name  = "qcom_rpm",
> +		.owner = THIS_MODULE,
> +		.of_match_table = qcom_rpm_of_match,
> +	},
> +};
> +
> +static int __init qcom_rpm_init(void)
> +{
> +	return platform_driver_register(&qcom_rpm_driver);
> +}
> +arch_initcall(qcom_rpm_init);
> +
> +static void __exit qcom_rpm_exit(void)
> +{
> +	platform_driver_unregister(&qcom_rpm_driver);
> +}
> +module_exit(qcom_rpm_exit)
> +
> +MODULE_DESCRIPTION("Qualcomm Resource Power Manager driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mfd/qcom_rpm.h b/include/linux/mfd/qcom_rpm.h
> new file mode 100644
> index 0000000..a52bc37
> --- /dev/null
> +++ b/include/linux/mfd/qcom_rpm.h
> @@ -0,0 +1,12 @@
> +#ifndef __QCOM_RPM_H__
> +#define __QCOM_RPM_H__
> +
> +#include <linux/types.h>
> +
> +struct device;
> +struct qcom_rpm;
> +
> +struct qcom_rpm *dev_get_qcom_rpm(struct device *dev);
> +int qcom_rpm_write(struct qcom_rpm *rpm, int resource, u32 *buf, size_t count);

IMHO, dummy functions for these are required, otherwise you would get a 
compilation errors on client drivers.

> +
> +#endif
>
thanks,
srini

Re: [PATCH 2/3] mfd: qcom-rpm: Driver for the Qualcomm RPM

[Bjorn Andersson]

On Thu, May 29, 2014 at 9:19 AM, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
>> +++ b/drivers/mfd/qcom_rpm.c

[...]

>> +struct qcom_rpm {
>> +       struct device *dev;
>> +       struct completion ack;
>> +       struct mutex lock;
>> +
>> +       void __iomem *status_regs;
>> +       void __iomem *ctrl_regs;
>> +       void __iomem *req_regs;
>> +
>> +       void __iomem *ipc_rpm_reg;
>> +
>> +       u32 ack_status;
>> +
>> +       u32 version;
>> +
>> +       const struct qcom_rpm_resource *resource_table;
>> +       unsigned n_resources;
>
>
> the line spacing looks bit odd.
>

I'll fix

>> +};
>> +
>> +#define RPM_STATUS_REG(rpm, i) ((rpm)->status_regs + (i) * 4)
>> +#define RPM_CTRL_REG(rpm, i)   ((rpm)->ctrl_regs + (i) * 4)
>> +#define RPM_REQ_REG(rpm, i)    ((rpm)->req_regs + (i) * 4)
>
>
> Probably you could make these macros bit more generic by removing the rpm
> and let the calling code dereference it.
>
>

I first open coded them, I then had separate writel/readl wrappers for them and
then I settled for this, as I figured it help clarifying the code. I can have
another look at it, but I don't think that below will make things clearer.

#define RPM_IDX_2_OFFSET(i) ((i) * 4)

[...]

>> +
>> +static irqreturn_t qcom_rpm_ack_interrupt(int irq, void *dev)
>> +{
>> +       struct qcom_rpm *rpm = dev;
>> +       u32 ack;
>> +       int i;
>> +
>> +       ack = readl_relaxed(RPM_CTRL_REG(rpm, RPM_ACK_CONTEXT));
>
> /n
>

Will try it out.

>> +       for (i = 0; i < RPM_SELECT_SIZE; i++)
>> +               writel_relaxed(0, RPM_CTRL_REG(rpm, RPM_ACK_SELECTOR +
>> i));
>
>
> /n
>

Will try it out, although to me this grouping says "write all selectors and the
context".

>> +       writel(0, RPM_CTRL_REG(rpm, RPM_ACK_CONTEXT));
>> +
>> +       if (ack & RPM_NOTIFICATION) {
>> +               dev_warn(rpm->dev, "ignoring notification!\n");
>> +       } else {
>> +               rpm->ack_status = ack;
>> +               complete(&rpm->ack);
>> +       }
>> +
>> +       return IRQ_HANDLED;
>> +}

[...]

>> +static int qcom_rpm_probe(struct platform_device *pdev)
>> +{
>> +       const struct of_device_id *match;
>> +       const struct qcom_rpm *template;
>> +       struct resource *res;
>> +       struct qcom_rpm *rpm;
>> +       u32 fw_version[3];
>> +       int irq_wakeup;
>> +       int irq_ack;
>> +       int irq_err;
>> +       int ret;
>> +
>> +       rpm = devm_kzalloc(&pdev->dev, sizeof(*rpm), GFP_KERNEL);
>
>
> Sorry If I missed somthing obvious, But why not just use the structure from
> of_data. Is the global structure going to be used for something else?
>
> Or make a seperate structure for of_data and not use struct qcom_rpm?
>
>
>

Although we will not have more than one rpm in a system and therefore not
instatiate this driver multiple times I do not want to run it off the global
state.

>> +       if (!rpm) {
>> +               dev_err(&pdev->dev, "Can't allocate qcom_rpm\n");
>
> message not necessary as kernel will print the alocation failures.
>

Thanks!

>> +               return -ENOMEM;
>> +       }

[...]

>> +
>> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>
> Should't you use the platform_get_resource_byname here?
>
> missed error case checks too.
>

This is a fairly commonly used construct, to have the error from
platform_get_resource being propagated through devm_ioremap_resource and catch
it there. It gives an extra error print in the log, but I find it very clean.

>> +       rpm->status_regs = devm_ioremap_resource(&pdev->dev, res);
>> +       rpm->ctrl_regs = rpm->status_regs + 0x400;
>> +       rpm->req_regs = rpm->status_regs + 0x600;
>> +       if (IS_ERR(rpm->status_regs))
>> +               return PTR_ERR(rpm->status_regs);
>> +
>> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>
> Dito.
>

[...]

>
>
> [ ..
>
>> +       ret = irq_set_irq_wake(irq_ack, 1);
>> +       if (ret)
>> +               dev_warn(&pdev->dev, "failed to mark ack irq as
>> wakeup\n");
>> +
>
> ..]
>
> Shouln't these be set as part of the pm suspend call, if the device is
> wakeup capable?
>
>

Is there any reason to toggle this?

I'm not sure when this interrupt will actually be fired, but I don't see any
harm in keeping it wakup enabled at all times.

[...]

>> +++ b/include/linux/mfd/qcom_rpm.h
>> @@ -0,0 +1,12 @@
>> +#ifndef __QCOM_RPM_H__
>> +#define __QCOM_RPM_H__
>> +
>> +#include <linux/types.h>
>> +
>> +struct device;
>> +struct qcom_rpm;
>> +
>> +struct qcom_rpm *dev_get_qcom_rpm(struct device *dev);
>> +int qcom_rpm_write(struct qcom_rpm *rpm, int resource, u32 *buf, size_t
>> count);
>
>
> IMHO, dummy functions for these are required, otherwise you would get a
> compilation errors on client drivers.
>

I didn't expect us to compile the children into a kernel that doesn't have the
rpm, as I see them as one entity. An exception would be if we want to add
COMPILE_TEST to the children, but that would require an extra change anyways.

Thanks for the review!

Regards,
Bjorn

Re: [PATCH 2/3] mfd: qcom-rpm: Driver for the Qualcomm RPM

[Srinivas Kandagatla]

On 29/05/14 20:45, Bjorn Andersson wrote:
> On Thu, May 29, 2014 at 9:19 AM, Srinivas Kandagatla
> <srinivas.kandagatla@linaro.org> wrote:
>>> +++ b/drivers/mfd/qcom_rpm.c
>
>> the line spacing looks bit odd.
>>
>
> I'll fix
>
>>> +};
>>> +
>>> +#define RPM_STATUS_REG(rpm, i) ((rpm)->status_regs + (i) * 4)
>>> +#define RPM_CTRL_REG(rpm, i)   ((rpm)->ctrl_regs + (i) * 4)
>>> +#define RPM_REQ_REG(rpm, i)    ((rpm)->req_regs + (i) * 4)
>>
>>
>> Probably you could make these macros bit more generic by removing the rpm
>> and let the calling code dereference it.
>>
>>
>
> I first open coded them, I then had separate writel/readl wrappers for them and
> then I settled for this, as I figured it help clarifying the code. I can have
> another look at it, but I don't think that below will make things clearer.
>
> #define RPM_IDX_2_OFFSET(i) ((i) * 4)
>

Yes, just leave it as it is.
> [...]
>
>>
>>> +static int qcom_rpm_probe(struct platform_device *pdev)
>>> +{
>>> +       const struct of_device_id *match;
>>> +       const struct qcom_rpm *template;
>>> +       struct resource *res;
>>> +       struct qcom_rpm *rpm;
>>> +       u32 fw_version[3];
>>> +       int irq_wakeup;
>>> +       int irq_ack;
>>> +       int irq_err;
>>> +       int ret;
>>> +
>>> +       rpm = devm_kzalloc(&pdev->dev, sizeof(*rpm), GFP_KERNEL);
>>
>>
>> Sorry If I missed somthing obvious, But why not just use the structure from
>> of_data. Is the global structure going to be used for something else?
>>
>> Or make a seperate structure for of_data and not use struct qcom_rpm?
>>
>>
>>
>
> Although we will not have more than one rpm in a system and therefore not
> instatiate this driver multiple times I do not want to run it off the global
> state.
>
I agree.

Why not make a separate data structure for the qcom_of_data?

>>> +       if (!rpm) {
>>> +               dev_err(&pdev->dev, "Can't allocate qcom_rpm\n");
>>
>> message not necessary as kernel will print the alocation failures.
>>
>
> Thanks!
>
>>> +               return -ENOMEM;
>>> +       }
>
> [...]
>
>>> +
>>> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>
>> Should't you use the platform_get_resource_byname here?
>>
>> missed error case checks too.
>>
>
> This is a fairly commonly used construct, to have the error from
> platform_get_resource being propagated through devm_ioremap_resource and catch
> it there. It gives an extra error print in the log, but I find it very clean.
Sorry I missed that point...


But my point on platform_get_resource_byname is to remove the dependency 
on the resource ordering, It is very difficult to catch errors resulting 
in wrong ordered resources in DT.

>
>>> +       rpm->status_regs = devm_ioremap_resource(&pdev->dev, res);
>>> +       rpm->ctrl_regs = rpm->status_regs + 0x400;
>>> +       rpm->req_regs = rpm->status_regs + 0x600;
>>> +       if (IS_ERR(rpm->status_regs))
>>> +               return PTR_ERR(rpm->status_regs);
>>> +
>>> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>>
>> Dito.
>>
>
> [...]
>
>>
>>
>> [ ..
>>
>>> +       ret = irq_set_irq_wake(irq_ack, 1);
>>> +       if (ret)
>>> +               dev_warn(&pdev->dev, "failed to mark ack irq as
>>> wakeup\n");
>>> +
>>
>> ..]
>>
>> Shouln't these be set as part of the pm suspend call, if the device is
>> wakeup capable?
>>
>>
>
> Is there any reason to toggle this?
>
> I'm not sure when this interrupt will actually be fired, but I don't see any
> harm in keeping it wakup enabled at all times.

Typically the wake-up source is driven/enabled by the user. When the 
system goes to low-power state it would enable the wakeup on the irq. 
And when there is an interrupt it would wake up the system as part of 
resuming from low-power state.

Again if you what this interrupt to wakeup the system, I would expect 
pm_wakeup_event/related calls in the interrupt handler too.

>
> [...]
>
>>> +++ b/include/linux/mfd/qcom_rpm.h
>>> @@ -0,0 +1,12 @@
>>> +#ifndef __QCOM_RPM_H__
>>> +#define __QCOM_RPM_H__
>>> +
>>> +#include <linux/types.h>
>>> +
>>> +struct device;
>>> +struct qcom_rpm;
>>> +
>>> +struct qcom_rpm *dev_get_qcom_rpm(struct device *dev);
>>> +int qcom_rpm_write(struct qcom_rpm *rpm, int resource, u32 *buf, size_t
>>> count);
>>
>>
>> IMHO, dummy functions for these are required, otherwise you would get a
>> compilation errors on client drivers.
>>
>
> I didn't expect us to compile the children into a kernel that doesn't have the
> rpm, as I see them as one entity. An exception would be if we want to add
> COMPILE_TEST to the children, but that would require an extra change anyways.
>
> Thanks for the review!
>
NP,

Thanks,
srini

> Regards,
> Bjorn
>

Re: [PATCH 2/3] mfd: qcom-rpm: Driver for the Qualcomm RPM

[Bjorn Andersson]

On Thu, May 29, 2014 at 2:41 PM, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
>> Although we will not have more than one rpm in a system and therefore not
>> instatiate this driver multiple times I do not want to run it off the
>> global
>> state.
>>
> I agree.
>
> Why not make a separate data structure for the qcom_of_data?
>

That might make sense, definitely worth a spin.

>>> Should't you use the platform_get_resource_byname here?
[...]
> But my point on platform_get_resource_byname is to remove the dependency on
> the resource ordering, It is very difficult to catch errors resulting in
> wrong ordered resources in DT.
>

Sorry, forgot to answer the first part of your question.

The order in dt must be fixed, hence there isn't really any reason to
use the byname; as long as it doesn't add any context or such here. We
had a long long discussion with the dt guys about this a few months
back.

>>>> +       ret = irq_set_irq_wake(irq_ack, 1);
>>>> +       if (ret)
>>>> +               dev_warn(&pdev->dev, "failed to mark ack irq as
>>>> wakeup\n");
>>>> +
>>>
>>>
>>> ..]
>>>
>>> Shouln't these be set as part of the pm suspend call, if the device is
>>> wakeup capable?
>>>
>>>
>>
>> Is there any reason to toggle this?
>>
>> I'm not sure when this interrupt will actually be fired, but I don't see
>> any
>> harm in keeping it wakup enabled at all times.
>
>
> Typically the wake-up source is driven/enabled by the user. When the system
> goes to low-power state it would enable the wakeup on the irq. And when
> there is an interrupt it would wake up the system as part of resuming from
> low-power state.
>

But the RPM is the user; the ack irq is used to acknowledge requests
or to notify the Linux system about some event. If these happens when
the system is asleep it's always expected (in current implementation
at least) to wake up the system; hence I mark it as a wake irq.

> Again if you what this interrupt to wakeup the system, I would expect
> pm_wakeup_event/related calls in the interrupt handler too.
>

There is no power management here. I'll have to look into what value
this would add. I have to get back to you on this.

Regards,
Bjorn

[PATCH 3/3] regulator: qcom-rpm: Regulator driver for the Qualcomm RPM

[Bjorn Andersson]

Driver for regulators exposed by the Resource Power Manager (RPM) found in
Qualcomm 8660, 8960 and 8064 based devices.

Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
---
 drivers/regulator/Kconfig              |  12 +
 drivers/regulator/Makefile             |   1 +
 drivers/regulator/qcom_rpm-regulator.c | 859 +++++++++++++++++++++++++++++++++
 3 files changed, 872 insertions(+)
 create mode 100644 drivers/regulator/qcom_rpm-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 903eb37..fb45d40 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -423,6 +423,18 @@ config REGULATOR_PFUZE100
 	  Say y here to support the regulators found on the Freescale
 	  PFUZE100/PFUZE200 PMIC.
 
+config REGULATOR_QCOM_RPM
+	tristate "Qualcomm RPM regulator driver"
+	depends on MFD_QCOM_RPM
+	help
+	  If you say yes to this option, support will be included for the
+	  regulators exposed by the Resource Power Manager found in Qualcomm
+	  8660, 8960 and 8064 based devices.
+
+	  Say M here if you want to include support for the regulators on the
+	  Qualcomm RPM as a module. The module will be named
+	  "qcom_rpm-regulator".
+
 config REGULATOR_RC5T583
 	tristate "RICOH RC5T583 Power regulators"
 	depends on MFD_RC5T583
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 12ef277..53b5ce8 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -52,6 +52,7 @@ obj-$(CONFIG_REGULATOR_MAX77693) += max77693.o
 obj-$(CONFIG_REGULATOR_MC13783) += mc13783-regulator.o
 obj-$(CONFIG_REGULATOR_MC13892) += mc13892-regulator.o
 obj-$(CONFIG_REGULATOR_MC13XXX_CORE) +=  mc13xxx-regulator-core.o
+obj-$(CONFIG_REGULATOR_QCOM_RPM) += qcom_rpm-regulator.o
 obj-$(CONFIG_REGULATOR_PALMAS) += palmas-regulator.o
 obj-$(CONFIG_REGULATOR_PFUZE100) += pfuze100-regulator.o
 obj-$(CONFIG_REGULATOR_TPS51632) += tps51632-regulator.o
diff --git a/drivers/regulator/qcom_rpm-regulator.c b/drivers/regulator/qcom_rpm-regulator.c
new file mode 100644
index 0000000..4ffbb64
--- /dev/null
+++ b/drivers/regulator/qcom_rpm-regulator.c
@@ -0,0 +1,859 @@
+/*
+ * Copyright (c) 2014, Sony Mobile Communications AB.
+ * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+
+#include <linux/mfd/qcom_rpm.h>
+
+#define MAX_REQUEST_LEN 2
+#define HPM_DEFAULT_THRESHOLD 10000
+
+struct request_member {
+	int		word;
+	unsigned int	mask;
+	int		shift;
+};
+
+struct rpm_reg_parts {
+	struct request_member mV;		/* used if voltage is in mV */
+	struct request_member uV;		/* used if voltage is in uV */
+	struct request_member ip;		/* peak current in mA */
+	struct request_member pd;		/* pull down enable */
+	struct request_member ia;		/* average current in mA */
+	struct request_member fm;		/* force mode */
+	struct request_member pm;		/* power mode */
+	struct request_member pc;		/* pin control */
+	struct request_member pf;		/* pin function */
+	struct request_member enable_state;	/* NCP and switch */
+	struct request_member comp_mode;	/* NCP */
+	struct request_member freq;		/* frequency: NCP and SMPS */
+	struct request_member freq_clk_src;	/* clock source: SMPS */
+	struct request_member hpm;		/* switch: control OCP and SS */
+	int request_len;
+};
+
+#define FORCE_MODE_IS_2_BITS(reg) \
+	((vreg->parts->fm.mask >> vreg->parts->fm.shift) == 3)
+#define FORCE_MODE_IS_3_BITS(reg) \
+	((vreg->parts->fm.mask >> vreg->parts->fm.shift) == 7)
+
+struct qcom_rpm_reg {
+	struct qcom_rpm *rpm;
+
+	struct mutex lock;
+	struct device *dev;
+	struct regulator_desc desc;
+
+	const struct rpm_reg_parts *parts;
+
+	int resource;
+
+	int is_enabled;
+
+	u32 val[MAX_REQUEST_LEN];
+
+	int uV;
+	unsigned int mode;
+
+	int hpm_threshold_uA;
+	int load_bias_uA;
+	int normal_uA;
+	int idle_uA;
+};
+
+static const struct rpm_reg_parts rpm8660_ldo_parts = {
+	.request_len    = 2,
+	.mV             = { 0, 0x00000FFF,  0 },
+	.ip             = { 0, 0x00FFF000, 12 },
+	.fm             = { 0, 0x03000000, 24 },
+	.pc             = { 0, 0x3C000000, 26 },
+	.pf             = { 0, 0xC0000000, 30 },
+	.pd             = { 1, 0x00000001,  0 },
+	.ia             = { 1, 0x00001FFE,  1 },
+};
+
+static const struct rpm_reg_parts rpm8660_smps_parts = {
+	.request_len    = 2,
+	.mV             = { 0, 0x00000FFF,  0 },
+	.ip             = { 0, 0x00FFF000, 12 },
+	.fm             = { 0, 0x03000000, 24 },
+	.pc             = { 0, 0x3C000000, 26 },
+	.pf             = { 0, 0xC0000000, 30 },
+	.pd             = { 1, 0x00000001,  0 },
+	.ia             = { 1, 0x00001FFE,  1 },
+	.freq           = { 1, 0x001FE000, 13 },
+	.freq_clk_src   = { 1, 0x00600000, 21 },
+};
+
+static const struct rpm_reg_parts rpm8660_switch_parts = {
+	.request_len    = 1,
+	.enable_state   = { 0, 0x00000001,  0 },
+	.pd             = { 0, 0x00000002,  1 },
+	.pc             = { 0, 0x0000003C,  2 },
+	.pf             = { 0, 0x000000C0,  6 },
+	.hpm            = { 0, 0x00000300,  8 },
+};
+
+static const struct rpm_reg_parts rpm8660_ncp_parts = {
+	.request_len    = 1,
+	.mV             = { 0, 0x00000FFF,  0 },
+	.enable_state   = { 0, 0x00001000, 12 },
+	.comp_mode      = { 0, 0x00002000, 13 },
+	.freq           = { 0, 0x003FC000, 14 },
+};
+
+static const struct rpm_reg_parts rpm8960_ldo_parts = {
+	.request_len    = 2,
+	.uV             = { 0, 0x007FFFFF,  0 },
+	.pd             = { 0, 0x00800000, 23 },
+	.pc             = { 0, 0x0F000000, 24 },
+	.pf             = { 0, 0xF0000000, 28 },
+	.ip             = { 1, 0x000003FF,  0 },
+	.ia             = { 1, 0x000FFC00, 10 },
+	.fm             = { 1, 0x00700000, 20 },
+};
+
+static const struct rpm_reg_parts rpm8960_smps_parts = {
+	.request_len    = 2,
+	.uV             = { 0, 0x007FFFFF,  0 },
+	.pd             = { 0, 0x00800000, 23 },
+	.pc             = { 0, 0x0F000000, 24 },
+	.pf             = { 0, 0xF0000000, 28 },
+	.ip             = { 1, 0x000003FF,  0 },
+	.ia             = { 1, 0x000FFC00, 10 },
+	.fm             = { 1, 0x00700000, 20 },
+	.pm             = { 1, 0x00800000, 23 },
+	.freq           = { 1, 0x1F000000, 24 },
+	.freq_clk_src   = { 1, 0x60000000, 29 },
+};
+
+static const struct rpm_reg_parts rpm8960_switch_parts = {
+	.request_len    = 1,
+	.enable_state   = { 0, 0x00000001,  0 },
+	.pd             = { 0, 0x00000002,  1 },
+	.pc             = { 0, 0x0000003C,  2 },
+	.pf             = { 0, 0x000003C0,  6 },
+	.hpm            = { 0, 0x00000C00, 10 },
+};
+
+static const struct rpm_reg_parts rpm8960_ncp_parts = {
+	.request_len    = 1,
+	.uV             = { 0, 0x007FFFFF,  0 },
+	.enable_state   = { 0, 0x00800000, 23 },
+	.comp_mode      = { 0, 0x01000000, 24 },
+	.freq           = { 0, 0x3E000000, 25 },
+};
+
+/*
+ * Physically available PMIC regulator voltage ranges
+ */
+static const struct regulator_linear_range pldo_ranges[] = {
+	REGULATOR_LINEAR_RANGE( 750000,   0,  59, 12500),
+	REGULATOR_LINEAR_RANGE(1500000,  60, 123, 25000),
+	REGULATOR_LINEAR_RANGE(3100000, 124, 160, 50000),
+};
+
+static const struct regulator_linear_range nldo_ranges[] = {
+	REGULATOR_LINEAR_RANGE( 750000,   0,  63, 12500),
+};
+
+static const struct regulator_linear_range nldo1200_ranges[] = {
+	REGULATOR_LINEAR_RANGE( 375000,   0,  59,  6250),
+	REGULATOR_LINEAR_RANGE( 750000,  60, 123, 12500),
+};
+
+static const struct regulator_linear_range smps_ranges[] = {
+	REGULATOR_LINEAR_RANGE( 375000,   0,  29, 12500),
+	REGULATOR_LINEAR_RANGE( 750000,  30,  89, 12500),
+	REGULATOR_LINEAR_RANGE(1500000,  90, 153, 25000),
+};
+
+static const struct regulator_linear_range ftsmps_ranges[] = {
+	REGULATOR_LINEAR_RANGE( 350000,   0,   6, 50000),
+	REGULATOR_LINEAR_RANGE( 700000,   7,  63, 12500),
+	REGULATOR_LINEAR_RANGE(1500000,  64, 100, 50000),
+};
+
+static const struct regulator_linear_range ncp_ranges[] = {
+	REGULATOR_LINEAR_RANGE(1500000,   0,  31, 50000),
+};
+
+static int rpm_reg_write(struct qcom_rpm_reg *vreg,
+			 const struct request_member *req,
+			 const int value)
+{
+	if (WARN_ON((value << req->shift) & ~req->mask))
+		return -EINVAL;
+
+	vreg->val[req->word] &= ~req->mask;
+	vreg->val[req->word] |= value << req->shift;
+
+	return qcom_rpm_write(vreg->rpm,
+			      vreg->resource,
+			      vreg->val,
+			      vreg->parts->request_len);
+}
+
+static int rpm_reg_set_voltage(struct regulator_dev *rdev,
+			       int min_uV, int max_uV,
+			       unsigned *selector)
+{
+	struct qcom_rpm_reg *vreg = rdev_get_drvdata(rdev);
+	const struct rpm_reg_parts *parts = vreg->parts;
+	const struct request_member *req;
+	int ret = 0;
+	int sel;
+	int val;
+	int uV;
+
+	dev_dbg(vreg->dev, "set_voltage(%d, %d)\n", min_uV, max_uV);
+
+	if (parts->uV.mask == 0 && parts->mV.mask == 0)
+		return -EINVAL;
+
+	/*
+	 * Snap to the voltage to a supported level.
+	 */
+	sel = regulator_map_voltage_linear_range(rdev, min_uV, max_uV);
+	if (sel < 0) {
+		dev_err(vreg->dev,
+			"requested voltage %d-%d outside possible ranges\n",
+			min_uV, max_uV);
+		return ret;
+	}
+	*selector = sel;
+
+	uV = regulator_list_voltage_linear_range(rdev, sel);
+	if (uV < 0)
+		return uV;
+
+	dev_dbg(vreg->dev, "snapped voltage %duV\n", uV);
+
+	/*
+	 * Update the register.
+	 */
+	mutex_lock(&vreg->lock);
+	vreg->uV = uV;
+	if (parts->uV.mask) {
+		req = &parts->uV;
+		val = vreg->uV;
+	} else {
+		req = &parts->mV;
+		val = vreg->uV / 1000;
+	}
+
+	if (vreg->is_enabled)
+		ret = rpm_reg_write(vreg, req, val);
+	mutex_unlock(&vreg->lock);
+
+	return ret;
+}
+
+static int rpm_reg_get_voltage(struct regulator_dev *rdev)
+{
+	struct qcom_rpm_reg *vreg = rdev_get_drvdata(rdev);
+
+	return vreg->uV;
+}
+
+static int rpm_reg_enable(struct regulator_dev *rdev)
+{
+	struct qcom_rpm_reg *vreg = rdev_get_drvdata(rdev);
+	const struct rpm_reg_parts *parts = vreg->parts;
+	const struct request_member *req;
+	int ret;
+	int val;
+
+	if (parts->uV.mask == 0 && parts->mV.mask == 0 &&
+	    parts->enable_state.mask == 0)
+		return -EINVAL;
+
+	mutex_lock(&vreg->lock);
+	if (parts->uV.mask) {
+		req = &parts->uV;
+		val = vreg->uV;
+	} else if (parts->mV.mask) {
+		req = &parts->mV;
+		val = vreg->uV / 1000;
+	} else {
+		req = &parts->enable_state;
+		val = 1;
+	}
+
+	ret = rpm_reg_write(vreg, req, val);
+	if (!ret)
+		vreg->is_enabled = 1;
+	mutex_unlock(&vreg->lock);
+
+	return ret;
+}
+
+static int rpm_reg_disable(struct regulator_dev *rdev)
+{
+	struct qcom_rpm_reg *vreg = rdev_get_drvdata(rdev);
+	const struct rpm_reg_parts *parts = vreg->parts;
+	const struct request_member *req;
+	int ret;
+
+	if (parts->uV.mask == 0 && parts->mV.mask == 0 &&
+	    parts->enable_state.mask == 0)
+		return -EINVAL;
+
+	mutex_lock(&vreg->lock);
+	if (parts->uV.mask)
+		req = &parts->uV;
+	else if (parts->mV.mask)
+		req = &parts->mV;
+	else
+		req = &parts->enable_state;
+
+	ret = rpm_reg_write(vreg, req, 0);
+	if (!ret)
+		vreg->is_enabled = 0;
+	mutex_unlock(&vreg->lock);
+
+	return ret;
+}
+
+static int rpm_reg_is_enabled(struct regulator_dev *rdev)
+{
+	struct qcom_rpm_reg *vreg = rdev_get_drvdata(rdev);
+
+	return vreg->is_enabled;
+}
+
+static int rpm_reg_set_mode(struct regulator_dev *rdev, unsigned int mode)
+{
+	struct qcom_rpm_reg *vreg = rdev_get_drvdata(rdev);
+	const struct rpm_reg_parts *parts = vreg->parts;
+	int max_mA = parts->ip.mask >> parts->ip.shift;
+	int load_mA;
+	int ret;
+
+	if (mode == REGULATOR_MODE_IDLE)
+		load_mA = vreg->idle_uA / 1000;
+	else
+		load_mA = vreg->normal_uA / 1000;
+
+	if (load_mA > max_mA)
+		load_mA = max_mA;
+
+	mutex_lock(&vreg->lock);
+	ret = rpm_reg_write(vreg, &parts->ip, load_mA);
+	if (!ret)
+		vreg->mode = mode;
+	mutex_unlock(&vreg->lock);
+
+	return ret;
+}
+
+static unsigned int rpm_reg_get_mode(struct regulator_dev *rdev)
+{
+	struct qcom_rpm_reg *vreg = rdev_get_drvdata(rdev);
+
+	return vreg->mode;
+}
+
+static unsigned int rpm_reg_get_optimum_mode(struct regulator_dev *rdev,
+					     int input_uV,
+					     int output_uV,
+					     int load_uA)
+{
+	struct qcom_rpm_reg *vreg = rdev_get_drvdata(rdev);
+
+	load_uA += vreg->load_bias_uA;
+
+	if (load_uA < vreg->hpm_threshold_uA) {
+		vreg->idle_uA = load_uA;
+		return REGULATOR_MODE_IDLE;
+	} else {
+		vreg->normal_uA = load_uA;
+		return REGULATOR_MODE_NORMAL;
+	}
+}
+
+static struct regulator_ops ldo_ops = {
+	.list_voltage = regulator_list_voltage_linear_range,
+
+	.set_voltage = rpm_reg_set_voltage,
+	.get_voltage = rpm_reg_get_voltage,
+
+	.enable = rpm_reg_enable,
+	.disable = rpm_reg_disable,
+	.is_enabled = rpm_reg_is_enabled,
+
+	.set_mode = rpm_reg_set_mode,
+	.get_mode = rpm_reg_get_mode,
+	.get_optimum_mode = rpm_reg_get_optimum_mode,
+};
+
+static struct regulator_ops smps_ops = {
+	.list_voltage = regulator_list_voltage_linear_range,
+
+	.set_voltage = rpm_reg_set_voltage,
+	.get_voltage = rpm_reg_get_voltage,
+
+	.enable = rpm_reg_enable,
+	.disable = rpm_reg_disable,
+	.is_enabled = rpm_reg_is_enabled,
+
+	.set_mode = rpm_reg_set_mode,
+	.get_mode = rpm_reg_get_mode,
+	.get_optimum_mode = rpm_reg_get_optimum_mode,
+};
+
+static struct regulator_ops ncp_ops = {
+	.list_voltage = regulator_list_voltage_linear_range,
+
+	.set_voltage = rpm_reg_set_voltage,
+	.get_voltage = rpm_reg_get_voltage,
+
+	.enable = rpm_reg_enable,
+	.disable = rpm_reg_disable,
+	.is_enabled = rpm_reg_is_enabled,
+};
+
+static struct regulator_ops switch_ops = {
+	.enable = rpm_reg_enable,
+	.disable = rpm_reg_disable,
+	.is_enabled = rpm_reg_is_enabled,
+};
+
+/*
+ * PM8058 regulators
+ */
+static const struct qcom_rpm_reg pm8058_pldo = {
+	.desc.linear_ranges = pldo_ranges,
+	.desc.n_linear_ranges = ARRAY_SIZE(pldo_ranges),
+	.desc.n_voltages = 161,
+	.desc.ops = &ldo_ops,
+	.parts = &rpm8660_ldo_parts,
+};
+
+static const struct qcom_rpm_reg pm8058_nldo = {
+	.desc.linear_ranges = nldo_ranges,
+	.desc.n_linear_ranges = ARRAY_SIZE(nldo_ranges),
+	.desc.n_voltages = 64,
+	.desc.ops = &ldo_ops,
+	.parts = &rpm8660_ldo_parts,
+};
+
+static const struct qcom_rpm_reg pm8058_smps = {
+	.desc.linear_ranges = smps_ranges,
+	.desc.n_linear_ranges = ARRAY_SIZE(smps_ranges),
+	.desc.n_voltages = 154,
+	.desc.ops = &smps_ops,
+	.parts = &rpm8660_smps_parts,
+};
+
+static const struct qcom_rpm_reg pm8058_ncp = {
+	.desc.linear_ranges = ncp_ranges,
+	.desc.n_linear_ranges = ARRAY_SIZE(ncp_ranges),
+	.desc.n_voltages = 32,
+	.desc.ops = &ncp_ops,
+	.parts = &rpm8660_ncp_parts,
+};
+
+static const struct qcom_rpm_reg pm8058_switch = {
+	.desc.ops = &switch_ops,
+	.parts = &rpm8660_switch_parts,
+};
+
+/*
+ * PM8901 regulators
+ */
+static const struct qcom_rpm_reg pm8901_pldo = {
+	.desc.linear_ranges = pldo_ranges,
+	.desc.n_linear_ranges = ARRAY_SIZE(pldo_ranges),
+	.desc.n_voltages = 161,
+	.desc.ops = &ldo_ops,
+	.parts = &rpm8660_ldo_parts,
+};
+
+static const struct qcom_rpm_reg pm8901_nldo = {
+	.desc.linear_ranges = nldo_ranges,
+	.desc.n_linear_ranges = ARRAY_SIZE(nldo_ranges),
+	.desc.n_voltages = 64,
+	.desc.ops = &ldo_ops,
+	.parts = &rpm8660_ldo_parts,
+};
+
+static const struct qcom_rpm_reg pm8901_ftsmps = {
+	.desc.linear_ranges = ftsmps_ranges,
+	.desc.n_linear_ranges = ARRAY_SIZE(ftsmps_ranges),
+	.desc.n_voltages = 101,
+	.desc.ops = &smps_ops,
+	.parts = &rpm8660_smps_parts,
+};
+
+static const struct qcom_rpm_reg pm8901_switch = {
+	.desc.ops = &switch_ops,
+	.parts = &rpm8660_switch_parts,
+};
+
+/*
+ * PM8921 regulators
+ */
+static const struct qcom_rpm_reg pm8921_pldo = {
+	.desc.linear_ranges = pldo_ranges,
+	.desc.n_linear_ranges = ARRAY_SIZE(pldo_ranges),
+	.desc.n_voltages = 161,
+	.desc.ops = &ldo_ops,
+	.parts = &rpm8960_ldo_parts,
+};
+
+static const struct qcom_rpm_reg pm8921_nldo = {
+	.desc.linear_ranges = nldo_ranges,
+	.desc.n_linear_ranges = ARRAY_SIZE(nldo_ranges),
+	.desc.n_voltages = 64,
+	.desc.ops = &ldo_ops,
+	.parts = &rpm8960_ldo_parts,
+};
+
+static const struct qcom_rpm_reg pm8921_nldo1200 = {
+	.desc.linear_ranges = nldo1200_ranges,
+	.desc.n_linear_ranges = ARRAY_SIZE(nldo1200_ranges),
+	.desc.n_voltages = 124,
+	.desc.ops = &ldo_ops,
+	.parts = &rpm8960_ldo_parts,
+};
+
+static const struct qcom_rpm_reg pm8921_smps = {
+	.desc.linear_ranges = smps_ranges,
+	.desc.n_linear_ranges = ARRAY_SIZE(smps_ranges),
+	.desc.n_voltages = 154,
+	.desc.ops = &smps_ops,
+	.parts = &rpm8960_smps_parts,
+};
+
+static const struct qcom_rpm_reg pm8921_ftsmps = {
+	.desc.linear_ranges = ftsmps_ranges,
+	.desc.n_linear_ranges = ARRAY_SIZE(ftsmps_ranges),
+	.desc.n_voltages = 101,
+	.desc.ops = &smps_ops,
+	.parts = &rpm8960_smps_parts,
+};
+
+static const struct qcom_rpm_reg pm8921_ncp = {
+	.desc.linear_ranges = ncp_ranges,
+	.desc.n_linear_ranges = ARRAY_SIZE(ncp_ranges),
+	.desc.n_voltages = 32,
+	.desc.ops = &ncp_ops,
+	.parts = &rpm8960_ncp_parts,
+};
+
+static const struct qcom_rpm_reg pm8921_switch = {
+	.desc.ops = &switch_ops,
+	.parts = &rpm8960_switch_parts,
+};
+
+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 },
+	{ }
+};
+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)
+{
+	if (req->mask == 0 || (value << req->shift) & ~req->mask)
+		return -EINVAL;
+
+	vreg->val[req->word] &= ~req->mask;
+	vreg->val[req->word] |= value << req->shift;
+
+	return 0;
+}
+
+static int rpm_reg_of_parse_freq(struct device *dev, struct qcom_rpm_reg *vreg)
+{
+	static const int freq_table[] = {
+		19200000, 9600000, 6400000, 4800000, 3840000, 3200000, 2740000,
+		2400000, 2130000, 1920000, 1750000, 1600000, 1480000, 1370000,
+		1280000, 1200000,
+
+	};
+	const char *key;
+	u32 freq;
+	int ret;
+	int i;
+
+	key = "qcom,switch-mode-frequency";
+	ret = of_property_read_u32(dev->of_node, key, &freq);
+	if (ret) {
+		dev_err(dev, "regulator requires %s property\n", key);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(freq_table); i++) {
+		if (freq == freq_table[i]) {
+			rpm_reg_set(vreg, &vreg->parts->freq, i + 1);
+			return 0;
+		}
+	}
+
+	dev_err(dev, "invalid frequency %d\n", freq);
+	return -EINVAL;
+}
+
+static int rpm_reg_of_select_one(struct device *dev, const char *keys[],
+				 const int count, int def)
+{
+	int found = -1;
+	int i;
+
+	for (i = 0; i < count; i++) {
+		if (!of_property_read_bool(dev->of_node, keys[i]))
+			continue;
+
+		if (found >= 0) {
+			dev_err(dev, "%s and %s are mutually exclusive\n",
+				keys[i], keys[found]);
+			return -EINVAL;
+		}
+		found = i;
+	}
+
+	if (found == -1)
+		return def;
+
+	return found;
+}
+
+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 = { 0 };
+	struct regulator_dev *rdev;
+	struct qcom_rpm_reg *vreg;
+	const char *key;
+	u32 boot_load;
+	u32 val;
+	int ret;
+
+	match = of_match_device(rpm_of_match, &pdev->dev);
+	template = match->data;
+
+	initdata = of_get_regulator_init_data(&pdev->dev, pdev->dev.of_node);
+	if (!initdata)
+		return -EINVAL;
+
+	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->rpm = dev_get_qcom_rpm(pdev->dev.parent);
+	if (!vreg->rpm) {
+		dev_err(&pdev->dev, "unable to retrieve handle to rpm\n");
+		return -ENODEV;
+	}
+
+	/* Enable pull down */
+	rpm_reg_set(vreg, &vreg->parts->pd, 1);
+
+	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;
+	}
+
+	if (vreg->parts->ip.mask) {
+		initdata->constraints.valid_ops_mask |= REGULATOR_CHANGE_DRMS;
+		initdata->constraints.valid_ops_mask |= REGULATOR_CHANGE_MODE;
+		initdata->constraints.valid_modes_mask |= REGULATOR_MODE_NORMAL;
+		initdata->constraints.valid_modes_mask |= REGULATOR_MODE_IDLE;
+
+		val = HPM_DEFAULT_THRESHOLD;
+		key = "qcom,hpm-threshold";
+		ret = of_property_read_u32(pdev->dev.of_node, key, &val);
+		if (ret && ret != -EINVAL) {
+			dev_err(&pdev->dev, "failed to read %s\n", key);
+			return ret;
+		}
+		vreg->hpm_threshold_uA = val;
+
+		key = "qcom,load-bias";
+		ret = of_property_read_u32(pdev->dev.of_node, key, &val);
+		if (ret && ret != -EINVAL) {
+			dev_err(&pdev->dev, "failed to read %s\n", key);
+			return ret;
+		} else if (ret != -EINVAL) {
+			vreg->load_bias_uA = val;
+		}
+
+		boot_load = 0;
+		key = "qcom,boot-load";
+		ret = of_property_read_u32(pdev->dev.of_node, key, &boot_load);
+		if (ret && ret != -EINVAL) {
+			dev_err(&pdev->dev, "failed to read %s\n", key);
+			return ret;
+		}
+
+		if (boot_load > vreg->hpm_threshold_uA) {
+			vreg->mode = REGULATOR_MODE_NORMAL;
+			vreg->normal_uA = boot_load;
+		} else {
+			vreg->mode = REGULATOR_MODE_IDLE;
+			vreg->idle_uA = boot_load;
+		}
+
+		ret = rpm_reg_set(vreg, &vreg->parts->ip, boot_load / 1000);
+		if (ret) {
+			dev_err(&pdev->dev, "invalid initial load %d\n",
+				boot_load);
+			return ret;
+		}
+	}
+
+	if (vreg->parts->freq.mask) {
+		ret = rpm_reg_of_parse_freq(&pdev->dev, vreg);
+		if (ret < 0)
+			return ret;
+	}
+
+	if (vreg->parts->pm.mask) {
+		static const char *keys[] = {
+			"qcom,power-mode-hysteretic",
+			"qcom,power-mode-pwm",
+		};
+
+		ret = rpm_reg_of_select_one(&pdev->dev,
+					    keys, ARRAY_SIZE(keys), 1);
+		if (ret < 0)
+			return ret;
+
+		ret = rpm_reg_set(vreg, &vreg->parts->pm, ret);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to set power mode\n");
+			return ret;
+		}
+	}
+
+	if (FORCE_MODE_IS_2_BITS(vreg)) {
+		static const char *keys[] = {
+			"qcom,force-mode-none",
+			"qcom,force-mode-lpm",
+			"qcom,force-mode-hpm"
+		};
+
+		ret = rpm_reg_of_select_one(&pdev->dev,
+					    keys, ARRAY_SIZE(keys), 0);
+		if (ret < 0)
+			return ret;
+
+		ret = rpm_reg_set(vreg, &vreg->parts->fm, ret);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to set force mode\n");
+			return ret;
+		}
+	} else if (FORCE_MODE_IS_3_BITS(vreg)) {
+		static const char *keys[] = {
+			"qcom,force-mode-none",
+			"qcom,force-mode-lpm",
+			"qcom,force-mode-auto",
+			"qcom,force-mode-hpm",
+			"qcom,force-mode-bypass",
+		};
+
+		ret = rpm_reg_of_select_one(&pdev->dev,
+					    keys, ARRAY_SIZE(keys), 0);
+		if (ret < 0)
+			return ret;
+
+		ret = rpm_reg_set(vreg, &vreg->parts->fm, ret);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to set force mode\n");
+			return ret;
+		}
+	}
+
+	config.dev = &pdev->dev;
+	config.init_data = initdata;
+	config.driver_data = vreg;
+	config.of_node = pdev->dev.of_node;
+	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;
+}
+
+static struct platform_driver rpm_reg_driver = {
+	.probe          = rpm_reg_probe,
+	.driver  = {
+		.name  = "qcom_rpm_reg",
+		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(rpm_of_match),
+	},
+};
+
+static int __init rpm_reg_init(void)
+{
+	return platform_driver_register(&rpm_reg_driver);
+}
+arch_initcall(rpm_reg_init);
+
+static void __exit rpm_reg_exit(void)
+{
+	platform_driver_unregister(&rpm_reg_driver);
+}
+module_exit(rpm_reg_exit)
+
+MODULE_DESCRIPTION("Qualcomm RPM regulator driver");
+MODULE_LICENSE("GPL v2");
-- 
1.8.2.2

Re: [PATCH 3/3] regulator: qcom-rpm: Regulator driver for the Qualcomm RPM

[Mark Brown]

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

On Tue, May 27, 2014 at 10:28:41AM -0700, Bjorn Andersson wrote:

> +static int rpm_reg_set_voltage(struct regulator_dev *rdev,
> +			       int min_uV, int max_uV,
> +			       unsigned *selector)
> +{
> +	struct qcom_rpm_reg *vreg = rdev_get_drvdata(rdev);
> +	const struct rpm_reg_parts *parts = vreg->parts;
> +	const struct request_member *req;
> +	int ret = 0;
> +	int sel;
> +	int val;
> +	int uV;
> +
> +	dev_dbg(vreg->dev, "set_voltage(%d, %d)\n", min_uV, max_uV);
> +
> +	if (parts->uV.mask == 0 && parts->mV.mask == 0)
> +		return -EINVAL;
> +
> +	/*
> +	 * Snap to the voltage to a supported level.
> +	 */

What is "snapping" a voltage?

> +	sel = regulator_map_voltage_linear_range(rdev, min_uV, max_uV);

Don't open code mapping the voltage, just implement set_voltage_sel().

> +	mutex_lock(&vreg->lock);
> +	if (parts->uV.mask)
> +		req = &parts->uV;
> +	else if (parts->mV.mask)
> +		req = &parts->mV;
> +	else
> +		req = &parts->enable_state;

Just implement separate operations for the regulators with different
register definitions.  It's going to simplify the code.

> +static int rpm_reg_set_mode(struct regulator_dev *rdev, unsigned int mode)
> +{
> +	struct qcom_rpm_reg *vreg = rdev_get_drvdata(rdev);
> +	const struct rpm_reg_parts *parts = vreg->parts;
> +	int max_mA = parts->ip.mask >> parts->ip.shift;
> +	int load_mA;
> +	int ret;
> +
> +	if (mode == REGULATOR_MODE_IDLE)
> +		load_mA = vreg->idle_uA / 1000;
> +	else
> +		load_mA = vreg->normal_uA / 1000;
> +
> +	if (load_mA > max_mA)
> +		load_mA = max_mA;
> +
> +	mutex_lock(&vreg->lock);
> +	ret = rpm_reg_write(vreg, &parts->ip, load_mA);
> +	if (!ret)
> +		vreg->mode = mode;
> +	mutex_unlock(&vreg->lock);

I'm not entirely sure what this is intended to do.  It looks like it's
doing something to do with current limits but it's a set_mode().  Some
documentation might help.  It should also be implementing only specific
modes and rejecting unsupported modes, if we do the same operation to
the device for two different modes that seems wrong.

> +static unsigned int rpm_reg_get_optimum_mode(struct regulator_dev *rdev,
> +					     int input_uV,
> +					     int output_uV,
> +					     int load_uA)
> +{
> +	struct qcom_rpm_reg *vreg = rdev_get_drvdata(rdev);
> +
> +	load_uA += vreg->load_bias_uA;
> +
> +	if (load_uA < vreg->hpm_threshold_uA) {
> +		vreg->idle_uA = load_uA;
> +		return REGULATOR_MODE_IDLE;
> +	} else {
> +		vreg->normal_uA = load_uA;
> +		return REGULATOR_MODE_NORMAL;
> +	}
> +}

This looks very broken - why are we storing anything here?  What is the
stored value supposed to do?

> +	if (vreg->parts->ip.mask) {
> +		initdata->constraints.valid_ops_mask |= REGULATOR_CHANGE_DRMS;
> +		initdata->constraints.valid_ops_mask |= REGULATOR_CHANGE_MODE;
> +		initdata->constraints.valid_modes_mask |= REGULATOR_MODE_NORMAL;
> +		initdata->constraints.valid_modes_mask |= REGULATOR_MODE_IDLE;

No, this is just plain broken.  Constraints are set by the *board*, you
don't know if these settings are safe on any given board.

> +static int __init rpm_reg_init(void)
> +{
> +	return platform_driver_register(&rpm_reg_driver);
> +}
> +arch_initcall(rpm_reg_init);
> +
> +static void __exit rpm_reg_exit(void)
> +{
> +	platform_driver_unregister(&rpm_reg_driver);
> +}
> +module_exit(rpm_reg_exit)

module_platform_driver() or if you must bodge the init order at least
use subsys_initcall() like everyone else.

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

Re: [PATCH 3/3] regulator: qcom-rpm: Regulator driver for the Qualcomm RPM

[Bjorn Andersson]

On Wed, May 28, 2014 at 9:55 AM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, May 27, 2014 at 10:28:41AM -0700, Bjorn Andersson wrote:
>
>> +static int rpm_reg_set_voltage(struct regulator_dev *rdev,
>> +                            int min_uV, int max_uV,
>> +                            unsigned *selector)
>> +{
>> +     struct qcom_rpm_reg *vreg = rdev_get_drvdata(rdev);
>> +     const struct rpm_reg_parts *parts = vreg->parts;
>> +     const struct request_member *req;
>> +     int ret = 0;
>> +     int sel;
>> +     int val;
>> +     int uV;
>> +
>> +     dev_dbg(vreg->dev, "set_voltage(%d, %d)\n", min_uV, max_uV);
>> +
>> +     if (parts->uV.mask == 0 && parts->mV.mask == 0)
>> +             return -EINVAL;
>> +
>> +     /*
>> +      * Snap to the voltage to a supported level.
>> +      */
>
> What is "snapping" a voltage?
>

I pass the requested voltage to the RPM, but it's only allowed to have valid
values, so I need to "clamp"/"snap"/"round" the voltage to a valid number.

>> +     sel = regulator_map_voltage_linear_range(rdev, min_uV, max_uV);
>
> Don't open code mapping the voltage, just implement set_voltage_sel().
>

I used ot open code this part, but changed it to
regulator_map_voltage_linear_range once I had implemented list_voltages. But as
you say I should be able to replace the first half of my set_voltage with
set_voltage_sel; and then do the transition from sel to requested voltage and
send that over.

[...]

>> +     mutex_lock(&vreg->lock);
>> +     if (parts->uV.mask)
>> +             req = &parts->uV;
>> +     else if (parts->mV.mask)
>> +             req = &parts->mV;
>> +     else
>> +             req = &parts->enable_state;
>
> Just implement separate operations for the regulators with different
> register definitions.  It's going to simplify the code.
>

Currently I can use e.g. ldo_ops for all the different ldos, if I split this I
need to split the ops as well. So it will for sure be more code, but I will
give it a spin and see how it looks like.

>> +static int rpm_reg_set_mode(struct regulator_dev *rdev, unsigned int mode)
>> +{
>> +     struct qcom_rpm_reg *vreg = rdev_get_drvdata(rdev);
>> +     const struct rpm_reg_parts *parts = vreg->parts;
>> +     int max_mA = parts->ip.mask >> parts->ip.shift;
>> +     int load_mA;
>> +     int ret;
>> +
>> +     if (mode == REGULATOR_MODE_IDLE)
>> +             load_mA = vreg->idle_uA / 1000;
>> +     else
>> +             load_mA = vreg->normal_uA / 1000;
>> +
>> +     if (load_mA > max_mA)
>> +             load_mA = max_mA;
>> +
>> +     mutex_lock(&vreg->lock);
>> +     ret = rpm_reg_write(vreg, &parts->ip, load_mA);
>> +     if (!ret)
>> +             vreg->mode = mode;
>> +     mutex_unlock(&vreg->lock);
>
> I'm not entirely sure what this is intended to do.  It looks like it's
> doing something to do with current limits but it's a set_mode().  Some
> documentation might help.  It should also be implementing only specific
> modes and rejecting unsupported modes, if we do the same operation to
> the device for two different modes that seems wrong.
>

The hardware in this case is a "pmic" shared by all cpus in the system, so when
we set the voltage, state or load of a regulator we merely case a vote. For
voltage and state this is not an issue, but for load the value that's
interesting for the "pmic" is the sum of the votes; i.e. the sum of the loads
from all systems on a single regulator.

What the code does here is to follow the hack found at codeaurora, that upon
get_optimum_mode we guess that the client will call get_optimum_mode followed
my set_mode. We keep the track of what load was last requested and use that in
our vote.


One alternative might be to use "force-mode" to actually set the mode of the
regulator, but I will need to run that by the Qualcomm guys to get an answer if
that would work (as it's not used by codeaurora).

>> +static unsigned int rpm_reg_get_optimum_mode(struct regulator_dev *rdev,
>> +                                          int input_uV,
>> +                                          int output_uV,
>> +                                          int load_uA)
>> +{
>> +     struct qcom_rpm_reg *vreg = rdev_get_drvdata(rdev);
>> +
>> +     load_uA += vreg->load_bias_uA;
>> +
>> +     if (load_uA < vreg->hpm_threshold_uA) {
>> +             vreg->idle_uA = load_uA;
>> +             return REGULATOR_MODE_IDLE;
>> +     } else {
>> +             vreg->normal_uA = load_uA;
>> +             return REGULATOR_MODE_NORMAL;
>> +     }
>> +}
>
> This looks very broken - why are we storing anything here?  What is the
> stored value supposed to do?
>

See above.

>> +     if (vreg->parts->ip.mask) {
>> +             initdata->constraints.valid_ops_mask |= REGULATOR_CHANGE_DRMS;
>> +             initdata->constraints.valid_ops_mask |= REGULATOR_CHANGE_MODE;
>> +             initdata->constraints.valid_modes_mask |= REGULATOR_MODE_NORMAL;
>> +             initdata->constraints.valid_modes_mask |= REGULATOR_MODE_IDLE;
>
> No, this is just plain broken.  Constraints are set by the *board*, you
> don't know if these settings are safe on any given board.
>

I can see that these are coming from board files, but I didn't find any example
of how these are supposed to be set when using DT only.

What's happening here is that given what compatible you use, different "parts"
will be selected and based on that this code will or will not be executed;
hence this is defined by what compatible you're using.

But this is of course not obvious, so unless I've missed something I can clean
this up slightly and make the connection to compatible more obvious. Okay?

>> +static int __init rpm_reg_init(void)
>> +{
>> +     return platform_driver_register(&rpm_reg_driver);
>> +}
>> +arch_initcall(rpm_reg_init);
>> +
>> +static void __exit rpm_reg_exit(void)
>> +{
>> +     platform_driver_unregister(&rpm_reg_driver);
>> +}
>> +module_exit(rpm_reg_exit)
>
> module_platform_driver() or if you must bodge the init order at least
> use subsys_initcall() like everyone else.

Okay

Thanks for the review!

Regards,
Bjorn

Re: [PATCH 3/3] regulator: qcom-rpm: Regulator driver for the Qualcomm RPM

[Mark Brown]

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

On Thu, May 29, 2014 at 02:03:40PM -0700, Bjorn Andersson wrote:

Please fix your mailer to word wrap at less than 80 columns so quoted
text is legible.

> The hardware in this case is a "pmic" shared by all cpus in the system, so when
> we set the voltage, state or load of a regulator we merely case a vote. For
> voltage and state this is not an issue, but for load the value that's
> interesting for the "pmic" is the sum of the votes; i.e. the sum of the loads
> from all systems on a single regulator.

> What the code does here is to follow the hack found at codeaurora, that upon
> get_optimum_mode we guess that the client will call get_optimum_mode followed
> my set_mode. We keep the track of what load was last requested and use that in
> our vote.

No, this is awful and there's no way in hell that stuff like this should
be implemented in a driver since there's clearly nothing at all hardware
specific about it.  The load tracking needs to be implemented in the
framework if it's going to be implemented, and passing it up through the
chain is obviously going to need some conversion and accounting for
hardware conversion losses which doesn't seem to be happening here.

I'm still unclear on what the summed current is going to be used for,
though...

> >> +     if (vreg->parts->ip.mask) {
> >> +             initdata->constraints.valid_ops_mask |= REGULATOR_CHANGE_DRMS;
> >> +             initdata->constraints.valid_ops_mask |= REGULATOR_CHANGE_MODE;
> >> +             initdata->constraints.valid_modes_mask |= REGULATOR_MODE_NORMAL;
> >> +             initdata->constraints.valid_modes_mask |= REGULATOR_MODE_IDLE;

> > No, this is just plain broken.  Constraints are set by the *board*, you
> > don't know if these settings are safe on any given board.

> I can see that these are coming from board files, but I didn't find any example
> of how these are supposed to be set when using DT only.

> What's happening here is that given what compatible you use, different "parts"
> will be selected and based on that this code will or will not be executed;
> hence this is defined by what compatible you're using.

> But this is of course not obvious, so unless I've missed something I can clean
> this up slightly and make the connection to compatible more obvious. Okay?

No, it's still just plain broken.  You've no idea if the settings you're
providing work or not on a given system (or set of drivers for that
matter) - mode configuration really is a part of the system integration
not an unchanging part of the PMIC.  This code appears to assume that
every client driver (plus passives) is going to accurately supply load
information which just isn't realistic except in very controlled cases
for a specific system.

The reason it's not supported in DT at the minute is that the definition
of modes is not at all clear in a generic fashion, plus of course the
fact that we have no users in mainline for dynamic mode setting.  Most
PMICs these days are smart enough to do this autonomously anyway so it's
not clear that this is something that it's worth spending time on.

Please look for the prior threads on this.

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

Re: [PATCH 3/3] regulator: qcom-rpm: Regulator driver for the Qualcomm RPM

[Bjorn Andersson]

On Thu, May 29, 2014 at 2:18 PM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, May 29, 2014 at 02:03:40PM -0700, Bjorn Andersson wrote:
>
> Please fix your mailer to word wrap at less than 80 columns so quoted
> text is legible.
>
>> The hardware in this case is a "pmic" shared by all cpus in the system, so when
>> we set the voltage, state or load of a regulator we merely case a vote. For
>> voltage and state this is not an issue, but for load the value that's
>> interesting for the "pmic" is the sum of the votes; i.e. the sum of the loads
>> from all systems on a single regulator.
>
>> What the code does here is to follow the hack found at codeaurora, that upon
>> get_optimum_mode we guess that the client will call get_optimum_mode followed
>> my set_mode. We keep the track of what load was last requested and use that in
>> our vote.
>
> No, this is awful and there's no way in hell that stuff like this should
> be implemented in a driver since there's clearly nothing at all hardware
> specific about it.  The load tracking needs to be implemented in the
> framework if it's going to be implemented, and passing it up through the
> chain is obviously going to need some conversion and accounting for
> hardware conversion losses which doesn't seem to be happening here.
>
> I'm still unclear on what the summed current is going to be used for,
> though...
>

You do load accumlation of all the requests from the drivers of the Linux
system, but in the Qualcomm system there might be load from the modem or the
sensor co-processor that we don't know about here. So additional accumulation
is done by the "pmic" - that is directly accessed by those other systems as
well.

So here we don't set the mode of the regulator, we tell the "pmic" our part and
then it can accumulate further.


I understand your strong opinions regarding this, so I will respin this to
forcefully set the regulator mode intead of merely casting a vote.  I.e.
implement set_mode to actually set the mode.

>> >> +     if (vreg->parts->ip.mask) {
>> >> +             initdata->constraints.valid_ops_mask |= REGULATOR_CHANGE_DRMS;
>> >> +             initdata->constraints.valid_ops_mask |= REGULATOR_CHANGE_MODE;
>> >> +             initdata->constraints.valid_modes_mask |= REGULATOR_MODE_NORMAL;
>> >> +             initdata->constraints.valid_modes_mask |= REGULATOR_MODE_IDLE;
>
>> > No, this is just plain broken.  Constraints are set by the *board*, you
>> > don't know if these settings are safe on any given board.
>
>> I can see that these are coming from board files, but I didn't find any example
>> of how these are supposed to be set when using DT only.
>
>> What's happening here is that given what compatible you use, different "parts"
>> will be selected and based on that this code will or will not be executed;
>> hence this is defined by what compatible you're using.
>
>> But this is of course not obvious, so unless I've missed something I can clean
>> this up slightly and make the connection to compatible more obvious. Okay?
>
> No, it's still just plain broken.  You've no idea if the settings you're
> providing work or not on a given system (or set of drivers for that
> matter) - mode configuration really is a part of the system integration
> not an unchanging part of the PMIC.  This code appears to assume that
> every client driver (plus passives) is going to accurately supply load
> information which just isn't realistic except in very controlled cases
> for a specific system.
>
> The reason it's not supported in DT at the minute is that the definition
> of modes is not at all clear in a generic fashion, plus of course the
> fact that we have no users in mainline for dynamic mode setting.  Most
> PMICs these days are smart enough to do this autonomously anyway so it's
> not clear that this is something that it's worth spending time on.
>
> Please look for the prior threads on this.

At the time of implementing this the proposed sdhci driver from Qualcomm called
regulator_set_mode, but that got redesigned not to fiddle with
regulators. So                                                     it
seems you're right, no-one ever calls regulator_set_mode in mainline
and
hence doesn't have this problem.

So the only example I can find is lp872x, that for buck* does this exactly like
I do.

But as there are no users anymore, I could just let the constraints part go for
now and once we've figured out the dt part there will be some way of setting
these. Okay?

Regards,
Bjorn

Re: [PATCH 3/3] regulator: qcom-rpm: Regulator driver for the Qualcomm RPM

[Mark Brown]

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

On Thu, May 29, 2014 at 02:59:38PM -0700, Bjorn Andersson wrote:
> On Thu, May 29, 2014 at 2:18 PM, Mark Brown <broonie@kernel.org> wrote:

> > No, this is awful and there's no way in hell that stuff like this should
> > be implemented in a driver since there's clearly nothing at all hardware
> > specific about it.  The load tracking needs to be implemented in the
> > framework if it's going to be implemented, and passing it up through the
> > chain is obviously going to need some conversion and accounting for
> > hardware conversion losses which doesn't seem to be happening here.
> >
> > I'm still unclear on what the summed current is going to be used for,
> > though...

> You do load accumlation of all the requests from the drivers of the Linux
> system, but in the Qualcomm system there might be load from the modem or the
> sensor co-processor that we don't know about here. So additional accumulation
> is done by the "pmic" - that is directly accessed by those other systems as
> well.

So the resulting load is then set directly in hardware instead of
setting a mode?  That would be totally fine but it doesn't free us from
having the logic for accumilating the current we know about in the core;
that's the bit that's just at completely the wrong abstraction layer.

> I understand your strong opinions regarding this, so I will respin this to
> forcefully set the regulator mode intead of merely casting a vote.  I.e.
> implement set_mode to actually set the mode.

Or just don't implement mode setting if it's only used by this crazy
stuff.

> But as there are no users anymore, I could just let the constraints part go for
> now and once we've figured out the dt part there will be some way of setting
> these. Okay?

Yes, that's fine.

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

Re: [PATCH 0/3] Qualcomm Resource Power Manager driver

[Kumar Gala]

On May 27, 2014, at 12:28 PM, Bjorn Andersson <bjorn.andersson@sonymobile.com> wrote:

> This series adds a regulator driver for the Resource Power Manager found in
> Qualcomm 8660, 8960 and 8064 based devices.
> 
> The RPM driver exposes resources to its child devices, that can be accessed to
> implement drivers for the regulators, clocks and bus frequency control that's
> owned by the RPM in these devices.

Rather than adding yet another mfd driver, how about we put this in drivers/soc/qcom as a much better location for the low level rpm code.  Some code already merged in arm-soc for creation of drivers/soc/qcom/

> 
> Bjorn Andersson (3):
>  mfd: devicetree: bindings: Add Qualcomm RPM DT binding
>  mfd: qcom-rpm: Driver for the Qualcomm RPM
>  regulator: qcom-rpm: Regulator driver for the Qualcomm RPM
> 
> Documentation/devicetree/bindings/mfd/qcom,rpm.txt | 283 +++++++
> drivers/mfd/Kconfig                                |  15 +
> drivers/mfd/Makefile                               |   1 +
> drivers/mfd/qcom_rpm.c                             | 554 ++++++++++++++
> drivers/regulator/Kconfig                          |  12 +
> drivers/regulator/Makefile                         |   1 +
> drivers/regulator/qcom_rpm-regulator.c             | 852 +++++++++++++++++++++
> include/dt-bindings/mfd/qcom_rpm.h                 | 148 ++++
> include/linux/mfd/qcom_rpm.h                       |  13 +
> 9 files changed, 1879 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/qcom,rpm.txt
> create mode 100644 drivers/mfd/qcom_rpm.c
> create mode 100644 drivers/regulator/qcom_rpm-regulator.c
> create mode 100644 include/dt-bindings/mfd/qcom_rpm.h
> create mode 100644 include/linux/mfd/qcom_rpm.h

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 0/3] Qualcomm Resource Power Manager driver

[Bjorn Andersson]

On Wed, May 28, 2014 at 9:23 AM, Kumar Gala <galak@codeaurora.org> wrote:
>
> On May 27, 2014, at 12:28 PM, Bjorn Andersson <bjorn.andersson@sonymobile.com> wrote:
>
>> This series adds a regulator driver for the Resource Power Manager found in
>> Qualcomm 8660, 8960 and 8064 based devices.
>>
>> The RPM driver exposes resources to its child devices, that can be accessed to
>> implement drivers for the regulators, clocks and bus frequency control that's
>> owned by the RPM in these devices.
>
> Rather than adding yet another mfd driver, how about we put this in drivers/soc/qcom as a much better location for the low level rpm code.  Some code already merged in arm-soc for creation of drivers/soc/qcom/

Hi Kumar,

I do see rpm as somewhat equivalent to a pmic and that was why I
followed suite and put it in mfd, but I can of course move it if you
prefer.


Lately I've been working on rpm, rpm-smd, smem, smd, smsm, smp2p
patches for mainline.
It could be argued that smd is a bus and should go in drivers/bus, but
for the rest I fear that we just created drivers/soc/qcom as another
dumping ground for things; a "Qualcomm specific drivers/mfd".

But maybe that is the purpose of it ;)


If I move the rpm driver, are there any conclusion to where I should
move the dt binding documentation?

Regards,
Bjorn

Re: [PATCH 0/3] Qualcomm Resource Power Manager driver

[Kumar Gala]

On May 28, 2014, at 11:59 AM, Bjorn Andersson <bjorn@kryo.se> wrote:

> On Wed, May 28, 2014 at 9:23 AM, Kumar Gala <galak@codeaurora.org> wrote:
>> 
>> On May 27, 2014, at 12:28 PM, Bjorn Andersson <bjorn.andersson@sonymobile.com> wrote:
>> 
>>> This series adds a regulator driver for the Resource Power Manager found in
>>> Qualcomm 8660, 8960 and 8064 based devices.
>>> 
>>> The RPM driver exposes resources to its child devices, that can be accessed to
>>> implement drivers for the regulators, clocks and bus frequency control that's
>>> owned by the RPM in these devices.
>> 
>> Rather than adding yet another mfd driver, how about we put this in drivers/soc/qcom as a much better location for the low level rpm code.  Some code already merged in arm-soc for creation of drivers/soc/qcom/
> 
> Hi Kumar,
> 
> I do see rpm as somewhat equivalent to a pmic and that was why I
> followed suite and put it in mfd, but I can of course move it if you
> prefer.
> 
> 
> Lately I've been working on rpm, rpm-smd, smem, smd, smsm, smp2p
> patches for mainline.
> It could be argued that smd is a bus and should go in drivers/bus, but
> for the rest I fear that we just created drivers/soc/qcom as another
> dumping ground for things; a "Qualcomm specific drivers/mfd".
> 
> But maybe that is the purpose of it ;)

It is the purpose so that as we see common patterns between either drivers/soc/<VENDOR> we can refactor in the future.  However, we need to all a little time for those patterns to emerge rather than shoe horning in drivers into places that don’t make sense.

> 
> If I move the rpm driver, are there any conclusion to where I should
> move the dt binding documentation?

devicetree/bindings/soc/qcom
include/dt-bindings/soc

> 
> Regards,
> Bjorn

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 0/3] Qualcomm Resource Power Manager driver

[Bjorn Andersson]

On Wed, May 28, 2014 at 10:06 AM, Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> It is the purpose so that as we see common patterns between either drivers/soc/<VENDOR> we can refactor in the future.  However, we need to all a little time for those patterns to emerge rather than shoe horning in drivers into places that don’t make sense.

Sounds reasonable, I'll move it for v2.

>
>>
>> If I move the rpm driver, are there any conclusion to where I should
>> move the dt binding documentation?
>
> devicetree/bindings/soc/qcom
> include/dt-bindings/soc

Thanks.

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] Qualcomm Resource Power Manager driver

[Stanimir Varbanov]

Hi Bjorn,

Thanks for the patches.

<snip>

> Lately I've been working on rpm, rpm-smd, smem, smd, smsm, smp2p
> patches for mainline.
> It could be argued that smd is a bus and should go in drivers/bus, but
> for the rest I fear that we just created drivers/soc/qcom as another
> dumping ground for things; a "Qualcomm specific drivers/mfd".

Do we have some obstacles to not use a "common mailbox framework" for
this IPC (shared memory) communication. It looks that it fits very well
with our needs for this rpm-regulator and future submitting of smd driver?

-- 
regards,
Stan

Re: [PATCH 0/3] Qualcomm Resource Power Manager driver

[Mark Brown]

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

On Mon, Jun 02, 2014 at 11:15:24AM +0300, Stanimir Varbanov wrote:

> > Lately I've been working on rpm, rpm-smd, smem, smd, smsm, smp2p
> > patches for mainline.
> > It could be argued that smd is a bus and should go in drivers/bus, but
> > for the rest I fear that we just created drivers/soc/qcom as another
> > dumping ground for things; a "Qualcomm specific drivers/mfd".

> Do we have some obstacles to not use a "common mailbox framework" for
> this IPC (shared memory) communication. It looks that it fits very well
> with our needs for this rpm-regulator and future submitting of smd driver?

Jassi (CCed) has been working on just such a thing.

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