Linux-ARM-MSM Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 0/7] PM8005 and PMS405 regulator support
@ 2019-06-13 14:21 Jeffrey Hugo
  2019-06-13 14:22 ` [PATCH v3 1/7] drivers: regulator: qcom_spmi: enable linear range info Jeffrey Hugo
                   ` (6 more replies)
  0 siblings, 7 replies; 18+ messages in thread
From: Jeffrey Hugo @ 2019-06-13 14:21 UTC (permalink / raw)
  To: unlisted-recipients:; (no To-header on input)
  Cc: agross, bjorn.andersson, lgirdwood, broonie, robh+dt,
	mark.rutland, linux-arm-msm, linux-kernel, devicetree,
	Jeffrey Hugo

The MSM8998 MTP reference platform supplies VDD_GFX from s1 of the
pm8005 PMIC.  VDD_GFX is needed to turn on the GPU.  As we are looking
to bring up the GPU, add the support for pm8005 and wire up s1 in a
basic manner so that we have this dependency out of the way and can
focus on enabling the GPU driver.

The s3 regulator of PMS405 is used for voltage scaling of the CPU on
QCS404.

Both PMICs are very similar in design, so add the base support with one,
and trivially add the support for the other on top.

v3:
-Allow PMS405 regulators to be enabled and disabled, instead of the
outdated "always on" concept

v2:
-Perform if statement cleanups per review discussion
-Pull in linear range support since its related, and simple
-Rework the PM8005 to minimize special cases in the driver
-"common2" is now ftsmps426 since that design first implemented it
-Reworked the PMS405 changes on top, since they are related to pm8005 and
trivial

Jeffrey Hugo (4):
  drivers: regulator: qcom_spmi: Refactor get_mode/set_mode
  dt-bindings: qcom_spmi: Document PM8005 regulators
  regulator: qcom_spmi: Add support for PM8005
  arm64: dts: msm8998-mtp: Add pm8005_s1 regulator

Jorge Ramirez (2):
  dt-bindings: qcom_spmi: Document pms405 support
  drivers: regulator: qcom: add PMS405 SPMI regulator

Jorge Ramirez-Ortiz (1):
  drivers: regulator: qcom_spmi: enable linear range info

 .../regulator/qcom,spmi-regulator.txt         |  28 +++
 arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi     |  17 ++
 drivers/regulator/qcom_spmi-regulator.c       | 233 +++++++++++++++++-
 3 files changed, 271 insertions(+), 7 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 1/7] drivers: regulator: qcom_spmi: enable linear range info
  2019-06-13 14:21 [PATCH v2 0/7] PM8005 and PMS405 regulator support Jeffrey Hugo
@ 2019-06-13 14:22 ` Jeffrey Hugo
  2019-06-13 15:10   ` Bjorn Andersson
  2019-06-13 14:22 ` [PATCH v3 2/7] drivers: regulator: qcom_spmi: Refactor get_mode/set_mode Jeffrey Hugo
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Jeffrey Hugo @ 2019-06-13 14:22 UTC (permalink / raw)
  To: lgirdwood, broonie
  Cc: agross, bjorn.andersson, robh+dt, mark.rutland, linux-arm-msm,
	linux-kernel, devicetree, Jorge Ramirez-Ortiz, Jeffrey Hugo

From: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
---
 drivers/regulator/qcom_spmi-regulator.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c
index 53a61fb65642..fd55438c25d6 100644
--- a/drivers/regulator/qcom_spmi-regulator.c
+++ b/drivers/regulator/qcom_spmi-regulator.c
@@ -1744,6 +1744,7 @@ MODULE_DEVICE_TABLE(of, qcom_spmi_regulator_match);
 static int qcom_spmi_regulator_probe(struct platform_device *pdev)
 {
 	const struct spmi_regulator_data *reg;
+	const struct spmi_voltage_range *range;
 	const struct of_device_id *match;
 	struct regulator_config config = { };
 	struct regulator_dev *rdev;
@@ -1833,6 +1834,12 @@ static int qcom_spmi_regulator_probe(struct platform_device *pdev)
 			}
 		}
 
+		if (vreg->logical_type == SPMI_REGULATOR_LOGICAL_TYPE_HFS430) {
+			/* since there is only one range */
+			range = spmi_regulator_find_range(vreg);
+			vreg->desc.uV_step = range->step_uV;
+		}
+
 		config.dev = dev;
 		config.driver_data = vreg;
 		config.regmap = regmap;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 2/7] drivers: regulator: qcom_spmi: Refactor get_mode/set_mode
  2019-06-13 14:21 [PATCH v2 0/7] PM8005 and PMS405 regulator support Jeffrey Hugo
  2019-06-13 14:22 ` [PATCH v3 1/7] drivers: regulator: qcom_spmi: enable linear range info Jeffrey Hugo
@ 2019-06-13 14:22 ` Jeffrey Hugo
  2019-06-13 15:12   ` Bjorn Andersson
  2019-06-13 14:23 ` [PATCH v3 3/7] dt-bindings: qcom_spmi: Document PM8005 regulators Jeffrey Hugo
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Jeffrey Hugo @ 2019-06-13 14:22 UTC (permalink / raw)
  To: lgirdwood, broonie
  Cc: agross, bjorn.andersson, robh+dt, mark.rutland, linux-arm-msm,
	linux-kernel, devicetree, Jeffrey Hugo

spmi_regulator_common_get_mode and spmi_regulator_common_set_mode use
multi-level ifs which mirror a switch statement.  Refactor to use a switch
statement to make the code flow more clear.

Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
---
 drivers/regulator/qcom_spmi-regulator.c | 28 ++++++++++++++++---------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c
index fd55438c25d6..1c18fe5969b5 100644
--- a/drivers/regulator/qcom_spmi-regulator.c
+++ b/drivers/regulator/qcom_spmi-regulator.c
@@ -911,13 +911,14 @@ static unsigned int spmi_regulator_common_get_mode(struct regulator_dev *rdev)
 
 	spmi_vreg_read(vreg, SPMI_COMMON_REG_MODE, &reg, 1);
 
-	if (reg & SPMI_COMMON_MODE_HPM_MASK)
+	switch (reg) {
+	case SPMI_COMMON_MODE_HPM_MASK:
 		return REGULATOR_MODE_NORMAL;
-
-	if (reg & SPMI_COMMON_MODE_AUTO_MASK)
+	case SPMI_COMMON_MODE_AUTO_MASK:
 		return REGULATOR_MODE_FAST;
-
-	return REGULATOR_MODE_IDLE;
+	default:
+		return REGULATOR_MODE_IDLE;
+	}
 }
 
 static int
@@ -925,12 +926,19 @@ spmi_regulator_common_set_mode(struct regulator_dev *rdev, unsigned int mode)
 {
 	struct spmi_regulator *vreg = rdev_get_drvdata(rdev);
 	u8 mask = SPMI_COMMON_MODE_HPM_MASK | SPMI_COMMON_MODE_AUTO_MASK;
-	u8 val = 0;
+	u8 val;
 
-	if (mode == REGULATOR_MODE_NORMAL)
+	switch (mode) {
+	case REGULATOR_MODE_NORMAL:
 		val = SPMI_COMMON_MODE_HPM_MASK;
-	else if (mode == REGULATOR_MODE_FAST)
+		break;
+	case REGULATOR_MODE_FAST:
 		val = SPMI_COMMON_MODE_AUTO_MASK;
+		break;
+	default:
+		val = 0;
+		break;
+	}
 
 	return spmi_vreg_update_bits(vreg, SPMI_COMMON_REG_MODE, val, mask);
 }
@@ -1834,9 +1842,9 @@ static int qcom_spmi_regulator_probe(struct platform_device *pdev)
 			}
 		}
 
-		if (vreg->logical_type == SPMI_REGULATOR_LOGICAL_TYPE_HFS430) {
+		if (vreg->set_points->count == 1) {
 			/* since there is only one range */
-			range = spmi_regulator_find_range(vreg);
+			range = vreg->set_points->range;
 			vreg->desc.uV_step = range->step_uV;
 		}
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 3/7] dt-bindings: qcom_spmi: Document PM8005 regulators
  2019-06-13 14:21 [PATCH v2 0/7] PM8005 and PMS405 regulator support Jeffrey Hugo
  2019-06-13 14:22 ` [PATCH v3 1/7] drivers: regulator: qcom_spmi: enable linear range info Jeffrey Hugo
  2019-06-13 14:22 ` [PATCH v3 2/7] drivers: regulator: qcom_spmi: Refactor get_mode/set_mode Jeffrey Hugo
@ 2019-06-13 14:23 ` Jeffrey Hugo
  2019-06-13 14:23 ` [PATCH v3 4/7] regulator: qcom_spmi: Add support for PM8005 Jeffrey Hugo
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Jeffrey Hugo @ 2019-06-13 14:23 UTC (permalink / raw)
  To: lgirdwood, broonie
  Cc: agross, bjorn.andersson, robh+dt, mark.rutland, linux-arm-msm,
	linux-kernel, devicetree, Jeffrey Hugo

Document the dt bindings for the PM8005 regulators which are usually used
for VDD of standalone blocks on a SoC like the GPU.

Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 .../devicetree/bindings/regulator/qcom,spmi-regulator.txt     | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt b/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
index 406f2e570c50..ba94bc2d407a 100644
--- a/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
@@ -4,6 +4,7 @@ Qualcomm SPMI Regulators
 	Usage: required
 	Value type: <string>
 	Definition: must be one of:
+			"qcom,pm8005-regulators"
 			"qcom,pm8841-regulators"
 			"qcom,pm8916-regulators"
 			"qcom,pm8941-regulators"
@@ -120,6 +121,9 @@ The regulator node houses sub-nodes for each regulator within the device. Each
 sub-node is identified using the node's name, with valid values listed for each
 of the PMICs below.
 
+pm8005:
+	s1, s2, s3, s4
+
 pm8841:
 	s1, s2, s3, s4, s5, s6, s7, s8
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 4/7] regulator: qcom_spmi: Add support for PM8005
  2019-06-13 14:21 [PATCH v2 0/7] PM8005 and PMS405 regulator support Jeffrey Hugo
                   ` (2 preceding siblings ...)
  2019-06-13 14:23 ` [PATCH v3 3/7] dt-bindings: qcom_spmi: Document PM8005 regulators Jeffrey Hugo
@ 2019-06-13 14:23 ` Jeffrey Hugo
  2019-06-13 15:25   ` Bjorn Andersson
  2019-06-13 14:24 ` [PATCH v3 5/7] arm64: dts: msm8998-mtp: Add pm8005_s1 regulator Jeffrey Hugo
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Jeffrey Hugo @ 2019-06-13 14:23 UTC (permalink / raw)
  To: lgirdwood, broonie
  Cc: agross, bjorn.andersson, robh+dt, mark.rutland, linux-arm-msm,
	linux-kernel, devicetree, Jeffrey Hugo

The PM8005 is used on the msm8998 MTP.  The S1 regulator is VDD_GFX, ie
it needs to be on and controlled inorder to use the GPU.  Add support to
drive the PM8005 regulators so that we can bring up the GPU on msm8998.

Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
---
 drivers/regulator/qcom_spmi-regulator.c | 169 ++++++++++++++++++++++++
 1 file changed, 169 insertions(+)

diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c
index 1c18fe5969b5..c7880c1d4bcd 100644
--- a/drivers/regulator/qcom_spmi-regulator.c
+++ b/drivers/regulator/qcom_spmi-regulator.c
@@ -104,6 +104,7 @@ enum spmi_regulator_logical_type {
 	SPMI_REGULATOR_LOGICAL_TYPE_ULT_LO_SMPS,
 	SPMI_REGULATOR_LOGICAL_TYPE_ULT_HO_SMPS,
 	SPMI_REGULATOR_LOGICAL_TYPE_ULT_LDO,
+	SPMI_REGULATOR_LOGICAL_TYPE_FTSMPS426,
 };
 
 enum spmi_regulator_type {
@@ -150,6 +151,7 @@ enum spmi_regulator_subtype {
 	SPMI_REGULATOR_SUBTYPE_5V_BOOST		= 0x01,
 	SPMI_REGULATOR_SUBTYPE_FTS_CTL		= 0x08,
 	SPMI_REGULATOR_SUBTYPE_FTS2p5_CTL	= 0x09,
+	SPMI_REGULATOR_SUBTYPE_FTS426_CTL	= 0x0a,
 	SPMI_REGULATOR_SUBTYPE_BB_2A		= 0x01,
 	SPMI_REGULATOR_SUBTYPE_ULT_HF_CTL1	= 0x0d,
 	SPMI_REGULATOR_SUBTYPE_ULT_HF_CTL2	= 0x0e,
@@ -170,6 +172,18 @@ enum spmi_common_regulator_registers {
 	SPMI_COMMON_REG_STEP_CTRL		= 0x61,
 };
 
+/*
+ * Second common register layout used by newer devices starting with ftsmps426
+ * Note that some of the registers from the first common layout remain
+ * unchanged and their definition is not duplicated.
+ */
+enum spmi_ftsmps426_regulator_registers {
+	SPMI_FTSMPS426_REG_VOLTAGE_LSB		= 0x40,
+	SPMI_FTSMPS426_REG_VOLTAGE_MSB		= 0x41,
+	SPMI_FTSMPS426_REG_VOLTAGE_ULS_LSB	= 0x68,
+	SPMI_FTSMPS426_REG_VOLTAGE_ULS_MSB	= 0x69,
+};
+
 enum spmi_vs_registers {
 	SPMI_VS_REG_OCP				= 0x4a,
 	SPMI_VS_REG_SOFT_START			= 0x4c,
@@ -229,6 +243,14 @@ enum spmi_common_control_register_index {
 #define SPMI_COMMON_MODE_FOLLOW_HW_EN0_MASK	0x01
 #define SPMI_COMMON_MODE_FOLLOW_ALL_MASK	0x1f
 
+#define SPMI_FTSMPS426_MODE_BYPASS_MASK		3
+#define SPMI_FTSMPS426_MODE_RETENTION_MASK	4
+#define SPMI_FTSMPS426_MODE_LPM_MASK		5
+#define SPMI_FTSMPS426_MODE_AUTO_MASK		6
+#define SPMI_FTSMPS426_MODE_HPM_MASK		7
+
+#define SPMI_FTSMPS426_MODE_MASK		0x07
+
 /* Common regulator pull down control register layout */
 #define SPMI_COMMON_PULL_DOWN_ENABLE_MASK	0x80
 
@@ -274,6 +296,23 @@ enum spmi_common_control_register_index {
 #define SPMI_FTSMPS_STEP_MARGIN_NUM	4
 #define SPMI_FTSMPS_STEP_MARGIN_DEN	5
 
+#define SPMI_FTSMPS426_STEP_CTRL_DELAY_MASK	0x03
+#define SPMI_FTSMPS426_STEP_CTRL_DELAY_SHIFT	0
+
+/* Clock rate in kHz of the FTSMPS426 regulator reference clock. */
+#define SPMI_FTSMPS426_CLOCK_RATE		4800
+
+/* Minimum voltage stepper delay for each step. */
+#define SPMI_FTSMPS426_STEP_DELAY		2
+
+/*
+ * The ratio SPMI_FTSMPS426_STEP_MARGIN_NUM/SPMI_FTSMPS426_STEP_MARGIN_DEN is
+ * used to adjust the step rate in order to account for oscillator variance.
+ */
+#define SPMI_FTSMPS426_STEP_MARGIN_NUM	10
+#define SPMI_FTSMPS426_STEP_MARGIN_DEN	11
+
+
 /* VSET value to decide the range of ULT SMPS */
 #define ULT_SMPS_RANGE_SPLIT 0x60
 
@@ -447,6 +486,10 @@ static struct spmi_voltage_range ftsmps2p5_ranges[] = {
 	SPMI_VOLTAGE_RANGE(1,  160000, 1360000, 2200000, 2200000, 10000),
 };
 
+static struct spmi_voltage_range ftsmps426_ranges[] = {
+	SPMI_VOLTAGE_RANGE(0,       0,  320000, 1352000, 1352000,  4000),
+};
+
 static struct spmi_voltage_range boost_ranges[] = {
 	SPMI_VOLTAGE_RANGE(0, 4000000, 4000000, 5550000, 5550000, 50000),
 };
@@ -480,6 +523,7 @@ static DEFINE_SPMI_SET_POINTS(ln_ldo);
 static DEFINE_SPMI_SET_POINTS(smps);
 static DEFINE_SPMI_SET_POINTS(ftsmps);
 static DEFINE_SPMI_SET_POINTS(ftsmps2p5);
+static DEFINE_SPMI_SET_POINTS(ftsmps426);
 static DEFINE_SPMI_SET_POINTS(boost);
 static DEFINE_SPMI_SET_POINTS(boost_byp);
 static DEFINE_SPMI_SET_POINTS(ult_lo_smps);
@@ -747,6 +791,23 @@ spmi_regulator_common_set_voltage(struct regulator_dev *rdev, unsigned selector)
 	return spmi_vreg_write(vreg, SPMI_COMMON_REG_VOLTAGE_RANGE, buf, 2);
 }
 
+static int spmi_regulator_common_list_voltage(struct regulator_dev *rdev,
+					      unsigned selector);
+
+static int spmi_regulator_ftsmps426_set_voltage(struct regulator_dev *rdev,
+					      unsigned selector)
+{
+	struct spmi_regulator *vreg = rdev_get_drvdata(rdev);
+	u8 buf[2];
+	int mV;
+
+	mV = spmi_regulator_common_list_voltage(rdev, selector) / 1000;
+
+	buf[0] = mV & 0xff;
+	buf[1] = mV >> 8;
+	return spmi_vreg_write(vreg, SPMI_FTSMPS426_REG_VOLTAGE_LSB, buf, 2);
+}
+
 static int spmi_regulator_set_voltage_time_sel(struct regulator_dev *rdev,
 		unsigned int old_selector, unsigned int new_selector)
 {
@@ -778,6 +839,16 @@ static int spmi_regulator_common_get_voltage(struct regulator_dev *rdev)
 	return spmi_hw_selector_to_sw(vreg, voltage_sel, range);
 }
 
+static int spmi_regulator_ftsmps426_get_voltage(struct regulator_dev *rdev)
+{
+	struct spmi_regulator *vreg = rdev_get_drvdata(rdev);
+	u8 buf[2];
+
+	spmi_vreg_read(vreg, SPMI_FTSMPS426_REG_VOLTAGE_LSB, buf, 2);
+
+	return (((unsigned int)buf[1] << 8) | (unsigned int)buf[0]) * 1000;
+}
+
 static int spmi_regulator_single_map_voltage(struct regulator_dev *rdev,
 		int min_uV, int max_uV)
 {
@@ -921,6 +992,23 @@ static unsigned int spmi_regulator_common_get_mode(struct regulator_dev *rdev)
 	}
 }
 
+static unsigned int spmi_regulator_ftsmps426_get_mode(struct regulator_dev *rdev)
+{
+	struct spmi_regulator *vreg = rdev_get_drvdata(rdev);
+	u8 reg;
+
+	spmi_vreg_read(vreg, SPMI_COMMON_REG_MODE, &reg, 1);
+
+	switch (reg) {
+	case SPMI_FTSMPS426_MODE_HPM_MASK:
+		return REGULATOR_MODE_NORMAL;
+	case SPMI_FTSMPS426_MODE_AUTO_MASK:
+		return REGULATOR_MODE_FAST;
+	default:
+		return REGULATOR_MODE_IDLE;
+	}
+}
+
 static int
 spmi_regulator_common_set_mode(struct regulator_dev *rdev, unsigned int mode)
 {
@@ -943,6 +1031,28 @@ spmi_regulator_common_set_mode(struct regulator_dev *rdev, unsigned int mode)
 	return spmi_vreg_update_bits(vreg, SPMI_COMMON_REG_MODE, val, mask);
 }
 
+static int
+spmi_regulator_ftsmps426_set_mode(struct regulator_dev *rdev, unsigned int mode)
+{
+	struct spmi_regulator *vreg = rdev_get_drvdata(rdev);
+	u8 mask = SPMI_FTSMPS426_MODE_MASK;
+	u8 val;
+
+	switch (mode) {
+	case REGULATOR_MODE_NORMAL:
+		val = SPMI_FTSMPS426_MODE_HPM_MASK;
+		break;
+	case REGULATOR_MODE_FAST:
+		val = SPMI_FTSMPS426_MODE_AUTO_MASK;
+		break;
+	default:
+		val = SPMI_FTSMPS426_MODE_LPM_MASK;
+		break;
+	}
+
+	return spmi_vreg_update_bits(vreg, SPMI_COMMON_REG_MODE, val, mask);
+}
+
 static int
 spmi_regulator_common_set_load(struct regulator_dev *rdev, int load_uA)
 {
@@ -1272,6 +1382,21 @@ static struct regulator_ops spmi_ult_ldo_ops = {
 	.set_soft_start		= spmi_regulator_common_set_soft_start,
 };
 
+static struct regulator_ops spmi_ftsmps426_ops = {
+	.enable			= regulator_enable_regmap,
+	.disable		= regulator_disable_regmap,
+	.is_enabled		= regulator_is_enabled_regmap,
+	.set_voltage_sel	= spmi_regulator_ftsmps426_set_voltage,
+	.set_voltage_time_sel	= spmi_regulator_set_voltage_time_sel,
+	.get_voltage		= spmi_regulator_ftsmps426_get_voltage,
+	.map_voltage		= spmi_regulator_single_map_voltage,
+	.list_voltage		= spmi_regulator_common_list_voltage,
+	.set_mode		= spmi_regulator_ftsmps426_set_mode,
+	.get_mode		= spmi_regulator_ftsmps426_get_mode,
+	.set_load		= spmi_regulator_common_set_load,
+	.set_pull_down		= spmi_regulator_common_set_pull_down,
+};
+
 /* Maximum possible digital major revision value */
 #define INF 0xFF
 
@@ -1307,6 +1432,7 @@ static const struct spmi_regulator_mapping supported_regulators[] = {
 	SPMI_VREG(BOOST, 5V_BOOST, 0, INF, BOOST,  boost,  boost,       0),
 	SPMI_VREG(FTS,   FTS_CTL,  0, INF, FTSMPS, ftsmps, ftsmps, 100000),
 	SPMI_VREG(FTS, FTS2p5_CTL, 0, INF, FTSMPS, ftsmps, ftsmps2p5, 100000),
+	SPMI_VREG(FTS, FTS426_CTL, 0, INF, FTSMPS426, ftsmps426, ftsmps426, 100000),
 	SPMI_VREG(BOOST_BYP, BB_2A, 0, INF, BOOST_BYP, boost, boost_byp, 0),
 	SPMI_VREG(ULT_BUCK, ULT_HF_CTL1, 0, INF, ULT_LO_SMPS, ult_lo_smps,
 						ult_lo_smps,   100000),
@@ -1444,6 +1570,34 @@ static int spmi_regulator_init_slew_rate(struct spmi_regulator *vreg)
 	return ret;
 }
 
+static int spmi_regulator_init_slew_rate_ftsmps426(struct spmi_regulator *vreg)
+{
+	int ret;
+	u8 reg = 0;
+	int delay, slew_rate;
+	const struct spmi_voltage_range *range = &vreg->set_points->range[0];
+
+	ret = spmi_vreg_read(vreg, SPMI_COMMON_REG_STEP_CTRL, &reg, 1);
+	if (ret) {
+		dev_err(vreg->dev, "spmi read failed, ret=%d\n", ret);
+		return ret;
+	}
+
+	delay = reg & SPMI_FTSMPS426_STEP_CTRL_DELAY_MASK;
+	delay >>= SPMI_FTSMPS426_STEP_CTRL_DELAY_SHIFT;
+
+	/* slew_rate has units of uV/us */
+	slew_rate = SPMI_FTSMPS426_CLOCK_RATE * range->step_uV;
+	slew_rate /= 1000 * (SPMI_FTSMPS426_STEP_DELAY << delay);
+	slew_rate *= SPMI_FTSMPS426_STEP_MARGIN_NUM;
+	slew_rate /= SPMI_FTSMPS426_STEP_MARGIN_DEN;
+
+	/* Ensure that the slew rate is greater than 0 */
+	vreg->slew_rate = max(slew_rate, 1);
+
+	return ret;
+}
+
 static int spmi_regulator_init_registers(struct spmi_regulator *vreg,
 				const struct spmi_regulator_init_data *data)
 {
@@ -1583,6 +1737,12 @@ static int spmi_regulator_of_parse(struct device_node *node,
 		ret = spmi_regulator_init_slew_rate(vreg);
 		if (ret)
 			return ret;
+		break;
+	case SPMI_REGULATOR_LOGICAL_TYPE_FTSMPS426:
+		ret = spmi_regulator_init_slew_rate_ftsmps426(vreg);
+		if (ret)
+			return ret;
+		break;
 	default:
 		break;
 	}
@@ -1739,7 +1899,16 @@ static const struct spmi_regulator_data pmi8994_regulators[] = {
 	{ }
 };
 
+static const struct spmi_regulator_data pm8005_regulators[] = {
+	{ "s1", 0x1400, "vdd_s1", },
+	{ "s2", 0x1700, "vdd_s2", },
+	{ "s3", 0x1a00, "vdd_s3", },
+	{ "s4", 0x1d00, "vdd_s4", },
+	{ }
+};
+
 static const struct of_device_id qcom_spmi_regulator_match[] = {
+	{ .compatible = "qcom,pm8005-regulators", .data = &pm8005_regulators },
 	{ .compatible = "qcom,pm8841-regulators", .data = &pm8841_regulators },
 	{ .compatible = "qcom,pm8916-regulators", .data = &pm8916_regulators },
 	{ .compatible = "qcom,pm8941-regulators", .data = &pm8941_regulators },
-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 5/7] arm64: dts: msm8998-mtp: Add pm8005_s1 regulator
  2019-06-13 14:21 [PATCH v2 0/7] PM8005 and PMS405 regulator support Jeffrey Hugo
                   ` (3 preceding siblings ...)
  2019-06-13 14:23 ` [PATCH v3 4/7] regulator: qcom_spmi: Add support for PM8005 Jeffrey Hugo
@ 2019-06-13 14:24 ` Jeffrey Hugo
  2019-06-13 14:24 ` [PATCH v3 6/7] dt-bindings: qcom_spmi: Document pms405 support Jeffrey Hugo
  2019-06-13 14:24 ` [PATCH v3 7/7] drivers: regulator: qcom: add PMS405 SPMI regulator Jeffrey Hugo
  6 siblings, 0 replies; 18+ messages in thread
From: Jeffrey Hugo @ 2019-06-13 14:24 UTC (permalink / raw)
  To: agross, bjorn.andersson
  Cc: lgirdwood, broonie, robh+dt, mark.rutland, linux-arm-msm,
	linux-kernel, devicetree, Jeffrey Hugo

The pm8005_s1 is VDD_GFX, and needs to be on to enable the GPU.
This should be hooked up to the GPU CPR, but we don't have support for that
yet, so until then, just turn on the regulator and keep it on so that we
can focus on basic GPU bringup.

Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
index f09f3e03f708..108667ce4f31 100644
--- a/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi
@@ -27,6 +27,23 @@
 	status = "okay";
 };
 
+&pm8005_lsid1 {
+	pm8005-regulators {
+		compatible = "qcom,pm8005-regulators";
+
+		vdd_s1-supply = <&vph_pwr>;
+
+		pm8005_s1: s1 { /* VDD_GFX supply */
+			regulator-min-microvolt = <524000>;
+			regulator-max-microvolt = <1100000>;
+			regulator-enable-ramp-delay = <500>;
+
+			/* hack until we rig up the gpu consumer */
+			regulator-always-on;
+		};
+	};
+};
+
 &qusb2phy {
 	status = "okay";
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 6/7] dt-bindings: qcom_spmi: Document pms405 support
  2019-06-13 14:21 [PATCH v2 0/7] PM8005 and PMS405 regulator support Jeffrey Hugo
                   ` (4 preceding siblings ...)
  2019-06-13 14:24 ` [PATCH v3 5/7] arm64: dts: msm8998-mtp: Add pm8005_s1 regulator Jeffrey Hugo
@ 2019-06-13 14:24 ` Jeffrey Hugo
  2019-06-13 15:36   ` Bjorn Andersson
  2019-06-13 14:24 ` [PATCH v3 7/7] drivers: regulator: qcom: add PMS405 SPMI regulator Jeffrey Hugo
  6 siblings, 1 reply; 18+ messages in thread
From: Jeffrey Hugo @ 2019-06-13 14:24 UTC (permalink / raw)
  To: lgirdwood, broonie
  Cc: agross, bjorn.andersson, robh+dt, mark.rutland, linux-arm-msm,
	linux-kernel, devicetree, Jorge Ramirez, Jeffrey Hugo

From: Jorge Ramirez <jorge.ramirez-ortiz@linaro.org>

The PMS405 supports 5 SMPS and 13 LDO regulators.

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
---
 .../regulator/qcom,spmi-regulator.txt         | 24 +++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt b/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
index ba94bc2d407a..19cffb239094 100644
--- a/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
@@ -10,6 +10,7 @@ Qualcomm SPMI Regulators
 			"qcom,pm8941-regulators"
 			"qcom,pm8994-regulators"
 			"qcom,pmi8994-regulators"
+			"qcom,pms405-regulators"
 
 - interrupts:
 	Usage: optional
@@ -111,6 +112,29 @@ Qualcomm SPMI Regulators
 	Definition: Reference to regulator supplying the input pin, as
 		    described in the data sheet.
 
+- vdd_s1-supply:
+- vdd_s2-supply:
+- vdd_s3-supply:
+- vdd_s4-supply:
+- vdd_s5-supply:
+- vdd_l1-supply:
+- vdd_l2-supply:
+- vdd_l3-supply:
+- vdd_l4-supply:
+- vdd_l5-supply:
+- vdd_l6-supply:
+- vdd_l7-supply:
+- vdd_l8-supply:
+- vdd_l9-supply:
+- vdd_l10-supply:
+- vdd_l11-supply:
+- vdd_l12-supply:
+- vdd_l13-supply:
+	Usage: optional (pms405 only)
+	Value type: <phandle>
+	Definition: Reference to regulator supplying the input pin, as
+		    described in the data sheet.
+
 - qcom,saw-reg:
 	Usage: optional
 	Value type: <phandle>
-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 7/7] drivers: regulator: qcom: add PMS405 SPMI regulator
  2019-06-13 14:21 [PATCH v2 0/7] PM8005 and PMS405 regulator support Jeffrey Hugo
                   ` (5 preceding siblings ...)
  2019-06-13 14:24 ` [PATCH v3 6/7] dt-bindings: qcom_spmi: Document pms405 support Jeffrey Hugo
@ 2019-06-13 14:24 ` Jeffrey Hugo
  2019-06-13 15:33   ` Bjorn Andersson
  6 siblings, 1 reply; 18+ messages in thread
From: Jeffrey Hugo @ 2019-06-13 14:24 UTC (permalink / raw)
  To: lgirdwood, broonie
  Cc: agross, bjorn.andersson, robh+dt, mark.rutland, linux-arm-msm,
	linux-kernel, devicetree, Jorge Ramirez, Jeffrey Hugo

From: Jorge Ramirez <jorge.ramirez-ortiz@linaro.org>

The PMS405 has 5 HFSMPS and 13 LDO regulators,

This commit adds support for one of the 5 HFSMPS regulators (s3) to
the spmi regulator driver.

The PMIC HFSMPS 430 regulators have 8 mV step size and a voltage
control scheme consisting of two  8-bit registers defining a 16-bit
voltage set point in units of millivolts

S3 controls the cpu voltages (s3 is a buck regulator of type HFS430);
it is therefore required so we can enable voltage scaling for safely
running cpufreq.

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
---
 drivers/regulator/qcom_spmi-regulator.c | 41 +++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c
index c7880c1d4bcd..975655e787fe 100644
--- a/drivers/regulator/qcom_spmi-regulator.c
+++ b/drivers/regulator/qcom_spmi-regulator.c
@@ -105,6 +105,7 @@ enum spmi_regulator_logical_type {
 	SPMI_REGULATOR_LOGICAL_TYPE_ULT_HO_SMPS,
 	SPMI_REGULATOR_LOGICAL_TYPE_ULT_LDO,
 	SPMI_REGULATOR_LOGICAL_TYPE_FTSMPS426,
+	SPMI_REGULATOR_LOGICAL_TYPE_HFS430,
 };
 
 enum spmi_regulator_type {
@@ -157,6 +158,7 @@ enum spmi_regulator_subtype {
 	SPMI_REGULATOR_SUBTYPE_ULT_HF_CTL2	= 0x0e,
 	SPMI_REGULATOR_SUBTYPE_ULT_HF_CTL3	= 0x0f,
 	SPMI_REGULATOR_SUBTYPE_ULT_HF_CTL4	= 0x10,
+	SPMI_REGULATOR_SUBTYPE_HFS430		= 0x0a,
 };
 
 enum spmi_common_regulator_registers {
@@ -302,6 +304,8 @@ enum spmi_common_control_register_index {
 /* Clock rate in kHz of the FTSMPS426 regulator reference clock. */
 #define SPMI_FTSMPS426_CLOCK_RATE		4800
 
+#define SPMI_HFS430_CLOCK_RATE			1600
+
 /* Minimum voltage stepper delay for each step. */
 #define SPMI_FTSMPS426_STEP_DELAY		2
 
@@ -515,6 +519,10 @@ static struct spmi_voltage_range ult_pldo_ranges[] = {
 	SPMI_VOLTAGE_RANGE(0, 1750000, 1750000, 3337500, 3337500, 12500),
 };
 
+static struct spmi_voltage_range hfs430_ranges[] = {
+	SPMI_VOLTAGE_RANGE(0, 320000, 320000, 2040000, 2040000, 8000),
+};
+
 static DEFINE_SPMI_SET_POINTS(pldo);
 static DEFINE_SPMI_SET_POINTS(nldo1);
 static DEFINE_SPMI_SET_POINTS(nldo2);
@@ -530,6 +538,7 @@ static DEFINE_SPMI_SET_POINTS(ult_lo_smps);
 static DEFINE_SPMI_SET_POINTS(ult_ho_smps);
 static DEFINE_SPMI_SET_POINTS(ult_nldo);
 static DEFINE_SPMI_SET_POINTS(ult_pldo);
+static DEFINE_SPMI_SET_POINTS(hfs430);
 
 static inline int spmi_vreg_read(struct spmi_regulator *vreg, u16 addr, u8 *buf,
 				 int len)
@@ -1397,12 +1406,24 @@ static struct regulator_ops spmi_ftsmps426_ops = {
 	.set_pull_down		= spmi_regulator_common_set_pull_down,
 };
 
+static struct regulator_ops spmi_hfs430_ops = {
+	/* always on regulators */
+	.set_voltage_sel	= spmi_regulator_ftsmps426_set_voltage,
+	.set_voltage_time_sel	= spmi_regulator_set_voltage_time_sel,
+	.get_voltage		= spmi_regulator_ftsmps426_get_voltage,
+	.map_voltage		= spmi_regulator_single_map_voltage,
+	.list_voltage		= spmi_regulator_common_list_voltage,
+	.set_mode		= spmi_regulator_ftsmps426_set_mode,
+	.get_mode		= spmi_regulator_ftsmps426_get_mode,
+};
+
 /* Maximum possible digital major revision value */
 #define INF 0xFF
 
 static const struct spmi_regulator_mapping supported_regulators[] = {
 	/*           type subtype dig_min dig_max ltype ops setpoints hpm_min */
 	SPMI_VREG(BUCK,  GP_CTL,   0, INF, SMPS,   smps,   smps,   100000),
+	SPMI_VREG(BUCK,  HFS430,   0, INF, HFS430, hfs430, hfs430,  10000),
 	SPMI_VREG(LDO,   N300,     0, INF, LDO,    ldo,    nldo1,   10000),
 	SPMI_VREG(LDO,   N600,     0,   0, LDO,    ldo,    nldo2,   10000),
 	SPMI_VREG(LDO,   N1200,    0,   0, LDO,    ldo,    nldo2,   10000),
@@ -1570,7 +1591,8 @@ static int spmi_regulator_init_slew_rate(struct spmi_regulator *vreg)
 	return ret;
 }
 
-static int spmi_regulator_init_slew_rate_ftsmps426(struct spmi_regulator *vreg)
+static int spmi_regulator_init_slew_rate_ftsmps426(struct spmi_regulator *vreg,
+						   int clock_rate)
 {
 	int ret;
 	u8 reg = 0;
@@ -1587,7 +1609,7 @@ static int spmi_regulator_init_slew_rate_ftsmps426(struct spmi_regulator *vreg)
 	delay >>= SPMI_FTSMPS426_STEP_CTRL_DELAY_SHIFT;
 
 	/* slew_rate has units of uV/us */
-	slew_rate = SPMI_FTSMPS426_CLOCK_RATE * range->step_uV;
+	slew_rate = clock_rate * range->step_uV;
 	slew_rate /= 1000 * (SPMI_FTSMPS426_STEP_DELAY << delay);
 	slew_rate *= SPMI_FTSMPS426_STEP_MARGIN_NUM;
 	slew_rate /= SPMI_FTSMPS426_STEP_MARGIN_DEN;
@@ -1739,7 +1761,14 @@ static int spmi_regulator_of_parse(struct device_node *node,
 			return ret;
 		break;
 	case SPMI_REGULATOR_LOGICAL_TYPE_FTSMPS426:
-		ret = spmi_regulator_init_slew_rate_ftsmps426(vreg);
+		ret = spmi_regulator_init_slew_rate_ftsmps426(vreg,
+						SPMI_FTSMPS426_CLOCK_RATE);
+		if (ret)
+			return ret;
+		break;
+	case SPMI_REGULATOR_LOGICAL_TYPE_HFS430:
+		ret = spmi_regulator_init_slew_rate_ftsmps426(vreg,
+							SPMI_HFS430_CLOCK_RATE);
 		if (ret)
 			return ret;
 		break;
@@ -1907,6 +1936,11 @@ static const struct spmi_regulator_data pm8005_regulators[] = {
 	{ }
 };
 
+static const struct spmi_regulator_data pms405_regulators[] = {
+	{ "s3", 0x1a00, }, /* supply name in the dts only */
+	{ }
+};
+
 static const struct of_device_id qcom_spmi_regulator_match[] = {
 	{ .compatible = "qcom,pm8005-regulators", .data = &pm8005_regulators },
 	{ .compatible = "qcom,pm8841-regulators", .data = &pm8841_regulators },
@@ -1914,6 +1948,7 @@ static const struct of_device_id qcom_spmi_regulator_match[] = {
 	{ .compatible = "qcom,pm8941-regulators", .data = &pm8941_regulators },
 	{ .compatible = "qcom,pm8994-regulators", .data = &pm8994_regulators },
 	{ .compatible = "qcom,pmi8994-regulators", .data = &pmi8994_regulators },
+	{ .compatible = "qcom,pms405-regulators", .data = &pms405_regulators },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, qcom_spmi_regulator_match);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 1/7] drivers: regulator: qcom_spmi: enable linear range info
  2019-06-13 14:22 ` [PATCH v3 1/7] drivers: regulator: qcom_spmi: enable linear range info Jeffrey Hugo
@ 2019-06-13 15:10   ` Bjorn Andersson
  2019-06-13 15:28     ` Jeffrey Hugo
  0 siblings, 1 reply; 18+ messages in thread
From: Bjorn Andersson @ 2019-06-13 15:10 UTC (permalink / raw)
  To: Jeffrey Hugo
  Cc: lgirdwood, broonie, agross, robh+dt, mark.rutland, linux-arm-msm,
	linux-kernel, devicetree, Jorge Ramirez-Ortiz

On Thu 13 Jun 07:22 PDT 2019, Jeffrey Hugo wrote:

> From: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> 
> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> ---
>  drivers/regulator/qcom_spmi-regulator.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c
> index 53a61fb65642..fd55438c25d6 100644
> --- a/drivers/regulator/qcom_spmi-regulator.c
> +++ b/drivers/regulator/qcom_spmi-regulator.c
> @@ -1744,6 +1744,7 @@ MODULE_DEVICE_TABLE(of, qcom_spmi_regulator_match);
>  static int qcom_spmi_regulator_probe(struct platform_device *pdev)
>  {
>  	const struct spmi_regulator_data *reg;
> +	const struct spmi_voltage_range *range;
>  	const struct of_device_id *match;
>  	struct regulator_config config = { };
>  	struct regulator_dev *rdev;
> @@ -1833,6 +1834,12 @@ static int qcom_spmi_regulator_probe(struct platform_device *pdev)
>  			}
>  		}
>  
> +		if (vreg->logical_type == SPMI_REGULATOR_LOGICAL_TYPE_HFS430) {

This doesn't compile, because HFS430 isn't defined until patch 7.

But in patch 2 you replace this with a check to see if there's just a
single range, which is a better solution. So squash the last hunk of
patch 2 into this.

Regards,
Bjorn

> +			/* since there is only one range */
> +			range = spmi_regulator_find_range(vreg);
> +			vreg->desc.uV_step = range->step_uV;
> +		}
> +
>  		config.dev = dev;
>  		config.driver_data = vreg;
>  		config.regmap = regmap;
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 2/7] drivers: regulator: qcom_spmi: Refactor get_mode/set_mode
  2019-06-13 14:22 ` [PATCH v3 2/7] drivers: regulator: qcom_spmi: Refactor get_mode/set_mode Jeffrey Hugo
@ 2019-06-13 15:12   ` Bjorn Andersson
  2019-06-13 15:24     ` Bjorn Andersson
  0 siblings, 1 reply; 18+ messages in thread
From: Bjorn Andersson @ 2019-06-13 15:12 UTC (permalink / raw)
  To: Jeffrey Hugo
  Cc: lgirdwood, broonie, agross, robh+dt, mark.rutland, linux-arm-msm,
	linux-kernel, devicetree

On Thu 13 Jun 07:22 PDT 2019, Jeffrey Hugo wrote:

> spmi_regulator_common_get_mode and spmi_regulator_common_set_mode use
> multi-level ifs which mirror a switch statement.  Refactor to use a switch
> statement to make the code flow more clear.
> 
> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> ---
>  drivers/regulator/qcom_spmi-regulator.c | 28 ++++++++++++++++---------
>  1 file changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c
> index fd55438c25d6..1c18fe5969b5 100644
> --- a/drivers/regulator/qcom_spmi-regulator.c
> +++ b/drivers/regulator/qcom_spmi-regulator.c
> @@ -911,13 +911,14 @@ static unsigned int spmi_regulator_common_get_mode(struct regulator_dev *rdev)
>  
>  	spmi_vreg_read(vreg, SPMI_COMMON_REG_MODE, &reg, 1);
>  
> -	if (reg & SPMI_COMMON_MODE_HPM_MASK)
> +	switch (reg) {
> +	case SPMI_COMMON_MODE_HPM_MASK:
>  		return REGULATOR_MODE_NORMAL;
> -
> -	if (reg & SPMI_COMMON_MODE_AUTO_MASK)
> +	case SPMI_COMMON_MODE_AUTO_MASK:
>  		return REGULATOR_MODE_FAST;
> -
> -	return REGULATOR_MODE_IDLE;
> +	default:
> +		return REGULATOR_MODE_IDLE;
> +	}
>  }
>  
>  static int
> @@ -925,12 +926,19 @@ spmi_regulator_common_set_mode(struct regulator_dev *rdev, unsigned int mode)
>  {
>  	struct spmi_regulator *vreg = rdev_get_drvdata(rdev);
>  	u8 mask = SPMI_COMMON_MODE_HPM_MASK | SPMI_COMMON_MODE_AUTO_MASK;
> -	u8 val = 0;
> +	u8 val;
>  
> -	if (mode == REGULATOR_MODE_NORMAL)
> +	switch (mode) {
> +	case REGULATOR_MODE_NORMAL:
>  		val = SPMI_COMMON_MODE_HPM_MASK;
> -	else if (mode == REGULATOR_MODE_FAST)
> +		break;
> +	case REGULATOR_MODE_FAST:
>  		val = SPMI_COMMON_MODE_AUTO_MASK;
> +		break;
> +	default:
> +		val = 0;
> +		break;
> +	}

For this part:
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>  
>  	return spmi_vreg_update_bits(vreg, SPMI_COMMON_REG_MODE, val, mask);
>  }
> @@ -1834,9 +1842,9 @@ static int qcom_spmi_regulator_probe(struct platform_device *pdev)
>  			}
>  		}
>  
> -		if (vreg->logical_type == SPMI_REGULATOR_LOGICAL_TYPE_HFS430) {

Squash this into patch 1.

Regards,
Bjorn

> +		if (vreg->set_points->count == 1) {
>  			/* since there is only one range */
> -			range = spmi_regulator_find_range(vreg);
> +			range = vreg->set_points->range;
>  			vreg->desc.uV_step = range->step_uV;
>  		}
>  
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 2/7] drivers: regulator: qcom_spmi: Refactor get_mode/set_mode
  2019-06-13 15:12   ` Bjorn Andersson
@ 2019-06-13 15:24     ` Bjorn Andersson
  2019-06-13 15:37       ` Jeffrey Hugo
  0 siblings, 1 reply; 18+ messages in thread
From: Bjorn Andersson @ 2019-06-13 15:24 UTC (permalink / raw)
  To: Jeffrey Hugo
  Cc: lgirdwood, broonie, agross, robh+dt, mark.rutland, linux-arm-msm,
	linux-kernel, devicetree

On Thu 13 Jun 08:12 PDT 2019, Bjorn Andersson wrote:

> On Thu 13 Jun 07:22 PDT 2019, Jeffrey Hugo wrote:
> 
> > spmi_regulator_common_get_mode and spmi_regulator_common_set_mode use
> > multi-level ifs which mirror a switch statement.  Refactor to use a switch
> > statement to make the code flow more clear.
> > 
> > Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> > ---
> >  drivers/regulator/qcom_spmi-regulator.c | 28 ++++++++++++++++---------
> >  1 file changed, 18 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c
> > index fd55438c25d6..1c18fe5969b5 100644
> > --- a/drivers/regulator/qcom_spmi-regulator.c
> > +++ b/drivers/regulator/qcom_spmi-regulator.c
> > @@ -911,13 +911,14 @@ static unsigned int spmi_regulator_common_get_mode(struct regulator_dev *rdev)
> >  
> >  	spmi_vreg_read(vreg, SPMI_COMMON_REG_MODE, &reg, 1);
> >  
> > -	if (reg & SPMI_COMMON_MODE_HPM_MASK)

Sorry, didn't see the & here. Don't you need to mask out the mode bits
before turning this into a switch?

> > +	switch (reg) {
> > +	case SPMI_COMMON_MODE_HPM_MASK:
> >  		return REGULATOR_MODE_NORMAL;
> > -
> > -	if (reg & SPMI_COMMON_MODE_AUTO_MASK)
> > +	case SPMI_COMMON_MODE_AUTO_MASK:
> >  		return REGULATOR_MODE_FAST;
> > -
> > -	return REGULATOR_MODE_IDLE;
> > +	default:
> > +		return REGULATOR_MODE_IDLE;
> > +	}
> >  }
> >  
> >  static int
> > @@ -925,12 +926,19 @@ spmi_regulator_common_set_mode(struct regulator_dev *rdev, unsigned int mode)
> >  {
> >  	struct spmi_regulator *vreg = rdev_get_drvdata(rdev);
> >  	u8 mask = SPMI_COMMON_MODE_HPM_MASK | SPMI_COMMON_MODE_AUTO_MASK;
> > -	u8 val = 0;
> > +	u8 val;
> >  
> > -	if (mode == REGULATOR_MODE_NORMAL)
> > +	switch (mode) {
> > +	case REGULATOR_MODE_NORMAL:
> >  		val = SPMI_COMMON_MODE_HPM_MASK;
> > -	else if (mode == REGULATOR_MODE_FAST)
> > +		break;
> > +	case REGULATOR_MODE_FAST:
> >  		val = SPMI_COMMON_MODE_AUTO_MASK;
> > +		break;
> > +	default:
> > +		val = 0;
> > +		break;
> > +	}
> 
> For this part:
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> >  
> >  	return spmi_vreg_update_bits(vreg, SPMI_COMMON_REG_MODE, val, mask);
> >  }
> > @@ -1834,9 +1842,9 @@ static int qcom_spmi_regulator_probe(struct platform_device *pdev)
> >  			}
> >  		}
> >  
> > -		if (vreg->logical_type == SPMI_REGULATOR_LOGICAL_TYPE_HFS430) {
> 
> Squash this into patch 1.
> 
> Regards,
> Bjorn
> 
> > +		if (vreg->set_points->count == 1) {
> >  			/* since there is only one range */
> > -			range = spmi_regulator_find_range(vreg);
> > +			range = vreg->set_points->range;
> >  			vreg->desc.uV_step = range->step_uV;
> >  		}
> >  
> > -- 
> > 2.17.1
> > 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 4/7] regulator: qcom_spmi: Add support for PM8005
  2019-06-13 14:23 ` [PATCH v3 4/7] regulator: qcom_spmi: Add support for PM8005 Jeffrey Hugo
@ 2019-06-13 15:25   ` Bjorn Andersson
  0 siblings, 0 replies; 18+ messages in thread
From: Bjorn Andersson @ 2019-06-13 15:25 UTC (permalink / raw)
  To: Jeffrey Hugo
  Cc: lgirdwood, broonie, agross, robh+dt, mark.rutland, linux-arm-msm,
	linux-kernel, devicetree

On Thu 13 Jun 07:23 PDT 2019, Jeffrey Hugo wrote:

> The PM8005 is used on the msm8998 MTP.  The S1 regulator is VDD_GFX, ie
> it needs to be on and controlled inorder to use the GPU.  Add support to
> drive the PM8005 regulators so that we can bring up the GPU on msm8998.
> 

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> ---
>  drivers/regulator/qcom_spmi-regulator.c | 169 ++++++++++++++++++++++++
>  1 file changed, 169 insertions(+)
> 
> diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c
> index 1c18fe5969b5..c7880c1d4bcd 100644
> --- a/drivers/regulator/qcom_spmi-regulator.c
> +++ b/drivers/regulator/qcom_spmi-regulator.c
> @@ -104,6 +104,7 @@ enum spmi_regulator_logical_type {
>  	SPMI_REGULATOR_LOGICAL_TYPE_ULT_LO_SMPS,
>  	SPMI_REGULATOR_LOGICAL_TYPE_ULT_HO_SMPS,
>  	SPMI_REGULATOR_LOGICAL_TYPE_ULT_LDO,
> +	SPMI_REGULATOR_LOGICAL_TYPE_FTSMPS426,
>  };
>  
>  enum spmi_regulator_type {
> @@ -150,6 +151,7 @@ enum spmi_regulator_subtype {
>  	SPMI_REGULATOR_SUBTYPE_5V_BOOST		= 0x01,
>  	SPMI_REGULATOR_SUBTYPE_FTS_CTL		= 0x08,
>  	SPMI_REGULATOR_SUBTYPE_FTS2p5_CTL	= 0x09,
> +	SPMI_REGULATOR_SUBTYPE_FTS426_CTL	= 0x0a,
>  	SPMI_REGULATOR_SUBTYPE_BB_2A		= 0x01,
>  	SPMI_REGULATOR_SUBTYPE_ULT_HF_CTL1	= 0x0d,
>  	SPMI_REGULATOR_SUBTYPE_ULT_HF_CTL2	= 0x0e,
> @@ -170,6 +172,18 @@ enum spmi_common_regulator_registers {
>  	SPMI_COMMON_REG_STEP_CTRL		= 0x61,
>  };
>  
> +/*
> + * Second common register layout used by newer devices starting with ftsmps426
> + * Note that some of the registers from the first common layout remain
> + * unchanged and their definition is not duplicated.
> + */
> +enum spmi_ftsmps426_regulator_registers {
> +	SPMI_FTSMPS426_REG_VOLTAGE_LSB		= 0x40,
> +	SPMI_FTSMPS426_REG_VOLTAGE_MSB		= 0x41,
> +	SPMI_FTSMPS426_REG_VOLTAGE_ULS_LSB	= 0x68,
> +	SPMI_FTSMPS426_REG_VOLTAGE_ULS_MSB	= 0x69,
> +};
> +
>  enum spmi_vs_registers {
>  	SPMI_VS_REG_OCP				= 0x4a,
>  	SPMI_VS_REG_SOFT_START			= 0x4c,
> @@ -229,6 +243,14 @@ enum spmi_common_control_register_index {
>  #define SPMI_COMMON_MODE_FOLLOW_HW_EN0_MASK	0x01
>  #define SPMI_COMMON_MODE_FOLLOW_ALL_MASK	0x1f
>  
> +#define SPMI_FTSMPS426_MODE_BYPASS_MASK		3
> +#define SPMI_FTSMPS426_MODE_RETENTION_MASK	4
> +#define SPMI_FTSMPS426_MODE_LPM_MASK		5
> +#define SPMI_FTSMPS426_MODE_AUTO_MASK		6
> +#define SPMI_FTSMPS426_MODE_HPM_MASK		7
> +
> +#define SPMI_FTSMPS426_MODE_MASK		0x07
> +
>  /* Common regulator pull down control register layout */
>  #define SPMI_COMMON_PULL_DOWN_ENABLE_MASK	0x80
>  
> @@ -274,6 +296,23 @@ enum spmi_common_control_register_index {
>  #define SPMI_FTSMPS_STEP_MARGIN_NUM	4
>  #define SPMI_FTSMPS_STEP_MARGIN_DEN	5
>  
> +#define SPMI_FTSMPS426_STEP_CTRL_DELAY_MASK	0x03
> +#define SPMI_FTSMPS426_STEP_CTRL_DELAY_SHIFT	0
> +
> +/* Clock rate in kHz of the FTSMPS426 regulator reference clock. */
> +#define SPMI_FTSMPS426_CLOCK_RATE		4800
> +
> +/* Minimum voltage stepper delay for each step. */
> +#define SPMI_FTSMPS426_STEP_DELAY		2
> +
> +/*
> + * The ratio SPMI_FTSMPS426_STEP_MARGIN_NUM/SPMI_FTSMPS426_STEP_MARGIN_DEN is
> + * used to adjust the step rate in order to account for oscillator variance.
> + */
> +#define SPMI_FTSMPS426_STEP_MARGIN_NUM	10
> +#define SPMI_FTSMPS426_STEP_MARGIN_DEN	11
> +
> +
>  /* VSET value to decide the range of ULT SMPS */
>  #define ULT_SMPS_RANGE_SPLIT 0x60
>  
> @@ -447,6 +486,10 @@ static struct spmi_voltage_range ftsmps2p5_ranges[] = {
>  	SPMI_VOLTAGE_RANGE(1,  160000, 1360000, 2200000, 2200000, 10000),
>  };
>  
> +static struct spmi_voltage_range ftsmps426_ranges[] = {
> +	SPMI_VOLTAGE_RANGE(0,       0,  320000, 1352000, 1352000,  4000),
> +};
> +
>  static struct spmi_voltage_range boost_ranges[] = {
>  	SPMI_VOLTAGE_RANGE(0, 4000000, 4000000, 5550000, 5550000, 50000),
>  };
> @@ -480,6 +523,7 @@ static DEFINE_SPMI_SET_POINTS(ln_ldo);
>  static DEFINE_SPMI_SET_POINTS(smps);
>  static DEFINE_SPMI_SET_POINTS(ftsmps);
>  static DEFINE_SPMI_SET_POINTS(ftsmps2p5);
> +static DEFINE_SPMI_SET_POINTS(ftsmps426);
>  static DEFINE_SPMI_SET_POINTS(boost);
>  static DEFINE_SPMI_SET_POINTS(boost_byp);
>  static DEFINE_SPMI_SET_POINTS(ult_lo_smps);
> @@ -747,6 +791,23 @@ spmi_regulator_common_set_voltage(struct regulator_dev *rdev, unsigned selector)
>  	return spmi_vreg_write(vreg, SPMI_COMMON_REG_VOLTAGE_RANGE, buf, 2);
>  }
>  
> +static int spmi_regulator_common_list_voltage(struct regulator_dev *rdev,
> +					      unsigned selector);
> +
> +static int spmi_regulator_ftsmps426_set_voltage(struct regulator_dev *rdev,
> +					      unsigned selector)
> +{
> +	struct spmi_regulator *vreg = rdev_get_drvdata(rdev);
> +	u8 buf[2];
> +	int mV;
> +
> +	mV = spmi_regulator_common_list_voltage(rdev, selector) / 1000;
> +
> +	buf[0] = mV & 0xff;
> +	buf[1] = mV >> 8;
> +	return spmi_vreg_write(vreg, SPMI_FTSMPS426_REG_VOLTAGE_LSB, buf, 2);
> +}
> +
>  static int spmi_regulator_set_voltage_time_sel(struct regulator_dev *rdev,
>  		unsigned int old_selector, unsigned int new_selector)
>  {
> @@ -778,6 +839,16 @@ static int spmi_regulator_common_get_voltage(struct regulator_dev *rdev)
>  	return spmi_hw_selector_to_sw(vreg, voltage_sel, range);
>  }
>  
> +static int spmi_regulator_ftsmps426_get_voltage(struct regulator_dev *rdev)
> +{
> +	struct spmi_regulator *vreg = rdev_get_drvdata(rdev);
> +	u8 buf[2];
> +
> +	spmi_vreg_read(vreg, SPMI_FTSMPS426_REG_VOLTAGE_LSB, buf, 2);
> +
> +	return (((unsigned int)buf[1] << 8) | (unsigned int)buf[0]) * 1000;
> +}
> +
>  static int spmi_regulator_single_map_voltage(struct regulator_dev *rdev,
>  		int min_uV, int max_uV)
>  {
> @@ -921,6 +992,23 @@ static unsigned int spmi_regulator_common_get_mode(struct regulator_dev *rdev)
>  	}
>  }
>  
> +static unsigned int spmi_regulator_ftsmps426_get_mode(struct regulator_dev *rdev)
> +{
> +	struct spmi_regulator *vreg = rdev_get_drvdata(rdev);
> +	u8 reg;
> +
> +	spmi_vreg_read(vreg, SPMI_COMMON_REG_MODE, &reg, 1);
> +
> +	switch (reg) {
> +	case SPMI_FTSMPS426_MODE_HPM_MASK:
> +		return REGULATOR_MODE_NORMAL;
> +	case SPMI_FTSMPS426_MODE_AUTO_MASK:
> +		return REGULATOR_MODE_FAST;
> +	default:
> +		return REGULATOR_MODE_IDLE;
> +	}
> +}
> +
>  static int
>  spmi_regulator_common_set_mode(struct regulator_dev *rdev, unsigned int mode)
>  {
> @@ -943,6 +1031,28 @@ spmi_regulator_common_set_mode(struct regulator_dev *rdev, unsigned int mode)
>  	return spmi_vreg_update_bits(vreg, SPMI_COMMON_REG_MODE, val, mask);
>  }
>  
> +static int
> +spmi_regulator_ftsmps426_set_mode(struct regulator_dev *rdev, unsigned int mode)
> +{
> +	struct spmi_regulator *vreg = rdev_get_drvdata(rdev);
> +	u8 mask = SPMI_FTSMPS426_MODE_MASK;
> +	u8 val;
> +
> +	switch (mode) {
> +	case REGULATOR_MODE_NORMAL:
> +		val = SPMI_FTSMPS426_MODE_HPM_MASK;
> +		break;
> +	case REGULATOR_MODE_FAST:
> +		val = SPMI_FTSMPS426_MODE_AUTO_MASK;
> +		break;
> +	default:
> +		val = SPMI_FTSMPS426_MODE_LPM_MASK;
> +		break;
> +	}
> +
> +	return spmi_vreg_update_bits(vreg, SPMI_COMMON_REG_MODE, val, mask);
> +}
> +
>  static int
>  spmi_regulator_common_set_load(struct regulator_dev *rdev, int load_uA)
>  {
> @@ -1272,6 +1382,21 @@ static struct regulator_ops spmi_ult_ldo_ops = {
>  	.set_soft_start		= spmi_regulator_common_set_soft_start,
>  };
>  
> +static struct regulator_ops spmi_ftsmps426_ops = {
> +	.enable			= regulator_enable_regmap,
> +	.disable		= regulator_disable_regmap,
> +	.is_enabled		= regulator_is_enabled_regmap,
> +	.set_voltage_sel	= spmi_regulator_ftsmps426_set_voltage,
> +	.set_voltage_time_sel	= spmi_regulator_set_voltage_time_sel,
> +	.get_voltage		= spmi_regulator_ftsmps426_get_voltage,
> +	.map_voltage		= spmi_regulator_single_map_voltage,
> +	.list_voltage		= spmi_regulator_common_list_voltage,
> +	.set_mode		= spmi_regulator_ftsmps426_set_mode,
> +	.get_mode		= spmi_regulator_ftsmps426_get_mode,
> +	.set_load		= spmi_regulator_common_set_load,
> +	.set_pull_down		= spmi_regulator_common_set_pull_down,
> +};
> +
>  /* Maximum possible digital major revision value */
>  #define INF 0xFF
>  
> @@ -1307,6 +1432,7 @@ static const struct spmi_regulator_mapping supported_regulators[] = {
>  	SPMI_VREG(BOOST, 5V_BOOST, 0, INF, BOOST,  boost,  boost,       0),
>  	SPMI_VREG(FTS,   FTS_CTL,  0, INF, FTSMPS, ftsmps, ftsmps, 100000),
>  	SPMI_VREG(FTS, FTS2p5_CTL, 0, INF, FTSMPS, ftsmps, ftsmps2p5, 100000),
> +	SPMI_VREG(FTS, FTS426_CTL, 0, INF, FTSMPS426, ftsmps426, ftsmps426, 100000),
>  	SPMI_VREG(BOOST_BYP, BB_2A, 0, INF, BOOST_BYP, boost, boost_byp, 0),
>  	SPMI_VREG(ULT_BUCK, ULT_HF_CTL1, 0, INF, ULT_LO_SMPS, ult_lo_smps,
>  						ult_lo_smps,   100000),
> @@ -1444,6 +1570,34 @@ static int spmi_regulator_init_slew_rate(struct spmi_regulator *vreg)
>  	return ret;
>  }
>  
> +static int spmi_regulator_init_slew_rate_ftsmps426(struct spmi_regulator *vreg)
> +{
> +	int ret;
> +	u8 reg = 0;
> +	int delay, slew_rate;
> +	const struct spmi_voltage_range *range = &vreg->set_points->range[0];
> +
> +	ret = spmi_vreg_read(vreg, SPMI_COMMON_REG_STEP_CTRL, &reg, 1);
> +	if (ret) {
> +		dev_err(vreg->dev, "spmi read failed, ret=%d\n", ret);
> +		return ret;
> +	}
> +
> +	delay = reg & SPMI_FTSMPS426_STEP_CTRL_DELAY_MASK;
> +	delay >>= SPMI_FTSMPS426_STEP_CTRL_DELAY_SHIFT;
> +
> +	/* slew_rate has units of uV/us */
> +	slew_rate = SPMI_FTSMPS426_CLOCK_RATE * range->step_uV;
> +	slew_rate /= 1000 * (SPMI_FTSMPS426_STEP_DELAY << delay);
> +	slew_rate *= SPMI_FTSMPS426_STEP_MARGIN_NUM;
> +	slew_rate /= SPMI_FTSMPS426_STEP_MARGIN_DEN;
> +
> +	/* Ensure that the slew rate is greater than 0 */
> +	vreg->slew_rate = max(slew_rate, 1);
> +
> +	return ret;
> +}
> +
>  static int spmi_regulator_init_registers(struct spmi_regulator *vreg,
>  				const struct spmi_regulator_init_data *data)
>  {
> @@ -1583,6 +1737,12 @@ static int spmi_regulator_of_parse(struct device_node *node,
>  		ret = spmi_regulator_init_slew_rate(vreg);
>  		if (ret)
>  			return ret;
> +		break;
> +	case SPMI_REGULATOR_LOGICAL_TYPE_FTSMPS426:
> +		ret = spmi_regulator_init_slew_rate_ftsmps426(vreg);
> +		if (ret)
> +			return ret;
> +		break;
>  	default:
>  		break;
>  	}
> @@ -1739,7 +1899,16 @@ static const struct spmi_regulator_data pmi8994_regulators[] = {
>  	{ }
>  };
>  
> +static const struct spmi_regulator_data pm8005_regulators[] = {
> +	{ "s1", 0x1400, "vdd_s1", },
> +	{ "s2", 0x1700, "vdd_s2", },
> +	{ "s3", 0x1a00, "vdd_s3", },
> +	{ "s4", 0x1d00, "vdd_s4", },
> +	{ }
> +};
> +
>  static const struct of_device_id qcom_spmi_regulator_match[] = {
> +	{ .compatible = "qcom,pm8005-regulators", .data = &pm8005_regulators },
>  	{ .compatible = "qcom,pm8841-regulators", .data = &pm8841_regulators },
>  	{ .compatible = "qcom,pm8916-regulators", .data = &pm8916_regulators },
>  	{ .compatible = "qcom,pm8941-regulators", .data = &pm8941_regulators },
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 1/7] drivers: regulator: qcom_spmi: enable linear range info
  2019-06-13 15:10   ` Bjorn Andersson
@ 2019-06-13 15:28     ` Jeffrey Hugo
  0 siblings, 0 replies; 18+ messages in thread
From: Jeffrey Hugo @ 2019-06-13 15:28 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: lgirdwood, broonie, Andy Gross, Rob Herring, Mark Rutland, MSM,
	lkml, devicetree, Jorge Ramirez-Ortiz

On Thu, Jun 13, 2019 at 9:10 AM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Thu 13 Jun 07:22 PDT 2019, Jeffrey Hugo wrote:
>
> > From: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> >
> > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> > Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> > ---
> >  drivers/regulator/qcom_spmi-regulator.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c
> > index 53a61fb65642..fd55438c25d6 100644
> > --- a/drivers/regulator/qcom_spmi-regulator.c
> > +++ b/drivers/regulator/qcom_spmi-regulator.c
> > @@ -1744,6 +1744,7 @@ MODULE_DEVICE_TABLE(of, qcom_spmi_regulator_match);
> >  static int qcom_spmi_regulator_probe(struct platform_device *pdev)
> >  {
> >       const struct spmi_regulator_data *reg;
> > +     const struct spmi_voltage_range *range;
> >       const struct of_device_id *match;
> >       struct regulator_config config = { };
> >       struct regulator_dev *rdev;
> > @@ -1833,6 +1834,12 @@ static int qcom_spmi_regulator_probe(struct platform_device *pdev)
> >                       }
> >               }
> >
> > +             if (vreg->logical_type == SPMI_REGULATOR_LOGICAL_TYPE_HFS430) {
>
> This doesn't compile, because HFS430 isn't defined until patch 7.
>
> But in patch 2 you replace this with a check to see if there's just a
> single range, which is a better solution. So squash the last hunk of
> patch 2 into this.

Huh, I did botch that.  The patch 2 hunk was intended to be in patch
1.  Will fix.

>
> Regards,
> Bjorn
>
> > +                     /* since there is only one range */
> > +                     range = spmi_regulator_find_range(vreg);
> > +                     vreg->desc.uV_step = range->step_uV;
> > +             }
> > +
> >               config.dev = dev;
> >               config.driver_data = vreg;
> >               config.regmap = regmap;
> > --
> > 2.17.1
> >

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 7/7] drivers: regulator: qcom: add PMS405 SPMI regulator
  2019-06-13 14:24 ` [PATCH v3 7/7] drivers: regulator: qcom: add PMS405 SPMI regulator Jeffrey Hugo
@ 2019-06-13 15:33   ` Bjorn Andersson
  2019-06-13 15:41     ` Jeffrey Hugo
  0 siblings, 1 reply; 18+ messages in thread
From: Bjorn Andersson @ 2019-06-13 15:33 UTC (permalink / raw)
  To: Jeffrey Hugo
  Cc: lgirdwood, broonie, agross, robh+dt, mark.rutland, linux-arm-msm,
	linux-kernel, devicetree, Jorge Ramirez

On Thu 13 Jun 07:24 PDT 2019, Jeffrey Hugo wrote:

> From: Jorge Ramirez <jorge.ramirez-ortiz@linaro.org>
> 
> The PMS405 has 5 HFSMPS and 13 LDO regulators,
> 
> This commit adds support for one of the 5 HFSMPS regulators (s3) to
> the spmi regulator driver.
> 
> The PMIC HFSMPS 430 regulators have 8 mV step size and a voltage
> control scheme consisting of two  8-bit registers defining a 16-bit
> voltage set point in units of millivolts
> 
> S3 controls the cpu voltages (s3 is a buck regulator of type HFS430);
> it is therefore required so we can enable voltage scaling for safely
> running cpufreq.
> 
> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> ---
>  drivers/regulator/qcom_spmi-regulator.c | 41 +++++++++++++++++++++++--
>  1 file changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c
> index c7880c1d4bcd..975655e787fe 100644
> --- a/drivers/regulator/qcom_spmi-regulator.c
> +++ b/drivers/regulator/qcom_spmi-regulator.c
> @@ -105,6 +105,7 @@ enum spmi_regulator_logical_type {
>  	SPMI_REGULATOR_LOGICAL_TYPE_ULT_HO_SMPS,
>  	SPMI_REGULATOR_LOGICAL_TYPE_ULT_LDO,
>  	SPMI_REGULATOR_LOGICAL_TYPE_FTSMPS426,
> +	SPMI_REGULATOR_LOGICAL_TYPE_HFS430,
>  };
>  
>  enum spmi_regulator_type {
> @@ -157,6 +158,7 @@ enum spmi_regulator_subtype {
>  	SPMI_REGULATOR_SUBTYPE_ULT_HF_CTL2	= 0x0e,
>  	SPMI_REGULATOR_SUBTYPE_ULT_HF_CTL3	= 0x0f,
>  	SPMI_REGULATOR_SUBTYPE_ULT_HF_CTL4	= 0x10,
> +	SPMI_REGULATOR_SUBTYPE_HFS430		= 0x0a,
>  };
>  
>  enum spmi_common_regulator_registers {
> @@ -302,6 +304,8 @@ enum spmi_common_control_register_index {
>  /* Clock rate in kHz of the FTSMPS426 regulator reference clock. */
>  #define SPMI_FTSMPS426_CLOCK_RATE		4800
>  
> +#define SPMI_HFS430_CLOCK_RATE			1600
> +
>  /* Minimum voltage stepper delay for each step. */
>  #define SPMI_FTSMPS426_STEP_DELAY		2
>  
> @@ -515,6 +519,10 @@ static struct spmi_voltage_range ult_pldo_ranges[] = {
>  	SPMI_VOLTAGE_RANGE(0, 1750000, 1750000, 3337500, 3337500, 12500),
>  };
>  
> +static struct spmi_voltage_range hfs430_ranges[] = {
> +	SPMI_VOLTAGE_RANGE(0, 320000, 320000, 2040000, 2040000, 8000),
> +};
> +
>  static DEFINE_SPMI_SET_POINTS(pldo);
>  static DEFINE_SPMI_SET_POINTS(nldo1);
>  static DEFINE_SPMI_SET_POINTS(nldo2);
> @@ -530,6 +538,7 @@ static DEFINE_SPMI_SET_POINTS(ult_lo_smps);
>  static DEFINE_SPMI_SET_POINTS(ult_ho_smps);
>  static DEFINE_SPMI_SET_POINTS(ult_nldo);
>  static DEFINE_SPMI_SET_POINTS(ult_pldo);
> +static DEFINE_SPMI_SET_POINTS(hfs430);
>  
>  static inline int spmi_vreg_read(struct spmi_regulator *vreg, u16 addr, u8 *buf,
>  				 int len)
> @@ -1397,12 +1406,24 @@ static struct regulator_ops spmi_ftsmps426_ops = {
>  	.set_pull_down		= spmi_regulator_common_set_pull_down,
>  };
>  
> +static struct regulator_ops spmi_hfs430_ops = {
> +	/* always on regulators */
> +	.set_voltage_sel	= spmi_regulator_ftsmps426_set_voltage,
> +	.set_voltage_time_sel	= spmi_regulator_set_voltage_time_sel,
> +	.get_voltage		= spmi_regulator_ftsmps426_get_voltage,
> +	.map_voltage		= spmi_regulator_single_map_voltage,
> +	.list_voltage		= spmi_regulator_common_list_voltage,
> +	.set_mode		= spmi_regulator_ftsmps426_set_mode,
> +	.get_mode		= spmi_regulator_ftsmps426_get_mode,
> +};
> +
>  /* Maximum possible digital major revision value */
>  #define INF 0xFF
>  
>  static const struct spmi_regulator_mapping supported_regulators[] = {
>  	/*           type subtype dig_min dig_max ltype ops setpoints hpm_min */
>  	SPMI_VREG(BUCK,  GP_CTL,   0, INF, SMPS,   smps,   smps,   100000),
> +	SPMI_VREG(BUCK,  HFS430,   0, INF, HFS430, hfs430, hfs430,  10000),
>  	SPMI_VREG(LDO,   N300,     0, INF, LDO,    ldo,    nldo1,   10000),
>  	SPMI_VREG(LDO,   N600,     0,   0, LDO,    ldo,    nldo2,   10000),
>  	SPMI_VREG(LDO,   N1200,    0,   0, LDO,    ldo,    nldo2,   10000),
> @@ -1570,7 +1591,8 @@ static int spmi_regulator_init_slew_rate(struct spmi_regulator *vreg)
>  	return ret;
>  }
>  
> -static int spmi_regulator_init_slew_rate_ftsmps426(struct spmi_regulator *vreg)
> +static int spmi_regulator_init_slew_rate_ftsmps426(struct spmi_regulator *vreg,
> +						   int clock_rate)
>  {
>  	int ret;
>  	u8 reg = 0;
> @@ -1587,7 +1609,7 @@ static int spmi_regulator_init_slew_rate_ftsmps426(struct spmi_regulator *vreg)
>  	delay >>= SPMI_FTSMPS426_STEP_CTRL_DELAY_SHIFT;
>  
>  	/* slew_rate has units of uV/us */
> -	slew_rate = SPMI_FTSMPS426_CLOCK_RATE * range->step_uV;
> +	slew_rate = clock_rate * range->step_uV;
>  	slew_rate /= 1000 * (SPMI_FTSMPS426_STEP_DELAY << delay);
>  	slew_rate *= SPMI_FTSMPS426_STEP_MARGIN_NUM;
>  	slew_rate /= SPMI_FTSMPS426_STEP_MARGIN_DEN;
> @@ -1739,7 +1761,14 @@ static int spmi_regulator_of_parse(struct device_node *node,
>  			return ret;
>  		break;
>  	case SPMI_REGULATOR_LOGICAL_TYPE_FTSMPS426:
> -		ret = spmi_regulator_init_slew_rate_ftsmps426(vreg);
> +		ret = spmi_regulator_init_slew_rate_ftsmps426(vreg,
> +						SPMI_FTSMPS426_CLOCK_RATE);
> +		if (ret)
> +			return ret;
> +		break;
> +	case SPMI_REGULATOR_LOGICAL_TYPE_HFS430:
> +		ret = spmi_regulator_init_slew_rate_ftsmps426(vreg,
> +							SPMI_HFS430_CLOCK_RATE);
>  		if (ret)
>  			return ret;
>  		break;
> @@ -1907,6 +1936,11 @@ static const struct spmi_regulator_data pm8005_regulators[] = {
>  	{ }
>  };
>  
> +static const struct spmi_regulator_data pms405_regulators[] = {
> +	{ "s3", 0x1a00, }, /* supply name in the dts only */

Not sure what this comment is trying to say. The third element here
should be the string that is used to find the supply as specified in DT.
For s3 this is "vdd_s3".


So please drop the comment and make this:
	{ "s3", 0x1a00, "s3" },

Regards,
Bjorn

> +	{ }
> +};
> +
>  static const struct of_device_id qcom_spmi_regulator_match[] = {
>  	{ .compatible = "qcom,pm8005-regulators", .data = &pm8005_regulators },
>  	{ .compatible = "qcom,pm8841-regulators", .data = &pm8841_regulators },
> @@ -1914,6 +1948,7 @@ static const struct of_device_id qcom_spmi_regulator_match[] = {
>  	{ .compatible = "qcom,pm8941-regulators", .data = &pm8941_regulators },
>  	{ .compatible = "qcom,pm8994-regulators", .data = &pm8994_regulators },
>  	{ .compatible = "qcom,pmi8994-regulators", .data = &pmi8994_regulators },
> +	{ .compatible = "qcom,pms405-regulators", .data = &pms405_regulators },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, qcom_spmi_regulator_match);
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 6/7] dt-bindings: qcom_spmi: Document pms405 support
  2019-06-13 14:24 ` [PATCH v3 6/7] dt-bindings: qcom_spmi: Document pms405 support Jeffrey Hugo
@ 2019-06-13 15:36   ` Bjorn Andersson
  2019-06-13 15:40     ` Jeffrey Hugo
  0 siblings, 1 reply; 18+ messages in thread
From: Bjorn Andersson @ 2019-06-13 15:36 UTC (permalink / raw)
  To: Jeffrey Hugo
  Cc: lgirdwood, broonie, agross, robh+dt, mark.rutland, linux-arm-msm,
	linux-kernel, devicetree, Jorge Ramirez

On Thu 13 Jun 07:24 PDT 2019, Jeffrey Hugo wrote:

> From: Jorge Ramirez <jorge.ramirez-ortiz@linaro.org>
> 
> The PMS405 supports 5 SMPS and 13 LDO regulators.
> 
> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> ---
>  .../regulator/qcom,spmi-regulator.txt         | 24 +++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt b/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
> index ba94bc2d407a..19cffb239094 100644
> --- a/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
> +++ b/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
> @@ -10,6 +10,7 @@ Qualcomm SPMI Regulators
>  			"qcom,pm8941-regulators"
>  			"qcom,pm8994-regulators"
>  			"qcom,pmi8994-regulators"
> +			"qcom,pms405-regulators"
>  
>  - interrupts:
>  	Usage: optional
> @@ -111,6 +112,29 @@ Qualcomm SPMI Regulators
>  	Definition: Reference to regulator supplying the input pin, as
>  		    described in the data sheet.
>  
> +- vdd_s1-supply:
> +- vdd_s2-supply:
> +- vdd_s3-supply:
> +- vdd_s4-supply:
> +- vdd_s5-supply:
> +- vdd_l1-supply:
> +- vdd_l2-supply:
> +- vdd_l3-supply:
> +- vdd_l4-supply:
> +- vdd_l5-supply:
> +- vdd_l6-supply:
> +- vdd_l7-supply:
> +- vdd_l8-supply:
> +- vdd_l9-supply:
> +- vdd_l10-supply:
> +- vdd_l11-supply:
> +- vdd_l12-supply:
> +- vdd_l13-supply:

No, the supply pins are as follows:

- vdd_l1_l2-supply:
- vdd_l3_l8-supply:
- vdd_l4-supply:
- vdd_l5_l6-supply:
- vdd_l10_l11_l12_l13-supply:
- vdd_l7-supply:
- vdd_l9-supply:
- vdd_s1-supply:
- vdd_s2-supply:
- vdd_s3-supply:
- vdd_s4-supply:
- vdd_s5-supply:


Regards,
Bjorn

> +	Usage: optional (pms405 only)
> +	Value type: <phandle>
> +	Definition: Reference to regulator supplying the input pin, as
> +		    described in the data sheet.
> +
>  - qcom,saw-reg:
>  	Usage: optional
>  	Value type: <phandle>
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 2/7] drivers: regulator: qcom_spmi: Refactor get_mode/set_mode
  2019-06-13 15:24     ` Bjorn Andersson
@ 2019-06-13 15:37       ` Jeffrey Hugo
  0 siblings, 0 replies; 18+ messages in thread
From: Jeffrey Hugo @ 2019-06-13 15:37 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: lgirdwood, broonie, Andy Gross, Rob Herring, Mark Rutland, MSM,
	lkml, devicetree

On Thu, Jun 13, 2019 at 9:24 AM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Thu 13 Jun 08:12 PDT 2019, Bjorn Andersson wrote:
>
> > On Thu 13 Jun 07:22 PDT 2019, Jeffrey Hugo wrote:
> >
> > > spmi_regulator_common_get_mode and spmi_regulator_common_set_mode use
> > > multi-level ifs which mirror a switch statement.  Refactor to use a switch
> > > statement to make the code flow more clear.
> > >
> > > Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> > > ---
> > >  drivers/regulator/qcom_spmi-regulator.c | 28 ++++++++++++++++---------
> > >  1 file changed, 18 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c
> > > index fd55438c25d6..1c18fe5969b5 100644
> > > --- a/drivers/regulator/qcom_spmi-regulator.c
> > > +++ b/drivers/regulator/qcom_spmi-regulator.c
> > > @@ -911,13 +911,14 @@ static unsigned int spmi_regulator_common_get_mode(struct regulator_dev *rdev)
> > >
> > >     spmi_vreg_read(vreg, SPMI_COMMON_REG_MODE, &reg, 1);
> > >
> > > -   if (reg & SPMI_COMMON_MODE_HPM_MASK)
>
> Sorry, didn't see the & here. Don't you need to mask out the mode bits
> before turning this into a switch?

Ah.  Yes.  I read the documentation wrong when doing this.  Will fix.

>
> > > +   switch (reg) {
> > > +   case SPMI_COMMON_MODE_HPM_MASK:
> > >             return REGULATOR_MODE_NORMAL;
> > > -
> > > -   if (reg & SPMI_COMMON_MODE_AUTO_MASK)
> > > +   case SPMI_COMMON_MODE_AUTO_MASK:
> > >             return REGULATOR_MODE_FAST;
> > > -
> > > -   return REGULATOR_MODE_IDLE;
> > > +   default:
> > > +           return REGULATOR_MODE_IDLE;
> > > +   }
> > >  }
> > >
> > >  static int
> > > @@ -925,12 +926,19 @@ spmi_regulator_common_set_mode(struct regulator_dev *rdev, unsigned int mode)
> > >  {
> > >     struct spmi_regulator *vreg = rdev_get_drvdata(rdev);
> > >     u8 mask = SPMI_COMMON_MODE_HPM_MASK | SPMI_COMMON_MODE_AUTO_MASK;
> > > -   u8 val = 0;
> > > +   u8 val;
> > >
> > > -   if (mode == REGULATOR_MODE_NORMAL)
> > > +   switch (mode) {
> > > +   case REGULATOR_MODE_NORMAL:
> > >             val = SPMI_COMMON_MODE_HPM_MASK;
> > > -   else if (mode == REGULATOR_MODE_FAST)
> > > +           break;
> > > +   case REGULATOR_MODE_FAST:
> > >             val = SPMI_COMMON_MODE_AUTO_MASK;
> > > +           break;
> > > +   default:
> > > +           val = 0;
> > > +           break;
> > > +   }
> >
> > For this part:
> > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > >
> > >     return spmi_vreg_update_bits(vreg, SPMI_COMMON_REG_MODE, val, mask);
> > >  }
> > > @@ -1834,9 +1842,9 @@ static int qcom_spmi_regulator_probe(struct platform_device *pdev)
> > >                     }
> > >             }
> > >
> > > -           if (vreg->logical_type == SPMI_REGULATOR_LOGICAL_TYPE_HFS430) {
> >
> > Squash this into patch 1.
> >
> > Regards,
> > Bjorn
> >
> > > +           if (vreg->set_points->count == 1) {
> > >                     /* since there is only one range */
> > > -                   range = spmi_regulator_find_range(vreg);
> > > +                   range = vreg->set_points->range;
> > >                     vreg->desc.uV_step = range->step_uV;
> > >             }
> > >
> > > --
> > > 2.17.1
> > >

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 6/7] dt-bindings: qcom_spmi: Document pms405 support
  2019-06-13 15:36   ` Bjorn Andersson
@ 2019-06-13 15:40     ` Jeffrey Hugo
  0 siblings, 0 replies; 18+ messages in thread
From: Jeffrey Hugo @ 2019-06-13 15:40 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: lgirdwood, broonie, Andy Gross, Rob Herring, Mark Rutland, MSM,
	lkml, devicetree, Jorge Ramirez

On Thu, Jun 13, 2019 at 9:36 AM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Thu 13 Jun 07:24 PDT 2019, Jeffrey Hugo wrote:
>
> > From: Jorge Ramirez <jorge.ramirez-ortiz@linaro.org>
> >
> > The PMS405 supports 5 SMPS and 13 LDO regulators.
> >
> > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> > ---
> >  .../regulator/qcom,spmi-regulator.txt         | 24 +++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt b/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
> > index ba94bc2d407a..19cffb239094 100644
> > --- a/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
> > +++ b/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt
> > @@ -10,6 +10,7 @@ Qualcomm SPMI Regulators
> >                       "qcom,pm8941-regulators"
> >                       "qcom,pm8994-regulators"
> >                       "qcom,pmi8994-regulators"
> > +                     "qcom,pms405-regulators"
> >
> >  - interrupts:
> >       Usage: optional
> > @@ -111,6 +112,29 @@ Qualcomm SPMI Regulators
> >       Definition: Reference to regulator supplying the input pin, as
> >                   described in the data sheet.
> >
> > +- vdd_s1-supply:
> > +- vdd_s2-supply:
> > +- vdd_s3-supply:
> > +- vdd_s4-supply:
> > +- vdd_s5-supply:
> > +- vdd_l1-supply:
> > +- vdd_l2-supply:
> > +- vdd_l3-supply:
> > +- vdd_l4-supply:
> > +- vdd_l5-supply:
> > +- vdd_l6-supply:
> > +- vdd_l7-supply:
> > +- vdd_l8-supply:
> > +- vdd_l9-supply:
> > +- vdd_l10-supply:
> > +- vdd_l11-supply:
> > +- vdd_l12-supply:
> > +- vdd_l13-supply:
>
> No, the supply pins are as follows:
>
> - vdd_l1_l2-supply:
> - vdd_l3_l8-supply:
> - vdd_l4-supply:
> - vdd_l5_l6-supply:
> - vdd_l10_l11_l12_l13-supply:
> - vdd_l7-supply:
> - vdd_l9-supply:
> - vdd_s1-supply:
> - vdd_s2-supply:
> - vdd_s3-supply:
> - vdd_s4-supply:
> - vdd_s5-supply:

Sure, will update.

>
>
> Regards,
> Bjorn
>
> > +     Usage: optional (pms405 only)
> > +     Value type: <phandle>
> > +     Definition: Reference to regulator supplying the input pin, as
> > +                 described in the data sheet.
> > +
> >  - qcom,saw-reg:
> >       Usage: optional
> >       Value type: <phandle>
> > --
> > 2.17.1
> >

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 7/7] drivers: regulator: qcom: add PMS405 SPMI regulator
  2019-06-13 15:33   ` Bjorn Andersson
@ 2019-06-13 15:41     ` Jeffrey Hugo
  0 siblings, 0 replies; 18+ messages in thread
From: Jeffrey Hugo @ 2019-06-13 15:41 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: lgirdwood, broonie, Andy Gross, Rob Herring, Mark Rutland, MSM,
	lkml, devicetree, Jorge Ramirez

On Thu, Jun 13, 2019 at 9:33 AM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Thu 13 Jun 07:24 PDT 2019, Jeffrey Hugo wrote:
>
> > From: Jorge Ramirez <jorge.ramirez-ortiz@linaro.org>
> >
> > The PMS405 has 5 HFSMPS and 13 LDO regulators,
> >
> > This commit adds support for one of the 5 HFSMPS regulators (s3) to
> > the spmi regulator driver.
> >
> > The PMIC HFSMPS 430 regulators have 8 mV step size and a voltage
> > control scheme consisting of two  8-bit registers defining a 16-bit
> > voltage set point in units of millivolts
> >
> > S3 controls the cpu voltages (s3 is a buck regulator of type HFS430);
> > it is therefore required so we can enable voltage scaling for safely
> > running cpufreq.
> >
> > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> > Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
> > ---
> >  drivers/regulator/qcom_spmi-regulator.c | 41 +++++++++++++++++++++++--
> >  1 file changed, 38 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c
> > index c7880c1d4bcd..975655e787fe 100644
> > --- a/drivers/regulator/qcom_spmi-regulator.c
> > +++ b/drivers/regulator/qcom_spmi-regulator.c
> > @@ -105,6 +105,7 @@ enum spmi_regulator_logical_type {
> >       SPMI_REGULATOR_LOGICAL_TYPE_ULT_HO_SMPS,
> >       SPMI_REGULATOR_LOGICAL_TYPE_ULT_LDO,
> >       SPMI_REGULATOR_LOGICAL_TYPE_FTSMPS426,
> > +     SPMI_REGULATOR_LOGICAL_TYPE_HFS430,
> >  };
> >
> >  enum spmi_regulator_type {
> > @@ -157,6 +158,7 @@ enum spmi_regulator_subtype {
> >       SPMI_REGULATOR_SUBTYPE_ULT_HF_CTL2      = 0x0e,
> >       SPMI_REGULATOR_SUBTYPE_ULT_HF_CTL3      = 0x0f,
> >       SPMI_REGULATOR_SUBTYPE_ULT_HF_CTL4      = 0x10,
> > +     SPMI_REGULATOR_SUBTYPE_HFS430           = 0x0a,
> >  };
> >
> >  enum spmi_common_regulator_registers {
> > @@ -302,6 +304,8 @@ enum spmi_common_control_register_index {
> >  /* Clock rate in kHz of the FTSMPS426 regulator reference clock. */
> >  #define SPMI_FTSMPS426_CLOCK_RATE            4800
> >
> > +#define SPMI_HFS430_CLOCK_RATE                       1600
> > +
> >  /* Minimum voltage stepper delay for each step. */
> >  #define SPMI_FTSMPS426_STEP_DELAY            2
> >
> > @@ -515,6 +519,10 @@ static struct spmi_voltage_range ult_pldo_ranges[] = {
> >       SPMI_VOLTAGE_RANGE(0, 1750000, 1750000, 3337500, 3337500, 12500),
> >  };
> >
> > +static struct spmi_voltage_range hfs430_ranges[] = {
> > +     SPMI_VOLTAGE_RANGE(0, 320000, 320000, 2040000, 2040000, 8000),
> > +};
> > +
> >  static DEFINE_SPMI_SET_POINTS(pldo);
> >  static DEFINE_SPMI_SET_POINTS(nldo1);
> >  static DEFINE_SPMI_SET_POINTS(nldo2);
> > @@ -530,6 +538,7 @@ static DEFINE_SPMI_SET_POINTS(ult_lo_smps);
> >  static DEFINE_SPMI_SET_POINTS(ult_ho_smps);
> >  static DEFINE_SPMI_SET_POINTS(ult_nldo);
> >  static DEFINE_SPMI_SET_POINTS(ult_pldo);
> > +static DEFINE_SPMI_SET_POINTS(hfs430);
> >
> >  static inline int spmi_vreg_read(struct spmi_regulator *vreg, u16 addr, u8 *buf,
> >                                int len)
> > @@ -1397,12 +1406,24 @@ static struct regulator_ops spmi_ftsmps426_ops = {
> >       .set_pull_down          = spmi_regulator_common_set_pull_down,
> >  };
> >
> > +static struct regulator_ops spmi_hfs430_ops = {
> > +     /* always on regulators */
> > +     .set_voltage_sel        = spmi_regulator_ftsmps426_set_voltage,
> > +     .set_voltage_time_sel   = spmi_regulator_set_voltage_time_sel,
> > +     .get_voltage            = spmi_regulator_ftsmps426_get_voltage,
> > +     .map_voltage            = spmi_regulator_single_map_voltage,
> > +     .list_voltage           = spmi_regulator_common_list_voltage,
> > +     .set_mode               = spmi_regulator_ftsmps426_set_mode,
> > +     .get_mode               = spmi_regulator_ftsmps426_get_mode,
> > +};
> > +
> >  /* Maximum possible digital major revision value */
> >  #define INF 0xFF
> >
> >  static const struct spmi_regulator_mapping supported_regulators[] = {
> >       /*           type subtype dig_min dig_max ltype ops setpoints hpm_min */
> >       SPMI_VREG(BUCK,  GP_CTL,   0, INF, SMPS,   smps,   smps,   100000),
> > +     SPMI_VREG(BUCK,  HFS430,   0, INF, HFS430, hfs430, hfs430,  10000),
> >       SPMI_VREG(LDO,   N300,     0, INF, LDO,    ldo,    nldo1,   10000),
> >       SPMI_VREG(LDO,   N600,     0,   0, LDO,    ldo,    nldo2,   10000),
> >       SPMI_VREG(LDO,   N1200,    0,   0, LDO,    ldo,    nldo2,   10000),
> > @@ -1570,7 +1591,8 @@ static int spmi_regulator_init_slew_rate(struct spmi_regulator *vreg)
> >       return ret;
> >  }
> >
> > -static int spmi_regulator_init_slew_rate_ftsmps426(struct spmi_regulator *vreg)
> > +static int spmi_regulator_init_slew_rate_ftsmps426(struct spmi_regulator *vreg,
> > +                                                int clock_rate)
> >  {
> >       int ret;
> >       u8 reg = 0;
> > @@ -1587,7 +1609,7 @@ static int spmi_regulator_init_slew_rate_ftsmps426(struct spmi_regulator *vreg)
> >       delay >>= SPMI_FTSMPS426_STEP_CTRL_DELAY_SHIFT;
> >
> >       /* slew_rate has units of uV/us */
> > -     slew_rate = SPMI_FTSMPS426_CLOCK_RATE * range->step_uV;
> > +     slew_rate = clock_rate * range->step_uV;
> >       slew_rate /= 1000 * (SPMI_FTSMPS426_STEP_DELAY << delay);
> >       slew_rate *= SPMI_FTSMPS426_STEP_MARGIN_NUM;
> >       slew_rate /= SPMI_FTSMPS426_STEP_MARGIN_DEN;
> > @@ -1739,7 +1761,14 @@ static int spmi_regulator_of_parse(struct device_node *node,
> >                       return ret;
> >               break;
> >       case SPMI_REGULATOR_LOGICAL_TYPE_FTSMPS426:
> > -             ret = spmi_regulator_init_slew_rate_ftsmps426(vreg);
> > +             ret = spmi_regulator_init_slew_rate_ftsmps426(vreg,
> > +                                             SPMI_FTSMPS426_CLOCK_RATE);
> > +             if (ret)
> > +                     return ret;
> > +             break;
> > +     case SPMI_REGULATOR_LOGICAL_TYPE_HFS430:
> > +             ret = spmi_regulator_init_slew_rate_ftsmps426(vreg,
> > +                                                     SPMI_HFS430_CLOCK_RATE);
> >               if (ret)
> >                       return ret;
> >               break;
> > @@ -1907,6 +1936,11 @@ static const struct spmi_regulator_data pm8005_regulators[] = {
> >       { }
> >  };
> >
> > +static const struct spmi_regulator_data pms405_regulators[] = {
> > +     { "s3", 0x1a00, }, /* supply name in the dts only */
>
> Not sure what this comment is trying to say. The third element here
> should be the string that is used to find the supply as specified in DT.
> For s3 this is "vdd_s3".
>
>
> So please drop the comment and make this:
>         { "s3", 0x1a00, "s3" },

Makes sense, will update.

>
> Regards,
> Bjorn
>
> > +     { }
> > +};
> > +
> >  static const struct of_device_id qcom_spmi_regulator_match[] = {
> >       { .compatible = "qcom,pm8005-regulators", .data = &pm8005_regulators },
> >       { .compatible = "qcom,pm8841-regulators", .data = &pm8841_regulators },
> > @@ -1914,6 +1948,7 @@ static const struct of_device_id qcom_spmi_regulator_match[] = {
> >       { .compatible = "qcom,pm8941-regulators", .data = &pm8941_regulators },
> >       { .compatible = "qcom,pm8994-regulators", .data = &pm8994_regulators },
> >       { .compatible = "qcom,pmi8994-regulators", .data = &pmi8994_regulators },
> > +     { .compatible = "qcom,pms405-regulators", .data = &pms405_regulators },
> >       { }
> >  };
> >  MODULE_DEVICE_TABLE(of, qcom_spmi_regulator_match);
> > --
> > 2.17.1
> >

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, back to index

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-13 14:21 [PATCH v2 0/7] PM8005 and PMS405 regulator support Jeffrey Hugo
2019-06-13 14:22 ` [PATCH v3 1/7] drivers: regulator: qcom_spmi: enable linear range info Jeffrey Hugo
2019-06-13 15:10   ` Bjorn Andersson
2019-06-13 15:28     ` Jeffrey Hugo
2019-06-13 14:22 ` [PATCH v3 2/7] drivers: regulator: qcom_spmi: Refactor get_mode/set_mode Jeffrey Hugo
2019-06-13 15:12   ` Bjorn Andersson
2019-06-13 15:24     ` Bjorn Andersson
2019-06-13 15:37       ` Jeffrey Hugo
2019-06-13 14:23 ` [PATCH v3 3/7] dt-bindings: qcom_spmi: Document PM8005 regulators Jeffrey Hugo
2019-06-13 14:23 ` [PATCH v3 4/7] regulator: qcom_spmi: Add support for PM8005 Jeffrey Hugo
2019-06-13 15:25   ` Bjorn Andersson
2019-06-13 14:24 ` [PATCH v3 5/7] arm64: dts: msm8998-mtp: Add pm8005_s1 regulator Jeffrey Hugo
2019-06-13 14:24 ` [PATCH v3 6/7] dt-bindings: qcom_spmi: Document pms405 support Jeffrey Hugo
2019-06-13 15:36   ` Bjorn Andersson
2019-06-13 15:40     ` Jeffrey Hugo
2019-06-13 14:24 ` [PATCH v3 7/7] drivers: regulator: qcom: add PMS405 SPMI regulator Jeffrey Hugo
2019-06-13 15:33   ` Bjorn Andersson
2019-06-13 15:41     ` Jeffrey Hugo

Linux-ARM-MSM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-msm/0 linux-arm-msm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-msm linux-arm-msm/ https://lore.kernel.org/linux-arm-msm \
		linux-arm-msm@vger.kernel.org linux-arm-msm@archiver.kernel.org
	public-inbox-index linux-arm-msm


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-arm-msm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox