* [PATCH 0/3] PM8005 regulator support for msm8998 GPU
@ 2019-05-21 16:49 Jeffrey Hugo
2019-05-21 16:52 ` [PATCH 1/3] dt-bindings: qcom_spmi: Document PM8005 regulators Jeffrey Hugo
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Jeffrey Hugo @ 2019-05-21 16:49 UTC (permalink / raw)
Cc: agross, david.brown, bjorn.andersson, jcrouse, 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.
Jeffrey Hugo (3):
dt-bindings: qcom_spmi: Document PM8005 regulators
regulator: qcom_spmi: Add support for PM8005
arm64: dts: msm8998-mtp: Add pm8005_s1 regulator
.../regulator/qcom,spmi-regulator.txt | 4 +
arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi | 17 ++
drivers/regulator/qcom_spmi-regulator.c | 203 +++++++++++++++++-
3 files changed, 219 insertions(+), 5 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/3] dt-bindings: qcom_spmi: Document PM8005 regulators 2019-05-21 16:49 [PATCH 0/3] PM8005 regulator support for msm8998 GPU Jeffrey Hugo @ 2019-05-21 16:52 ` Jeffrey Hugo 2019-05-21 17:26 ` Bjorn Andersson 2019-05-21 16:53 ` [PATCH 2/3] regulator: qcom_spmi: Add support for PM8005 Jeffrey Hugo 2019-05-21 16:53 ` [PATCH 3/3] arm64: dts: msm8998-mtp: Add pm8005_s1 regulator Jeffrey Hugo 2 siblings, 1 reply; 13+ messages in thread From: Jeffrey Hugo @ 2019-05-21 16:52 UTC (permalink / raw) To: lgirdwood, broonie, robh+dt, mark.rutland Cc: agross, david.brown, bjorn.andersson, jcrouse, 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> --- .../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 related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] dt-bindings: qcom_spmi: Document PM8005 regulators 2019-05-21 16:52 ` [PATCH 1/3] dt-bindings: qcom_spmi: Document PM8005 regulators Jeffrey Hugo @ 2019-05-21 17:26 ` Bjorn Andersson 0 siblings, 0 replies; 13+ messages in thread From: Bjorn Andersson @ 2019-05-21 17:26 UTC (permalink / raw) To: Jeffrey Hugo Cc: lgirdwood, broonie, robh+dt, mark.rutland, agross, david.brown, jcrouse, linux-arm-msm, linux-kernel, devicetree On Tue 21 May 09:52 PDT 2019, Jeffrey Hugo wrote: > Document the dt bindings for the PM8005 regulators which are usually used > for VDD of standalone blocks on a SoC like the GPU. > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com> > --- > .../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] 13+ messages in thread
* [PATCH 2/3] regulator: qcom_spmi: Add support for PM8005 2019-05-21 16:49 [PATCH 0/3] PM8005 regulator support for msm8998 GPU Jeffrey Hugo 2019-05-21 16:52 ` [PATCH 1/3] dt-bindings: qcom_spmi: Document PM8005 regulators Jeffrey Hugo @ 2019-05-21 16:53 ` Jeffrey Hugo 2019-05-21 18:50 ` Mark Brown 2019-05-21 19:09 ` Bjorn Andersson 2019-05-21 16:53 ` [PATCH 3/3] arm64: dts: msm8998-mtp: Add pm8005_s1 regulator Jeffrey Hugo 2 siblings, 2 replies; 13+ messages in thread From: Jeffrey Hugo @ 2019-05-21 16:53 UTC (permalink / raw) To: lgirdwood, broonie Cc: agross, david.brown, bjorn.andersson, jcrouse, 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 | 203 +++++++++++++++++++++++- 1 file changed, 198 insertions(+), 5 deletions(-) diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c index 53a61fb65642..e4b89fbaf426 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_FTSMPS2, }; 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,20 @@ enum spmi_common_regulator_registers { SPMI_COMMON_REG_STEP_CTRL = 0x61, }; +/* + * Second common register layout used by newer devices + * Note that some of the registers from the first common layout remain + * unchanged and their definition is not duplicated. + */ +enum qpnp_common2_regulator_registers { + SPMI_COMMON2_REG_VOLTAGE_LSB = 0x40, + SPMI_COMMON2_REG_VOLTAGE_MSB = 0x41, + SPMI_COMMON2_REG_MODE = 0x45, + SPMI_COMMON2_REG_STEP_CTRL = 0x61, + SPMI_COMMON2_REG_VOLTAGE_ULS_LSB = 0x68, + SPMI_COMMON2_REG_VOLTAGE_ULS_MSB = 0x69, +}; + enum spmi_vs_registers { SPMI_VS_REG_OCP = 0x4a, SPMI_VS_REG_SOFT_START = 0x4c, @@ -229,6 +245,15 @@ enum spmi_common_control_register_index { #define SPMI_COMMON_MODE_FOLLOW_HW_EN0_MASK 0x01 #define SPMI_COMMON_MODE_FOLLOW_ALL_MASK 0x1f +/* Second common regulator mode register values */ +#define SPMI_COMMON2_MODE_BYPASS_MASK 3 +#define SPMI_COMMON2_MODE_RETENTION_MASK 4 +#define SPMI_COMMON2_MODE_LPM_MASK 5 +#define SPMI_COMMON2_MODE_AUTO_MASK 6 +#define SPMI_COMMON2_MODE_HPM_MASK 7 + +#define SPMI_COMMON2_MODE_MASK 0x07 + /* Common regulator pull down control register layout */ #define SPMI_COMMON_PULL_DOWN_ENABLE_MASK 0x80 @@ -274,6 +299,23 @@ enum spmi_common_control_register_index { #define SPMI_FTSMPS_STEP_MARGIN_NUM 4 #define SPMI_FTSMPS_STEP_MARGIN_DEN 5 +#define SPMI_FTSMPS2_STEP_CTRL_DELAY_MASK 0x03 +#define SPMI_FTSMPS2_STEP_CTRL_DELAY_SHIFT 0 + +/* Clock rate in kHz of the FTSMPS2 regulator reference clock. */ +#define SPMI_FTSMPS2_CLOCK_RATE 4800 + +/* Minimum voltage stepper delay for each step. */ +#define SPMI_FTSMPS2_STEP_DELAY 2 + +/* + * The ratio SPMI_FTSMPS2_STEP_MARGIN_NUM/SPMI_FTSMPS2_STEP_MARGIN_DEN is used + * to adjust the step rate in order to account for oscillator variance. + */ +#define SPMI_FTSMPS2_STEP_MARGIN_NUM 10 +#define SPMI_FTSMPS2_STEP_MARGIN_DEN 11 + + /* VSET value to decide the range of ULT SMPS */ #define ULT_SMPS_RANGE_SPLIT 0x60 @@ -447,6 +489,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 +526,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); @@ -647,17 +694,31 @@ static int spmi_hw_selector_to_sw(struct spmi_regulator *vreg, u8 hw_sel, static const struct spmi_voltage_range * spmi_regulator_find_range(struct spmi_regulator *vreg) { - u8 range_sel; + int uV; + u8 range_sel, lsb, msb; const struct spmi_voltage_range *range, *end; range = vreg->set_points->range; end = range + vreg->set_points->count; - spmi_vreg_read(vreg, SPMI_COMMON_REG_VOLTAGE_RANGE, &range_sel, 1); + /* second common devices don't have VOLTAGE_RANGE register */ + if (vreg->logical_type == SPMI_REGULATOR_LOGICAL_TYPE_FTSMPS2) { + spmi_vreg_read(vreg, SPMI_COMMON2_REG_VOLTAGE_LSB, &lsb, 1); + spmi_vreg_read(vreg, SPMI_COMMON2_REG_VOLTAGE_MSB, &msb, 1); + + uV = (((int)msb << 8) | (int)lsb) * 1000; - for (; range < end; range++) - if (range->range_sel == range_sel) - return range; + for (; range < end; range++) + if (range->min_uV <= uV && range->max_uV >= uV) + return range; + } else { + spmi_vreg_read(vreg, SPMI_COMMON_REG_VOLTAGE_RANGE, &range_sel, + 1); + + for (; range < end; range++) + if (range->range_sel == range_sel) + return range; + } return NULL; } @@ -747,6 +808,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_common2_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) & 0xff; + return spmi_vreg_write(vreg, SPMI_COMMON2_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 +856,17 @@ 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_common2_get_voltage(struct regulator_dev *rdev) +{ + struct spmi_regulator *vreg = rdev_get_drvdata(rdev); + u8 lsb, msb; + + spmi_vreg_read(vreg, SPMI_COMMON2_REG_VOLTAGE_LSB, &lsb, 1); + spmi_vreg_read(vreg, SPMI_COMMON2_REG_VOLTAGE_MSB, &msb, 1); + + return (((int)msb << 8) | (int)lsb) * 1000; +} + static int spmi_regulator_single_map_voltage(struct regulator_dev *rdev, int min_uV, int max_uV) { @@ -920,6 +1009,22 @@ static unsigned int spmi_regulator_common_get_mode(struct regulator_dev *rdev) return REGULATOR_MODE_IDLE; } +static unsigned int spmi_regulator_common2_get_mode(struct regulator_dev *rdev) +{ + struct spmi_regulator *vreg = rdev_get_drvdata(rdev); + u8 reg; + + spmi_vreg_read(vreg, SPMI_COMMON2_REG_MODE, ®, 1); + + if (reg == SPMI_COMMON2_MODE_HPM_MASK) + return REGULATOR_MODE_NORMAL; + + if (reg == SPMI_COMMON2_MODE_AUTO_MASK) + return REGULATOR_MODE_FAST; + + return REGULATOR_MODE_IDLE; +} + static int spmi_regulator_common_set_mode(struct regulator_dev *rdev, unsigned int mode) { @@ -935,6 +1040,21 @@ 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_common2_set_mode(struct regulator_dev *rdev, unsigned int mode) +{ + struct spmi_regulator *vreg = rdev_get_drvdata(rdev); + u8 mask = SPMI_COMMON2_MODE_MASK; + u8 val = SPMI_COMMON2_MODE_LPM_MASK; + + if (mode == REGULATOR_MODE_NORMAL) + val = SPMI_COMMON2_MODE_HPM_MASK; + else if (mode == REGULATOR_MODE_FAST) + val = SPMI_COMMON2_MODE_AUTO_MASK; + + return spmi_vreg_update_bits(vreg, SPMI_COMMON2_REG_MODE, val, mask); +} + static int spmi_regulator_common_set_load(struct regulator_dev *rdev, int load_uA) { @@ -1264,6 +1384,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_common2_set_voltage, + .set_voltage_time_sel = spmi_regulator_set_voltage_time_sel, + .get_voltage_sel = spmi_regulator_common2_get_voltage, + .map_voltage = spmi_regulator_common_map_voltage, + .list_voltage = spmi_regulator_common_list_voltage, + .set_mode = spmi_regulator_common2_set_mode, + .get_mode = spmi_regulator_common2_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 @@ -1299,6 +1434,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, FTSMPS2, 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), @@ -1436,6 +1572,48 @@ static int spmi_regulator_init_slew_rate(struct spmi_regulator *vreg) return ret; } +/* slew rate init for common register layout #2 */ +static int spmi_regulator_init_slew_rate2(struct spmi_regulator *vreg) +{ + int ret, i; + u8 reg = 0; + int delay, slew_rate; + const struct spmi_voltage_range *range = NULL; + + ret = spmi_vreg_read(vreg, SPMI_COMMON2_REG_STEP_CTRL, ®, 1); + if (ret) { + dev_err(vreg->dev, "spmi read failed, ret=%d\n", ret); + return ret; + } + + /* + * Regulators using the common #2 register layout do not have a voltage + * range select register. Choose the lowest possible step size to be + * conservative in the slew rate calculation. + */ + for (i = 0; i < vreg->set_points->count; i++) + if (!range || vreg->set_points->range[i].step_uV < + range->step_uV) + range = &vreg->set_points->range[i]; + + if (!range) + return -EINVAL; + + delay = reg & SPMI_FTSMPS2_STEP_CTRL_DELAY_MASK; + delay >>= SPMI_FTSMPS2_STEP_CTRL_DELAY_SHIFT; + + /* slew_rate has units of uV/us */ + slew_rate = SPMI_FTSMPS2_CLOCK_RATE * range->step_uV; + slew_rate /= 1000 * (SPMI_FTSMPS2_STEP_DELAY << delay); + slew_rate *= SPMI_FTSMPS2_STEP_MARGIN_NUM; + slew_rate /= SPMI_FTSMPS2_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) { @@ -1575,6 +1753,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_FTSMPS2: + ret = spmi_regulator_init_slew_rate2(vreg); + if (ret) + return ret; + break; default: break; } @@ -1731,7 +1915,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 related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] regulator: qcom_spmi: Add support for PM8005 2019-05-21 16:53 ` [PATCH 2/3] regulator: qcom_spmi: Add support for PM8005 Jeffrey Hugo @ 2019-05-21 18:50 ` Mark Brown 2019-05-21 23:16 ` Jeffrey Hugo 2019-05-21 19:09 ` Bjorn Andersson 1 sibling, 1 reply; 13+ messages in thread From: Mark Brown @ 2019-05-21 18:50 UTC (permalink / raw) To: Jeffrey Hugo Cc: lgirdwood, agross, david.brown, bjorn.andersson, jcrouse, robh+dt, mark.rutland, linux-arm-msm, linux-kernel, devicetree, Jorge Ramirez-Ortiz [-- Attachment #1: Type: text/plain, Size: 2023 bytes --] On Tue, May 21, 2019 at 09:53:15AM -0700, Jeffrey Hugo wrote: > - spmi_vreg_read(vreg, SPMI_COMMON_REG_VOLTAGE_RANGE, &range_sel, 1); > + /* second common devices don't have VOLTAGE_RANGE register */ > + if (vreg->logical_type == SPMI_REGULATOR_LOGICAL_TYPE_FTSMPS2) { > + spmi_vreg_read(vreg, SPMI_COMMON2_REG_VOLTAGE_LSB, &lsb, 1); > + spmi_vreg_read(vreg, SPMI_COMMON2_REG_VOLTAGE_MSB, &msb, 1); > + > + uV = (((int)msb << 8) | (int)lsb) * 1000; This overlaps with some changes that Jorge (CCed) was sending for the PMS405. As I was saying to him rather than shoving special cases for different regulator types into the ops (especially ones that don't have any of the range stuff) it'd be better to just define separate ops for the regulators that look quite different to the existing ones. > +static int spmi_regulator_common_list_voltage(struct regulator_dev *rdev, > + unsigned selector); > + > +static int spmi_regulator_common2_set_voltage(struct regulator_dev *rdev, > + unsigned selector) Eeew, can we not have better names? > +static unsigned int spmi_regulator_common2_get_mode(struct regulator_dev *rdev) > +{ > + struct spmi_regulator *vreg = rdev_get_drvdata(rdev); > + u8 reg; > + > + spmi_vreg_read(vreg, SPMI_COMMON2_REG_MODE, ®, 1); > + > + if (reg == SPMI_COMMON2_MODE_HPM_MASK) > + return REGULATOR_MODE_NORMAL; > + > + if (reg == SPMI_COMMON2_MODE_AUTO_MASK) > + return REGULATOR_MODE_FAST; > + > + return REGULATOR_MODE_IDLE; > +} This looks like you want to write a switch statement. > +spmi_regulator_common2_set_mode(struct regulator_dev *rdev, unsigned int mode) > +{ > + struct spmi_regulator *vreg = rdev_get_drvdata(rdev); > + u8 mask = SPMI_COMMON2_MODE_MASK; > + u8 val = SPMI_COMMON2_MODE_LPM_MASK; > + > + if (mode == REGULATOR_MODE_NORMAL) > + val = SPMI_COMMON2_MODE_HPM_MASK; > + else if (mode == REGULATOR_MODE_FAST) > + val = SPMI_COMMON2_MODE_AUTO_MASK; This needs to be a switch statement, then it can have a default case to catch errors too. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] regulator: qcom_spmi: Add support for PM8005 2019-05-21 18:50 ` Mark Brown @ 2019-05-21 23:16 ` Jeffrey Hugo 2019-05-22 11:01 ` Mark Brown 0 siblings, 1 reply; 13+ messages in thread From: Jeffrey Hugo @ 2019-05-21 23:16 UTC (permalink / raw) To: Mark Brown, Jeffrey Hugo Cc: lgirdwood, agross, david.brown, bjorn.andersson, jcrouse, robh+dt, mark.rutland, linux-arm-msm, linux-kernel, devicetree, Jorge Ramirez-Ortiz On 5/21/2019 12:50 PM, Mark Brown wrote: > On Tue, May 21, 2019 at 09:53:15AM -0700, Jeffrey Hugo wrote: > >> - spmi_vreg_read(vreg, SPMI_COMMON_REG_VOLTAGE_RANGE, &range_sel, 1); >> + /* second common devices don't have VOLTAGE_RANGE register */ >> + if (vreg->logical_type == SPMI_REGULATOR_LOGICAL_TYPE_FTSMPS2) { >> + spmi_vreg_read(vreg, SPMI_COMMON2_REG_VOLTAGE_LSB, &lsb, 1); >> + spmi_vreg_read(vreg, SPMI_COMMON2_REG_VOLTAGE_MSB, &msb, 1); >> + >> + uV = (((int)msb << 8) | (int)lsb) * 1000; > > This overlaps with some changes that Jorge (CCed) was sending for the > PMS405. As I was saying to him rather than shoving special cases for > different regulator types into the ops (especially ones that don't have > any of the range stuff) it'd be better to just define separate ops for > the regulators that look quite different to the existing ones. Sorry, I hadn't paid attention to that discussion. Reviewing it now. > >> +static int spmi_regulator_common_list_voltage(struct regulator_dev *rdev, >> + unsigned selector); >> + >> +static int spmi_regulator_common2_set_voltage(struct regulator_dev *rdev, >> + unsigned selector) > > Eeew, can we not have better names? I'm open to suggestions. Apparently there are two register common register schemes - the old one and the new one. PMIC designs after some random point in time are all the new register scheme per the documentation I see. As far as I an aware, the FT426 design is the first design to be added to this driver to make use of the new scheme, but I expect more to be supported in future, thus I'm reluctant to make these ft426 specific in the name. > >> +static unsigned int spmi_regulator_common2_get_mode(struct regulator_dev *rdev) >> +{ >> + struct spmi_regulator *vreg = rdev_get_drvdata(rdev); >> + u8 reg; >> + >> + spmi_vreg_read(vreg, SPMI_COMMON2_REG_MODE, ®, 1); >> + >> + if (reg == SPMI_COMMON2_MODE_HPM_MASK) >> + return REGULATOR_MODE_NORMAL; >> + >> + if (reg == SPMI_COMMON2_MODE_AUTO_MASK) >> + return REGULATOR_MODE_FAST; >> + >> + return REGULATOR_MODE_IDLE; >> +} > > This looks like you want to write a switch statement. It follows the existing style in the driver, but sure I can make this a switch. > >> +spmi_regulator_common2_set_mode(struct regulator_dev *rdev, unsigned int mode) >> +{ >> + struct spmi_regulator *vreg = rdev_get_drvdata(rdev); >> + u8 mask = SPMI_COMMON2_MODE_MASK; >> + u8 val = SPMI_COMMON2_MODE_LPM_MASK; >> + >> + if (mode == REGULATOR_MODE_NORMAL) >> + val = SPMI_COMMON2_MODE_HPM_MASK; >> + else if (mode == REGULATOR_MODE_FAST) >> + val = SPMI_COMMON2_MODE_AUTO_MASK; > > This needs to be a switch statement, then it can have a default case to > catch errors too. > -- Jeffrey Hugo Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] regulator: qcom_spmi: Add support for PM8005 2019-05-21 23:16 ` Jeffrey Hugo @ 2019-05-22 11:01 ` Mark Brown 2019-05-22 14:16 ` Jeffrey Hugo 0 siblings, 1 reply; 13+ messages in thread From: Mark Brown @ 2019-05-22 11:01 UTC (permalink / raw) To: Jeffrey Hugo Cc: Jeffrey Hugo, lgirdwood, agross, david.brown, bjorn.andersson, jcrouse, robh+dt, mark.rutland, linux-arm-msm, linux-kernel, devicetree, Jorge Ramirez-Ortiz [-- Attachment #1: Type: text/plain, Size: 1358 bytes --] On Tue, May 21, 2019 at 05:16:06PM -0600, Jeffrey Hugo wrote: > On 5/21/2019 12:50 PM, Mark Brown wrote: > > > +static int spmi_regulator_common_list_voltage(struct regulator_dev *rdev, > > > + unsigned selector); > > > + > > > +static int spmi_regulator_common2_set_voltage(struct regulator_dev *rdev, > > > + unsigned selector) > > Eeew, can we not have better names? > I'm open to suggestions. Apparently there are two register common register > schemes - the old one and the new one. PMIC designs after some random point > in time are all the new register scheme per the documentation I see. > As far as I an aware, the FT426 design is the first design to be added to > this driver to make use of the new scheme, but I expect more to be supported > in future, thus I'm reluctant to make these ft426 specific in the name. If there's a completely new register map why are these even in the same driver? > > > + if (reg == SPMI_COMMON2_MODE_HPM_MASK) > > > + return REGULATOR_MODE_NORMAL; > > > + > > > + if (reg == SPMI_COMMON2_MODE_AUTO_MASK) > > > + return REGULATOR_MODE_FAST; > > > + > > > + return REGULATOR_MODE_IDLE; > > > +} > > This looks like you want to write a switch statement. > It follows the existing style in the driver, but sure I can make this a > switch. Please fix the rest of the driver as well then. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] regulator: qcom_spmi: Add support for PM8005 2019-05-22 11:01 ` Mark Brown @ 2019-05-22 14:16 ` Jeffrey Hugo 2019-05-22 14:28 ` Mark Brown 0 siblings, 1 reply; 13+ messages in thread From: Jeffrey Hugo @ 2019-05-22 14:16 UTC (permalink / raw) To: Mark Brown Cc: Jeffrey Hugo, lgirdwood, agross, david.brown, bjorn.andersson, jcrouse, robh+dt, mark.rutland, linux-arm-msm, linux-kernel, devicetree, Jorge Ramirez-Ortiz On 5/22/2019 5:01 AM, Mark Brown wrote: > On Tue, May 21, 2019 at 05:16:06PM -0600, Jeffrey Hugo wrote: >> On 5/21/2019 12:50 PM, Mark Brown wrote: > >>>> +static int spmi_regulator_common_list_voltage(struct regulator_dev *rdev, >>>> + unsigned selector); >>>> + >>>> +static int spmi_regulator_common2_set_voltage(struct regulator_dev *rdev, >>>> + unsigned selector) > >>> Eeew, can we not have better names? > >> I'm open to suggestions. Apparently there are two register common register >> schemes - the old one and the new one. PMIC designs after some random point >> in time are all the new register scheme per the documentation I see. > >> As far as I an aware, the FT426 design is the first design to be added to >> this driver to make use of the new scheme, but I expect more to be supported >> in future, thus I'm reluctant to make these ft426 specific in the name. > > If there's a completely new register map why are these even in the same > driver? Its not completely new, its a derivative of the old scheme. Of the 104 registers, approximately 5 have been modified, therefore the new scheme is 95% compatible with the old one. Duplicating a 1883 line driver to handle a change in 5% of the register space seems less than ideal. Particularly considering your previous comments seem to indicate that you feel its pretty trivial to handle the quirks associated with the changes in this driver. > >>>> + if (reg == SPMI_COMMON2_MODE_HPM_MASK) >>>> + return REGULATOR_MODE_NORMAL; >>>> + >>>> + if (reg == SPMI_COMMON2_MODE_AUTO_MASK) >>>> + return REGULATOR_MODE_FAST; >>>> + >>>> + return REGULATOR_MODE_IDLE; >>>> +} > >>> This looks like you want to write a switch statement. > >> It follows the existing style in the driver, but sure I can make this a >> switch. > > Please fix the rest of the driver as well then. > -- Jeffrey Hugo Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] regulator: qcom_spmi: Add support for PM8005 2019-05-22 14:16 ` Jeffrey Hugo @ 2019-05-22 14:28 ` Mark Brown 2019-05-22 14:58 ` Jeffrey Hugo 0 siblings, 1 reply; 13+ messages in thread From: Mark Brown @ 2019-05-22 14:28 UTC (permalink / raw) To: Jeffrey Hugo Cc: Jeffrey Hugo, lgirdwood, agross, david.brown, bjorn.andersson, jcrouse, robh+dt, mark.rutland, linux-arm-msm, linux-kernel, devicetree, Jorge Ramirez-Ortiz [-- Attachment #1: Type: text/plain, Size: 1529 bytes --] On Wed, May 22, 2019 at 08:16:38AM -0600, Jeffrey Hugo wrote: > On 5/22/2019 5:01 AM, Mark Brown wrote: > > On Tue, May 21, 2019 at 05:16:06PM -0600, Jeffrey Hugo wrote: > > > I'm open to suggestions. Apparently there are two register common register > > > schemes - the old one and the new one. PMIC designs after some random point > > > in time are all the new register scheme per the documentation I see. > > > As far as I an aware, the FT426 design is the first design to be added to > > > this driver to make use of the new scheme, but I expect more to be supported > > > in future, thus I'm reluctant to make these ft426 specific in the name. > > If there's a completely new register map why are these even in the same > > driver? > Its not completely new, its a derivative of the old scheme. Of the 104 > registers, approximately 5 have been modified, therefore the new scheme is > 95% compatible with the old one. Duplicating a 1883 line driver to handle a > change in 5% of the register space seems less than ideal. Particularly > considering your previous comments seem to indicate that you feel its pretty > trivial to handle the quirks associated with the changes in this driver. Ah, so it's not a completely new scheme but rather just a couple of registers that have changed. Sharing the driver is fine then. Ideally there would be some documentation from the vendor about this, a mention of IP revisions or some such. If not what the DT bindings do for names is use the first chip things were found in. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] regulator: qcom_spmi: Add support for PM8005 2019-05-22 14:28 ` Mark Brown @ 2019-05-22 14:58 ` Jeffrey Hugo 0 siblings, 0 replies; 13+ messages in thread From: Jeffrey Hugo @ 2019-05-22 14:58 UTC (permalink / raw) To: Mark Brown Cc: Jeffrey Hugo, lgirdwood, agross, david.brown, bjorn.andersson, jcrouse, robh+dt, mark.rutland, linux-arm-msm, linux-kernel, devicetree, Jorge Ramirez-Ortiz On 5/22/2019 8:28 AM, Mark Brown wrote: > On Wed, May 22, 2019 at 08:16:38AM -0600, Jeffrey Hugo wrote: >> On 5/22/2019 5:01 AM, Mark Brown wrote: >>> On Tue, May 21, 2019 at 05:16:06PM -0600, Jeffrey Hugo wrote: > >>>> I'm open to suggestions. Apparently there are two register common register >>>> schemes - the old one and the new one. PMIC designs after some random point >>>> in time are all the new register scheme per the documentation I see. > >>>> As far as I an aware, the FT426 design is the first design to be added to >>>> this driver to make use of the new scheme, but I expect more to be supported >>>> in future, thus I'm reluctant to make these ft426 specific in the name. > >>> If there's a completely new register map why are these even in the same >>> driver? > >> Its not completely new, its a derivative of the old scheme. Of the 104 >> registers, approximately 5 have been modified, therefore the new scheme is >> 95% compatible with the old one. Duplicating a 1883 line driver to handle a >> change in 5% of the register space seems less than ideal. Particularly >> considering your previous comments seem to indicate that you feel its pretty >> trivial to handle the quirks associated with the changes in this driver. > > Ah, so it's not a completely new scheme but rather just a couple of > registers that have changed. Sharing the driver is fine then. Ideally > there would be some documentation from the vendor about this, a mention > of IP revisions or some such. If not what the DT bindings do for names > is use the first chip things were found in. > The documentation isn't great. There isn't really an IP revision this started with. Looking at all of the PMIC designs, this actually started with PM8005, so I guess "common2" becomes "pm8005", and the PMS405 should reuse that. I'll coordinate with Jorge. -- Jeffrey Hugo Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] regulator: qcom_spmi: Add support for PM8005 2019-05-21 16:53 ` [PATCH 2/3] regulator: qcom_spmi: Add support for PM8005 Jeffrey Hugo 2019-05-21 18:50 ` Mark Brown @ 2019-05-21 19:09 ` Bjorn Andersson 1 sibling, 0 replies; 13+ messages in thread From: Bjorn Andersson @ 2019-05-21 19:09 UTC (permalink / raw) To: Jeffrey Hugo Cc: lgirdwood, broonie, agross, david.brown, jcrouse, robh+dt, mark.rutland, linux-arm-msm, linux-kernel, devicetree On Tue 21 May 09:53 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. > > Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com> > --- > drivers/regulator/qcom_spmi-regulator.c | 203 +++++++++++++++++++++++- > 1 file changed, 198 insertions(+), 5 deletions(-) > > diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c > index 53a61fb65642..e4b89fbaf426 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_FTSMPS2, > }; > > 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,20 @@ enum spmi_common_regulator_registers { > SPMI_COMMON_REG_STEP_CTRL = 0x61, > }; > > +/* > + * Second common register layout used by newer devices > + * Note that some of the registers from the first common layout remain > + * unchanged and their definition is not duplicated. > + */ > +enum qpnp_common2_regulator_registers { > + SPMI_COMMON2_REG_VOLTAGE_LSB = 0x40, > + SPMI_COMMON2_REG_VOLTAGE_MSB = 0x41, > + SPMI_COMMON2_REG_MODE = 0x45, > + SPMI_COMMON2_REG_STEP_CTRL = 0x61, > + SPMI_COMMON2_REG_VOLTAGE_ULS_LSB = 0x68, > + SPMI_COMMON2_REG_VOLTAGE_ULS_MSB = 0x69, > +}; > + > enum spmi_vs_registers { > SPMI_VS_REG_OCP = 0x4a, > SPMI_VS_REG_SOFT_START = 0x4c, > @@ -229,6 +245,15 @@ enum spmi_common_control_register_index { > #define SPMI_COMMON_MODE_FOLLOW_HW_EN0_MASK 0x01 > #define SPMI_COMMON_MODE_FOLLOW_ALL_MASK 0x1f > > +/* Second common regulator mode register values */ > +#define SPMI_COMMON2_MODE_BYPASS_MASK 3 > +#define SPMI_COMMON2_MODE_RETENTION_MASK 4 > +#define SPMI_COMMON2_MODE_LPM_MASK 5 > +#define SPMI_COMMON2_MODE_AUTO_MASK 6 > +#define SPMI_COMMON2_MODE_HPM_MASK 7 > + > +#define SPMI_COMMON2_MODE_MASK 0x07 > + > /* Common regulator pull down control register layout */ > #define SPMI_COMMON_PULL_DOWN_ENABLE_MASK 0x80 > > @@ -274,6 +299,23 @@ enum spmi_common_control_register_index { > #define SPMI_FTSMPS_STEP_MARGIN_NUM 4 > #define SPMI_FTSMPS_STEP_MARGIN_DEN 5 > > +#define SPMI_FTSMPS2_STEP_CTRL_DELAY_MASK 0x03 > +#define SPMI_FTSMPS2_STEP_CTRL_DELAY_SHIFT 0 > + > +/* Clock rate in kHz of the FTSMPS2 regulator reference clock. */ > +#define SPMI_FTSMPS2_CLOCK_RATE 4800 > + > +/* Minimum voltage stepper delay for each step. */ > +#define SPMI_FTSMPS2_STEP_DELAY 2 > + > +/* > + * The ratio SPMI_FTSMPS2_STEP_MARGIN_NUM/SPMI_FTSMPS2_STEP_MARGIN_DEN is used > + * to adjust the step rate in order to account for oscillator variance. > + */ > +#define SPMI_FTSMPS2_STEP_MARGIN_NUM 10 > +#define SPMI_FTSMPS2_STEP_MARGIN_DEN 11 > + > + > /* VSET value to decide the range of ULT SMPS */ > #define ULT_SMPS_RANGE_SPLIT 0x60 > > @@ -447,6 +489,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 +526,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); > @@ -647,17 +694,31 @@ static int spmi_hw_selector_to_sw(struct spmi_regulator *vreg, u8 hw_sel, > static const struct spmi_voltage_range * > spmi_regulator_find_range(struct spmi_regulator *vreg) > { > - u8 range_sel; > + int uV; > + u8 range_sel, lsb, msb; > const struct spmi_voltage_range *range, *end; > > range = vreg->set_points->range; > end = range + vreg->set_points->count; > > - spmi_vreg_read(vreg, SPMI_COMMON_REG_VOLTAGE_RANGE, &range_sel, 1); > + /* second common devices don't have VOLTAGE_RANGE register */ > + if (vreg->logical_type == SPMI_REGULATOR_LOGICAL_TYPE_FTSMPS2) { > + spmi_vreg_read(vreg, SPMI_COMMON2_REG_VOLTAGE_LSB, &lsb, 1); > + spmi_vreg_read(vreg, SPMI_COMMON2_REG_VOLTAGE_MSB, &msb, 1); > + > + uV = (((int)msb << 8) | (int)lsb) * 1000; > > - for (; range < end; range++) > - if (range->range_sel == range_sel) > - return range; > + for (; range < end; range++) > + if (range->min_uV <= uV && range->max_uV >= uV) > + return range; > + } else { > + spmi_vreg_read(vreg, SPMI_COMMON_REG_VOLTAGE_RANGE, &range_sel, > + 1); > + > + for (; range < end; range++) > + if (range->range_sel == range_sel) > + return range; > + } While I think the patch looks good it suffers from the same design issue that Mark objected to in: https://lore.kernel.org/lkml/1548675904-18324-3-git-send-email-jorge.ramirez-ortiz@linaro.org/ I think we need a better strategy, as we now at least have to support the three cases of: * range selector + voltage selector * multi range voltage selector * single range voltage selector Regards, Bjorn > > return NULL; > } > @@ -747,6 +808,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_common2_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) & 0xff; > + return spmi_vreg_write(vreg, SPMI_COMMON2_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 +856,17 @@ 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_common2_get_voltage(struct regulator_dev *rdev) > +{ > + struct spmi_regulator *vreg = rdev_get_drvdata(rdev); > + u8 lsb, msb; > + > + spmi_vreg_read(vreg, SPMI_COMMON2_REG_VOLTAGE_LSB, &lsb, 1); > + spmi_vreg_read(vreg, SPMI_COMMON2_REG_VOLTAGE_MSB, &msb, 1); > + > + return (((int)msb << 8) | (int)lsb) * 1000; > +} > + > static int spmi_regulator_single_map_voltage(struct regulator_dev *rdev, > int min_uV, int max_uV) > { > @@ -920,6 +1009,22 @@ static unsigned int spmi_regulator_common_get_mode(struct regulator_dev *rdev) > return REGULATOR_MODE_IDLE; > } > > +static unsigned int spmi_regulator_common2_get_mode(struct regulator_dev *rdev) > +{ > + struct spmi_regulator *vreg = rdev_get_drvdata(rdev); > + u8 reg; > + > + spmi_vreg_read(vreg, SPMI_COMMON2_REG_MODE, ®, 1); > + > + if (reg == SPMI_COMMON2_MODE_HPM_MASK) > + return REGULATOR_MODE_NORMAL; > + > + if (reg == SPMI_COMMON2_MODE_AUTO_MASK) > + return REGULATOR_MODE_FAST; > + > + return REGULATOR_MODE_IDLE; > +} > + > static int > spmi_regulator_common_set_mode(struct regulator_dev *rdev, unsigned int mode) > { > @@ -935,6 +1040,21 @@ 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_common2_set_mode(struct regulator_dev *rdev, unsigned int mode) > +{ > + struct spmi_regulator *vreg = rdev_get_drvdata(rdev); > + u8 mask = SPMI_COMMON2_MODE_MASK; > + u8 val = SPMI_COMMON2_MODE_LPM_MASK; > + > + if (mode == REGULATOR_MODE_NORMAL) > + val = SPMI_COMMON2_MODE_HPM_MASK; > + else if (mode == REGULATOR_MODE_FAST) > + val = SPMI_COMMON2_MODE_AUTO_MASK; > + > + return spmi_vreg_update_bits(vreg, SPMI_COMMON2_REG_MODE, val, mask); > +} > + > static int > spmi_regulator_common_set_load(struct regulator_dev *rdev, int load_uA) > { > @@ -1264,6 +1384,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_common2_set_voltage, > + .set_voltage_time_sel = spmi_regulator_set_voltage_time_sel, > + .get_voltage_sel = spmi_regulator_common2_get_voltage, > + .map_voltage = spmi_regulator_common_map_voltage, > + .list_voltage = spmi_regulator_common_list_voltage, > + .set_mode = spmi_regulator_common2_set_mode, > + .get_mode = spmi_regulator_common2_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 > > @@ -1299,6 +1434,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, FTSMPS2, 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), > @@ -1436,6 +1572,48 @@ static int spmi_regulator_init_slew_rate(struct spmi_regulator *vreg) > return ret; > } > > +/* slew rate init for common register layout #2 */ > +static int spmi_regulator_init_slew_rate2(struct spmi_regulator *vreg) > +{ > + int ret, i; > + u8 reg = 0; > + int delay, slew_rate; > + const struct spmi_voltage_range *range = NULL; > + > + ret = spmi_vreg_read(vreg, SPMI_COMMON2_REG_STEP_CTRL, ®, 1); > + if (ret) { > + dev_err(vreg->dev, "spmi read failed, ret=%d\n", ret); > + return ret; > + } > + > + /* > + * Regulators using the common #2 register layout do not have a voltage > + * range select register. Choose the lowest possible step size to be > + * conservative in the slew rate calculation. > + */ > + for (i = 0; i < vreg->set_points->count; i++) > + if (!range || vreg->set_points->range[i].step_uV < > + range->step_uV) > + range = &vreg->set_points->range[i]; > + > + if (!range) > + return -EINVAL; > + > + delay = reg & SPMI_FTSMPS2_STEP_CTRL_DELAY_MASK; > + delay >>= SPMI_FTSMPS2_STEP_CTRL_DELAY_SHIFT; > + > + /* slew_rate has units of uV/us */ > + slew_rate = SPMI_FTSMPS2_CLOCK_RATE * range->step_uV; > + slew_rate /= 1000 * (SPMI_FTSMPS2_STEP_DELAY << delay); > + slew_rate *= SPMI_FTSMPS2_STEP_MARGIN_NUM; > + slew_rate /= SPMI_FTSMPS2_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) > { > @@ -1575,6 +1753,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_FTSMPS2: > + ret = spmi_regulator_init_slew_rate2(vreg); > + if (ret) > + return ret; > + break; > default: > break; > } > @@ -1731,7 +1915,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] 13+ messages in thread
* [PATCH 3/3] arm64: dts: msm8998-mtp: Add pm8005_s1 regulator 2019-05-21 16:49 [PATCH 0/3] PM8005 regulator support for msm8998 GPU Jeffrey Hugo 2019-05-21 16:52 ` [PATCH 1/3] dt-bindings: qcom_spmi: Document PM8005 regulators Jeffrey Hugo 2019-05-21 16:53 ` [PATCH 2/3] regulator: qcom_spmi: Add support for PM8005 Jeffrey Hugo @ 2019-05-21 16:53 ` Jeffrey Hugo 2019-05-21 18:16 ` Bjorn Andersson 2 siblings, 1 reply; 13+ messages in thread From: Jeffrey Hugo @ 2019-05-21 16:53 UTC (permalink / raw) To: agross, david.brown, bjorn.andersson Cc: jcrouse, 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> --- 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 related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] arm64: dts: msm8998-mtp: Add pm8005_s1 regulator 2019-05-21 16:53 ` [PATCH 3/3] arm64: dts: msm8998-mtp: Add pm8005_s1 regulator Jeffrey Hugo @ 2019-05-21 18:16 ` Bjorn Andersson 0 siblings, 0 replies; 13+ messages in thread From: Bjorn Andersson @ 2019-05-21 18:16 UTC (permalink / raw) To: Jeffrey Hugo Cc: agross, david.brown, jcrouse, lgirdwood, broonie, robh+dt, mark.rutland, linux-arm-msm, linux-kernel, devicetree On Tue 21 May 09:53 PDT 2019, Jeffrey Hugo wrote: > 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. > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com> > --- > 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] 13+ messages in thread
end of thread, other threads:[~2019-05-22 14:58 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-21 16:49 [PATCH 0/3] PM8005 regulator support for msm8998 GPU Jeffrey Hugo 2019-05-21 16:52 ` [PATCH 1/3] dt-bindings: qcom_spmi: Document PM8005 regulators Jeffrey Hugo 2019-05-21 17:26 ` Bjorn Andersson 2019-05-21 16:53 ` [PATCH 2/3] regulator: qcom_spmi: Add support for PM8005 Jeffrey Hugo 2019-05-21 18:50 ` Mark Brown 2019-05-21 23:16 ` Jeffrey Hugo 2019-05-22 11:01 ` Mark Brown 2019-05-22 14:16 ` Jeffrey Hugo 2019-05-22 14:28 ` Mark Brown 2019-05-22 14:58 ` Jeffrey Hugo 2019-05-21 19:09 ` Bjorn Andersson 2019-05-21 16:53 ` [PATCH 3/3] arm64: dts: msm8998-mtp: Add pm8005_s1 regulator Jeffrey Hugo 2019-05-21 18:16 ` Bjorn Andersson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).