[PATCH v4 0/2] regulator: add QCOM RPMh regulator driver

[PATCH v4 0/2] regulator: add QCOM RPMh regulator driver

[David Collins]

This patch series adds a driver and device tree binding documentation for
PMIC regulator control via Resource Power Manager-hardened (RPMh) on some
Qualcomm Technologies, Inc. SoCs such as SDM845.  RPMh is a hardware block
which contains several accelerators which are used to manage various
hardware resources that are shared between the processors of the SoC.  The
final hardware state of a regulator is determined within RPMh by performing
max aggregation of the requests made by all of the processors.

The RPMh regulator driver depends upon the RPMh driver [1] and command DB
driver [2] which are both still undergoing review.  It also depends upon
three recent regulator changes: [3], [4], and [5].

Changes since v3 [6]:
 - Removed support for DT properties qcom,regulator-initial-microvolt
   and qcom,headroom-microvolt
 - Renamed DT property qcom,allowed-drms-modes to be
   qcom,regulator-drms-modes
 - Updated DT binding documentation to mention which common regulator
   bindings can be used for qcom-rpmh-regulator devices
 - Added voltage caching so that voltage requests are only sent to RPMh
   after the regulator has been enabled at least once
 - Changed 'voltage_selector' default value to be -ENOTRECOVERABLE to
   interact with [5]
 - Initialized 'enabled' to -EINVAL so that unused regulators are
   disabled by regulator_late_cleanup()
 - Removed rpmh_regulator_load_default_parameters() as it is no longer
   needed
 - Updated the mode usage description in qcom,rpmh-regulator.h

Changes since v2 [7]:
 - Replaced '_' with '-' in device tree supply property names
 - Renamed qcom_rpmh-regulator.c to be qcom-rpmh-regulator.c
 - Updated various DT property names to use "microvolt" and "microamp"
 - Moved allowed modes constraint specification out of the driver [4]
 - Replaced rpmh_client with device pointer to match new RPMh API [1]
 - Corrected drms mode threshold checking
 - Initialized voltage_selector to -EINVAL when not specified in DT
 - Added constants for PMIC regulator hardware modes
 - Corrected type sign of mode mapping tables
 - Made variable names for mode arrays plural
 - Simplified Kconfig depends on
 - Removed unnecessary constants and struct fields
 - Added some descriptive comments

Changes since v1 [8]:
 - Addressed review feedback from Doug, Mark, and Stephen
 - Replaced set_voltage()/get_voltage() callbacks with set_voltage_sel()/
   get_voltage_sel()
 - Added set_bypass()/get_bypass() callbacks for BOB pass-through mode
   control
 - Removed top-level PMIC data structures
 - Removed initialization variables from structs and passed them as
   function parameters
 - Removed various comments and error messages
 - Simplified mode handling
 - Refactored per-PMIC rpmh-regulator data specification
 - Simplified probe function
 - Moved header into DT patch
 - Removed redundant property listings from DT binding documentation

[1]: https://lkml.org/lkml/2018/5/9/729
[2]: https://lkml.org/lkml/2018/4/10/714
[3]: https://lkml.org/lkml/2018/4/18/556
[4]: https://lkml.org/lkml/2018/5/11/696
[5]: https://lkml.org/lkml/2018/5/15/1005
[6]: https://lkml.org/lkml/2018/5/11/701
[7]: https://lkml.org/lkml/2018/4/13/687
[8]: https://lkml.org/lkml/2018/3/16/1431

David Collins (2):
  regulator: dt-bindings: add QCOM RPMh regulator bindings
  regulator: add QCOM RPMh regulator driver

 .../bindings/regulator/qcom,rpmh-regulator.txt     | 185 +++++
 drivers/regulator/Kconfig                          |   9 +
 drivers/regulator/Makefile                         |   1 +
 drivers/regulator/qcom-rpmh-regulator.c            | 865 +++++++++++++++++++++
 .../dt-bindings/regulator/qcom,rpmh-regulator.h    |  36 +
 5 files changed, 1096 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
 create mode 100644 drivers/regulator/qcom-rpmh-regulator.c
 create mode 100644 include/dt-bindings/regulator/qcom,rpmh-regulator.h

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

[PATCH v4 0/2] regulator: add QCOM RPMh regulator driver

[David Collins]

This patch series adds a driver and device tree binding documentation for
PMIC regulator control via Resource Power Manager-hardened (RPMh) on some
Qualcomm Technologies, Inc. SoCs such as SDM845.  RPMh is a hardware block
which contains several accelerators which are used to manage various
hardware resources that are shared between the processors of the SoC.  The
final hardware state of a regulator is determined within RPMh by performing
max aggregation of the requests made by all of the processors.

The RPMh regulator driver depends upon the RPMh driver [1] and command DB
driver [2] which are both still undergoing review.  It also depends upon
three recent regulator changes: [3], [4], and [5].

Changes since v3 [6]:
 - Removed support for DT properties qcom,regulator-initial-microvolt
   and qcom,headroom-microvolt
 - Renamed DT property qcom,allowed-drms-modes to be
   qcom,regulator-drms-modes
 - Updated DT binding documentation to mention which common regulator
   bindings can be used for qcom-rpmh-regulator devices
 - Added voltage caching so that voltage requests are only sent to RPMh
   after the regulator has been enabled at least once
 - Changed 'voltage_selector' default value to be -ENOTRECOVERABLE to
   interact with [5]
 - Initialized 'enabled' to -EINVAL so that unused regulators are
   disabled by regulator_late_cleanup()
 - Removed rpmh_regulator_load_default_parameters() as it is no longer
   needed
 - Updated the mode usage description in qcom,rpmh-regulator.h

Changes since v2 [7]:
 - Replaced '_' with '-' in device tree supply property names
 - Renamed qcom_rpmh-regulator.c to be qcom-rpmh-regulator.c
 - Updated various DT property names to use "microvolt" and "microamp"
 - Moved allowed modes constraint specification out of the driver [4]
 - Replaced rpmh_client with device pointer to match new RPMh API [1]
 - Corrected drms mode threshold checking
 - Initialized voltage_selector to -EINVAL when not specified in DT
 - Added constants for PMIC regulator hardware modes
 - Corrected type sign of mode mapping tables
 - Made variable names for mode arrays plural
 - Simplified Kconfig depends on
 - Removed unnecessary constants and struct fields
 - Added some descriptive comments

Changes since v1 [8]:
 - Addressed review feedback from Doug, Mark, and Stephen
 - Replaced set_voltage()/get_voltage() callbacks with set_voltage_sel()/
   get_voltage_sel()
 - Added set_bypass()/get_bypass() callbacks for BOB pass-through mode
   control
 - Removed top-level PMIC data structures
 - Removed initialization variables from structs and passed them as
   function parameters
 - Removed various comments and error messages
 - Simplified mode handling
 - Refactored per-PMIC rpmh-regulator data specification
 - Simplified probe function
 - Moved header into DT patch
 - Removed redundant property listings from DT binding documentation

[1]: https://lkml.org/lkml/2018/5/9/729
[2]: https://lkml.org/lkml/2018/4/10/714
[3]: https://lkml.org/lkml/2018/4/18/556
[4]: https://lkml.org/lkml/2018/5/11/696
[5]: https://lkml.org/lkml/2018/5/15/1005
[6]: https://lkml.org/lkml/2018/5/11/701
[7]: https://lkml.org/lkml/2018/4/13/687
[8]: https://lkml.org/lkml/2018/3/16/1431

David Collins (2):
  regulator: dt-bindings: add QCOM RPMh regulator bindings
  regulator: add QCOM RPMh regulator driver

 .../bindings/regulator/qcom,rpmh-regulator.txt     | 185 +++++
 drivers/regulator/Kconfig                          |   9 +
 drivers/regulator/Makefile                         |   1 +
 drivers/regulator/qcom-rpmh-regulator.c            | 865 +++++++++++++++++++++
 .../dt-bindings/regulator/qcom,rpmh-regulator.h    |  36 +
 5 files changed, 1096 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
 create mode 100644 drivers/regulator/qcom-rpmh-regulator.c
 create mode 100644 include/dt-bindings/regulator/qcom,rpmh-regulator.h

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

[PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[David Collins]

Introduce bindings for RPMh regulator devices found on some
Qualcomm Technlogies, Inc. SoCs.  These devices allow a given
processor within the SoC to make PMIC regulator requests which
are aggregated within the RPMh hardware block along with requests
from other processors in the SoC to determine the final PMIC
regulator hardware state.

Signed-off-by: David Collins <collinsd@codeaurora.org>
---
 .../bindings/regulator/qcom,rpmh-regulator.txt     | 185 +++++++++++++++++++++
 .../dt-bindings/regulator/qcom,rpmh-regulator.h    |  36 ++++
 2 files changed, 221 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
 create mode 100644 include/dt-bindings/regulator/qcom,rpmh-regulator.h

diff --git a/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
new file mode 100644
index 0000000..aaac975
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
@@ -0,0 +1,185 @@
+Qualcomm Technologies, Inc. RPMh Regulators
+
+rpmh-regulator devices support PMIC regulator management via the Voltage
+Regulator Manager (VRM) and Oscillator Buffer (XOB) RPMh accelerators.  The APPS
+processor communicates with these hardware blocks via a Resource State
+Coordinator (RSC) using command packets.  The VRM allows changing three
+parameters for a given regulator: enable state, output voltage, and operating
+mode.  The XOB allows changing only a single parameter for a given regulator:
+its enable state.  Despite its name, the XOB is capable of controlling the
+enable state of any PMIC peripheral.  It is used for clock buffers, low-voltage
+switches, and LDO/SMPS regulators which have a fixed voltage and mode.
+
+=======================
+Required Node Structure
+=======================
+
+RPMh regulators must be described in two levels of device nodes.  The first
+level describes the PMIC containing the regulators and must reside within an
+RPMh device node.  The second level describes each regulator within the PMIC
+which is to be used on the board.  Each of these regulators maps to a single
+RPMh resource.
+
+The names used for regulator nodes must match those supported by a given PMIC.
+Supported regulator node names:
+	PM8998:		smps1 - smps13, ldo1 - ldo28, lvs1 - lvs2
+	PMI8998:	bob
+	PM8005:		smps1 - smps4
+
+========================
+First Level Nodes - PMIC
+========================
+
+- compatible
+	Usage:      required
+	Value type: <string>
+	Definition: Must be one of: "qcom,pm8998-rpmh-regulators",
+		    "qcom,pmi8998-rpmh-regulators" or
+		    "qcom,pm8005-rpmh-regulators".
+
+- qcom,pmic-id
+	Usage:      required
+	Value type: <string>
+	Definition: RPMh resource name suffix used for the regulators found on
+		    this PMIC.  Typical values: "a", "b", "c", "d", "e", "f".
+
+- vdd-s1-supply
+- vdd-s2-supply
+- vdd-s3-supply
+- vdd-s4-supply
+	Usage:      optional (PM8998 and PM8005 only)
+	Value type: <phandle>
+	Definition: phandle of the parent supply regulator of one or more of the
+		    regulators for this PMIC.
+
+- vdd-s5-supply
+- vdd-s6-supply
+- vdd-s7-supply
+- vdd-s8-supply
+- vdd-s9-supply
+- vdd-s10-supply
+- vdd-s11-supply
+- vdd-s12-supply
+- vdd-s13-supply
+- vdd-l1-l27-supply
+- vdd-l2-l8-l17-supply
+- vdd-l3-l11-supply
+- vdd-l4-l5-supply
+- vdd-l6-supply
+- vdd-l7-l12-l14-l15-supply
+- vdd-l9-supply
+- vdd-l10-l23-l25-supply
+- vdd-l13-l19-l21-supply
+- vdd-l16-l28-supply
+- vdd-l18-l22-supply
+- vdd-l20-l24-supply
+- vdd-l26-supply
+- vin-lvs-1-2-supply
+	Usage:      optional (PM8998 only)
+	Value type: <phandle>
+	Definition: phandle of the parent supply regulator of one or more of the
+		    regulators for this PMIC.
+
+- vdd-bob-supply
+	Usage:      optional (PMI8998 only)
+	Value type: <phandle>
+	Definition: BOB regulator parent supply phandle
+
+===============================
+Second Level Nodes - Regulators
+===============================
+
+- qcom,regulator-drms-modes
+	Usage:      required if regulator-allow-set-load is specified;
+		    VRM regulators only
+	Value type: <prop-encoded-array>
+	Definition: A list of integers specifying the PMIC regulator modes which
+		    can be configured at runtime based upon consumer load needs.
+		    Supported values are RPMH_REGULATOR_MODE_* which are defined
+		    in [1] (i.e. 0 to 3).
+
+- qcom,drms-mode-max-microamps
+	Usage:      required if regulator-allow-set-load is specified;
+		    VRM regulators only
+	Value type: <prop-encoded-array>
+	Definition: A list of integers specifying the maximum allowed load
+		    current in microamps for each of the modes listed in
+		    qcom,regulator-drms-modes (matched 1-to-1 in order).
+		    Elements must be specified in order from lowest to highest
+		    value.
+
+- qcom,always-wait-for-ack
+	Usage:      optional
+	Value type: <empty>
+	Definition: Boolean flag which indicates that the application processor
+		    must wait for an ACK or a NACK from RPMh for every request
+		    sent for this regulator including those which are for a
+		    strictly lower power state.
+
+Other properties defined in Documentation/devicetree/bindings/regulator.txt
+may also be used.  regulator-initial-mode and regulator-allowed-modes may be
+specified for VRM regulators using mode values from [1].  regulator-allow-bypass
+may be specified for BOB type regulators managed via VRM.
+regulator-allow-set-load may be specified for VRM regulators.  It is used in
+conjunction with qcom,regulator-drms-modes and qcom,drms-mode-max-microamps to
+define a dynamic total load current to mode mapping.
+
+[1] include/dt-bindings/regulator/qcom,rpmh-regulator.h
+
+========
+Examples
+========
+
+#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
+
+&apps_rsc {
+	pm8998-rpmh-regulators {
+		compatible = "qcom,pm8998-rpmh-regulators";
+		qcom,pmic-id = "a";
+
+		vdd-l7-l12-l14-l15-supply = <&pm8998_s5>;
+
+		smps2 {
+			regulator-min-microvolt = <1100000>;
+			regulator-max-microvolt = <1100000>;
+		};
+
+		pm8998_s5: smps5 {
+			regulator-min-microvolt = <1904000>;
+			regulator-max-microvolt = <2040000>;
+		};
+
+		ldo7 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_LPM>;
+			regulator-allowed-modes =
+				<RPMH_REGULATOR_MODE_LPM
+				 RPMH_REGULATOR_MODE_HPM>;
+			regulator-allow-set-load;
+			qcom,regulator-drms-modes =
+				<RPMH_REGULATOR_MODE_LPM
+				 RPMH_REGULATOR_MODE_HPM>;
+			qcom,drms-mode-max-microamps = <10000 1000000>;
+		};
+
+		lvs1 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+	};
+
+	pmi8998-rpmh-regulators {
+		compatible = "qcom,pmi8998-rpmh-regulators";
+		qcom,pmic-id = "b";
+
+		bob {
+			regulator-min-microvolt = <3312000>;
+			regulator-max-microvolt = <3600000>;
+			regulator-allowed-modes =
+				<RPMH_REGULATOR_MODE_AUTO
+				 RPMH_REGULATOR_MODE_HPM>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_AUTO>;
+		};
+	};
+};
diff --git a/include/dt-bindings/regulator/qcom,rpmh-regulator.h b/include/dt-bindings/regulator/qcom,rpmh-regulator.h
new file mode 100644
index 0000000..86713dc
--- /dev/null
+++ b/include/dt-bindings/regulator/qcom,rpmh-regulator.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2018, The Linux Foundation. All rights reserved. */
+
+#ifndef __QCOM_RPMH_REGULATOR_H
+#define __QCOM_RPMH_REGULATOR_H
+
+/*
+ * These mode constants may be used to specify modes for various RPMh regulator
+ * device tree properties (e.g. regulator-initial-mode).  Each type of regulator
+ * supports a subset of the possible modes.
+ *
+ * %RPMH_REGULATOR_MODE_RET:	Retention mode in which only an extremely small
+ *				load current is allowed.  This mode is supported
+ *				by LDO and SMPS type regulators.
+ * %RPMH_REGULATOR_MODE_LPM:	Low power mode in which a small load current is
+ *				allowed.  This mode corresponds to PFM for SMPS
+ *				and BOB type regulators.  This mode is supported
+ *				by LDO, HFSMPS, BOB, and PMIC4 FTSMPS type
+ *				regulators.
+ * %RPMH_REGULATOR_MODE_AUTO:	Auto mode in which the regulator hardware
+ *				automatically switches between LPM and HPM based
+ *				upon the real-time load current.  This mode is
+ *				supported by HFSMPS, BOB, and PMIC4 FTSMPS type
+ *				regulators.
+ * %RPMH_REGULATOR_MODE_HPM:	High power mode in which the full rated current
+ *				of the regulator is allowed.  This mode
+ *				corresponds to PWM for SMPS and BOB type
+ *				regulators.  This mode is supported by all types
+ *				of regulators.
+ */
+#define RPMH_REGULATOR_MODE_RET		0
+#define RPMH_REGULATOR_MODE_LPM		1
+#define RPMH_REGULATOR_MODE_AUTO	2
+#define RPMH_REGULATOR_MODE_HPM		3
+
+#endif
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[David Collins]

Introduce bindings for RPMh regulator devices found on some
Qualcomm Technlogies, Inc. SoCs.  These devices allow a given
processor within the SoC to make PMIC regulator requests which
are aggregated within the RPMh hardware block along with requests
from other processors in the SoC to determine the final PMIC
regulator hardware state.

Signed-off-by: David Collins <collinsd@codeaurora.org>
---
 .../bindings/regulator/qcom,rpmh-regulator.txt     | 185 +++++++++++++++++++++
 .../dt-bindings/regulator/qcom,rpmh-regulator.h    |  36 ++++
 2 files changed, 221 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
 create mode 100644 include/dt-bindings/regulator/qcom,rpmh-regulator.h

diff --git a/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
new file mode 100644
index 0000000..aaac975
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
@@ -0,0 +1,185 @@
+Qualcomm Technologies, Inc. RPMh Regulators
+
+rpmh-regulator devices support PMIC regulator management via the Voltage
+Regulator Manager (VRM) and Oscillator Buffer (XOB) RPMh accelerators.  The APPS
+processor communicates with these hardware blocks via a Resource State
+Coordinator (RSC) using command packets.  The VRM allows changing three
+parameters for a given regulator: enable state, output voltage, and operating
+mode.  The XOB allows changing only a single parameter for a given regulator:
+its enable state.  Despite its name, the XOB is capable of controlling the
+enable state of any PMIC peripheral.  It is used for clock buffers, low-voltage
+switches, and LDO/SMPS regulators which have a fixed voltage and mode.
+
+=======================
+Required Node Structure
+=======================
+
+RPMh regulators must be described in two levels of device nodes.  The first
+level describes the PMIC containing the regulators and must reside within an
+RPMh device node.  The second level describes each regulator within the PMIC
+which is to be used on the board.  Each of these regulators maps to a single
+RPMh resource.
+
+The names used for regulator nodes must match those supported by a given PMIC.
+Supported regulator node names:
+	PM8998:		smps1 - smps13, ldo1 - ldo28, lvs1 - lvs2
+	PMI8998:	bob
+	PM8005:		smps1 - smps4
+
+========================
+First Level Nodes - PMIC
+========================
+
+- compatible
+	Usage:      required
+	Value type: <string>
+	Definition: Must be one of: "qcom,pm8998-rpmh-regulators",
+		    "qcom,pmi8998-rpmh-regulators" or
+		    "qcom,pm8005-rpmh-regulators".
+
+- qcom,pmic-id
+	Usage:      required
+	Value type: <string>
+	Definition: RPMh resource name suffix used for the regulators found on
+		    this PMIC.  Typical values: "a", "b", "c", "d", "e", "f".
+
+- vdd-s1-supply
+- vdd-s2-supply
+- vdd-s3-supply
+- vdd-s4-supply
+	Usage:      optional (PM8998 and PM8005 only)
+	Value type: <phandle>
+	Definition: phandle of the parent supply regulator of one or more of the
+		    regulators for this PMIC.
+
+- vdd-s5-supply
+- vdd-s6-supply
+- vdd-s7-supply
+- vdd-s8-supply
+- vdd-s9-supply
+- vdd-s10-supply
+- vdd-s11-supply
+- vdd-s12-supply
+- vdd-s13-supply
+- vdd-l1-l27-supply
+- vdd-l2-l8-l17-supply
+- vdd-l3-l11-supply
+- vdd-l4-l5-supply
+- vdd-l6-supply
+- vdd-l7-l12-l14-l15-supply
+- vdd-l9-supply
+- vdd-l10-l23-l25-supply
+- vdd-l13-l19-l21-supply
+- vdd-l16-l28-supply
+- vdd-l18-l22-supply
+- vdd-l20-l24-supply
+- vdd-l26-supply
+- vin-lvs-1-2-supply
+	Usage:      optional (PM8998 only)
+	Value type: <phandle>
+	Definition: phandle of the parent supply regulator of one or more of the
+		    regulators for this PMIC.
+
+- vdd-bob-supply
+	Usage:      optional (PMI8998 only)
+	Value type: <phandle>
+	Definition: BOB regulator parent supply phandle
+
+===============================
+Second Level Nodes - Regulators
+===============================
+
+- qcom,regulator-drms-modes
+	Usage:      required if regulator-allow-set-load is specified;
+		    VRM regulators only
+	Value type: <prop-encoded-array>
+	Definition: A list of integers specifying the PMIC regulator modes which
+		    can be configured at runtime based upon consumer load needs.
+		    Supported values are RPMH_REGULATOR_MODE_* which are defined
+		    in [1] (i.e. 0 to 3).
+
+- qcom,drms-mode-max-microamps
+	Usage:      required if regulator-allow-set-load is specified;
+		    VRM regulators only
+	Value type: <prop-encoded-array>
+	Definition: A list of integers specifying the maximum allowed load
+		    current in microamps for each of the modes listed in
+		    qcom,regulator-drms-modes (matched 1-to-1 in order).
+		    Elements must be specified in order from lowest to highest
+		    value.
+
+- qcom,always-wait-for-ack
+	Usage:      optional
+	Value type: <empty>
+	Definition: Boolean flag which indicates that the application processor
+		    must wait for an ACK or a NACK from RPMh for every request
+		    sent for this regulator including those which are for a
+		    strictly lower power state.
+
+Other properties defined in Documentation/devicetree/bindings/regulator.txt
+may also be used.  regulator-initial-mode and regulator-allowed-modes may be
+specified for VRM regulators using mode values from [1].  regulator-allow-bypass
+may be specified for BOB type regulators managed via VRM.
+regulator-allow-set-load may be specified for VRM regulators.  It is used in
+conjunction with qcom,regulator-drms-modes and qcom,drms-mode-max-microamps to
+define a dynamic total load current to mode mapping.
+
+[1] include/dt-bindings/regulator/qcom,rpmh-regulator.h
+
+========
+Examples
+========
+
+#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
+
+&apps_rsc {
+	pm8998-rpmh-regulators {
+		compatible = "qcom,pm8998-rpmh-regulators";
+		qcom,pmic-id = "a";
+
+		vdd-l7-l12-l14-l15-supply = <&pm8998_s5>;
+
+		smps2 {
+			regulator-min-microvolt = <1100000>;
+			regulator-max-microvolt = <1100000>;
+		};
+
+		pm8998_s5: smps5 {
+			regulator-min-microvolt = <1904000>;
+			regulator-max-microvolt = <2040000>;
+		};
+
+		ldo7 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_LPM>;
+			regulator-allowed-modes =
+				<RPMH_REGULATOR_MODE_LPM
+				 RPMH_REGULATOR_MODE_HPM>;
+			regulator-allow-set-load;
+			qcom,regulator-drms-modes =
+				<RPMH_REGULATOR_MODE_LPM
+				 RPMH_REGULATOR_MODE_HPM>;
+			qcom,drms-mode-max-microamps = <10000 1000000>;
+		};
+
+		lvs1 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+		};
+	};
+
+	pmi8998-rpmh-regulators {
+		compatible = "qcom,pmi8998-rpmh-regulators";
+		qcom,pmic-id = "b";
+
+		bob {
+			regulator-min-microvolt = <3312000>;
+			regulator-max-microvolt = <3600000>;
+			regulator-allowed-modes =
+				<RPMH_REGULATOR_MODE_AUTO
+				 RPMH_REGULATOR_MODE_HPM>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_AUTO>;
+		};
+	};
+};
diff --git a/include/dt-bindings/regulator/qcom,rpmh-regulator.h b/include/dt-bindings/regulator/qcom,rpmh-regulator.h
new file mode 100644
index 0000000..86713dc
--- /dev/null
+++ b/include/dt-bindings/regulator/qcom,rpmh-regulator.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2018, The Linux Foundation. All rights reserved. */
+
+#ifndef __QCOM_RPMH_REGULATOR_H
+#define __QCOM_RPMH_REGULATOR_H
+
+/*
+ * These mode constants may be used to specify modes for various RPMh regulator
+ * device tree properties (e.g. regulator-initial-mode).  Each type of regulator
+ * supports a subset of the possible modes.
+ *
+ * %RPMH_REGULATOR_MODE_RET:	Retention mode in which only an extremely small
+ *				load current is allowed.  This mode is supported
+ *				by LDO and SMPS type regulators.
+ * %RPMH_REGULATOR_MODE_LPM:	Low power mode in which a small load current is
+ *				allowed.  This mode corresponds to PFM for SMPS
+ *				and BOB type regulators.  This mode is supported
+ *				by LDO, HFSMPS, BOB, and PMIC4 FTSMPS type
+ *				regulators.
+ * %RPMH_REGULATOR_MODE_AUTO:	Auto mode in which the regulator hardware
+ *				automatically switches between LPM and HPM based
+ *				upon the real-time load current.  This mode is
+ *				supported by HFSMPS, BOB, and PMIC4 FTSMPS type
+ *				regulators.
+ * %RPMH_REGULATOR_MODE_HPM:	High power mode in which the full rated current
+ *				of the regulator is allowed.  This mode
+ *				corresponds to PWM for SMPS and BOB type
+ *				regulators.  This mode is supported by all types
+ *				of regulators.
+ */
+#define RPMH_REGULATOR_MODE_RET		0
+#define RPMH_REGULATOR_MODE_LPM		1
+#define RPMH_REGULATOR_MODE_AUTO	2
+#define RPMH_REGULATOR_MODE_HPM		3
+
+#endif
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[Rob Herring]

On Tue, May 22, 2018 at 07:43:17PM -0700, David Collins wrote:
> Introduce bindings for RPMh regulator devices found on some
> Qualcomm Technlogies, Inc. SoCs.  These devices allow a given
> processor within the SoC to make PMIC regulator requests which
> are aggregated within the RPMh hardware block along with requests
> from other processors in the SoC to determine the final PMIC
> regulator hardware state.
> 
> Signed-off-by: David Collins <collinsd@codeaurora.org>
> ---
>  .../bindings/regulator/qcom,rpmh-regulator.txt     | 185 +++++++++++++++++++++
>  .../dt-bindings/regulator/qcom,rpmh-regulator.h    |  36 ++++
>  2 files changed, 221 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
>  create mode 100644 include/dt-bindings/regulator/qcom,rpmh-regulator.h

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[Rob Herring]

On Tue, May 22, 2018 at 07:43:17PM -0700, David Collins wrote:
> Introduce bindings for RPMh regulator devices found on some
> Qualcomm Technlogies, Inc. SoCs.  These devices allow a given
> processor within the SoC to make PMIC regulator requests which
> are aggregated within the RPMh hardware block along with requests
> from other processors in the SoC to determine the final PMIC
> regulator hardware state.
> 
> Signed-off-by: David Collins <collinsd@codeaurora.org>
> ---
>  .../bindings/regulator/qcom,rpmh-regulator.txt     | 185 +++++++++++++++++++++
>  .../dt-bindings/regulator/qcom,rpmh-regulator.h    |  36 ++++
>  2 files changed, 221 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
>  create mode 100644 include/dt-bindings/regulator/qcom,rpmh-regulator.h

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[Doug Anderson]

Hi,

On Tue, May 22, 2018 at 7:43 PM, David Collins <collinsd@codeaurora.org> wrote:
> +========
> +Examples
> +========
> +
> +#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> +
> +&apps_rsc {
> +       pm8998-rpmh-regulators {
> +               compatible = "qcom,pm8998-rpmh-regulators";
> +               qcom,pmic-id = "a";
> +
> +               vdd-l7-l12-l14-l15-supply = <&pm8998_s5>;
> +
> +               smps2 {
> +                       regulator-min-microvolt = <1100000>;
> +                       regulator-max-microvolt = <1100000>;
> +               };
> +
> +               pm8998_s5: smps5 {
> +                       regulator-min-microvolt = <1904000>;
> +                       regulator-max-microvolt = <2040000>;
> +               };
> +
> +               ldo7 {
> +                       regulator-min-microvolt = <1800000>;
> +                       regulator-max-microvolt = <1800000>;
> +                       regulator-initial-mode = <RPMH_REGULATOR_MODE_LPM>;
> +                       regulator-allowed-modes =
> +                               <RPMH_REGULATOR_MODE_LPM
> +                                RPMH_REGULATOR_MODE_HPM>;
> +                       regulator-allow-set-load;
> +                       qcom,regulator-drms-modes =
> +                               <RPMH_REGULATOR_MODE_LPM
> +                                RPMH_REGULATOR_MODE_HPM>;
> +                       qcom,drms-mode-max-microamps = <10000 1000000>;

Things look pretty good to me now.  I'm still hesitant about the whole
need to list the modes twice (once using the unordered
"regulator-allowed-modes" and once to match up against the ordered
"qcom,drms-mode-max-microamps").  I'm also still of the opinion that
the whole "drms-mode-max-microamps" ought to be a standard property
(not a qcom specific one) and handled in the regulator core.

However, for both of these things I leave it to the discretion of Mark
to choose what he wants.  Thus assuming Mark is OK with these two
things, feel free to add my Reviewed-by.

-Doug

[PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[Doug Anderson]

Hi,

On Tue, May 22, 2018 at 7:43 PM, David Collins <collinsd@codeaurora.org> wrote:
> +========
> +Examples
> +========
> +
> +#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> +
> +&apps_rsc {
> +       pm8998-rpmh-regulators {
> +               compatible = "qcom,pm8998-rpmh-regulators";
> +               qcom,pmic-id = "a";
> +
> +               vdd-l7-l12-l14-l15-supply = <&pm8998_s5>;
> +
> +               smps2 {
> +                       regulator-min-microvolt = <1100000>;
> +                       regulator-max-microvolt = <1100000>;
> +               };
> +
> +               pm8998_s5: smps5 {
> +                       regulator-min-microvolt = <1904000>;
> +                       regulator-max-microvolt = <2040000>;
> +               };
> +
> +               ldo7 {
> +                       regulator-min-microvolt = <1800000>;
> +                       regulator-max-microvolt = <1800000>;
> +                       regulator-initial-mode = <RPMH_REGULATOR_MODE_LPM>;
> +                       regulator-allowed-modes =
> +                               <RPMH_REGULATOR_MODE_LPM
> +                                RPMH_REGULATOR_MODE_HPM>;
> +                       regulator-allow-set-load;
> +                       qcom,regulator-drms-modes =
> +                               <RPMH_REGULATOR_MODE_LPM
> +                                RPMH_REGULATOR_MODE_HPM>;
> +                       qcom,drms-mode-max-microamps = <10000 1000000>;

Things look pretty good to me now.  I'm still hesitant about the whole
need to list the modes twice (once using the unordered
"regulator-allowed-modes" and once to match up against the ordered
"qcom,drms-mode-max-microamps").  I'm also still of the opinion that
the whole "drms-mode-max-microamps" ought to be a standard property
(not a qcom specific one) and handled in the regulator core.

However, for both of these things I leave it to the discretion of Mark
to choose what he wants.  Thus assuming Mark is OK with these two
things, feel free to add my Reviewed-by.

-Doug

Re: [PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[Mark Brown]

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

On Tue, May 29, 2018 at 10:23:20PM -0700, Doug Anderson wrote:

> > +                       qcom,drms-mode-max-microamps = <10000 1000000>;

> Things look pretty good to me now.  I'm still hesitant about the whole
> need to list the modes twice (once using the unordered
> "regulator-allowed-modes" and once to match up against the ordered
> "qcom,drms-mode-max-microamps").  I'm also still of the opinion that
> the whole "drms-mode-max-microamps" ought to be a standard property
> (not a qcom specific one) and handled in the regulator core.

I'm confused as to why we are specifying the maximum current the device
can deliver in a given mode in the DT - surely that's a fixed property
of the hardware?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[Mark Brown]

On Tue, May 29, 2018 at 10:23:20PM -0700, Doug Anderson wrote:

> > +                       qcom,drms-mode-max-microamps = <10000 1000000>;

> Things look pretty good to me now.  I'm still hesitant about the whole
> need to list the modes twice (once using the unordered
> "regulator-allowed-modes" and once to match up against the ordered
> "qcom,drms-mode-max-microamps").  I'm also still of the opinion that
> the whole "drms-mode-max-microamps" ought to be a standard property
> (not a qcom specific one) and handled in the regulator core.

I'm confused as to why we are specifying the maximum current the device
can deliver in a given mode in the DT - surely that's a fixed property
of the hardware?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180530/a7eec198/attachment-0001.sig>

Re: [PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[Doug Anderson]

Hi,

On Wed, May 30, 2018 at 3:37 AM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, May 29, 2018 at 10:23:20PM -0700, Doug Anderson wrote:
>
>> > +                       qcom,drms-mode-max-microamps = <10000 1000000>;
>
>> Things look pretty good to me now.  I'm still hesitant about the whole
>> need to list the modes twice (once using the unordered
>> "regulator-allowed-modes" and once to match up against the ordered
>> "qcom,drms-mode-max-microamps").  I'm also still of the opinion that
>> the whole "drms-mode-max-microamps" ought to be a standard property
>> (not a qcom specific one) and handled in the regulator core.
>
> I'm confused as to why we are specifying the maximum current the device
> can deliver in a given mode in the DT - surely that's a fixed property
> of the hardware?

Said another way: you're saying that you'd expect the "max-microamps"
table to have one fewer element than the listed modes?  So in the
above example you'd have:

drms-modes: LPM, HPM
max-microamps: 10000

...or in a more complicated case, you could have:

drms-modes: RET, LPM, AUTO, HPM
max-microamps: 1, 100, 10000


Did I understand you correctly?

I'm personally OK with that color for the shed.  I kinda like the
symmetry of having the same number of elements in both lists (and
being able to print an error message if someone ends up asking for too
much current), but it's not a big deal for me either way.

-Doug

[PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[Doug Anderson]

Hi,

On Wed, May 30, 2018 at 3:37 AM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, May 29, 2018 at 10:23:20PM -0700, Doug Anderson wrote:
>
>> > +                       qcom,drms-mode-max-microamps = <10000 1000000>;
>
>> Things look pretty good to me now.  I'm still hesitant about the whole
>> need to list the modes twice (once using the unordered
>> "regulator-allowed-modes" and once to match up against the ordered
>> "qcom,drms-mode-max-microamps").  I'm also still of the opinion that
>> the whole "drms-mode-max-microamps" ought to be a standard property
>> (not a qcom specific one) and handled in the regulator core.
>
> I'm confused as to why we are specifying the maximum current the device
> can deliver in a given mode in the DT - surely that's a fixed property
> of the hardware?

Said another way: you're saying that you'd expect the "max-microamps"
table to have one fewer element than the listed modes?  So in the
above example you'd have:

drms-modes: LPM, HPM
max-microamps: 10000

...or in a more complicated case, you could have:

drms-modes: RET, LPM, AUTO, HPM
max-microamps: 1, 100, 10000


Did I understand you correctly?

I'm personally OK with that color for the shed.  I kinda like the
symmetry of having the same number of elements in both lists (and
being able to print an error message if someone ends up asking for too
much current), but it's not a big deal for me either way.

-Doug

Re: [PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[Mark Brown]

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

On Wed, May 30, 2018 at 07:54:47AM -0700, Doug Anderson wrote:
> On Wed, May 30, 2018 at 3:37 AM, Mark Brown <broonie@kernel.org> wrote:

> > I'm confused as to why we are specifying the maximum current the device
> > can deliver in a given mode in the DT - surely that's a fixed property
> > of the hardware?

> Said another way: you're saying that you'd expect the "max-microamps"
> table to have one fewer element than the listed modes?  So in the
> above example you'd have:

No, I'm saying that I don't know why that property exists at all.  This
sounds like it's intended to be the amount of current the regulator can
deliver in each mode which is normally a design property of the silicon.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[Mark Brown]

On Wed, May 30, 2018 at 07:54:47AM -0700, Doug Anderson wrote:
> On Wed, May 30, 2018 at 3:37 AM, Mark Brown <broonie@kernel.org> wrote:

> > I'm confused as to why we are specifying the maximum current the device
> > can deliver in a given mode in the DT - surely that's a fixed property
> > of the hardware?

> Said another way: you're saying that you'd expect the "max-microamps"
> table to have one fewer element than the listed modes?  So in the
> above example you'd have:

No, I'm saying that I don't know why that property exists at all.  This
sounds like it's intended to be the amount of current the regulator can
deliver in each mode which is normally a design property of the silicon.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180530/ae2343e4/attachment.sig>

Re: [PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[Doug Anderson]

Hi,

On Wed, May 30, 2018 at 8:50 AM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, May 30, 2018 at 07:54:47AM -0700, Doug Anderson wrote:
>> On Wed, May 30, 2018 at 3:37 AM, Mark Brown <broonie@kernel.org> wrote:
>
>> > I'm confused as to why we are specifying the maximum current the device
>> > can deliver in a given mode in the DT - surely that's a fixed property
>> > of the hardware?
>
>> Said another way: you're saying that you'd expect the "max-microamps"
>> table to have one fewer element than the listed modes?  So in the
>> above example you'd have:
>
> No, I'm saying that I don't know why that property exists at all.  This
> sounds like it's intended to be the amount of current the regulator can
> deliver in each mode which is normally a design property of the silicon.

Ah, got it.  So the whole point here is to be able to implement either
the function "set_load" or the function "get_optimum_mode".  We need
some sort of table to convert from current to mode.  That's what this
table does.

My argument to David was that since set_load / get_optimum_mode were
features of the regulator core these should actually be standard
properties and not Qualcomm-specific ones.

-Doug

[PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[Doug Anderson]

Hi,

On Wed, May 30, 2018 at 8:50 AM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, May 30, 2018 at 07:54:47AM -0700, Doug Anderson wrote:
>> On Wed, May 30, 2018 at 3:37 AM, Mark Brown <broonie@kernel.org> wrote:
>
>> > I'm confused as to why we are specifying the maximum current the device
>> > can deliver in a given mode in the DT - surely that's a fixed property
>> > of the hardware?
>
>> Said another way: you're saying that you'd expect the "max-microamps"
>> table to have one fewer element than the listed modes?  So in the
>> above example you'd have:
>
> No, I'm saying that I don't know why that property exists at all.  This
> sounds like it's intended to be the amount of current the regulator can
> deliver in each mode which is normally a design property of the silicon.

Ah, got it.  So the whole point here is to be able to implement either
the function "set_load" or the function "get_optimum_mode".  We need
some sort of table to convert from current to mode.  That's what this
table does.

My argument to David was that since set_load / get_optimum_mode were
features of the regulator core these should actually be standard
properties and not Qualcomm-specific ones.

-Doug

Re: [PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[Mark Brown]

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

On Wed, May 30, 2018 at 09:12:25AM -0700, Doug Anderson wrote:
> On Wed, May 30, 2018 at 8:50 AM, Mark Brown <broonie@kernel.org> wrote:

> > No, I'm saying that I don't know why that property exists at all.  This
> > sounds like it's intended to be the amount of current the regulator can
> > deliver in each mode which is normally a design property of the silicon.

> Ah, got it.  So the whole point here is to be able to implement either
> the function "set_load" or the function "get_optimum_mode".  We need
> some sort of table to convert from current to mode.  That's what this
> table does.

We do need that table, my expectation would be that this table would be
in the driver as it's not something I'd expect to vary between different
systems but rather be a property of the silicon design.  No sense in
every single board having to copy it in.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[Mark Brown]

On Wed, May 30, 2018 at 09:12:25AM -0700, Doug Anderson wrote:
> On Wed, May 30, 2018 at 8:50 AM, Mark Brown <broonie@kernel.org> wrote:

> > No, I'm saying that I don't know why that property exists at all.  This
> > sounds like it's intended to be the amount of current the regulator can
> > deliver in each mode which is normally a design property of the silicon.

> Ah, got it.  So the whole point here is to be able to implement either
> the function "set_load" or the function "get_optimum_mode".  We need
> some sort of table to convert from current to mode.  That's what this
> table does.

We do need that table, my expectation would be that this table would be
in the driver as it's not something I'd expect to vary between different
systems but rather be a property of the silicon design.  No sense in
every single board having to copy it in.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180530/e99e0527/attachment-0001.sig>

Re: [PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[Doug Anderson]

Hi,

On Wed, May 30, 2018 at 9:20 AM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, May 30, 2018 at 09:12:25AM -0700, Doug Anderson wrote:
>> On Wed, May 30, 2018 at 8:50 AM, Mark Brown <broonie@kernel.org> wrote:
>
>> > No, I'm saying that I don't know why that property exists at all.  This
>> > sounds like it's intended to be the amount of current the regulator can
>> > deliver in each mode which is normally a design property of the silicon.
>
>> Ah, got it.  So the whole point here is to be able to implement either
>> the function "set_load" or the function "get_optimum_mode".  We need
>> some sort of table to convert from current to mode.  That's what this
>> table does.
>
> We do need that table, my expectation would be that this table would be
> in the driver as it's not something I'd expect to vary between different
> systems but rather be a property of the silicon design.  No sense in
> every single board having to copy it in.

Ah, got it!  I'd be OK with it being hardcoded in the driver.

At one point I think David was making the argument that some boards
have different noise needs for the rails and thus you might want to
change modes at different currents.  I don't know if this is realistic
but I believe it was part of his original argument for why this needed
to be specified.  If we can hardcode this in the driver I'm fine with
it...  That would actually solve many of my objections too...


-Doug

[PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[Doug Anderson]

Hi,

On Wed, May 30, 2018 at 9:20 AM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, May 30, 2018 at 09:12:25AM -0700, Doug Anderson wrote:
>> On Wed, May 30, 2018 at 8:50 AM, Mark Brown <broonie@kernel.org> wrote:
>
>> > No, I'm saying that I don't know why that property exists at all.  This
>> > sounds like it's intended to be the amount of current the regulator can
>> > deliver in each mode which is normally a design property of the silicon.
>
>> Ah, got it.  So the whole point here is to be able to implement either
>> the function "set_load" or the function "get_optimum_mode".  We need
>> some sort of table to convert from current to mode.  That's what this
>> table does.
>
> We do need that table, my expectation would be that this table would be
> in the driver as it's not something I'd expect to vary between different
> systems but rather be a property of the silicon design.  No sense in
> every single board having to copy it in.

Ah, got it!  I'd be OK with it being hardcoded in the driver.

At one point I think David was making the argument that some boards
have different noise needs for the rails and thus you might want to
change modes at different currents.  I don't know if this is realistic
but I believe it was part of his original argument for why this needed
to be specified.  If we can hardcode this in the driver I'm fine with
it...  That would actually solve many of my objections too...


-Doug

Re: [PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[David Collins]

Hello Mark and Doug,

On 05/30/2018 09:24 AM, Doug Anderson wrote:
> On Wed, May 30, 2018 at 9:20 AM, Mark Brown <broonie@kernel.org> wrote:
>> On Wed, May 30, 2018 at 09:12:25AM -0700, Doug Anderson wrote:
>>> On Wed, May 30, 2018 at 8:50 AM, Mark Brown <broonie@kernel.org> wrote:
>>
>>>> No, I'm saying that I don't know why that property exists at all.  This
>>>> sounds like it's intended to be the amount of current the regulator can
>>>> deliver in each mode which is normally a design property of the silicon.
>>
>>> Ah, got it.  So the whole point here is to be able to implement either
>>> the function "set_load" or the function "get_optimum_mode".  We need
>>> some sort of table to convert from current to mode.  That's what this
>>> table does.
>>
>> We do need that table, my expectation would be that this table would be
>> in the driver as it's not something I'd expect to vary between different
>> systems but rather be a property of the silicon design.  No sense in
>> every single board having to copy it in.
> 
> Ah, got it!  I'd be OK with it being hardcoded in the driver.
> 
> At one point I think David was making the argument that some boards
> have different noise needs for the rails and thus you might want to
> change modes at different currents.  I don't know if this is realistic
> but I believe it was part of his original argument for why this needed
> to be specified.  If we can hardcode this in the driver I'm fine with
> it...  That would actually solve many of my objections too...

The DRMS modes to use and max allowed current per mode need to be
specified at the board level in device tree instead of hard-coded per
regulator type in the driver.  There are at least two use cases driving
this need: LDOs shared between RPMh client processors and SMPSes requiring
PWM mode in certain circumstances.

For LDOs the maximum low power mode (LPM) current is typically 10 mA or 30
mA (depending upon subtype) per hardware documentation. Unfortunately,
sharing control of regulators with other processors adds some subtlety to
the LPM current limit that should actually be applied at runtime.

Consider the case of a regulator with physical 10 mA LPM max current. Say
that modem and application processors each have a load on the regulator
that draws 9 mA. If they each respect the 10 mA limit, then they'd each
vote for LPM. The VRM block in RPMh hardware will aggregate these requests
together using a max function which will result in the regulator being set
to LPM, even though the total load is 18 mA (which would require high
power mode (HPM)). To get around this corner case, a LPM max current of 1
uA can be used for all LDO supplies that have non-application processor
consumers. Thus, any non-zero regulator_set_load() current request will
result in setting the regulator to HPM (which is always safe).

The second situation that needs board-level DRMS mode and current limit
specification is SMPS regulator AUTO mode to PWM (HPM) mode switching.
SMPS regulators should theoretically always be able to operate in AUTO
mode as it switches automatically between PWM mode (which can provide the
maximum current) and PFM mode (which supports lower current but has higher
efficiency). However, there may be board/system issues that require
switching to PWM mode for certain use cases as it has better load
regulation (i.e. no PFM ripple for lower loads) and supports more
aggressive load current steps (i.e. greater A/ns).

If a Linux consumer requires the ability to force a given SMPS regulator
from AUTO mode into PWM mode and that SMPS is shared by other Linux
consumers (which may be the case, but at least must be guaranteed to work
architecturally), then regulator_set_load() is the only option since it
provides aggregation, where as regulator_set_mode() does not.
regulator_set_load() can be utilized in this case by specifying AUTO mode
and PWM mode as drms modes and specifying some particular AUTO mode
maximum current (that is known by the consumer) in device tree.  The
consumer can then call regulator_set_load() with the imposed AUTO mode
limit + delta when PWM mode is required and a lower value when AUTO mode
is sufficient.

Note that I previously mentioned the need for board-level drms mode and
current limit specification in [1].

Take care,
David

[1]: https://lkml.org/lkml/2018/3/22/802

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

[PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[David Collins]

Hello Mark and Doug,

On 05/30/2018 09:24 AM, Doug Anderson wrote:
> On Wed, May 30, 2018 at 9:20 AM, Mark Brown <broonie@kernel.org> wrote:
>> On Wed, May 30, 2018 at 09:12:25AM -0700, Doug Anderson wrote:
>>> On Wed, May 30, 2018 at 8:50 AM, Mark Brown <broonie@kernel.org> wrote:
>>
>>>> No, I'm saying that I don't know why that property exists at all.  This
>>>> sounds like it's intended to be the amount of current the regulator can
>>>> deliver in each mode which is normally a design property of the silicon.
>>
>>> Ah, got it.  So the whole point here is to be able to implement either
>>> the function "set_load" or the function "get_optimum_mode".  We need
>>> some sort of table to convert from current to mode.  That's what this
>>> table does.
>>
>> We do need that table, my expectation would be that this table would be
>> in the driver as it's not something I'd expect to vary between different
>> systems but rather be a property of the silicon design.  No sense in
>> every single board having to copy it in.
> 
> Ah, got it!  I'd be OK with it being hardcoded in the driver.
> 
> At one point I think David was making the argument that some boards
> have different noise needs for the rails and thus you might want to
> change modes at different currents.  I don't know if this is realistic
> but I believe it was part of his original argument for why this needed
> to be specified.  If we can hardcode this in the driver I'm fine with
> it...  That would actually solve many of my objections too...

The DRMS modes to use and max allowed current per mode need to be
specified at the board level in device tree instead of hard-coded per
regulator type in the driver.  There are at least two use cases driving
this need: LDOs shared between RPMh client processors and SMPSes requiring
PWM mode in certain circumstances.

For LDOs the maximum low power mode (LPM) current is typically 10 mA or 30
mA (depending upon subtype) per hardware documentation. Unfortunately,
sharing control of regulators with other processors adds some subtlety to
the LPM current limit that should actually be applied at runtime.

Consider the case of a regulator with physical 10 mA LPM max current. Say
that modem and application processors each have a load on the regulator
that draws 9 mA. If they each respect the 10 mA limit, then they'd each
vote for LPM. The VRM block in RPMh hardware will aggregate these requests
together using a max function which will result in the regulator being set
to LPM, even though the total load is 18 mA (which would require high
power mode (HPM)). To get around this corner case, a LPM max current of 1
uA can be used for all LDO supplies that have non-application processor
consumers. Thus, any non-zero regulator_set_load() current request will
result in setting the regulator to HPM (which is always safe).

The second situation that needs board-level DRMS mode and current limit
specification is SMPS regulator AUTO mode to PWM (HPM) mode switching.
SMPS regulators should theoretically always be able to operate in AUTO
mode as it switches automatically between PWM mode (which can provide the
maximum current) and PFM mode (which supports lower current but has higher
efficiency). However, there may be board/system issues that require
switching to PWM mode for certain use cases as it has better load
regulation (i.e. no PFM ripple for lower loads) and supports more
aggressive load current steps (i.e. greater A/ns).

If a Linux consumer requires the ability to force a given SMPS regulator
from AUTO mode into PWM mode and that SMPS is shared by other Linux
consumers (which may be the case, but at least must be guaranteed to work
architecturally), then regulator_set_load() is the only option since it
provides aggregation, where as regulator_set_mode() does not.
regulator_set_load() can be utilized in this case by specifying AUTO mode
and PWM mode as drms modes and specifying some particular AUTO mode
maximum current (that is known by the consumer) in device tree.  The
consumer can then call regulator_set_load() with the imposed AUTO mode
limit + delta when PWM mode is required and a lower value when AUTO mode
is sufficient.

Note that I previously mentioned the need for board-level drms mode and
current limit specification in [1].

Take care,
David

[1]: https://lkml.org/lkml/2018/3/22/802

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

Re: [PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[Doug Anderson]

Hi,

On Wed, May 30, 2018 at 4:39 PM, David Collins <collinsd@codeaurora.org> wrote:
> Consider the case of a regulator with physical 10 mA LPM max current. Say
> that modem and application processors each have a load on the regulator
> that draws 9 mA. If they each respect the 10 mA limit, then they'd each
> vote for LPM. The VRM block in RPMh hardware will aggregate these requests
> together using a max function which will result in the regulator being set
> to LPM, even though the total load is 18 mA (which would require high
> power mode (HPM)). To get around this corner case, a LPM max current of 1
> uA can be used for all LDO supplies that have non-application processor
> consumers. Thus, any non-zero regulator_set_load() current request will
> result in setting the regulator to HPM (which is always safe).

Is there any plan to change the way this works for future versions of RPMh?


> The second situation that needs board-level DRMS mode and current limit
> specification is SMPS regulator AUTO mode to PWM (HPM) mode switching.
> SMPS regulators should theoretically always be able to operate in AUTO
> mode as it switches automatically between PWM mode (which can provide the
> maximum current) and PFM mode (which supports lower current but has higher
> efficiency). However, there may be board/system issues that require
> switching to PWM mode for certain use cases as it has better load
> regulation (i.e. no PFM ripple for lower loads) and supports more
> aggressive load current steps (i.e. greater A/ns).
>
> If a Linux consumer requires the ability to force a given SMPS regulator
> from AUTO mode into PWM mode and that SMPS is shared by other Linux
> consumers (which may be the case, but at least must be guaranteed to work
> architecturally), then regulator_set_load() is the only option since it
> provides aggregation, where as regulator_set_mode() does not.
> regulator_set_load() can be utilized in this case by specifying AUTO mode
> and PWM mode as drms modes and specifying some particular AUTO mode
> maximum current (that is known by the consumer) in device tree.  The
> consumer can then call regulator_set_load() with the imposed AUTO mode
> limit + delta when PWM mode is required and a lower value when AUTO mode
> is sufficient.

Mark: I'm leaving this firmly in your hands.  I can see David's points
here.  I could even believe that some of this stuff could be board
specific where one board might have slightly different capacitors or
they might be placed differently and might need a higher power mode to
keep the signal clean.

-Doug

[PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[Doug Anderson]

Hi,

On Wed, May 30, 2018 at 4:39 PM, David Collins <collinsd@codeaurora.org> wrote:
> Consider the case of a regulator with physical 10 mA LPM max current. Say
> that modem and application processors each have a load on the regulator
> that draws 9 mA. If they each respect the 10 mA limit, then they'd each
> vote for LPM. The VRM block in RPMh hardware will aggregate these requests
> together using a max function which will result in the regulator being set
> to LPM, even though the total load is 18 mA (which would require high
> power mode (HPM)). To get around this corner case, a LPM max current of 1
> uA can be used for all LDO supplies that have non-application processor
> consumers. Thus, any non-zero regulator_set_load() current request will
> result in setting the regulator to HPM (which is always safe).

Is there any plan to change the way this works for future versions of RPMh?


> The second situation that needs board-level DRMS mode and current limit
> specification is SMPS regulator AUTO mode to PWM (HPM) mode switching.
> SMPS regulators should theoretically always be able to operate in AUTO
> mode as it switches automatically between PWM mode (which can provide the
> maximum current) and PFM mode (which supports lower current but has higher
> efficiency). However, there may be board/system issues that require
> switching to PWM mode for certain use cases as it has better load
> regulation (i.e. no PFM ripple for lower loads) and supports more
> aggressive load current steps (i.e. greater A/ns).
>
> If a Linux consumer requires the ability to force a given SMPS regulator
> from AUTO mode into PWM mode and that SMPS is shared by other Linux
> consumers (which may be the case, but at least must be guaranteed to work
> architecturally), then regulator_set_load() is the only option since it
> provides aggregation, where as regulator_set_mode() does not.
> regulator_set_load() can be utilized in this case by specifying AUTO mode
> and PWM mode as drms modes and specifying some particular AUTO mode
> maximum current (that is known by the consumer) in device tree.  The
> consumer can then call regulator_set_load() with the imposed AUTO mode
> limit + delta when PWM mode is required and a lower value when AUTO mode
> is sufficient.

Mark: I'm leaving this firmly in your hands.  I can see David's points
here.  I could even believe that some of this stuff could be board
specific where one board might have slightly different capacitors or
they might be placed differently and might need a higher power mode to
keep the signal clean.

-Doug

Re: [PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[Doug Anderson]

Hi,

On Wed, May 30, 2018 at 4:39 PM, David Collins <collinsd@codeaurora.org> wrote:
> Consider the case of a regulator with physical 10 mA LPM max current. Say
> that modem and application processors each have a load on the regulator
> that draws 9 mA. If they each respect the 10 mA limit, then they'd each
> vote for LPM. The VRM block in RPMh hardware will aggregate these requests
> together using a max function which will result in the regulator being set
> to LPM, even though the total load is 18 mA (which would require high
> power mode (HPM)). To get around this corner case, a LPM max current of 1
> uA can be used for all LDO supplies that have non-application processor
> consumers. Thus, any non-zero regulator_set_load() current request will
> result in setting the regulator to HPM (which is always safe).

Is there any plan to change the way this works for future versions of RPMh?


> The second situation that needs board-level DRMS mode and current limit
> specification is SMPS regulator AUTO mode to PWM (HPM) mode switching.
> SMPS regulators should theoretically always be able to operate in AUTO
> mode as it switches automatically between PWM mode (which can provide the
> maximum current) and PFM mode (which supports lower current but has higher
> efficiency). However, there may be board/system issues that require
> switching to PWM mode for certain use cases as it has better load
> regulation (i.e. no PFM ripple for lower loads) and supports more
> aggressive load current steps (i.e. greater A/ns).
>
> If a Linux consumer requires the ability to force a given SMPS regulator
> from AUTO mode into PWM mode and that SMPS is shared by other Linux
> consumers (which may be the case, but at least must be guaranteed to work
> architecturally), then regulator_set_load() is the only option since it
> provides aggregation, where as regulator_set_mode() does not.
> regulator_set_load() can be utilized in this case by specifying AUTO mode
> and PWM mode as drms modes and specifying some particular AUTO mode
> maximum current (that is known by the consumer) in device tree.  The
> consumer can then call regulator_set_load() with the imposed AUTO mode
> limit + delta when PWM mode is required and a lower value when AUTO mode
> is sufficient.

Mark: I'm leaving this firmly in your hands.  I can see David's points
here.  I could even believe that some of this stuff could be board
specific where one board might have slightly different capacitors or
they might be placed differently and might need a higher power mode to
keep the signal clean.

-Doug

Re: [PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[David Collins]

Hi Doug,

On 05/30/2018 05:34 PM, Doug Anderson wrote:
> On Wed, May 30, 2018 at 4:39 PM, David Collins <collinsd@codeaurora.org> wrote:
>> Consider the case of a regulator with physical 10 mA LPM max current. Say
>> that modem and application processors each have a load on the regulator
>> that draws 9 mA. If they each respect the 10 mA limit, then they'd each
>> vote for LPM. The VRM block in RPMh hardware will aggregate these requests
>> together using a max function which will result in the regulator being set
>> to LPM, even though the total load is 18 mA (which would require high
>> power mode (HPM)). To get around this corner case, a LPM max current of 1
>> uA can be used for all LDO supplies that have non-application processor
>> consumers. Thus, any non-zero regulator_set_load() current request will
>> result in setting the regulator to HPM (which is always safe).
> 
> Is there any plan to change the way this works for future versions of RPMh?

As far as I know, no.  Adding VRM logic to sum RPMh client load current
requests, compare the sums to thresholds, select corresponding modes, and
override with explicit modes was determined to be too complex for RPMh.
Instead, much simpler max aggregation was implemented along with design
guidelines for PMICs to ensure that mode values are ordered from strictly
lower to higher power.

Take care,
David

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

[PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[David Collins]

Hi Doug,

On 05/30/2018 05:34 PM, Doug Anderson wrote:
> On Wed, May 30, 2018 at 4:39 PM, David Collins <collinsd@codeaurora.org> wrote:
>> Consider the case of a regulator with physical 10 mA LPM max current. Say
>> that modem and application processors each have a load on the regulator
>> that draws 9 mA. If they each respect the 10 mA limit, then they'd each
>> vote for LPM. The VRM block in RPMh hardware will aggregate these requests
>> together using a max function which will result in the regulator being set
>> to LPM, even though the total load is 18 mA (which would require high
>> power mode (HPM)). To get around this corner case, a LPM max current of 1
>> uA can be used for all LDO supplies that have non-application processor
>> consumers. Thus, any non-zero regulator_set_load() current request will
>> result in setting the regulator to HPM (which is always safe).
> 
> Is there any plan to change the way this works for future versions of RPMh?

As far as I know, no.  Adding VRM logic to sum RPMh client load current
requests, compare the sums to thresholds, select corresponding modes, and
override with explicit modes was determined to be too complex for RPMh.
Instead, much simpler max aggregation was implemented along with design
guidelines for PMICs to ensure that mode values are ordered from strictly
lower to higher power.

Take care,
David

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

Re: [PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[Mark Brown]

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

On Wed, May 30, 2018 at 04:39:10PM -0700, David Collins wrote:

> The DRMS modes to use and max allowed current per mode need to be
> specified at the board level in device tree instead of hard-coded per
> regulator type in the driver.  There are at least two use cases driving
> this need: LDOs shared between RPMh client processors and SMPSes requiring
> PWM mode in certain circumstances.

Is there really no way to improve the RPM firmware?

> Consider the case of a regulator with physical 10 mA LPM max current. Say
> that modem and application processors each have a load on the regulator
> that draws 9 mA. If they each respect the 10 mA limit, then they'd each
> vote for LPM. The VRM block in RPMh hardware will aggregate these requests

This is, of course, why the regulator API aggregates this stuff based on
the current not based on having every individual user select a mode.

> together using a max function which will result in the regulator being set
> to LPM, even though the total load is 18 mA (which would require high
> power mode (HPM)). To get around this corner case, a LPM max current of 1
> uA can be used for all LDO supplies that have non-application processor
> consumers. Thus, any non-zero regulator_set_load() current request will
> result in setting the regulator to HPM (which is always safe).

That's obviously just never going to work well, the best you can do here
is just pretend that the other components are always operating at full
power (which I assume all the other components are doing too...) or not
try to use load based mode switching in the first place.

> The second situation that needs board-level DRMS mode and current limit
> specification is SMPS regulator AUTO mode to PWM (HPM) mode switching.
> SMPS regulators should theoretically always be able to operate in AUTO
> mode as it switches automatically between PWM mode (which can provide the
> maximum current) and PFM mode (which supports lower current but has higher
> efficiency). However, there may be board/system issues that require
> switching to PWM mode for certain use cases as it has better load
> regulation (i.e. no PFM ripple for lower loads) and supports more
> aggressive load current steps (i.e. greater A/ns).

> If a Linux consumer requires the ability to force a given SMPS regulator
> from AUTO mode into PWM mode and that SMPS is shared by other Linux
> consumers (which may be the case, but at least must be guaranteed to work
> architecturally), then regulator_set_load() is the only option since it
> provides aggregation, where as regulator_set_mode() does not.

That's obviously broken though, what you're describing is just clearly
nothing to do with load so trying to configure it using load is just
going to lead to problems later on.  Honestly it sounds like you just
want to force the regulator into forced PWM mode all the time, otherwise
you need to look into implementing support for describing the thing
you're actually trying to do and add a mechanism for per consumer mode
configuration.

This has been a frequent pattern with these RPM drivers, trying to find
some way to shoehorn something that happens to work right now into the
code.  That's going to make things fragile and hard to maintain, we need
code that does the thing it's saying it does so that it's easier to
understand and work with - getting things running isn't enough, they
need to be clear.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[Mark Brown]

On Wed, May 30, 2018 at 04:39:10PM -0700, David Collins wrote:

> The DRMS modes to use and max allowed current per mode need to be
> specified at the board level in device tree instead of hard-coded per
> regulator type in the driver.  There are at least two use cases driving
> this need: LDOs shared between RPMh client processors and SMPSes requiring
> PWM mode in certain circumstances.

Is there really no way to improve the RPM firmware?

> Consider the case of a regulator with physical 10 mA LPM max current. Say
> that modem and application processors each have a load on the regulator
> that draws 9 mA. If they each respect the 10 mA limit, then they'd each
> vote for LPM. The VRM block in RPMh hardware will aggregate these requests

This is, of course, why the regulator API aggregates this stuff based on
the current not based on having every individual user select a mode.

> together using a max function which will result in the regulator being set
> to LPM, even though the total load is 18 mA (which would require high
> power mode (HPM)). To get around this corner case, a LPM max current of 1
> uA can be used for all LDO supplies that have non-application processor
> consumers. Thus, any non-zero regulator_set_load() current request will
> result in setting the regulator to HPM (which is always safe).

That's obviously just never going to work well, the best you can do here
is just pretend that the other components are always operating at full
power (which I assume all the other components are doing too...) or not
try to use load based mode switching in the first place.

> The second situation that needs board-level DRMS mode and current limit
> specification is SMPS regulator AUTO mode to PWM (HPM) mode switching.
> SMPS regulators should theoretically always be able to operate in AUTO
> mode as it switches automatically between PWM mode (which can provide the
> maximum current) and PFM mode (which supports lower current but has higher
> efficiency). However, there may be board/system issues that require
> switching to PWM mode for certain use cases as it has better load
> regulation (i.e. no PFM ripple for lower loads) and supports more
> aggressive load current steps (i.e. greater A/ns).

> If a Linux consumer requires the ability to force a given SMPS regulator
> from AUTO mode into PWM mode and that SMPS is shared by other Linux
> consumers (which may be the case, but at least must be guaranteed to work
> architecturally), then regulator_set_load() is the only option since it
> provides aggregation, where as regulator_set_mode() does not.

That's obviously broken though, what you're describing is just clearly
nothing to do with load so trying to configure it using load is just
going to lead to problems later on.  Honestly it sounds like you just
want to force the regulator into forced PWM mode all the time, otherwise
you need to look into implementing support for describing the thing
you're actually trying to do and add a mechanism for per consumer mode
configuration.

This has been a frequent pattern with these RPM drivers, trying to find
some way to shoehorn something that happens to work right now into the
code.  That's going to make things fragile and hard to maintain, we need
code that does the thing it's saying it does so that it's easier to
understand and work with - getting things running isn't enough, they
need to be clear.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180531/53042fa7/attachment.sig>

Re: [PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[David Collins]

Hello Mark,

On 05/31/2018 04:48 AM, Mark Brown wrote:
> On Wed, May 30, 2018 at 04:39:10PM -0700, David Collins wrote:
>> The DRMS modes to use and max allowed current per mode need to be
>> specified at the board level in device tree instead of hard-coded per
>> regulator type in the driver.  There are at least two use cases driving
>> this need: LDOs shared between RPMh client processors and SMPSes requiring
>> PWM mode in certain circumstances.
> 
> Is there really no way to improve the RPM firmware?

This aggregation takes place in a discrete hardware block, not in a
general purpose processor.  Theoretically, future chips could have
redesigned VRM hardware; however, there is no plan to make such a change.


>> Consider the case of a regulator with physical 10 mA LPM max current. Say
>> that modem and application processors each have a load on the regulator
>> that draws 9 mA. If they each respect the 10 mA limit, then they'd each
>> vote for LPM. The VRM block in RPMh hardware will aggregate these requests
> 
> This is, of course, why the regulator API aggregates this stuff based on
> the current not based on having every individual user select a mode.
> 
>> together using a max function which will result in the regulator being set
>> to LPM, even though the total load is 18 mA (which would require high
>> power mode (HPM)). To get around this corner case, a LPM max current of 1
>> uA can be used for all LDO supplies that have non-application processor
>> consumers. Thus, any non-zero regulator_set_load() current request will
>> result in setting the regulator to HPM (which is always safe).
> 
> That's obviously just never going to work well, the best you can do here
> is just pretend that the other components are always operating at full
> power (which I assume all the other components are doing too...) or not
> try to use load based mode switching in the first place.

I will remove the DT-based mode and max load current mapping support.  In
its place, I'll implement hard coded LPM current limits for LDOs of 10 mA
or 30 mA (depending upon subtype) like is done in other regulator drivers.

If we actually encounter an issue caused by the APPS processor and another
RPMh client both voting for LPM when HPM is needed for the combination,
then we can disable load-based mode control for the affected regulator in
DT and configure its initial mode as HPM.


>> The second situation that needs board-level DRMS mode and current limit
>> specification is SMPS regulator AUTO mode to PWM (HPM) mode switching.
>> SMPS regulators should theoretically always be able to operate in AUTO
>> mode as it switches automatically between PWM mode (which can provide the
>> maximum current) and PFM mode (which supports lower current but has higher
>> efficiency). However, there may be board/system issues that require
>> switching to PWM mode for certain use cases as it has better load
>> regulation (i.e. no PFM ripple for lower loads) and supports more
>> aggressive load current steps (i.e. greater A/ns).
> 
>> If a Linux consumer requires the ability to force a given SMPS regulator
>> from AUTO mode into PWM mode and that SMPS is shared by other Linux
>> consumers (which may be the case, but at least must be guaranteed to work
>> architecturally), then regulator_set_load() is the only option since it
>> provides aggregation, where as regulator_set_mode() does not.
> 
> That's obviously broken though, what you're describing is just clearly
> nothing to do with load so trying to configure it using load is just
> going to lead to problems later on.  Honestly it sounds like you just
> want to force the regulator into forced PWM mode all the time, otherwise
> you need to look into implementing support for describing the thing
> you're actually trying to do and add a mechanism for per consumer mode
> configuration.
> 
> This has been a frequent pattern with these RPM drivers, trying to find
> some way to shoehorn something that happens to work right now into the
> code.  That's going to make things fragile and hard to maintain, we need
> code that does the thing it's saying it does so that it's easier to
> understand and work with - getting things running isn't enough, they
> need to be clear.

I agree that using regulator_set_load() to handle SMPS AUTO mode to PWM
mode switching is hacky.  Since there is no natural way to pick SMPS modes
based on load current, I will remove the functionality completely.  In its
place, we can configure the SMPSes to have an initial mode of AUTO.  If a
use case requiring PWM mode arises, then the consumer driver responsible
for it can call regulator_set_mode() to switch between AUTO and PWM modes
explicitly.

Since regulator_set_mode() does not support aggregation currently, this
technique would work only if there is exactly one consumer per regulator
that needs explicit mode control.  Should we hit a situation that needs
multiple consumers to have such mode control, then we could simply default
the SMPS to PWM mode.

I'll also start looking into per-consumer mode configuration and
regulator_set_mode() aggregation within the regulator framework.  I think
that we should be able to function without it for now; however, it would
be good to have going forward.

Take care,
David

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

[PATCH v4 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

[David Collins]

Hello Mark,

On 05/31/2018 04:48 AM, Mark Brown wrote:
> On Wed, May 30, 2018 at 04:39:10PM -0700, David Collins wrote:
>> The DRMS modes to use and max allowed current per mode need to be
>> specified at the board level in device tree instead of hard-coded per
>> regulator type in the driver.  There are at least two use cases driving
>> this need: LDOs shared between RPMh client processors and SMPSes requiring
>> PWM mode in certain circumstances.
> 
> Is there really no way to improve the RPM firmware?

This aggregation takes place in a discrete hardware block, not in a
general purpose processor.  Theoretically, future chips could have
redesigned VRM hardware; however, there is no plan to make such a change.


>> Consider the case of a regulator with physical 10 mA LPM max current. Say
>> that modem and application processors each have a load on the regulator
>> that draws 9 mA. If they each respect the 10 mA limit, then they'd each
>> vote for LPM. The VRM block in RPMh hardware will aggregate these requests
> 
> This is, of course, why the regulator API aggregates this stuff based on
> the current not based on having every individual user select a mode.
> 
>> together using a max function which will result in the regulator being set
>> to LPM, even though the total load is 18 mA (which would require high
>> power mode (HPM)). To get around this corner case, a LPM max current of 1
>> uA can be used for all LDO supplies that have non-application processor
>> consumers. Thus, any non-zero regulator_set_load() current request will
>> result in setting the regulator to HPM (which is always safe).
> 
> That's obviously just never going to work well, the best you can do here
> is just pretend that the other components are always operating at full
> power (which I assume all the other components are doing too...) or not
> try to use load based mode switching in the first place.

I will remove the DT-based mode and max load current mapping support.  In
its place, I'll implement hard coded LPM current limits for LDOs of 10 mA
or 30 mA (depending upon subtype) like is done in other regulator drivers.

If we actually encounter an issue caused by the APPS processor and another
RPMh client both voting for LPM when HPM is needed for the combination,
then we can disable load-based mode control for the affected regulator in
DT and configure its initial mode as HPM.


>> The second situation that needs board-level DRMS mode and current limit
>> specification is SMPS regulator AUTO mode to PWM (HPM) mode switching.
>> SMPS regulators should theoretically always be able to operate in AUTO
>> mode as it switches automatically between PWM mode (which can provide the
>> maximum current) and PFM mode (which supports lower current but has higher
>> efficiency). However, there may be board/system issues that require
>> switching to PWM mode for certain use cases as it has better load
>> regulation (i.e. no PFM ripple for lower loads) and supports more
>> aggressive load current steps (i.e. greater A/ns).
> 
>> If a Linux consumer requires the ability to force a given SMPS regulator
>> from AUTO mode into PWM mode and that SMPS is shared by other Linux
>> consumers (which may be the case, but at least must be guaranteed to work
>> architecturally), then regulator_set_load() is the only option since it
>> provides aggregation, where as regulator_set_mode() does not.
> 
> That's obviously broken though, what you're describing is just clearly
> nothing to do with load so trying to configure it using load is just
> going to lead to problems later on.  Honestly it sounds like you just
> want to force the regulator into forced PWM mode all the time, otherwise
> you need to look into implementing support for describing the thing
> you're actually trying to do and add a mechanism for per consumer mode
> configuration.
> 
> This has been a frequent pattern with these RPM drivers, trying to find
> some way to shoehorn something that happens to work right now into the
> code.  That's going to make things fragile and hard to maintain, we need
> code that does the thing it's saying it does so that it's easier to
> understand and work with - getting things running isn't enough, they
> need to be clear.

I agree that using regulator_set_load() to handle SMPS AUTO mode to PWM
mode switching is hacky.  Since there is no natural way to pick SMPS modes
based on load current, I will remove the functionality completely.  In its
place, we can configure the SMPSes to have an initial mode of AUTO.  If a
use case requiring PWM mode arises, then the consumer driver responsible
for it can call regulator_set_mode() to switch between AUTO and PWM modes
explicitly.

Since regulator_set_mode() does not support aggregation currently, this
technique would work only if there is exactly one consumer per regulator
that needs explicit mode control.  Should we hit a situation that needs
multiple consumers to have such mode control, then we could simply default
the SMPS to PWM mode.

I'll also start looking into per-consumer mode configuration and
regulator_set_mode() aggregation within the regulator framework.  I think
that we should be able to function without it for now; however, it would
be good to have going forward.

Take care,
David

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

[PATCH v4 2/2] regulator: add QCOM RPMh regulator driver

[David Collins]

Add the QCOM RPMh regulator driver to manage PMIC regulators
which are controlled via RPMh on some Qualcomm Technologies, Inc.
SoCs.  RPMh is a hardware block which contains several
accelerators which are used to manage various hardware resources
that are shared between the processors of the SoC.  The final
hardware state of a regulator is determined within RPMh by
performing max aggregation of the requests made by all of the
processors.

Add support for PMIC regulator control via the voltage regulator
manager (VRM) and oscillator buffer (XOB) RPMh accelerators.
VRM supports manipulation of enable state, voltage, and mode.
XOB supports manipulation of enable state.

Signed-off-by: David Collins <collinsd@codeaurora.org>
---
 drivers/regulator/Kconfig               |   9 +
 drivers/regulator/Makefile              |   1 +
 drivers/regulator/qcom-rpmh-regulator.c | 865 ++++++++++++++++++++++++++++++++
 3 files changed, 875 insertions(+)
 create mode 100644 drivers/regulator/qcom-rpmh-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 4efae3b..1a69bdc 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -671,6 +671,15 @@ config REGULATOR_QCOM_RPM
 	  Qualcomm RPM as a module. The module will be named
 	  "qcom_rpm-regulator".
 
+config REGULATOR_QCOM_RPMH
+	tristate "Qualcomm Technologies, Inc. RPMh regulator driver"
+	depends on QCOM_RPMH || COMPILE_TEST
+	help
+	  This driver supports control of PMIC regulators via the RPMh hardware
+	  block found on Qualcomm Technologies Inc. SoCs.  RPMh regulator
+	  control allows for voting on regulator state between multiple
+	  processors within the SoC.
+
 config REGULATOR_QCOM_SMD_RPM
 	tristate "Qualcomm SMD based RPM regulator driver"
 	depends on QCOM_SMD_RPM
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index d81fb02..906f048 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -77,6 +77,7 @@ obj-$(CONFIG_REGULATOR_MT6323)	+= mt6323-regulator.o
 obj-$(CONFIG_REGULATOR_MT6380)	+= mt6380-regulator.o
 obj-$(CONFIG_REGULATOR_MT6397)	+= mt6397-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_RPM) += qcom_rpm-regulator.o
+obj-$(CONFIG_REGULATOR_QCOM_RPMH) += qcom-rpmh-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_SMD_RPM) += qcom_smd-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_SPMI) += qcom_spmi-regulator.o
 obj-$(CONFIG_REGULATOR_PALMAS) += palmas-regulator.o
diff --git a/drivers/regulator/qcom-rpmh-regulator.c b/drivers/regulator/qcom-rpmh-regulator.c
new file mode 100644
index 0000000..0217dcc
--- /dev/null
+++ b/drivers/regulator/qcom-rpmh-regulator.c
@@ -0,0 +1,865 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2018, The Linux Foundation. All rights reserved. */
+
+#define pr_fmt(fmt) "%s: " fmt, __func__
+
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+
+#include <soc/qcom/cmd-db.h>
+#include <soc/qcom/rpmh.h>
+
+#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
+
+/**
+ * enum rpmh_regulator_type - supported RPMh accelerator types
+ * %VRM:	RPMh VRM accelerator which supports voting on enable, voltage,
+ *		and mode of LDO, SMPS, and BOB type PMIC regulators.
+ * %XOB:	RPMh XOB accelerator which supports voting on the enable state
+ *		of PMIC regulators.
+ */
+enum rpmh_regulator_type {
+	VRM,
+	XOB,
+};
+
+#define RPMH_VRM_HEADROOM_MAX_UV		511000
+
+#define RPMH_REGULATOR_REG_VRM_VOLTAGE		0x0
+#define RPMH_REGULATOR_REG_ENABLE		0x4
+#define RPMH_REGULATOR_REG_VRM_MODE		0x8
+#define RPMH_REGULATOR_REG_VRM_HEADROOM		0xC
+
+#define RPMH_REGULATOR_MODE_COUNT		4
+
+#define PMIC4_LDO_MODE_RETENTION		4
+#define PMIC4_LDO_MODE_LPM			5
+#define PMIC4_LDO_MODE_HPM			7
+
+#define PMIC4_SMPS_MODE_RETENTION		4
+#define PMIC4_SMPS_MODE_PFM			5
+#define PMIC4_SMPS_MODE_AUTO			6
+#define PMIC4_SMPS_MODE_PWM			7
+
+#define PMIC4_BOB_MODE_PASS			0
+#define PMIC4_BOB_MODE_PFM			1
+#define PMIC4_BOB_MODE_AUTO			2
+#define PMIC4_BOB_MODE_PWM			3
+
+/**
+ * struct rpmh_vreg_hw_data - RPMh regulator hardware configurations
+ * @regulator_type:		RPMh accelerator type used to manage this
+ *				regulator
+ * @ops:			Pointer to regulator ops callback structure
+ * @voltage_range:		The single range of voltages supported by this
+ *				PMIC regulator type
+ * @n_voltages:			The number of unique voltage set points defined
+ *				by voltage_range
+ * @pmic_mode_map:		Array indexed by regulator framework mode
+ *				containing PMIC hardware modes.  Must be large
+ *				enough to index all framework modes supported
+ *				by this regulator hardware type.
+ * @of_map_mode:		Maps an RPMH_REGULATOR_MODE_* mode value defined
+ *				in device tree to a regulator framework mode
+ */
+struct rpmh_vreg_hw_data {
+	enum rpmh_regulator_type		regulator_type;
+	const struct regulator_ops		*ops;
+	const struct regulator_linear_range	voltage_range;
+	int					n_voltages;
+	const int				*pmic_mode_map;
+	unsigned int			      (*of_map_mode)(unsigned int mode);
+};
+
+/**
+ * struct rpmh_vreg - individual RPMh regulator data structure encapsulating a
+ *		single regulator device
+ * @dev:			Device pointer for the top-level PMIC RPMh
+ *				regulator parent device.  This is used as a
+ *				handle in RPMh write requests.
+ * @addr:			Base address of the regulator resource within
+ *				an RPMh accelerator
+ * @rdesc:			Regulator descriptor
+ * @hw_data:			PMIC regulator configuration data for this RPMh
+ *				regulator
+ * @always_wait_for_ack:	Boolean flag indicating if a request must always
+ *				wait for an ACK from RPMh before continuing even
+ *				if it corresponds to a strictly lower power
+ *				state (e.g. enabled --> disabled).
+ * @drms_modes:			Array of regulator framework modes which can
+ *				be configured dynamically for this regulator
+ *				via the set_load() callback.
+ * @drms_mode_max_uAs:		Array of maximum load currents in microamps
+ *				supported by the corresponding modes in
+ *				drms_mode.  Elements must be specified in
+ *				strictly increasing order.
+ * @drms_mode_count:		The number of elements in drms_mode array.
+ * @enabled:			Flag indicating if the regulator is enabled or
+ *				not
+ * @ever_enabled:		Boolean indicating that the regulator has been
+ *				explicitly enabled at least once.  Voltage
+ *				requests should be cached when this flag is not
+ *				set.
+ * @bypassed:			Boolean indicating if the regulator is in
+ *				bypass (pass-through) mode or not.  This is
+ *				only used by BOB rpmh-regulator resources.
+ * @voltage_selector:		Selector used for get_voltage_sel() and
+ *				set_voltage_sel() callbacks
+ * @mode:			RPMh VRM regulator current framework mode
+ */
+struct rpmh_vreg {
+	struct device			*dev;
+	u32				addr;
+	struct regulator_desc		rdesc;
+	const struct rpmh_vreg_hw_data	*hw_data;
+	bool				always_wait_for_ack;
+	unsigned int			*drms_modes;
+	int				*drms_mode_max_uAs;
+	size_t				drms_mode_count;
+
+	int				enabled;
+	bool				ever_enabled;
+	bool				bypassed;
+	int				voltage_selector;
+	unsigned int			mode;
+};
+
+/**
+ * struct rpmh_vreg_init_data - initialization data for an RPMh regulator
+ * @name:			Name for the regulator which also corresponds
+ *				to the device tree subnode name of the regulator
+ * @resource_name:		RPMh regulator resource name format string.
+ *				This must include exactly one field: '%s' which
+ *				is filled at run-time with the PMIC ID provided
+ *				by device tree property qcom,pmic-id.  Example:
+ *				"ldo%s1" for RPMh resource "ldoa1".
+ * @supply_name:		Parent supply regulator name
+ * @hw_data:			Configuration data for this PMIC regulator type
+ */
+struct rpmh_vreg_init_data {
+	const char			*name;
+	const char			*resource_name;
+	const char			*supply_name;
+	const struct rpmh_vreg_hw_data	*hw_data;
+};
+
+/**
+ * rpmh_regulator_send_request() - send the request to RPMh
+ * @vreg:		Pointer to the RPMh regulator
+ * @cmd:		RPMh commands to send
+ * @count:		Size of cmd array
+ * @wait_for_ack:	Boolean indicating if execution must wait until the
+ *			request has been acknowledged as complete
+ *
+ * Return: 0 on success, errno on failure
+ */
+static int rpmh_regulator_send_request(struct rpmh_vreg *vreg,
+			struct tcs_cmd *cmd, int count, bool wait_for_ack)
+{
+	int ret;
+
+	if (wait_for_ack || vreg->always_wait_for_ack)
+		ret = rpmh_write(vreg->dev, RPMH_ACTIVE_ONLY_STATE, cmd, count);
+	else
+		ret = rpmh_write_async(vreg->dev, RPMH_ACTIVE_ONLY_STATE, cmd,
+					count);
+
+	return ret;
+}
+
+static int _rpmh_regulator_vrm_set_voltage_sel(struct regulator_dev *rdev,
+				unsigned int selector, bool wait_for_ack)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+	struct tcs_cmd cmd = {
+		.addr = vreg->addr + RPMH_REGULATOR_REG_VRM_VOLTAGE,
+	};
+	int ret;
+
+	/* VRM voltage control register is set with voltage in millivolts. */
+	cmd.data = DIV_ROUND_UP(regulator_list_voltage_linear_range(rdev,
+							selector), 1000);
+
+	ret = rpmh_regulator_send_request(vreg, &cmd, 1, wait_for_ack);
+	if (!ret)
+		vreg->voltage_selector = selector;
+
+	return 0;
+}
+
+static int rpmh_regulator_vrm_set_voltage_sel(struct regulator_dev *rdev,
+				unsigned int selector)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+
+	if (vreg->ever_enabled) {
+		return _rpmh_regulator_vrm_set_voltage_sel(rdev, selector,
+					selector > vreg->voltage_selector);
+	} else {
+		/*
+		 * Cache the voltage and send it later when the regulator is
+		 * enabled.
+		 */
+		vreg->voltage_selector = selector;
+	}
+
+	return 0;
+}
+
+static int rpmh_regulator_vrm_get_voltage_sel(struct regulator_dev *rdev)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+
+	return vreg->voltage_selector;
+}
+
+static int rpmh_regulator_is_enabled(struct regulator_dev *rdev)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+
+	return vreg->enabled;
+}
+
+static int rpmh_regulator_enable(struct regulator_dev *rdev)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+	struct tcs_cmd cmd = {
+		.addr = vreg->addr + RPMH_REGULATOR_REG_ENABLE,
+		.data = 1,
+	};
+	int ret;
+
+	if (!vreg->ever_enabled && vreg->voltage_selector != -ENOTRECOVERABLE) {
+		ret = _rpmh_regulator_vrm_set_voltage_sel(rdev,
+						vreg->voltage_selector, true);
+		if (ret < 0)
+			return ret;
+	}
+
+	ret = rpmh_regulator_send_request(vreg, &cmd, 1, true);
+	if (!ret) {
+		vreg->enabled = true;
+		vreg->ever_enabled = true;
+	}
+
+	return ret;
+}
+
+static int rpmh_regulator_disable(struct regulator_dev *rdev)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+	struct tcs_cmd cmd = {
+		.addr = vreg->addr + RPMH_REGULATOR_REG_ENABLE,
+		.data = 0,
+	};
+	int ret;
+
+	ret = rpmh_regulator_send_request(vreg, &cmd, 1, false);
+	if (!ret)
+		vreg->enabled = false;
+
+	return ret;
+}
+
+static int rpmh_regulator_vrm_set_mode_bypass(struct rpmh_vreg *vreg,
+					unsigned int mode, bool bypassed)
+{
+	struct tcs_cmd cmd = {
+		.addr = vreg->addr + RPMH_REGULATOR_REG_VRM_MODE,
+	};
+	int pmic_mode;
+
+	if (mode > REGULATOR_MODE_STANDBY)
+		return -EINVAL;
+
+	pmic_mode = vreg->hw_data->pmic_mode_map[mode];
+	if (pmic_mode < 0)
+		return pmic_mode;
+
+	cmd.data = bypassed ? PMIC4_BOB_MODE_PASS : pmic_mode;
+
+	return rpmh_regulator_send_request(vreg, &cmd, 1, true);
+}
+
+static int rpmh_regulator_vrm_set_mode(struct regulator_dev *rdev,
+					unsigned int mode)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+	int ret;
+
+	if (mode == vreg->mode)
+		return 0;
+
+	ret = rpmh_regulator_vrm_set_mode_bypass(vreg, mode, vreg->bypassed);
+	if (!ret)
+		vreg->mode = mode;
+
+	return ret;
+}
+
+static unsigned int rpmh_regulator_vrm_get_mode(struct regulator_dev *rdev)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+
+	return vreg->mode;
+}
+
+/**
+ * rpmh_regulator_vrm_set_load() - set the regulator mode based upon the load
+ *		current requested
+ * @rdev:		Regulator device pointer for the rpmh-regulator
+ * @load_uA:		Aggregated load current in microamps
+ *
+ * This function is used in the regulator_ops for VRM type RPMh regulator
+ * devices.  It updates the mode of the regulator using a table of maximum
+ * load currents per mode specified in device tree properties.
+ *
+ * Return: 0 on success, errno on failure
+ */
+static int rpmh_regulator_vrm_set_load(struct regulator_dev *rdev, int load_uA)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+	int i;
+
+	for (i = 0; i < vreg->drms_mode_count - 1; i++)
+		if (vreg->drms_mode_max_uAs[i] >= load_uA)
+			break;
+	if (load_uA > vreg->drms_mode_max_uAs[vreg->drms_mode_count - 1])
+		dev_warn(vreg->dev, "%s: requested load=%d uA greater than max=%d uA\n",
+			vreg->rdesc.name, load_uA,
+			vreg->drms_mode_max_uAs[vreg->drms_mode_count - 1]);
+
+	return rpmh_regulator_vrm_set_mode(rdev, vreg->drms_modes[i]);
+}
+
+static int rpmh_regulator_vrm_set_bypass(struct regulator_dev *rdev,
+				bool enable)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+	int ret;
+
+	if (vreg->bypassed == enable)
+		return 0;
+
+	ret = rpmh_regulator_vrm_set_mode_bypass(vreg, vreg->mode, enable);
+	if (!ret)
+		vreg->bypassed = enable;
+
+	return ret;
+}
+
+static int rpmh_regulator_vrm_get_bypass(struct regulator_dev *rdev,
+				bool *enable)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+
+	*enable = vreg->bypassed;
+
+	return 0;
+}
+
+static const struct regulator_ops rpmh_regulator_vrm_ops = {
+	.enable			= rpmh_regulator_enable,
+	.disable		= rpmh_regulator_disable,
+	.is_enabled		= rpmh_regulator_is_enabled,
+	.set_voltage_sel	= rpmh_regulator_vrm_set_voltage_sel,
+	.get_voltage_sel	= rpmh_regulator_vrm_get_voltage_sel,
+	.list_voltage		= regulator_list_voltage_linear_range,
+	.set_mode		= rpmh_regulator_vrm_set_mode,
+	.get_mode		= rpmh_regulator_vrm_get_mode,
+	.set_load		= rpmh_regulator_vrm_set_load,
+};
+
+static const struct regulator_ops rpmh_regulator_vrm_bypass_ops = {
+	.enable			= rpmh_regulator_enable,
+	.disable		= rpmh_regulator_disable,
+	.is_enabled		= rpmh_regulator_is_enabled,
+	.set_voltage_sel	= rpmh_regulator_vrm_set_voltage_sel,
+	.get_voltage_sel	= rpmh_regulator_vrm_get_voltage_sel,
+	.list_voltage		= regulator_list_voltage_linear_range,
+	.set_mode		= rpmh_regulator_vrm_set_mode,
+	.get_mode		= rpmh_regulator_vrm_get_mode,
+	.set_load		= rpmh_regulator_vrm_set_load,
+	.set_bypass		= rpmh_regulator_vrm_set_bypass,
+	.get_bypass		= rpmh_regulator_vrm_get_bypass,
+};
+
+static const struct regulator_ops rpmh_regulator_xob_ops = {
+	.enable			= rpmh_regulator_enable,
+	.disable		= rpmh_regulator_disable,
+	.is_enabled		= rpmh_regulator_is_enabled,
+};
+
+/**
+ * rpmh_regulator_parse_vrm_modes() - parse the supported mode configurations
+ *		for a VRM RPMh resource from device tree
+ * vreg:		Pointer to the individual rpmh-regulator resource
+ * dev:			Pointer to the top level rpmh-regulator PMIC device
+ * node:		Pointer to the individual rpmh-regulator resource
+ *			device node
+ *
+ * This function initializes the drms_modes[] and drms_mode_max_uAs[] arrays of
+ * vreg based upon the values of optional device tree properties.
+ *
+ * Return: 0 on success, errno on failure
+ */
+static int rpmh_regulator_parse_vrm_modes(struct rpmh_vreg *vreg,
+				struct device *dev, struct device_node *node)
+{
+	const char *prop;
+	int i, len, ret, mode;
+	u32 *buf;
+
+	/* qcom,regulator-drms-modes is optional */
+	prop = "qcom,regulator-drms-modes";
+	len = of_property_count_elems_of_size(node, prop, sizeof(u32));
+	if (len < 0)
+		return 0;
+
+	vreg->drms_modes = devm_kcalloc(dev, len, sizeof(*vreg->drms_modes),
+					GFP_KERNEL);
+	vreg->drms_mode_max_uAs = devm_kcalloc(dev, len,
+					       sizeof(*vreg->drms_mode_max_uAs),
+					       GFP_KERNEL);
+	if (!vreg->drms_modes || !vreg->drms_mode_max_uAs)
+		return -ENOMEM;
+	vreg->drms_mode_count = len;
+
+	buf = kcalloc(len, sizeof(*buf), GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = of_property_read_u32_array(node, prop, buf, len);
+	if (ret < 0) {
+		dev_err(dev, "%s: unable to read %s, ret=%d\n",
+			node->name, prop, ret);
+		goto done;
+	}
+
+	for (i = 0; i < len; i++) {
+		mode = vreg->hw_data->of_map_mode(buf[i]);
+		if (mode == REGULATOR_MODE_INVALID) {
+			dev_err(dev, "%s: element %d of %s = %u is invalid for this regulator\n",
+				node->name, i, prop, buf[i]);
+			ret = -EINVAL;
+			goto done;
+		}
+
+		vreg->drms_modes[i] = mode;
+	}
+
+	prop = "qcom,drms-mode-max-microamps";
+	len = of_property_count_elems_of_size(node, prop, sizeof(u32));
+	if (len != vreg->drms_mode_count) {
+		dev_err(dev, "%s: invalid element count=%d for %s\n",
+			node->name, len, prop);
+		ret = -EINVAL;
+		goto done;
+	}
+
+	ret = of_property_read_u32_array(node, prop, buf, len);
+	if (ret < 0) {
+		dev_err(dev, "%s: unable to read %s, ret=%d\n",
+			node->name, prop, ret);
+		goto done;
+	}
+
+	for (i = 0; i < len; i++) {
+		vreg->drms_mode_max_uAs[i] = buf[i];
+
+		if (i > 0 && vreg->drms_mode_max_uAs[i]
+				<= vreg->drms_mode_max_uAs[i - 1]) {
+			dev_err(dev, "%s: %s elements are not in ascending order\n",
+				node->name, prop);
+			ret = -EINVAL;
+			goto done;
+		}
+	}
+
+done:
+	kfree(buf);
+	return ret;
+}
+
+/**
+ * rpmh_regulator_init_vreg() - initialize all attributes of an rpmh-regulator
+ * vreg:		Pointer to the individual rpmh-regulator resource
+ * dev:			Pointer to the top level rpmh-regulator PMIC device
+ * node:		Pointer to the individual rpmh-regulator resource
+ *			device node
+ * pmic_id:		String used to identify the top level rpmh-regulator
+ *			PMIC device on the board
+ * rpmh_data:		Pointer to a null-terminated array of rpmh-regulator
+ *			resources defined for the top level PMIC device
+ *
+ * Return: 0 on success, errno on failure
+ */
+static int rpmh_regulator_init_vreg(struct rpmh_vreg *vreg, struct device *dev,
+				struct device_node *node, const char *pmic_id,
+				const struct rpmh_vreg_init_data *rpmh_data)
+{
+	struct regulator_config reg_config = {};
+	char rpmh_resource_name[20] = "";
+	struct regulator_dev *rdev;
+	enum rpmh_regulator_type type;
+	struct regulator_init_data *init_data;
+	int ret;
+
+	vreg->dev = dev;
+
+	for (; rpmh_data->name; rpmh_data++)
+		if (!strcmp(rpmh_data->name, node->name))
+			break;
+
+	if (!rpmh_data->name) {
+		dev_err(dev, "Unknown regulator %s\n", node->name);
+		return -EINVAL;
+	}
+
+	scnprintf(rpmh_resource_name, sizeof(rpmh_resource_name),
+		rpmh_data->resource_name, pmic_id);
+
+	vreg->addr = cmd_db_read_addr(rpmh_resource_name);
+	if (!vreg->addr) {
+		dev_err(dev, "%s: could not find RPMh address for resource %s\n",
+			node->name, rpmh_resource_name);
+		return -ENODEV;
+	}
+
+	vreg->rdesc.name = rpmh_data->name;
+	vreg->rdesc.supply_name = rpmh_data->supply_name;
+	vreg->hw_data = rpmh_data->hw_data;
+
+	vreg->enabled = -EINVAL;
+	vreg->voltage_selector = -ENOTRECOVERABLE;
+	vreg->mode = REGULATOR_MODE_INVALID;
+
+	if (rpmh_data->hw_data->n_voltages) {
+		vreg->rdesc.linear_ranges = &rpmh_data->hw_data->voltage_range;
+		vreg->rdesc.n_linear_ranges = 1;
+		vreg->rdesc.n_voltages = rpmh_data->hw_data->n_voltages;
+	}
+
+	type = rpmh_data->hw_data->regulator_type;
+	if (type == VRM) {
+		ret = rpmh_regulator_parse_vrm_modes(vreg, dev, node);
+		if (ret < 0)
+			return ret;
+	}
+
+	vreg->always_wait_for_ack = of_property_read_bool(node,
+						"qcom,always-wait-for-ack");
+
+	vreg->rdesc.owner	= THIS_MODULE;
+	vreg->rdesc.type	= REGULATOR_VOLTAGE;
+	vreg->rdesc.ops		= vreg->hw_data->ops;
+	vreg->rdesc.of_map_mode	= vreg->hw_data->of_map_mode;
+
+	init_data = of_get_regulator_init_data(dev, node, &vreg->rdesc);
+	if (!init_data)
+		return -ENOMEM;
+
+	if (type == XOB && init_data->constraints.min_uV &&
+	    init_data->constraints.min_uV == init_data->constraints.max_uV) {
+		vreg->rdesc.fixed_uV = init_data->constraints.min_uV;
+		vreg->rdesc.n_voltages = 1;
+	}
+
+	reg_config.dev		= dev;
+	reg_config.init_data	= init_data;
+	reg_config.of_node	= node;
+	reg_config.driver_data	= vreg;
+
+	rdev = devm_regulator_register(dev, &vreg->rdesc, &reg_config);
+	if (IS_ERR(rdev)) {
+		ret = PTR_ERR(rdev);
+		rdev = NULL;
+		dev_err(dev, "%s: devm_regulator_register() failed, ret=%d\n",
+			node->name, ret);
+		return ret;
+	}
+
+	dev_dbg(dev, "%s regulator registered for RPMh resource %s @ 0x%05X\n",
+		node->name, rpmh_resource_name, vreg->addr);
+
+	return ret;
+}
+
+static const int pmic_mode_map_pmic4_ldo[REGULATOR_MODE_STANDBY + 1] = {
+	[REGULATOR_MODE_INVALID] = -EINVAL,
+	[REGULATOR_MODE_STANDBY] = PMIC4_LDO_MODE_RETENTION,
+	[REGULATOR_MODE_IDLE]    = PMIC4_LDO_MODE_LPM,
+	[REGULATOR_MODE_NORMAL]  = -EINVAL,
+	[REGULATOR_MODE_FAST]    = PMIC4_LDO_MODE_HPM,
+};
+
+static unsigned int rpmh_regulator_pmic4_ldo_of_map_mode(unsigned int mode)
+{
+	static const unsigned int of_mode_map[RPMH_REGULATOR_MODE_COUNT] = {
+		[RPMH_REGULATOR_MODE_RET]  = REGULATOR_MODE_STANDBY,
+		[RPMH_REGULATOR_MODE_LPM]  = REGULATOR_MODE_IDLE,
+		[RPMH_REGULATOR_MODE_AUTO] = REGULATOR_MODE_INVALID,
+		[RPMH_REGULATOR_MODE_HPM]  = REGULATOR_MODE_FAST,
+	};
+
+	if (mode >= RPMH_REGULATOR_MODE_COUNT)
+		return -EINVAL;
+
+	return of_mode_map[mode];
+}
+
+static const int pmic_mode_map_pmic4_smps[REGULATOR_MODE_STANDBY + 1] = {
+	[REGULATOR_MODE_INVALID] = -EINVAL,
+	[REGULATOR_MODE_STANDBY] = PMIC4_SMPS_MODE_RETENTION,
+	[REGULATOR_MODE_IDLE]    = PMIC4_SMPS_MODE_PFM,
+	[REGULATOR_MODE_NORMAL]  = PMIC4_SMPS_MODE_AUTO,
+	[REGULATOR_MODE_FAST]    = PMIC4_SMPS_MODE_PWM,
+};
+
+static unsigned int rpmh_regulator_pmic4_smps_of_map_mode(unsigned int mode)
+{
+	static const unsigned int of_mode_map[RPMH_REGULATOR_MODE_COUNT] = {
+		[RPMH_REGULATOR_MODE_RET]  = REGULATOR_MODE_STANDBY,
+		[RPMH_REGULATOR_MODE_LPM]  = REGULATOR_MODE_IDLE,
+		[RPMH_REGULATOR_MODE_AUTO] = REGULATOR_MODE_NORMAL,
+		[RPMH_REGULATOR_MODE_HPM]  = REGULATOR_MODE_FAST,
+	};
+
+	if (mode >= RPMH_REGULATOR_MODE_COUNT)
+		return -EINVAL;
+
+	return of_mode_map[mode];
+}
+
+static const int pmic_mode_map_pmic4_bob[REGULATOR_MODE_STANDBY + 1] = {
+	[REGULATOR_MODE_INVALID] = -EINVAL,
+	[REGULATOR_MODE_STANDBY] = -EINVAL,
+	[REGULATOR_MODE_IDLE]    = PMIC4_BOB_MODE_PFM,
+	[REGULATOR_MODE_NORMAL]  = PMIC4_BOB_MODE_AUTO,
+	[REGULATOR_MODE_FAST]    = PMIC4_BOB_MODE_PWM,
+};
+
+static unsigned int rpmh_regulator_pmic4_bob_of_map_mode(unsigned int mode)
+{
+	static const unsigned int of_mode_map[RPMH_REGULATOR_MODE_COUNT] = {
+		[RPMH_REGULATOR_MODE_RET]  = REGULATOR_MODE_INVALID,
+		[RPMH_REGULATOR_MODE_LPM]  = REGULATOR_MODE_IDLE,
+		[RPMH_REGULATOR_MODE_AUTO] = REGULATOR_MODE_NORMAL,
+		[RPMH_REGULATOR_MODE_HPM]  = REGULATOR_MODE_FAST,
+	};
+
+	if (mode >= RPMH_REGULATOR_MODE_COUNT)
+		return -EINVAL;
+
+	return of_mode_map[mode];
+}
+
+static const struct rpmh_vreg_hw_data pmic4_pldo = {
+	.regulator_type = VRM,
+	.ops = &rpmh_regulator_vrm_ops,
+	.voltage_range = REGULATOR_LINEAR_RANGE(1664000, 0, 255, 8000),
+	.n_voltages = 256,
+	.pmic_mode_map = pmic_mode_map_pmic4_ldo,
+	.of_map_mode = rpmh_regulator_pmic4_ldo_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_pldo_lv = {
+	.regulator_type = VRM,
+	.ops = &rpmh_regulator_vrm_ops,
+	.voltage_range = REGULATOR_LINEAR_RANGE(1256000, 0, 127, 8000),
+	.n_voltages = 128,
+	.pmic_mode_map = pmic_mode_map_pmic4_ldo,
+	.of_map_mode = rpmh_regulator_pmic4_ldo_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_nldo = {
+	.regulator_type = VRM,
+	.ops = &rpmh_regulator_vrm_ops,
+	.voltage_range = REGULATOR_LINEAR_RANGE(312000, 0, 127, 8000),
+	.n_voltages = 128,
+	.pmic_mode_map = pmic_mode_map_pmic4_ldo,
+	.of_map_mode = rpmh_regulator_pmic4_ldo_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_hfsmps3 = {
+	.regulator_type = VRM,
+	.ops = &rpmh_regulator_vrm_ops,
+	.voltage_range = REGULATOR_LINEAR_RANGE(320000, 0, 215, 8000),
+	.n_voltages = 216,
+	.pmic_mode_map = pmic_mode_map_pmic4_smps,
+	.of_map_mode = rpmh_regulator_pmic4_smps_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_ftsmps426 = {
+	.regulator_type = VRM,
+	.ops = &rpmh_regulator_vrm_ops,
+	.voltage_range = REGULATOR_LINEAR_RANGE(320000, 0, 258, 4000),
+	.n_voltages = 259,
+	.pmic_mode_map = pmic_mode_map_pmic4_smps,
+	.of_map_mode = rpmh_regulator_pmic4_smps_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_bob = {
+	.regulator_type = VRM,
+	.ops = &rpmh_regulator_vrm_bypass_ops,
+	.voltage_range = REGULATOR_LINEAR_RANGE(1824000, 0, 83, 32000),
+	.n_voltages = 84,
+	.pmic_mode_map = pmic_mode_map_pmic4_bob,
+	.of_map_mode = rpmh_regulator_pmic4_bob_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_lvs = {
+	.regulator_type = XOB,
+	.ops = &rpmh_regulator_xob_ops,
+	/* LVS hardware does not support voltage or mode configuration. */
+};
+
+#define RPMH_VREG(_name, _resource_name, _hw_data, _supply_name) \
+{ \
+	.name		= _name, \
+	.resource_name	= _resource_name, \
+	.hw_data	= _hw_data, \
+	.supply_name	= _supply_name, \
+}
+
+static const struct rpmh_vreg_init_data pm8998_vreg_data[] = {
+	RPMH_VREG("smps1",  "smp%s1",  &pmic4_ftsmps426, "vdd-s1"),
+	RPMH_VREG("smps2",  "smp%s2",  &pmic4_ftsmps426, "vdd-s2"),
+	RPMH_VREG("smps3",  "smp%s3",  &pmic4_hfsmps3,   "vdd-s3"),
+	RPMH_VREG("smps4",  "smp%s4",  &pmic4_hfsmps3,   "vdd-s4"),
+	RPMH_VREG("smps5",  "smp%s5",  &pmic4_hfsmps3,   "vdd-s5"),
+	RPMH_VREG("smps6",  "smp%s6",  &pmic4_ftsmps426, "vdd-s6"),
+	RPMH_VREG("smps7",  "smp%s7",  &pmic4_ftsmps426, "vdd-s7"),
+	RPMH_VREG("smps8",  "smp%s8",  &pmic4_ftsmps426, "vdd-s8"),
+	RPMH_VREG("smps9",  "smp%s9",  &pmic4_ftsmps426, "vdd-s9"),
+	RPMH_VREG("smps10", "smp%s10", &pmic4_ftsmps426, "vdd-s10"),
+	RPMH_VREG("smps11", "smp%s11", &pmic4_ftsmps426, "vdd-s11"),
+	RPMH_VREG("smps12", "smp%s12", &pmic4_ftsmps426, "vdd-s12"),
+	RPMH_VREG("smps13", "smp%s13", &pmic4_ftsmps426, "vdd-s13"),
+	RPMH_VREG("ldo1",   "ldo%s1",  &pmic4_nldo,      "vdd-l1-l27"),
+	RPMH_VREG("ldo2",   "ldo%s2",  &pmic4_nldo,      "vdd-l2-l8-l17"),
+	RPMH_VREG("ldo3",   "ldo%s3",  &pmic4_nldo,      "vdd-l3-l11"),
+	RPMH_VREG("ldo4",   "ldo%s4",  &pmic4_nldo,      "vdd-l4-l5"),
+	RPMH_VREG("ldo5",   "ldo%s5",  &pmic4_nldo,      "vdd-l4-l5"),
+	RPMH_VREG("ldo6",   "ldo%s6",  &pmic4_pldo,      "vdd-l6"),
+	RPMH_VREG("ldo7",   "ldo%s7",  &pmic4_pldo_lv,   "vdd-l7-l12-l14-l15"),
+	RPMH_VREG("ldo8",   "ldo%s8",  &pmic4_nldo,      "vdd-l2-l8-l17"),
+	RPMH_VREG("ldo9",   "ldo%s9",  &pmic4_pldo,      "vdd-l9"),
+	RPMH_VREG("ldo10",  "ldo%s10", &pmic4_pldo,      "vdd-l10-l23-l25"),
+	RPMH_VREG("ldo11",  "ldo%s11", &pmic4_nldo,      "vdd-l3-l11"),
+	RPMH_VREG("ldo12",  "ldo%s12", &pmic4_pldo_lv,   "vdd-l7-l12-l14-l15"),
+	RPMH_VREG("ldo13",  "ldo%s13", &pmic4_pldo,      "vdd-l13-l19-l21"),
+	RPMH_VREG("ldo14",  "ldo%s14", &pmic4_pldo_lv,   "vdd-l7-l12-l14-l15"),
+	RPMH_VREG("ldo15",  "ldo%s15", &pmic4_pldo_lv,   "vdd-l7-l12-l14-l15"),
+	RPMH_VREG("ldo16",  "ldo%s16", &pmic4_pldo,      "vdd-l16-l28"),
+	RPMH_VREG("ldo17",  "ldo%s17", &pmic4_nldo,      "vdd-l2-l8-l17"),
+	RPMH_VREG("ldo18",  "ldo%s18", &pmic4_pldo,      "vdd-l18-l22"),
+	RPMH_VREG("ldo19",  "ldo%s19", &pmic4_pldo,      "vdd-l13-l19-l21"),
+	RPMH_VREG("ldo20",  "ldo%s20", &pmic4_pldo,      "vdd-l20-l24"),
+	RPMH_VREG("ldo21",  "ldo%s21", &pmic4_pldo,      "vdd-l13-l19-l21"),
+	RPMH_VREG("ldo22",  "ldo%s22", &pmic4_pldo,      "vdd-l18-l22"),
+	RPMH_VREG("ldo23",  "ldo%s23", &pmic4_pldo,      "vdd-l10-l23-l25"),
+	RPMH_VREG("ldo24",  "ldo%s24", &pmic4_pldo,      "vdd-l20-l24"),
+	RPMH_VREG("ldo25",  "ldo%s25", &pmic4_pldo,      "vdd-l10-l23-l25"),
+	RPMH_VREG("ldo26",  "ldo%s26", &pmic4_nldo,      "vdd-l26"),
+	RPMH_VREG("ldo27",  "ldo%s27", &pmic4_nldo,      "vdd-l1-l27"),
+	RPMH_VREG("ldo28",  "ldo%s28", &pmic4_pldo,      "vdd-l16-l28"),
+	RPMH_VREG("lvs1",   "vs%s1",   &pmic4_lvs,       "vin-lvs-1-2"),
+	RPMH_VREG("lvs2",   "vs%s2",   &pmic4_lvs,       "vin-lvs-1-2"),
+	{},
+};
+
+static const struct rpmh_vreg_init_data pmi8998_vreg_data[] = {
+	RPMH_VREG("bob",    "bob%s1",  &pmic4_bob,       "vdd-bob"),
+	{},
+};
+
+static const struct rpmh_vreg_init_data pm8005_vreg_data[] = {
+	RPMH_VREG("smps1",  "smp%s1",  &pmic4_ftsmps426, "vdd-s1"),
+	RPMH_VREG("smps2",  "smp%s2",  &pmic4_ftsmps426, "vdd-s2"),
+	RPMH_VREG("smps3",  "smp%s3",  &pmic4_ftsmps426, "vdd-s3"),
+	RPMH_VREG("smps4",  "smp%s4",  &pmic4_ftsmps426, "vdd-s4"),
+	{},
+};
+
+static int rpmh_regulator_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct rpmh_vreg_init_data *vreg_data;
+	struct device_node *node;
+	struct rpmh_vreg *vreg;
+	const char *pmic_id;
+	int ret;
+
+	ret = cmd_db_ready();
+	if (ret < 0) {
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Command DB not available, ret=%d\n", ret);
+		return ret;
+	}
+
+	vreg_data = of_device_get_match_data(dev);
+	if (!vreg_data)
+		return -ENODEV;
+
+	ret = of_property_read_string(dev->of_node, "qcom,pmic-id", &pmic_id);
+	if (ret < 0) {
+		dev_err(dev, "qcom,pmic-id missing in DT node\n");
+		return ret;
+	}
+
+	for_each_available_child_of_node(dev->of_node, node) {
+		vreg = devm_kzalloc(dev, sizeof(*vreg), GFP_KERNEL);
+		if (!vreg) {
+			of_node_put(node);
+			return -ENOMEM;
+		}
+
+		ret = rpmh_regulator_init_vreg(vreg, dev, node, pmic_id,
+						vreg_data);
+		if (ret < 0) {
+			of_node_put(node);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static const struct of_device_id rpmh_regulator_match_table[] = {
+	{
+		.compatible = "qcom,pm8998-rpmh-regulators",
+		.data = pm8998_vreg_data,
+	},
+	{
+		.compatible = "qcom,pmi8998-rpmh-regulators",
+		.data = pmi8998_vreg_data,
+	},
+	{
+		.compatible = "qcom,pm8005-rpmh-regulators",
+		.data = pm8005_vreg_data,
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(of, rpmh_regulator_match_table);
+
+static struct platform_driver rpmh_regulator_driver = {
+	.driver = {
+		.name = "qcom-rpmh-regulator",
+		.of_match_table	= of_match_ptr(rpmh_regulator_match_table),
+	},
+	.probe = rpmh_regulator_probe,
+};
+module_platform_driver(rpmh_regulator_driver);
+
+MODULE_DESCRIPTION("Qualcomm RPMh regulator driver");
+MODULE_LICENSE("GPL v2");
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v4 2/2] regulator: add QCOM RPMh regulator driver

[David Collins]

Add the QCOM RPMh regulator driver to manage PMIC regulators
which are controlled via RPMh on some Qualcomm Technologies, Inc.
SoCs.  RPMh is a hardware block which contains several
accelerators which are used to manage various hardware resources
that are shared between the processors of the SoC.  The final
hardware state of a regulator is determined within RPMh by
performing max aggregation of the requests made by all of the
processors.

Add support for PMIC regulator control via the voltage regulator
manager (VRM) and oscillator buffer (XOB) RPMh accelerators.
VRM supports manipulation of enable state, voltage, and mode.
XOB supports manipulation of enable state.

Signed-off-by: David Collins <collinsd@codeaurora.org>
---
 drivers/regulator/Kconfig               |   9 +
 drivers/regulator/Makefile              |   1 +
 drivers/regulator/qcom-rpmh-regulator.c | 865 ++++++++++++++++++++++++++++++++
 3 files changed, 875 insertions(+)
 create mode 100644 drivers/regulator/qcom-rpmh-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 4efae3b..1a69bdc 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -671,6 +671,15 @@ config REGULATOR_QCOM_RPM
 	  Qualcomm RPM as a module. The module will be named
 	  "qcom_rpm-regulator".
 
+config REGULATOR_QCOM_RPMH
+	tristate "Qualcomm Technologies, Inc. RPMh regulator driver"
+	depends on QCOM_RPMH || COMPILE_TEST
+	help
+	  This driver supports control of PMIC regulators via the RPMh hardware
+	  block found on Qualcomm Technologies Inc. SoCs.  RPMh regulator
+	  control allows for voting on regulator state between multiple
+	  processors within the SoC.
+
 config REGULATOR_QCOM_SMD_RPM
 	tristate "Qualcomm SMD based RPM regulator driver"
 	depends on QCOM_SMD_RPM
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index d81fb02..906f048 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -77,6 +77,7 @@ obj-$(CONFIG_REGULATOR_MT6323)	+= mt6323-regulator.o
 obj-$(CONFIG_REGULATOR_MT6380)	+= mt6380-regulator.o
 obj-$(CONFIG_REGULATOR_MT6397)	+= mt6397-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_RPM) += qcom_rpm-regulator.o
+obj-$(CONFIG_REGULATOR_QCOM_RPMH) += qcom-rpmh-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_SMD_RPM) += qcom_smd-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_SPMI) += qcom_spmi-regulator.o
 obj-$(CONFIG_REGULATOR_PALMAS) += palmas-regulator.o
diff --git a/drivers/regulator/qcom-rpmh-regulator.c b/drivers/regulator/qcom-rpmh-regulator.c
new file mode 100644
index 0000000..0217dcc
--- /dev/null
+++ b/drivers/regulator/qcom-rpmh-regulator.c
@@ -0,0 +1,865 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2018, The Linux Foundation. All rights reserved. */
+
+#define pr_fmt(fmt) "%s: " fmt, __func__
+
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+
+#include <soc/qcom/cmd-db.h>
+#include <soc/qcom/rpmh.h>
+
+#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
+
+/**
+ * enum rpmh_regulator_type - supported RPMh accelerator types
+ * %VRM:	RPMh VRM accelerator which supports voting on enable, voltage,
+ *		and mode of LDO, SMPS, and BOB type PMIC regulators.
+ * %XOB:	RPMh XOB accelerator which supports voting on the enable state
+ *		of PMIC regulators.
+ */
+enum rpmh_regulator_type {
+	VRM,
+	XOB,
+};
+
+#define RPMH_VRM_HEADROOM_MAX_UV		511000
+
+#define RPMH_REGULATOR_REG_VRM_VOLTAGE		0x0
+#define RPMH_REGULATOR_REG_ENABLE		0x4
+#define RPMH_REGULATOR_REG_VRM_MODE		0x8
+#define RPMH_REGULATOR_REG_VRM_HEADROOM		0xC
+
+#define RPMH_REGULATOR_MODE_COUNT		4
+
+#define PMIC4_LDO_MODE_RETENTION		4
+#define PMIC4_LDO_MODE_LPM			5
+#define PMIC4_LDO_MODE_HPM			7
+
+#define PMIC4_SMPS_MODE_RETENTION		4
+#define PMIC4_SMPS_MODE_PFM			5
+#define PMIC4_SMPS_MODE_AUTO			6
+#define PMIC4_SMPS_MODE_PWM			7
+
+#define PMIC4_BOB_MODE_PASS			0
+#define PMIC4_BOB_MODE_PFM			1
+#define PMIC4_BOB_MODE_AUTO			2
+#define PMIC4_BOB_MODE_PWM			3
+
+/**
+ * struct rpmh_vreg_hw_data - RPMh regulator hardware configurations
+ * @regulator_type:		RPMh accelerator type used to manage this
+ *				regulator
+ * @ops:			Pointer to regulator ops callback structure
+ * @voltage_range:		The single range of voltages supported by this
+ *				PMIC regulator type
+ * @n_voltages:			The number of unique voltage set points defined
+ *				by voltage_range
+ * @pmic_mode_map:		Array indexed by regulator framework mode
+ *				containing PMIC hardware modes.  Must be large
+ *				enough to index all framework modes supported
+ *				by this regulator hardware type.
+ * @of_map_mode:		Maps an RPMH_REGULATOR_MODE_* mode value defined
+ *				in device tree to a regulator framework mode
+ */
+struct rpmh_vreg_hw_data {
+	enum rpmh_regulator_type		regulator_type;
+	const struct regulator_ops		*ops;
+	const struct regulator_linear_range	voltage_range;
+	int					n_voltages;
+	const int				*pmic_mode_map;
+	unsigned int			      (*of_map_mode)(unsigned int mode);
+};
+
+/**
+ * struct rpmh_vreg - individual RPMh regulator data structure encapsulating a
+ *		single regulator device
+ * @dev:			Device pointer for the top-level PMIC RPMh
+ *				regulator parent device.  This is used as a
+ *				handle in RPMh write requests.
+ * @addr:			Base address of the regulator resource within
+ *				an RPMh accelerator
+ * @rdesc:			Regulator descriptor
+ * @hw_data:			PMIC regulator configuration data for this RPMh
+ *				regulator
+ * @always_wait_for_ack:	Boolean flag indicating if a request must always
+ *				wait for an ACK from RPMh before continuing even
+ *				if it corresponds to a strictly lower power
+ *				state (e.g. enabled --> disabled).
+ * @drms_modes:			Array of regulator framework modes which can
+ *				be configured dynamically for this regulator
+ *				via the set_load() callback.
+ * @drms_mode_max_uAs:		Array of maximum load currents in microamps
+ *				supported by the corresponding modes in
+ *				drms_mode.  Elements must be specified in
+ *				strictly increasing order.
+ * @drms_mode_count:		The number of elements in drms_mode array.
+ * @enabled:			Flag indicating if the regulator is enabled or
+ *				not
+ * @ever_enabled:		Boolean indicating that the regulator has been
+ *				explicitly enabled at least once.  Voltage
+ *				requests should be cached when this flag is not
+ *				set.
+ * @bypassed:			Boolean indicating if the regulator is in
+ *				bypass (pass-through) mode or not.  This is
+ *				only used by BOB rpmh-regulator resources.
+ * @voltage_selector:		Selector used for get_voltage_sel() and
+ *				set_voltage_sel() callbacks
+ * @mode:			RPMh VRM regulator current framework mode
+ */
+struct rpmh_vreg {
+	struct device			*dev;
+	u32				addr;
+	struct regulator_desc		rdesc;
+	const struct rpmh_vreg_hw_data	*hw_data;
+	bool				always_wait_for_ack;
+	unsigned int			*drms_modes;
+	int				*drms_mode_max_uAs;
+	size_t				drms_mode_count;
+
+	int				enabled;
+	bool				ever_enabled;
+	bool				bypassed;
+	int				voltage_selector;
+	unsigned int			mode;
+};
+
+/**
+ * struct rpmh_vreg_init_data - initialization data for an RPMh regulator
+ * @name:			Name for the regulator which also corresponds
+ *				to the device tree subnode name of the regulator
+ * @resource_name:		RPMh regulator resource name format string.
+ *				This must include exactly one field: '%s' which
+ *				is filled at run-time with the PMIC ID provided
+ *				by device tree property qcom,pmic-id.  Example:
+ *				"ldo%s1" for RPMh resource "ldoa1".
+ * @supply_name:		Parent supply regulator name
+ * @hw_data:			Configuration data for this PMIC regulator type
+ */
+struct rpmh_vreg_init_data {
+	const char			*name;
+	const char			*resource_name;
+	const char			*supply_name;
+	const struct rpmh_vreg_hw_data	*hw_data;
+};
+
+/**
+ * rpmh_regulator_send_request() - send the request to RPMh
+ * @vreg:		Pointer to the RPMh regulator
+ * @cmd:		RPMh commands to send
+ * @count:		Size of cmd array
+ * @wait_for_ack:	Boolean indicating if execution must wait until the
+ *			request has been acknowledged as complete
+ *
+ * Return: 0 on success, errno on failure
+ */
+static int rpmh_regulator_send_request(struct rpmh_vreg *vreg,
+			struct tcs_cmd *cmd, int count, bool wait_for_ack)
+{
+	int ret;
+
+	if (wait_for_ack || vreg->always_wait_for_ack)
+		ret = rpmh_write(vreg->dev, RPMH_ACTIVE_ONLY_STATE, cmd, count);
+	else
+		ret = rpmh_write_async(vreg->dev, RPMH_ACTIVE_ONLY_STATE, cmd,
+					count);
+
+	return ret;
+}
+
+static int _rpmh_regulator_vrm_set_voltage_sel(struct regulator_dev *rdev,
+				unsigned int selector, bool wait_for_ack)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+	struct tcs_cmd cmd = {
+		.addr = vreg->addr + RPMH_REGULATOR_REG_VRM_VOLTAGE,
+	};
+	int ret;
+
+	/* VRM voltage control register is set with voltage in millivolts. */
+	cmd.data = DIV_ROUND_UP(regulator_list_voltage_linear_range(rdev,
+							selector), 1000);
+
+	ret = rpmh_regulator_send_request(vreg, &cmd, 1, wait_for_ack);
+	if (!ret)
+		vreg->voltage_selector = selector;
+
+	return 0;
+}
+
+static int rpmh_regulator_vrm_set_voltage_sel(struct regulator_dev *rdev,
+				unsigned int selector)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+
+	if (vreg->ever_enabled) {
+		return _rpmh_regulator_vrm_set_voltage_sel(rdev, selector,
+					selector > vreg->voltage_selector);
+	} else {
+		/*
+		 * Cache the voltage and send it later when the regulator is
+		 * enabled.
+		 */
+		vreg->voltage_selector = selector;
+	}
+
+	return 0;
+}
+
+static int rpmh_regulator_vrm_get_voltage_sel(struct regulator_dev *rdev)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+
+	return vreg->voltage_selector;
+}
+
+static int rpmh_regulator_is_enabled(struct regulator_dev *rdev)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+
+	return vreg->enabled;
+}
+
+static int rpmh_regulator_enable(struct regulator_dev *rdev)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+	struct tcs_cmd cmd = {
+		.addr = vreg->addr + RPMH_REGULATOR_REG_ENABLE,
+		.data = 1,
+	};
+	int ret;
+
+	if (!vreg->ever_enabled && vreg->voltage_selector != -ENOTRECOVERABLE) {
+		ret = _rpmh_regulator_vrm_set_voltage_sel(rdev,
+						vreg->voltage_selector, true);
+		if (ret < 0)
+			return ret;
+	}
+
+	ret = rpmh_regulator_send_request(vreg, &cmd, 1, true);
+	if (!ret) {
+		vreg->enabled = true;
+		vreg->ever_enabled = true;
+	}
+
+	return ret;
+}
+
+static int rpmh_regulator_disable(struct regulator_dev *rdev)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+	struct tcs_cmd cmd = {
+		.addr = vreg->addr + RPMH_REGULATOR_REG_ENABLE,
+		.data = 0,
+	};
+	int ret;
+
+	ret = rpmh_regulator_send_request(vreg, &cmd, 1, false);
+	if (!ret)
+		vreg->enabled = false;
+
+	return ret;
+}
+
+static int rpmh_regulator_vrm_set_mode_bypass(struct rpmh_vreg *vreg,
+					unsigned int mode, bool bypassed)
+{
+	struct tcs_cmd cmd = {
+		.addr = vreg->addr + RPMH_REGULATOR_REG_VRM_MODE,
+	};
+	int pmic_mode;
+
+	if (mode > REGULATOR_MODE_STANDBY)
+		return -EINVAL;
+
+	pmic_mode = vreg->hw_data->pmic_mode_map[mode];
+	if (pmic_mode < 0)
+		return pmic_mode;
+
+	cmd.data = bypassed ? PMIC4_BOB_MODE_PASS : pmic_mode;
+
+	return rpmh_regulator_send_request(vreg, &cmd, 1, true);
+}
+
+static int rpmh_regulator_vrm_set_mode(struct regulator_dev *rdev,
+					unsigned int mode)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+	int ret;
+
+	if (mode == vreg->mode)
+		return 0;
+
+	ret = rpmh_regulator_vrm_set_mode_bypass(vreg, mode, vreg->bypassed);
+	if (!ret)
+		vreg->mode = mode;
+
+	return ret;
+}
+
+static unsigned int rpmh_regulator_vrm_get_mode(struct regulator_dev *rdev)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+
+	return vreg->mode;
+}
+
+/**
+ * rpmh_regulator_vrm_set_load() - set the regulator mode based upon the load
+ *		current requested
+ * @rdev:		Regulator device pointer for the rpmh-regulator
+ * @load_uA:		Aggregated load current in microamps
+ *
+ * This function is used in the regulator_ops for VRM type RPMh regulator
+ * devices.  It updates the mode of the regulator using a table of maximum
+ * load currents per mode specified in device tree properties.
+ *
+ * Return: 0 on success, errno on failure
+ */
+static int rpmh_regulator_vrm_set_load(struct regulator_dev *rdev, int load_uA)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+	int i;
+
+	for (i = 0; i < vreg->drms_mode_count - 1; i++)
+		if (vreg->drms_mode_max_uAs[i] >= load_uA)
+			break;
+	if (load_uA > vreg->drms_mode_max_uAs[vreg->drms_mode_count - 1])
+		dev_warn(vreg->dev, "%s: requested load=%d uA greater than max=%d uA\n",
+			vreg->rdesc.name, load_uA,
+			vreg->drms_mode_max_uAs[vreg->drms_mode_count - 1]);
+
+	return rpmh_regulator_vrm_set_mode(rdev, vreg->drms_modes[i]);
+}
+
+static int rpmh_regulator_vrm_set_bypass(struct regulator_dev *rdev,
+				bool enable)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+	int ret;
+
+	if (vreg->bypassed == enable)
+		return 0;
+
+	ret = rpmh_regulator_vrm_set_mode_bypass(vreg, vreg->mode, enable);
+	if (!ret)
+		vreg->bypassed = enable;
+
+	return ret;
+}
+
+static int rpmh_regulator_vrm_get_bypass(struct regulator_dev *rdev,
+				bool *enable)
+{
+	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
+
+	*enable = vreg->bypassed;
+
+	return 0;
+}
+
+static const struct regulator_ops rpmh_regulator_vrm_ops = {
+	.enable			= rpmh_regulator_enable,
+	.disable		= rpmh_regulator_disable,
+	.is_enabled		= rpmh_regulator_is_enabled,
+	.set_voltage_sel	= rpmh_regulator_vrm_set_voltage_sel,
+	.get_voltage_sel	= rpmh_regulator_vrm_get_voltage_sel,
+	.list_voltage		= regulator_list_voltage_linear_range,
+	.set_mode		= rpmh_regulator_vrm_set_mode,
+	.get_mode		= rpmh_regulator_vrm_get_mode,
+	.set_load		= rpmh_regulator_vrm_set_load,
+};
+
+static const struct regulator_ops rpmh_regulator_vrm_bypass_ops = {
+	.enable			= rpmh_regulator_enable,
+	.disable		= rpmh_regulator_disable,
+	.is_enabled		= rpmh_regulator_is_enabled,
+	.set_voltage_sel	= rpmh_regulator_vrm_set_voltage_sel,
+	.get_voltage_sel	= rpmh_regulator_vrm_get_voltage_sel,
+	.list_voltage		= regulator_list_voltage_linear_range,
+	.set_mode		= rpmh_regulator_vrm_set_mode,
+	.get_mode		= rpmh_regulator_vrm_get_mode,
+	.set_load		= rpmh_regulator_vrm_set_load,
+	.set_bypass		= rpmh_regulator_vrm_set_bypass,
+	.get_bypass		= rpmh_regulator_vrm_get_bypass,
+};
+
+static const struct regulator_ops rpmh_regulator_xob_ops = {
+	.enable			= rpmh_regulator_enable,
+	.disable		= rpmh_regulator_disable,
+	.is_enabled		= rpmh_regulator_is_enabled,
+};
+
+/**
+ * rpmh_regulator_parse_vrm_modes() - parse the supported mode configurations
+ *		for a VRM RPMh resource from device tree
+ * vreg:		Pointer to the individual rpmh-regulator resource
+ * dev:			Pointer to the top level rpmh-regulator PMIC device
+ * node:		Pointer to the individual rpmh-regulator resource
+ *			device node
+ *
+ * This function initializes the drms_modes[] and drms_mode_max_uAs[] arrays of
+ * vreg based upon the values of optional device tree properties.
+ *
+ * Return: 0 on success, errno on failure
+ */
+static int rpmh_regulator_parse_vrm_modes(struct rpmh_vreg *vreg,
+				struct device *dev, struct device_node *node)
+{
+	const char *prop;
+	int i, len, ret, mode;
+	u32 *buf;
+
+	/* qcom,regulator-drms-modes is optional */
+	prop = "qcom,regulator-drms-modes";
+	len = of_property_count_elems_of_size(node, prop, sizeof(u32));
+	if (len < 0)
+		return 0;
+
+	vreg->drms_modes = devm_kcalloc(dev, len, sizeof(*vreg->drms_modes),
+					GFP_KERNEL);
+	vreg->drms_mode_max_uAs = devm_kcalloc(dev, len,
+					       sizeof(*vreg->drms_mode_max_uAs),
+					       GFP_KERNEL);
+	if (!vreg->drms_modes || !vreg->drms_mode_max_uAs)
+		return -ENOMEM;
+	vreg->drms_mode_count = len;
+
+	buf = kcalloc(len, sizeof(*buf), GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = of_property_read_u32_array(node, prop, buf, len);
+	if (ret < 0) {
+		dev_err(dev, "%s: unable to read %s, ret=%d\n",
+			node->name, prop, ret);
+		goto done;
+	}
+
+	for (i = 0; i < len; i++) {
+		mode = vreg->hw_data->of_map_mode(buf[i]);
+		if (mode == REGULATOR_MODE_INVALID) {
+			dev_err(dev, "%s: element %d of %s = %u is invalid for this regulator\n",
+				node->name, i, prop, buf[i]);
+			ret = -EINVAL;
+			goto done;
+		}
+
+		vreg->drms_modes[i] = mode;
+	}
+
+	prop = "qcom,drms-mode-max-microamps";
+	len = of_property_count_elems_of_size(node, prop, sizeof(u32));
+	if (len != vreg->drms_mode_count) {
+		dev_err(dev, "%s: invalid element count=%d for %s\n",
+			node->name, len, prop);
+		ret = -EINVAL;
+		goto done;
+	}
+
+	ret = of_property_read_u32_array(node, prop, buf, len);
+	if (ret < 0) {
+		dev_err(dev, "%s: unable to read %s, ret=%d\n",
+			node->name, prop, ret);
+		goto done;
+	}
+
+	for (i = 0; i < len; i++) {
+		vreg->drms_mode_max_uAs[i] = buf[i];
+
+		if (i > 0 && vreg->drms_mode_max_uAs[i]
+				<= vreg->drms_mode_max_uAs[i - 1]) {
+			dev_err(dev, "%s: %s elements are not in ascending order\n",
+				node->name, prop);
+			ret = -EINVAL;
+			goto done;
+		}
+	}
+
+done:
+	kfree(buf);
+	return ret;
+}
+
+/**
+ * rpmh_regulator_init_vreg() - initialize all attributes of an rpmh-regulator
+ * vreg:		Pointer to the individual rpmh-regulator resource
+ * dev:			Pointer to the top level rpmh-regulator PMIC device
+ * node:		Pointer to the individual rpmh-regulator resource
+ *			device node
+ * pmic_id:		String used to identify the top level rpmh-regulator
+ *			PMIC device on the board
+ * rpmh_data:		Pointer to a null-terminated array of rpmh-regulator
+ *			resources defined for the top level PMIC device
+ *
+ * Return: 0 on success, errno on failure
+ */
+static int rpmh_regulator_init_vreg(struct rpmh_vreg *vreg, struct device *dev,
+				struct device_node *node, const char *pmic_id,
+				const struct rpmh_vreg_init_data *rpmh_data)
+{
+	struct regulator_config reg_config = {};
+	char rpmh_resource_name[20] = "";
+	struct regulator_dev *rdev;
+	enum rpmh_regulator_type type;
+	struct regulator_init_data *init_data;
+	int ret;
+
+	vreg->dev = dev;
+
+	for (; rpmh_data->name; rpmh_data++)
+		if (!strcmp(rpmh_data->name, node->name))
+			break;
+
+	if (!rpmh_data->name) {
+		dev_err(dev, "Unknown regulator %s\n", node->name);
+		return -EINVAL;
+	}
+
+	scnprintf(rpmh_resource_name, sizeof(rpmh_resource_name),
+		rpmh_data->resource_name, pmic_id);
+
+	vreg->addr = cmd_db_read_addr(rpmh_resource_name);
+	if (!vreg->addr) {
+		dev_err(dev, "%s: could not find RPMh address for resource %s\n",
+			node->name, rpmh_resource_name);
+		return -ENODEV;
+	}
+
+	vreg->rdesc.name = rpmh_data->name;
+	vreg->rdesc.supply_name = rpmh_data->supply_name;
+	vreg->hw_data = rpmh_data->hw_data;
+
+	vreg->enabled = -EINVAL;
+	vreg->voltage_selector = -ENOTRECOVERABLE;
+	vreg->mode = REGULATOR_MODE_INVALID;
+
+	if (rpmh_data->hw_data->n_voltages) {
+		vreg->rdesc.linear_ranges = &rpmh_data->hw_data->voltage_range;
+		vreg->rdesc.n_linear_ranges = 1;
+		vreg->rdesc.n_voltages = rpmh_data->hw_data->n_voltages;
+	}
+
+	type = rpmh_data->hw_data->regulator_type;
+	if (type == VRM) {
+		ret = rpmh_regulator_parse_vrm_modes(vreg, dev, node);
+		if (ret < 0)
+			return ret;
+	}
+
+	vreg->always_wait_for_ack = of_property_read_bool(node,
+						"qcom,always-wait-for-ack");
+
+	vreg->rdesc.owner	= THIS_MODULE;
+	vreg->rdesc.type	= REGULATOR_VOLTAGE;
+	vreg->rdesc.ops		= vreg->hw_data->ops;
+	vreg->rdesc.of_map_mode	= vreg->hw_data->of_map_mode;
+
+	init_data = of_get_regulator_init_data(dev, node, &vreg->rdesc);
+	if (!init_data)
+		return -ENOMEM;
+
+	if (type == XOB && init_data->constraints.min_uV &&
+	    init_data->constraints.min_uV == init_data->constraints.max_uV) {
+		vreg->rdesc.fixed_uV = init_data->constraints.min_uV;
+		vreg->rdesc.n_voltages = 1;
+	}
+
+	reg_config.dev		= dev;
+	reg_config.init_data	= init_data;
+	reg_config.of_node	= node;
+	reg_config.driver_data	= vreg;
+
+	rdev = devm_regulator_register(dev, &vreg->rdesc, &reg_config);
+	if (IS_ERR(rdev)) {
+		ret = PTR_ERR(rdev);
+		rdev = NULL;
+		dev_err(dev, "%s: devm_regulator_register() failed, ret=%d\n",
+			node->name, ret);
+		return ret;
+	}
+
+	dev_dbg(dev, "%s regulator registered for RPMh resource %s @ 0x%05X\n",
+		node->name, rpmh_resource_name, vreg->addr);
+
+	return ret;
+}
+
+static const int pmic_mode_map_pmic4_ldo[REGULATOR_MODE_STANDBY + 1] = {
+	[REGULATOR_MODE_INVALID] = -EINVAL,
+	[REGULATOR_MODE_STANDBY] = PMIC4_LDO_MODE_RETENTION,
+	[REGULATOR_MODE_IDLE]    = PMIC4_LDO_MODE_LPM,
+	[REGULATOR_MODE_NORMAL]  = -EINVAL,
+	[REGULATOR_MODE_FAST]    = PMIC4_LDO_MODE_HPM,
+};
+
+static unsigned int rpmh_regulator_pmic4_ldo_of_map_mode(unsigned int mode)
+{
+	static const unsigned int of_mode_map[RPMH_REGULATOR_MODE_COUNT] = {
+		[RPMH_REGULATOR_MODE_RET]  = REGULATOR_MODE_STANDBY,
+		[RPMH_REGULATOR_MODE_LPM]  = REGULATOR_MODE_IDLE,
+		[RPMH_REGULATOR_MODE_AUTO] = REGULATOR_MODE_INVALID,
+		[RPMH_REGULATOR_MODE_HPM]  = REGULATOR_MODE_FAST,
+	};
+
+	if (mode >= RPMH_REGULATOR_MODE_COUNT)
+		return -EINVAL;
+
+	return of_mode_map[mode];
+}
+
+static const int pmic_mode_map_pmic4_smps[REGULATOR_MODE_STANDBY + 1] = {
+	[REGULATOR_MODE_INVALID] = -EINVAL,
+	[REGULATOR_MODE_STANDBY] = PMIC4_SMPS_MODE_RETENTION,
+	[REGULATOR_MODE_IDLE]    = PMIC4_SMPS_MODE_PFM,
+	[REGULATOR_MODE_NORMAL]  = PMIC4_SMPS_MODE_AUTO,
+	[REGULATOR_MODE_FAST]    = PMIC4_SMPS_MODE_PWM,
+};
+
+static unsigned int rpmh_regulator_pmic4_smps_of_map_mode(unsigned int mode)
+{
+	static const unsigned int of_mode_map[RPMH_REGULATOR_MODE_COUNT] = {
+		[RPMH_REGULATOR_MODE_RET]  = REGULATOR_MODE_STANDBY,
+		[RPMH_REGULATOR_MODE_LPM]  = REGULATOR_MODE_IDLE,
+		[RPMH_REGULATOR_MODE_AUTO] = REGULATOR_MODE_NORMAL,
+		[RPMH_REGULATOR_MODE_HPM]  = REGULATOR_MODE_FAST,
+	};
+
+	if (mode >= RPMH_REGULATOR_MODE_COUNT)
+		return -EINVAL;
+
+	return of_mode_map[mode];
+}
+
+static const int pmic_mode_map_pmic4_bob[REGULATOR_MODE_STANDBY + 1] = {
+	[REGULATOR_MODE_INVALID] = -EINVAL,
+	[REGULATOR_MODE_STANDBY] = -EINVAL,
+	[REGULATOR_MODE_IDLE]    = PMIC4_BOB_MODE_PFM,
+	[REGULATOR_MODE_NORMAL]  = PMIC4_BOB_MODE_AUTO,
+	[REGULATOR_MODE_FAST]    = PMIC4_BOB_MODE_PWM,
+};
+
+static unsigned int rpmh_regulator_pmic4_bob_of_map_mode(unsigned int mode)
+{
+	static const unsigned int of_mode_map[RPMH_REGULATOR_MODE_COUNT] = {
+		[RPMH_REGULATOR_MODE_RET]  = REGULATOR_MODE_INVALID,
+		[RPMH_REGULATOR_MODE_LPM]  = REGULATOR_MODE_IDLE,
+		[RPMH_REGULATOR_MODE_AUTO] = REGULATOR_MODE_NORMAL,
+		[RPMH_REGULATOR_MODE_HPM]  = REGULATOR_MODE_FAST,
+	};
+
+	if (mode >= RPMH_REGULATOR_MODE_COUNT)
+		return -EINVAL;
+
+	return of_mode_map[mode];
+}
+
+static const struct rpmh_vreg_hw_data pmic4_pldo = {
+	.regulator_type = VRM,
+	.ops = &rpmh_regulator_vrm_ops,
+	.voltage_range = REGULATOR_LINEAR_RANGE(1664000, 0, 255, 8000),
+	.n_voltages = 256,
+	.pmic_mode_map = pmic_mode_map_pmic4_ldo,
+	.of_map_mode = rpmh_regulator_pmic4_ldo_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_pldo_lv = {
+	.regulator_type = VRM,
+	.ops = &rpmh_regulator_vrm_ops,
+	.voltage_range = REGULATOR_LINEAR_RANGE(1256000, 0, 127, 8000),
+	.n_voltages = 128,
+	.pmic_mode_map = pmic_mode_map_pmic4_ldo,
+	.of_map_mode = rpmh_regulator_pmic4_ldo_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_nldo = {
+	.regulator_type = VRM,
+	.ops = &rpmh_regulator_vrm_ops,
+	.voltage_range = REGULATOR_LINEAR_RANGE(312000, 0, 127, 8000),
+	.n_voltages = 128,
+	.pmic_mode_map = pmic_mode_map_pmic4_ldo,
+	.of_map_mode = rpmh_regulator_pmic4_ldo_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_hfsmps3 = {
+	.regulator_type = VRM,
+	.ops = &rpmh_regulator_vrm_ops,
+	.voltage_range = REGULATOR_LINEAR_RANGE(320000, 0, 215, 8000),
+	.n_voltages = 216,
+	.pmic_mode_map = pmic_mode_map_pmic4_smps,
+	.of_map_mode = rpmh_regulator_pmic4_smps_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_ftsmps426 = {
+	.regulator_type = VRM,
+	.ops = &rpmh_regulator_vrm_ops,
+	.voltage_range = REGULATOR_LINEAR_RANGE(320000, 0, 258, 4000),
+	.n_voltages = 259,
+	.pmic_mode_map = pmic_mode_map_pmic4_smps,
+	.of_map_mode = rpmh_regulator_pmic4_smps_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_bob = {
+	.regulator_type = VRM,
+	.ops = &rpmh_regulator_vrm_bypass_ops,
+	.voltage_range = REGULATOR_LINEAR_RANGE(1824000, 0, 83, 32000),
+	.n_voltages = 84,
+	.pmic_mode_map = pmic_mode_map_pmic4_bob,
+	.of_map_mode = rpmh_regulator_pmic4_bob_of_map_mode,
+};
+
+static const struct rpmh_vreg_hw_data pmic4_lvs = {
+	.regulator_type = XOB,
+	.ops = &rpmh_regulator_xob_ops,
+	/* LVS hardware does not support voltage or mode configuration. */
+};
+
+#define RPMH_VREG(_name, _resource_name, _hw_data, _supply_name) \
+{ \
+	.name		= _name, \
+	.resource_name	= _resource_name, \
+	.hw_data	= _hw_data, \
+	.supply_name	= _supply_name, \
+}
+
+static const struct rpmh_vreg_init_data pm8998_vreg_data[] = {
+	RPMH_VREG("smps1",  "smp%s1",  &pmic4_ftsmps426, "vdd-s1"),
+	RPMH_VREG("smps2",  "smp%s2",  &pmic4_ftsmps426, "vdd-s2"),
+	RPMH_VREG("smps3",  "smp%s3",  &pmic4_hfsmps3,   "vdd-s3"),
+	RPMH_VREG("smps4",  "smp%s4",  &pmic4_hfsmps3,   "vdd-s4"),
+	RPMH_VREG("smps5",  "smp%s5",  &pmic4_hfsmps3,   "vdd-s5"),
+	RPMH_VREG("smps6",  "smp%s6",  &pmic4_ftsmps426, "vdd-s6"),
+	RPMH_VREG("smps7",  "smp%s7",  &pmic4_ftsmps426, "vdd-s7"),
+	RPMH_VREG("smps8",  "smp%s8",  &pmic4_ftsmps426, "vdd-s8"),
+	RPMH_VREG("smps9",  "smp%s9",  &pmic4_ftsmps426, "vdd-s9"),
+	RPMH_VREG("smps10", "smp%s10", &pmic4_ftsmps426, "vdd-s10"),
+	RPMH_VREG("smps11", "smp%s11", &pmic4_ftsmps426, "vdd-s11"),
+	RPMH_VREG("smps12", "smp%s12", &pmic4_ftsmps426, "vdd-s12"),
+	RPMH_VREG("smps13", "smp%s13", &pmic4_ftsmps426, "vdd-s13"),
+	RPMH_VREG("ldo1",   "ldo%s1",  &pmic4_nldo,      "vdd-l1-l27"),
+	RPMH_VREG("ldo2",   "ldo%s2",  &pmic4_nldo,      "vdd-l2-l8-l17"),
+	RPMH_VREG("ldo3",   "ldo%s3",  &pmic4_nldo,      "vdd-l3-l11"),
+	RPMH_VREG("ldo4",   "ldo%s4",  &pmic4_nldo,      "vdd-l4-l5"),
+	RPMH_VREG("ldo5",   "ldo%s5",  &pmic4_nldo,      "vdd-l4-l5"),
+	RPMH_VREG("ldo6",   "ldo%s6",  &pmic4_pldo,      "vdd-l6"),
+	RPMH_VREG("ldo7",   "ldo%s7",  &pmic4_pldo_lv,   "vdd-l7-l12-l14-l15"),
+	RPMH_VREG("ldo8",   "ldo%s8",  &pmic4_nldo,      "vdd-l2-l8-l17"),
+	RPMH_VREG("ldo9",   "ldo%s9",  &pmic4_pldo,      "vdd-l9"),
+	RPMH_VREG("ldo10",  "ldo%s10", &pmic4_pldo,      "vdd-l10-l23-l25"),
+	RPMH_VREG("ldo11",  "ldo%s11", &pmic4_nldo,      "vdd-l3-l11"),
+	RPMH_VREG("ldo12",  "ldo%s12", &pmic4_pldo_lv,   "vdd-l7-l12-l14-l15"),
+	RPMH_VREG("ldo13",  "ldo%s13", &pmic4_pldo,      "vdd-l13-l19-l21"),
+	RPMH_VREG("ldo14",  "ldo%s14", &pmic4_pldo_lv,   "vdd-l7-l12-l14-l15"),
+	RPMH_VREG("ldo15",  "ldo%s15", &pmic4_pldo_lv,   "vdd-l7-l12-l14-l15"),
+	RPMH_VREG("ldo16",  "ldo%s16", &pmic4_pldo,      "vdd-l16-l28"),
+	RPMH_VREG("ldo17",  "ldo%s17", &pmic4_nldo,      "vdd-l2-l8-l17"),
+	RPMH_VREG("ldo18",  "ldo%s18", &pmic4_pldo,      "vdd-l18-l22"),
+	RPMH_VREG("ldo19",  "ldo%s19", &pmic4_pldo,      "vdd-l13-l19-l21"),
+	RPMH_VREG("ldo20",  "ldo%s20", &pmic4_pldo,      "vdd-l20-l24"),
+	RPMH_VREG("ldo21",  "ldo%s21", &pmic4_pldo,      "vdd-l13-l19-l21"),
+	RPMH_VREG("ldo22",  "ldo%s22", &pmic4_pldo,      "vdd-l18-l22"),
+	RPMH_VREG("ldo23",  "ldo%s23", &pmic4_pldo,      "vdd-l10-l23-l25"),
+	RPMH_VREG("ldo24",  "ldo%s24", &pmic4_pldo,      "vdd-l20-l24"),
+	RPMH_VREG("ldo25",  "ldo%s25", &pmic4_pldo,      "vdd-l10-l23-l25"),
+	RPMH_VREG("ldo26",  "ldo%s26", &pmic4_nldo,      "vdd-l26"),
+	RPMH_VREG("ldo27",  "ldo%s27", &pmic4_nldo,      "vdd-l1-l27"),
+	RPMH_VREG("ldo28",  "ldo%s28", &pmic4_pldo,      "vdd-l16-l28"),
+	RPMH_VREG("lvs1",   "vs%s1",   &pmic4_lvs,       "vin-lvs-1-2"),
+	RPMH_VREG("lvs2",   "vs%s2",   &pmic4_lvs,       "vin-lvs-1-2"),
+	{},
+};
+
+static const struct rpmh_vreg_init_data pmi8998_vreg_data[] = {
+	RPMH_VREG("bob",    "bob%s1",  &pmic4_bob,       "vdd-bob"),
+	{},
+};
+
+static const struct rpmh_vreg_init_data pm8005_vreg_data[] = {
+	RPMH_VREG("smps1",  "smp%s1",  &pmic4_ftsmps426, "vdd-s1"),
+	RPMH_VREG("smps2",  "smp%s2",  &pmic4_ftsmps426, "vdd-s2"),
+	RPMH_VREG("smps3",  "smp%s3",  &pmic4_ftsmps426, "vdd-s3"),
+	RPMH_VREG("smps4",  "smp%s4",  &pmic4_ftsmps426, "vdd-s4"),
+	{},
+};
+
+static int rpmh_regulator_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct rpmh_vreg_init_data *vreg_data;
+	struct device_node *node;
+	struct rpmh_vreg *vreg;
+	const char *pmic_id;
+	int ret;
+
+	ret = cmd_db_ready();
+	if (ret < 0) {
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Command DB not available, ret=%d\n", ret);
+		return ret;
+	}
+
+	vreg_data = of_device_get_match_data(dev);
+	if (!vreg_data)
+		return -ENODEV;
+
+	ret = of_property_read_string(dev->of_node, "qcom,pmic-id", &pmic_id);
+	if (ret < 0) {
+		dev_err(dev, "qcom,pmic-id missing in DT node\n");
+		return ret;
+	}
+
+	for_each_available_child_of_node(dev->of_node, node) {
+		vreg = devm_kzalloc(dev, sizeof(*vreg), GFP_KERNEL);
+		if (!vreg) {
+			of_node_put(node);
+			return -ENOMEM;
+		}
+
+		ret = rpmh_regulator_init_vreg(vreg, dev, node, pmic_id,
+						vreg_data);
+		if (ret < 0) {
+			of_node_put(node);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static const struct of_device_id rpmh_regulator_match_table[] = {
+	{
+		.compatible = "qcom,pm8998-rpmh-regulators",
+		.data = pm8998_vreg_data,
+	},
+	{
+		.compatible = "qcom,pmi8998-rpmh-regulators",
+		.data = pmi8998_vreg_data,
+	},
+	{
+		.compatible = "qcom,pm8005-rpmh-regulators",
+		.data = pm8005_vreg_data,
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(of, rpmh_regulator_match_table);
+
+static struct platform_driver rpmh_regulator_driver = {
+	.driver = {
+		.name = "qcom-rpmh-regulator",
+		.of_match_table	= of_match_ptr(rpmh_regulator_match_table),
+	},
+	.probe = rpmh_regulator_probe,
+};
+module_platform_driver(rpmh_regulator_driver);
+
+MODULE_DESCRIPTION("Qualcomm RPMh regulator driver");
+MODULE_LICENSE("GPL v2");
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 2/2] regulator: add QCOM RPMh regulator driver

[Doug Anderson]

Hi,

On Tue, May 22, 2018 at 7:43 PM, David Collins <collinsd@codeaurora.org> wrote:
> + * @ever_enabled:              Boolean indicating that the regulator has been
> + *                             explicitly enabled at least once.  Voltage
> + *                             requests should be cached when this flag is not
> + *                             set.

Do you really need this extra boolean?  Can't you just check if
"enabled" is still "-EINVAL"?  If it is then you don't pass the
voltage along.

...this would mean that you'd also need to send the voltage vote when
the regulator core tries to disable unused regulators at the end of
bootup, but that should be OK right?  If we never touched a regulator
anywhere at probe time and we're about to vote to disable it, we know
there's nobody requiring it to still be on.  We can vote for the
voltage now without fear of messing up a vote that the BIOS left in
place.

In theory this should also allow you to assert your vote about the
voltage of a regulator that has never been enabled, which (if I
understand correctly) you consider to be a feature.

---

Other than that comment, everything else here looks good to go if Mark
is good with it.  As per the previous threads there are some things
that I don't like a ton, but I feel it is between you and Mark to
decide if you're good with it.  A summary of those issues are:

1. I still believe that when we disable a regulator in Linux we should
tell RPMh that we vote for the lowest voltage.  ...but if Mark is
happy with the way the driver works now I won't push it.

2. As per my comments in the bindings patch, I still believe that
"qcom,drms-mode-max-microamps" belongs in the core.  Again, up to
Mark.

3. As per my comments in the bindings patch, I still believe that
we're just adding lots of noise to the DT most of the time by
specifying both "qcom,regulator-drms-modes" and
"regulator-allowed-modes".  Again, up to Mark.


-Doug

[PATCH v4 2/2] regulator: add QCOM RPMh regulator driver

[Doug Anderson]

Hi,

On Tue, May 22, 2018 at 7:43 PM, David Collins <collinsd@codeaurora.org> wrote:
> + * @ever_enabled:              Boolean indicating that the regulator has been
> + *                             explicitly enabled at least once.  Voltage
> + *                             requests should be cached when this flag is not
> + *                             set.

Do you really need this extra boolean?  Can't you just check if
"enabled" is still "-EINVAL"?  If it is then you don't pass the
voltage along.

...this would mean that you'd also need to send the voltage vote when
the regulator core tries to disable unused regulators at the end of
bootup, but that should be OK right?  If we never touched a regulator
anywhere at probe time and we're about to vote to disable it, we know
there's nobody requiring it to still be on.  We can vote for the
voltage now without fear of messing up a vote that the BIOS left in
place.

In theory this should also allow you to assert your vote about the
voltage of a regulator that has never been enabled, which (if I
understand correctly) you consider to be a feature.

---

Other than that comment, everything else here looks good to go if Mark
is good with it.  As per the previous threads there are some things
that I don't like a ton, but I feel it is between you and Mark to
decide if you're good with it.  A summary of those issues are:

1. I still believe that when we disable a regulator in Linux we should
tell RPMh that we vote for the lowest voltage.  ...but if Mark is
happy with the way the driver works now I won't push it.

2. As per my comments in the bindings patch, I still believe that
"qcom,drms-mode-max-microamps" belongs in the core.  Again, up to
Mark.

3. As per my comments in the bindings patch, I still believe that
we're just adding lots of noise to the DT most of the time by
specifying both "qcom,regulator-drms-modes" and
"regulator-allowed-modes".  Again, up to Mark.


-Doug

Re: [PATCH v4 2/2] regulator: add QCOM RPMh regulator driver

[David Collins]

Hello Doug,

On 05/29/2018 10:32 PM, Doug Anderson wrote:
> On Tue, May 22, 2018 at 7:43 PM, David Collins <collinsd@codeaurora.org> wrote:
>> + * @ever_enabled:              Boolean indicating that the regulator has been
>> + *                             explicitly enabled at least once.  Voltage
>> + *                             requests should be cached when this flag is not
>> + *                             set.
> 
> Do you really need this extra boolean?  Can't you just check if
> "enabled" is still "-EINVAL"?  If it is then you don't pass the
> voltage along.
> 
> ...this would mean that you'd also need to send the voltage vote when
> the regulator core tries to disable unused regulators at the end of
> bootup, but that should be OK right?  If we never touched a regulator
> anywhere at probe time and we're about to vote to disable it, we know
> there's nobody requiring it to still be on.  We can vote for the
> voltage now without fear of messing up a vote that the BIOS left in
> place.
> 
> In theory this should also allow you to assert your vote about the
> voltage of a regulator that has never been enabled, which (if I
> understand correctly) you consider to be a feature.

Removing 'ever_enabled' and caching the voltage when 'enabled == -EINVAL'
seems workable.  I'm a little concerned about this resulting in voltage =
regulator-min-microvolt requests being sent for all regulators that are
not explicitly enabled by Linux consumers before late_initcall_sync().
Theoretically all of the boot loader hand-off cases should be taken care
of by this point so it should be safe.

I'll make this change.

Take care,
David

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

[PATCH v4 2/2] regulator: add QCOM RPMh regulator driver

[David Collins]

Hello Doug,

On 05/29/2018 10:32 PM, Doug Anderson wrote:
> On Tue, May 22, 2018 at 7:43 PM, David Collins <collinsd@codeaurora.org> wrote:
>> + * @ever_enabled:              Boolean indicating that the regulator has been
>> + *                             explicitly enabled at least once.  Voltage
>> + *                             requests should be cached when this flag is not
>> + *                             set.
> 
> Do you really need this extra boolean?  Can't you just check if
> "enabled" is still "-EINVAL"?  If it is then you don't pass the
> voltage along.
> 
> ...this would mean that you'd also need to send the voltage vote when
> the regulator core tries to disable unused regulators at the end of
> bootup, but that should be OK right?  If we never touched a regulator
> anywhere at probe time and we're about to vote to disable it, we know
> there's nobody requiring it to still be on.  We can vote for the
> voltage now without fear of messing up a vote that the BIOS left in
> place.
> 
> In theory this should also allow you to assert your vote about the
> voltage of a regulator that has never been enabled, which (if I
> understand correctly) you consider to be a feature.

Removing 'ever_enabled' and caching the voltage when 'enabled == -EINVAL'
seems workable.  I'm a little concerned about this resulting in voltage =
regulator-min-microvolt requests being sent for all regulators that are
not explicitly enabled by Linux consumers before late_initcall_sync().
Theoretically all of the boot loader hand-off cases should be taken care
of by this point so it should be safe.

I'll make this change.

Take care,
David

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

Re: [PATCH v4 0/2] regulator: add QCOM RPMh regulator driver

[Mark Brown]

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

On Tue, May 22, 2018 at 07:43:16PM -0700, David Collins wrote:
> This patch series adds a driver and device tree binding documentation for
> PMIC regulator control via Resource Power Manager-hardened (RPMh) on some
> Qualcomm Technologies, Inc. SoCs such as SDM845.  RPMh is a hardware block

So, this is a very big driver and obviously it being RPM based it
doesn't look like other regulators which is causing problems, especially
when coupled with the desire to implement a bunch of more exotic
features like the mode setting.  I think this review is going to go a
lot more smoothly if you split this up into a base driver with just
normal, standard stuff that doesn't add too many custom properties or
unusual ways of working and then a series of patches on top of that
adding things like the mode adjustment and interaction with other RPM
clients.

We've got other RPM based regulators in tree already so the baseline bit
shouldn't be too hard, that'll make the rest of the patches much smaller
and easier to review and mean that the bits that are simpler and easier
to cope with don't need to be reposted.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v4 0/2] regulator: add QCOM RPMh regulator driver

[Mark Brown]

On Tue, May 22, 2018 at 07:43:16PM -0700, David Collins wrote:
> This patch series adds a driver and device tree binding documentation for
> PMIC regulator control via Resource Power Manager-hardened (RPMh) on some
> Qualcomm Technologies, Inc. SoCs such as SDM845.  RPMh is a hardware block

So, this is a very big driver and obviously it being RPM based it
doesn't look like other regulators which is causing problems, especially
when coupled with the desire to implement a bunch of more exotic
features like the mode setting.  I think this review is going to go a
lot more smoothly if you split this up into a base driver with just
normal, standard stuff that doesn't add too many custom properties or
unusual ways of working and then a series of patches on top of that
adding things like the mode adjustment and interaction with other RPM
clients.

We've got other RPM based regulators in tree already so the baseline bit
shouldn't be too hard, that'll make the rest of the patches much smaller
and easier to review and mean that the bits that are simpler and easier
to cope with don't need to be reposted.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180530/fd8d3adf/attachment.sig>

Re: [PATCH v4 0/2] regulator: add QCOM RPMh regulator driver

[David Collins]

Hello Mark,

On 05/30/2018 09:33 AM, Mark Brown wrote:
> On Tue, May 22, 2018 at 07:43:16PM -0700, David Collins wrote:
>> This patch series adds a driver and device tree binding documentation for
>> PMIC regulator control via Resource Power Manager-hardened (RPMh) on some
>> Qualcomm Technologies, Inc. SoCs such as SDM845.  RPMh is a hardware block
> 
> So, this is a very big driver and obviously it being RPM based it
> doesn't look like other regulators which is causing problems, especially
> when coupled with the desire to implement a bunch of more exotic
> features like the mode setting.  I think this review is going to go a
> lot more smoothly if you split this up into a base driver with just
> normal, standard stuff that doesn't add too many custom properties or
> unusual ways of working and then a series of patches on top of that
> adding things like the mode adjustment and interaction with other RPM
> clients.
> 
> We've got other RPM based regulators in tree already so the baseline bit
> shouldn't be too hard, that'll make the rest of the patches much smaller
> and easier to review and mean that the bits that are simpler and easier
> to cope with don't need to be reposted.

I'll split up the patches so that reviewing is easier.  For the base
patch, would you prefer that I remove *all* mode support (handled by
generic regulator framework DT properties) or only remove the special
purpose drms mode handling support (i.e. qcom,regulator-drms-modes and
qcom,drms-mode-max-microamps)?

Thanks,
David

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

[PATCH v4 0/2] regulator: add QCOM RPMh regulator driver

[David Collins]

Hello Mark,

On 05/30/2018 09:33 AM, Mark Brown wrote:
> On Tue, May 22, 2018 at 07:43:16PM -0700, David Collins wrote:
>> This patch series adds a driver and device tree binding documentation for
>> PMIC regulator control via Resource Power Manager-hardened (RPMh) on some
>> Qualcomm Technologies, Inc. SoCs such as SDM845.  RPMh is a hardware block
> 
> So, this is a very big driver and obviously it being RPM based it
> doesn't look like other regulators which is causing problems, especially
> when coupled with the desire to implement a bunch of more exotic
> features like the mode setting.  I think this review is going to go a
> lot more smoothly if you split this up into a base driver with just
> normal, standard stuff that doesn't add too many custom properties or
> unusual ways of working and then a series of patches on top of that
> adding things like the mode adjustment and interaction with other RPM
> clients.
> 
> We've got other RPM based regulators in tree already so the baseline bit
> shouldn't be too hard, that'll make the rest of the patches much smaller
> and easier to review and mean that the bits that are simpler and easier
> to cope with don't need to be reposted.

I'll split up the patches so that reviewing is easier.  For the base
patch, would you prefer that I remove *all* mode support (handled by
generic regulator framework DT properties) or only remove the special
purpose drms mode handling support (i.e. qcom,regulator-drms-modes and
qcom,drms-mode-max-microamps)?

Thanks,
David

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

Re: [PATCH v4 0/2] regulator: add QCOM RPMh regulator driver

[Mark Brown]

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

On Wed, May 30, 2018 at 05:11:54PM -0700, David Collins wrote:

> I'll split up the patches so that reviewing is easier.  For the base
> patch, would you prefer that I remove *all* mode support (handled by
> generic regulator framework DT properties) or only remove the special
> purpose drms mode handling support (i.e. qcom,regulator-drms-modes and
> qcom,drms-mode-max-microamps)?

Standard operations should be fine, just weird custom stuff that's been
causing issues.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v4 0/2] regulator: add QCOM RPMh regulator driver

[Mark Brown]

On Wed, May 30, 2018 at 05:11:54PM -0700, David Collins wrote:

> I'll split up the patches so that reviewing is easier.  For the base
> patch, would you prefer that I remove *all* mode support (handled by
> generic regulator framework DT properties) or only remove the special
> purpose drms mode handling support (i.e. qcom,regulator-drms-modes and
> qcom,drms-mode-max-microamps)?

Standard operations should be fine, just weird custom stuff that's been
causing issues.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180531/cc95ff7c/attachment.sig>