* [PATCH v4 0/7] PM8005 and PMS405 regulator support @ 2019-06-13 21:24 Jeffrey Hugo 2019-06-13 21:25 ` [PATCH v4 1/7] regulator: qcom_spmi: enable linear range info Jeffrey Hugo ` (4 more replies) 0 siblings, 5 replies; 22+ messages in thread From: Jeffrey Hugo @ 2019-06-13 21:24 UTC (permalink / raw) 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. v4: -fix the linear range change to use the correct implementation -mask out the non-mode bits when reading the hardware reg -correct the pms405 supply pins listing -correct the pms405 s3 supply name in the match struct -correct subject names to be more aligned with the subsystem history 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 | 22 ++ arch/arm64/boot/dts/qcom/msm8998-mtp.dtsi | 17 ++ drivers/regulator/qcom_spmi-regulator.c | 237 +++++++++++++++++- 3 files changed, 269 insertions(+), 7 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 1/7] regulator: qcom_spmi: enable linear range info 2019-06-13 21:24 [PATCH v4 0/7] PM8005 and PMS405 regulator support Jeffrey Hugo @ 2019-06-13 21:25 ` Jeffrey Hugo 2019-06-13 21:25 ` [PATCH v4 2/7] regulator: qcom_spmi: Refactor get_mode/set_mode Jeffrey Hugo 2019-06-17 15:24 ` Applied "regulator: qcom_spmi: enable linear range info" " Mark Brown 2019-06-13 21:25 ` [PATCH v4 3/7] dt-bindings: qcom_spmi: Document PM8005 regulators Jeffrey Hugo ` (3 subsequent siblings) 4 siblings, 2 replies; 22+ messages in thread From: Jeffrey Hugo @ 2019-06-13 21:25 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..42c429d50743 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->set_points->count == 1) { + /* since there is only one range */ + range = vreg->set_points->range; + vreg->desc.uV_step = range->step_uV; + } + config.dev = dev; config.driver_data = vreg; config.regmap = regmap; -- 2.17.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v4 2/7] regulator: qcom_spmi: Refactor get_mode/set_mode 2019-06-13 21:25 ` [PATCH v4 1/7] regulator: qcom_spmi: enable linear range info Jeffrey Hugo @ 2019-06-13 21:25 ` Jeffrey Hugo 2019-06-13 21:32 ` Bjorn Andersson 2019-06-17 15:24 ` Applied "regulator: qcom_spmi: Refactor get_mode/set_mode" to the regulator tree Mark Brown 2019-06-17 15:24 ` Applied "regulator: qcom_spmi: enable linear range info" " Mark Brown 1 sibling, 2 replies; 22+ messages in thread From: Jeffrey Hugo @ 2019-06-13 21:25 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 | 26 +++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c index 42c429d50743..1b3383a24c9d 100644 --- a/drivers/regulator/qcom_spmi-regulator.c +++ b/drivers/regulator/qcom_spmi-regulator.c @@ -911,13 +911,16 @@ static unsigned int spmi_regulator_common_get_mode(struct regulator_dev *rdev) spmi_vreg_read(vreg, SPMI_COMMON_REG_MODE, ®, 1); - if (reg & SPMI_COMMON_MODE_HPM_MASK) - return REGULATOR_MODE_NORMAL; + reg &= SPMI_COMMON_MODE_HPM_MASK | SPMI_COMMON_MODE_AUTO_MASK; - if (reg & SPMI_COMMON_MODE_AUTO_MASK) + switch (reg) { + case SPMI_COMMON_MODE_HPM_MASK: + return REGULATOR_MODE_NORMAL; + case SPMI_COMMON_MODE_AUTO_MASK: return REGULATOR_MODE_FAST; - - return REGULATOR_MODE_IDLE; + default: + return REGULATOR_MODE_IDLE; + } } static int @@ -925,12 +928,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); } -- 2.17.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/7] regulator: qcom_spmi: Refactor get_mode/set_mode 2019-06-13 21:25 ` [PATCH v4 2/7] regulator: qcom_spmi: Refactor get_mode/set_mode Jeffrey Hugo @ 2019-06-13 21:32 ` Bjorn Andersson 2019-06-17 15:24 ` Applied "regulator: qcom_spmi: Refactor get_mode/set_mode" to the regulator tree Mark Brown 1 sibling, 0 replies; 22+ messages in thread From: Bjorn Andersson @ 2019-06-13 21:32 UTC (permalink / raw) To: Jeffrey Hugo Cc: lgirdwood, broonie, agross, robh+dt, mark.rutland, linux-arm-msm, linux-kernel, devicetree On Thu 13 Jun 14:25 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. > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com> > --- > drivers/regulator/qcom_spmi-regulator.c | 26 +++++++++++++++++-------- > 1 file changed, 18 insertions(+), 8 deletions(-) > > diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c > index 42c429d50743..1b3383a24c9d 100644 > --- a/drivers/regulator/qcom_spmi-regulator.c > +++ b/drivers/regulator/qcom_spmi-regulator.c > @@ -911,13 +911,16 @@ static unsigned int spmi_regulator_common_get_mode(struct regulator_dev *rdev) > > spmi_vreg_read(vreg, SPMI_COMMON_REG_MODE, ®, 1); > > - if (reg & SPMI_COMMON_MODE_HPM_MASK) > - return REGULATOR_MODE_NORMAL; > + reg &= SPMI_COMMON_MODE_HPM_MASK | SPMI_COMMON_MODE_AUTO_MASK; > > - if (reg & SPMI_COMMON_MODE_AUTO_MASK) > + switch (reg) { > + case SPMI_COMMON_MODE_HPM_MASK: > + return REGULATOR_MODE_NORMAL; > + case SPMI_COMMON_MODE_AUTO_MASK: > return REGULATOR_MODE_FAST; > - > - return REGULATOR_MODE_IDLE; > + default: > + return REGULATOR_MODE_IDLE; > + } > } > > static int > @@ -925,12 +928,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); > } > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Applied "regulator: qcom_spmi: Refactor get_mode/set_mode" to the regulator tree 2019-06-13 21:25 ` [PATCH v4 2/7] regulator: qcom_spmi: Refactor get_mode/set_mode Jeffrey Hugo 2019-06-13 21:32 ` Bjorn Andersson @ 2019-06-17 15:24 ` Mark Brown 1 sibling, 0 replies; 22+ messages in thread From: Mark Brown @ 2019-06-17 15:24 UTC (permalink / raw) To: Jeffrey Hugo Cc: agross, bjorn.andersson, broonie, devicetree, lgirdwood, linux-arm-msm, linux-kernel, Mark Brown, mark.rutland, robh+dt The patch regulator: qcom_spmi: Refactor get_mode/set_mode has been applied to the regulator tree at https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-5.3 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark From ba576a6232dc06605f4edfaeea9b526ba7724f84 Mon Sep 17 00:00:00 2001 From: Jeffrey Hugo <jeffrey.l.hugo@gmail.com> Date: Thu, 13 Jun 2019 14:25:31 -0700 Subject: [PATCH] regulator: qcom_spmi: Refactor get_mode/set_mode 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> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> Signed-off-by: Mark Brown <broonie@kernel.org> --- drivers/regulator/qcom_spmi-regulator.c | 26 +++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c index 42c429d50743..1b3383a24c9d 100644 --- a/drivers/regulator/qcom_spmi-regulator.c +++ b/drivers/regulator/qcom_spmi-regulator.c @@ -911,13 +911,16 @@ static unsigned int spmi_regulator_common_get_mode(struct regulator_dev *rdev) spmi_vreg_read(vreg, SPMI_COMMON_REG_MODE, ®, 1); - if (reg & SPMI_COMMON_MODE_HPM_MASK) - return REGULATOR_MODE_NORMAL; + reg &= SPMI_COMMON_MODE_HPM_MASK | SPMI_COMMON_MODE_AUTO_MASK; - if (reg & SPMI_COMMON_MODE_AUTO_MASK) + switch (reg) { + case SPMI_COMMON_MODE_HPM_MASK: + return REGULATOR_MODE_NORMAL; + case SPMI_COMMON_MODE_AUTO_MASK: return REGULATOR_MODE_FAST; - - return REGULATOR_MODE_IDLE; + default: + return REGULATOR_MODE_IDLE; + } } static int @@ -925,12 +928,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); } -- 2.20.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Applied "regulator: qcom_spmi: enable linear range info" to the regulator tree 2019-06-13 21:25 ` [PATCH v4 1/7] regulator: qcom_spmi: enable linear range info Jeffrey Hugo 2019-06-13 21:25 ` [PATCH v4 2/7] regulator: qcom_spmi: Refactor get_mode/set_mode Jeffrey Hugo @ 2019-06-17 15:24 ` Mark Brown 1 sibling, 0 replies; 22+ messages in thread From: Mark Brown @ 2019-06-17 15:24 UTC (permalink / raw) To: Jorge Ramirez-Ortiz Cc: agross, bjorn.andersson, broonie, devicetree, Jeffrey Hugo, lgirdwood, linux-arm-msm, linux-kernel, Mark Brown, mark.rutland, robh+dt The patch regulator: qcom_spmi: enable linear range info has been applied to the regulator tree at https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-5.3 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark From 86f4ff7a0c0cd8e391ffa4dd0edb82ae0eedcc9e Mon Sep 17 00:00:00 2001 From: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org> Date: Thu, 13 Jun 2019 14:25:30 -0700 Subject: [PATCH] regulator: qcom_spmi: enable linear range info Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com> Signed-off-by: Mark Brown <broonie@kernel.org> --- 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..42c429d50743 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->set_points->count == 1) { + /* since there is only one range */ + range = vreg->set_points->range; + vreg->desc.uV_step = range->step_uV; + } + config.dev = dev; config.driver_data = vreg; config.regmap = regmap; -- 2.20.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v4 3/7] dt-bindings: qcom_spmi: Document PM8005 regulators 2019-06-13 21:24 [PATCH v4 0/7] PM8005 and PMS405 regulator support Jeffrey Hugo 2019-06-13 21:25 ` [PATCH v4 1/7] regulator: qcom_spmi: enable linear range info Jeffrey Hugo @ 2019-06-13 21:25 ` Jeffrey Hugo 2019-06-13 21:25 ` [PATCH v4 4/7] regulator: qcom_spmi: Add support for PM8005 Jeffrey Hugo 2019-06-13 21:26 ` [PATCH v4 5/7] arm64: dts: msm8998-mtp: Add pm8005_s1 regulator Jeffrey Hugo ` (2 subsequent siblings) 4 siblings, 1 reply; 22+ messages in thread From: Jeffrey Hugo @ 2019-06-13 21:25 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 related [flat|nested] 22+ messages in thread
* [PATCH v4 4/7] regulator: qcom_spmi: Add support for PM8005 2019-06-13 21:25 ` [PATCH v4 3/7] dt-bindings: qcom_spmi: Document PM8005 regulators Jeffrey Hugo @ 2019-06-13 21:25 ` Jeffrey Hugo 2019-06-17 15:05 ` Mark Brown 0 siblings, 1 reply; 22+ messages in thread From: Jeffrey Hugo @ 2019-06-13 21:25 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> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> --- 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 1b3383a24c9d..c8791b036c53 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) { @@ -923,6 +994,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, ®, 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) { @@ -945,6 +1033,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) { @@ -1274,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_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 @@ -1309,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, 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), @@ -1446,6 +1572,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, ®, 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) { @@ -1585,6 +1739,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; } @@ -1741,7 +1901,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] 22+ messages in thread
* Re: [PATCH v4 4/7] regulator: qcom_spmi: Add support for PM8005 2019-06-13 21:25 ` [PATCH v4 4/7] regulator: qcom_spmi: Add support for PM8005 Jeffrey Hugo @ 2019-06-17 15:05 ` Mark Brown 2019-06-17 15:17 ` Jeffrey Hugo 0 siblings, 1 reply; 22+ messages in thread From: Mark Brown @ 2019-06-17 15:05 UTC (permalink / raw) To: Jeffrey Hugo Cc: lgirdwood, agross, bjorn.andersson, robh+dt, mark.rutland, linux-arm-msm, linux-kernel, devicetree [-- Attachment #1: Type: text/plain, Size: 1496 bytes --] On Thu, Jun 13, 2019 at 02:25:53PM -0700, Jeffrey Hugo wrote: > +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); > +} This could just be a set_voltage_sel(), no need for it to be a set_voltage() operation.... > +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; > +} ...or if the conversion is this trivial why do the list_voltage() lookup above? > +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; > + } This should validate, it shouldn't just translate invalid values into valid ones. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 4/7] regulator: qcom_spmi: Add support for PM8005 2019-06-17 15:05 ` Mark Brown @ 2019-06-17 15:17 ` Jeffrey Hugo 2019-06-17 16:03 ` Mark Brown 0 siblings, 1 reply; 22+ messages in thread From: Jeffrey Hugo @ 2019-06-17 15:17 UTC (permalink / raw) To: Mark Brown Cc: lgirdwood, Andy Gross, Bjorn Andersson, Rob Herring, Mark Rutland, MSM, lkml, devicetree On Mon, Jun 17, 2019 at 9:05 AM Mark Brown <broonie@kernel.org> wrote: > > On Thu, Jun 13, 2019 at 02:25:53PM -0700, Jeffrey Hugo wrote: > > > +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); > > +} > > This could just be a set_voltage_sel(), no need for it to be a > set_voltage() operation.... This is a set_voltage_sel() in spmi_ftsmps426_ops. Is the issue because this function is "spmi_regulator_ftsmps426_set_voltage" and not "spmi_regulator_ftsmps426_set_voltage_sel"? > > > +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; > > +} > > ...or if the conversion is this trivial why do the list_voltage() lookup > above? We already have code in the driver to convert a selector to the voltage. Why duplicate that inline in spmi_regulator_ftsmps426_set_voltage? > > > +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; > > + } > > This should validate, it shouldn't just translate invalid values into > valid ones. Validate what? The other defines are REGULATOR_MODE_IDLE and REGULATOR_MODE_STANDBY which correspond to the LPM mode. Or are you suggesting that regulator framework is going to pass REGULATOR_MODE_INVALID to this operation? ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 4/7] regulator: qcom_spmi: Add support for PM8005 2019-06-17 15:17 ` Jeffrey Hugo @ 2019-06-17 16:03 ` Mark Brown 2019-06-17 17:07 ` Jeffrey Hugo 0 siblings, 1 reply; 22+ messages in thread From: Mark Brown @ 2019-06-17 16:03 UTC (permalink / raw) To: Jeffrey Hugo Cc: lgirdwood, Andy Gross, Bjorn Andersson, Rob Herring, Mark Rutland, MSM, lkml, devicetree [-- Attachment #1: Type: text/plain, Size: 2177 bytes --] On Mon, Jun 17, 2019 at 09:17:21AM -0600, Jeffrey Hugo wrote: > On Mon, Jun 17, 2019 at 9:05 AM Mark Brown <broonie@kernel.org> wrote: > > > +static int spmi_regulator_ftsmps426_set_voltage(struct regulator_dev *rdev, > > > + unsigned selector) > > > +{ > > > + mV = spmi_regulator_common_list_voltage(rdev, selector) / 1000; > > This could just be a set_voltage_sel(), no need for it to be a > > set_voltage() operation.... > This is a set_voltage_sel() in spmi_ftsmps426_ops. Is the issue because this > function is "spmi_regulator_ftsmps426_set_voltage" and not > "spmi_regulator_ftsmps426_set_voltage_sel"? Well, that's certainly confusing naming and there's some confusion in the code about what a selector is - it's supposed to be a raw register value so if you're having to convert it into a voltage something is going wrong. Just implement a set_voltage() operation? > We already have code in the driver to convert a selector to the > voltage. Why duplicate > that inline in spmi_regulator_ftsmps426_set_voltage? Either work with selectors or work with voltages, don't mix and match the two. > > > + 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; > > > + } > > This should validate, it shouldn't just translate invalid values into > > valid ones. > Validate what? The other defines are REGULATOR_MODE_IDLE > and REGULATOR_MODE_STANDBY which correspond to the LPM > mode. Or are you suggesting that regulator framework is going to pass > REGULATOR_MODE_INVALID to this operation? You should be validating that the argument passed in is one that the driver understands, your assumption will break if we add any new modes and in any case there should be a 1:1 mapping between hardware and API modes so you shouldn't be translating two different API modes into the same hardware mode. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 4/7] regulator: qcom_spmi: Add support for PM8005 2019-06-17 16:03 ` Mark Brown @ 2019-06-17 17:07 ` Jeffrey Hugo 2019-06-17 18:37 ` Mark Brown 0 siblings, 1 reply; 22+ messages in thread From: Jeffrey Hugo @ 2019-06-17 17:07 UTC (permalink / raw) To: Mark Brown Cc: lgirdwood, Andy Gross, Bjorn Andersson, Rob Herring, Mark Rutland, MSM, lkml, devicetree On Mon, Jun 17, 2019 at 10:04 AM Mark Brown <broonie@kernel.org> wrote: > > On Mon, Jun 17, 2019 at 09:17:21AM -0600, Jeffrey Hugo wrote: > > On Mon, Jun 17, 2019 at 9:05 AM Mark Brown <broonie@kernel.org> wrote: > > > > > +static int spmi_regulator_ftsmps426_set_voltage(struct regulator_dev *rdev, > > > > + unsigned selector) > > > > +{ > > > > > + mV = spmi_regulator_common_list_voltage(rdev, selector) / 1000; > > > > This could just be a set_voltage_sel(), no need for it to be a > > > set_voltage() operation.... > > > This is a set_voltage_sel() in spmi_ftsmps426_ops. Is the issue because this > > function is "spmi_regulator_ftsmps426_set_voltage" and not > > "spmi_regulator_ftsmps426_set_voltage_sel"? > > Well, that's certainly confusing naming and there's some confusion in > the code about what a selector is - it's supposed to be a raw register > value so if you're having to convert it into a voltage something is > going wrong. Just implement a set_voltage() operation? No. Is what a selector is documented anywhere? I just looked again and I haven't found documentation explaining that a selector is the raw register value. Now I understand why this driver has the hardware to software selector translation. The selector being the raw register value seems to be a very limited assumption that I don't see working for more than very basic implementations. We've already figured out a virtualized selector mapping, I don't want to reimplement the complicated math to correctly map a requested voltage range to what the regulator can provide, and possibly get it wrong, or at the very least have two duplicate implementations. The naming is consistent with the rest of the driver, and the name seems long enough already. Lets just keep this. > > > We already have code in the driver to convert a selector to the > > voltage. Why duplicate > > that inline in spmi_regulator_ftsmps426_set_voltage? > > Either work with selectors or work with voltages, don't mix and match > the two. Fine. I'll fix up the get() to return the selector, and not the voltage since that works better with everything else that is implemented. Again, it would be nice if the documentation for regulator_ops indicated that a driver should only implement the voltage operations or the selector operations, not mix and match if that is your expectation. > > > > > + 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; > > > > + } > > > > This should validate, it shouldn't just translate invalid values into > > > valid ones. > > > Validate what? The other defines are REGULATOR_MODE_IDLE > > and REGULATOR_MODE_STANDBY which correspond to the LPM > > mode. Or are you suggesting that regulator framework is going to pass > > REGULATOR_MODE_INVALID to this operation? > > You should be validating that the argument passed in is one that the > driver understands, your assumption will break if we add any new modes > and in any case there should be a 1:1 mapping between hardware and API > modes so you shouldn't be translating two different API modes into the > same hardware mode. Fine. I'll fix this per what you've stated. Again, would be nice if the documentation for the API modes clearly indicated they should match to one specific HW setting incases where the HW doesn't match the API 1:1. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 4/7] regulator: qcom_spmi: Add support for PM8005 2019-06-17 17:07 ` Jeffrey Hugo @ 2019-06-17 18:37 ` Mark Brown [not found] ` <CAOCk7NpbZwAreGpVCvF2yFBDJKbAxBZ23oncfF_SyEwoiC2+PQ@mail.gmail.com> 0 siblings, 1 reply; 22+ messages in thread From: Mark Brown @ 2019-06-17 18:37 UTC (permalink / raw) To: Jeffrey Hugo Cc: lgirdwood, Andy Gross, Bjorn Andersson, Rob Herring, Mark Rutland, MSM, lkml, devicetree [-- Attachment #1: Type: text/plain, Size: 3551 bytes --] On Mon, Jun 17, 2019 at 11:07:18AM -0600, Jeffrey Hugo wrote: > On Mon, Jun 17, 2019 at 10:04 AM Mark Brown <broonie@kernel.org> wrote: > > On Mon, Jun 17, 2019 at 09:17:21AM -0600, Jeffrey Hugo wrote: Something really weird is going on with the word wrapping in your mail, it looks like you're writing lines longer than 80 characters (120?) and they're getting a newline added in the middle of the line to wrap them without reflowing the paragraph. > > > This is a set_voltage_sel() in spmi_ftsmps426_ops. Is the issue because this > > > function is "spmi_regulator_ftsmps426_set_voltage" and not > > > "spmi_regulator_ftsmps426_set_voltage_sel"? > > Well, that's certainly confusing naming and there's some confusion in > > the code about what a selector is - it's supposed to be a raw register > > value so if you're having to convert it into a voltage something is > > going wrong. Just implement a set_voltage() operation? > No. > Is what a selector is documented anywhere? I just looked again and I > haven't found > documentation explaining that a selector is the raw register value. Well, it doesn't *have* to be the raw register value, more accurately it's some token which is useful for passing to and from the hardware; The documentation such as it is is in the documentation of the list_voltage() operation (which is what defines the selector values for a given driver). > Now I understand why this driver has the hardware to software selector > translation. > The selector being the raw register value seems to be a very limited > assumption that > I don't see working for more than very basic implementations. Your idea of very basic implementations is how the overwhelming majority of hardware is implemented. Regulators with register maps will tend to just have a bitfield where you set the voltage with each valid value in that bitfield mapped to a single voltage, the exceptions tend to use direct voltage values instead (and not support enumeration at all). Looking at the driver I think it's got what the helpers are terming pickable linear ranges (naming is hard) and could use those helpers. > We've already figured out a virtualized selector mapping, I don't want > to reimplement > the complicated math to correctly map a requested voltage range to > what the regulator > can provide, and possibly get it wrong, or at the very least have two duplicate > implementations. I don't know what a "virtualized selector mapping" is... We do have an extensive range of helper implementations which cover the majority of cases, are you sure that none of these cover what the hardware is doing? > The naming is consistent with the rest of the driver, and the name > seems long enough > already. Lets just keep this. I appreciate that the existing code is a bit of a mess but as you can see it's making it difficult to maintain so I'd rather push towards making it cleaner, it looks like we're getting more and more devices added to the driver so it's actually an issue. > Again, it would be nice if the documentation for regulator_ops > indicated that a driver > should only implement the voltage operations or the selector > operations, not mix and > match if that is your expectation. Please add whatever documentation you're looking for here. I genuinely don't know where people would look for that and TBH I'm having a hard time figuring out why you'd implement the operations asymmetrically. It's not an issue that ever comes up. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <CAOCk7NpbZwAreGpVCvF2yFBDJKbAxBZ23oncfF_SyEwoiC2+PQ@mail.gmail.com>]
[parent not found: <20190617192413.GI5316@sirena.org.uk>]
* Re: [PATCH v4 4/7] regulator: qcom_spmi: Add support for PM8005 [not found] ` <20190617192413.GI5316@sirena.org.uk> @ 2019-06-17 19:41 ` Jeffrey Hugo 0 siblings, 0 replies; 22+ messages in thread From: Jeffrey Hugo @ 2019-06-17 19:41 UTC (permalink / raw) To: Mark Brown Cc: lgirdwood, Andy Gross, Bjorn Andersson, Rob Herring, Mark Rutland, MSM, lkml, devicetree On Mon, Jun 17, 2019 at 1:24 PM Mark Brown <broonie@kernel.org> wrote: > > On Mon, Jun 17, 2019 at 01:06:42PM -0600, Jeffrey Hugo wrote: > > On Mon, Jun 17, 2019 at 12:37 PM Mark Brown <broonie@kernel.org> wrote: > > > > Something really weird is going on with the word wrapping in your mail, > > > it looks like you're writing lines longer than 80 characters (120?) and > > > they're getting a newline added in the middle of the line to wrap them > > > without reflowing the paragraph. > > > Doh. Hopefully this one is better. > > Yes, thanks! Though now it's off-list :/ Doh. Added everyone back. > > > > Well, it doesn't *have* to be the raw register value, more accurately > > > it's some token which is useful for passing to and from the hardware; > > > The documentation such as it > > > is is in the documentation of the list_voltage() operation (which is > > > what defines the selector values for a given driver). > > > Ok, so this one bit - > > Selectors range from zero to one less than regulator_desc.n_voltages. > > Voltages may be reported in any order. > > Yes. > > > So, I understand that. Its an indexing of the supported voltages. > > From my perspective, that has nothing to do with hardware. Its just a > > remapping of the values to a different set. Voltages X, Y, and Z map > > to 0, 1, and 2. Its a token so that the driver and the framework can > > use a common value. > > > I really think we are on the same page here, its just I was getting > > confused by how you were describing it in your replies. > > Yes, I was just coming from the perspective that for almost all hardware > the selectors are chosen to be the values that are in the bitfield that > the hardware uses to specify the voltage since that's the most common > thing and tends to make things simpler for people, it's also the primary > place where the concept came from. > > > > Your idea of very basic implementations is how the overwhelming majority > > > of hardware is implemented. Regulators with register maps will tend to > > > just have a bitfield where you set the voltage with each valid value in > > > that bitfield mapped to a single voltage, the exceptions tend to use > > > direct voltage values instead (and not support enumeration at all). > > > > Looking at the driver I think it's got what the helpers are terming > > > pickable linear ranges (naming is hard) and could use those helpers. > > > I'm pretty sure Stephen Boyd and Bjorn just investigated that a few > > weeks ago, and came to the conclusion that it can't because of things > > like the hardware really wants to stay in the same range if at all > > possible, which is not behavior the pickable linear ranges seems to > > support. > > Sounds like it just needs a custom map_voltage() function? Though > thinking about it it's possibly worth just making the standard map try > to keep things in the same range as that will if nothing else reduce the > number of I/O operations. Probably also reduce glitches if there's > overlapping ranges. I didn't think so when I was paying attention to their discussion. However maybe I missed something. I think I'll take another look. Might make for a nice cleanup, but if its just in the driver, or if it involves updating the framework, it seems to be outside scope for this series of changes. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 5/7] arm64: dts: msm8998-mtp: Add pm8005_s1 regulator 2019-06-13 21:24 [PATCH v4 0/7] PM8005 and PMS405 regulator support Jeffrey Hugo 2019-06-13 21:25 ` [PATCH v4 1/7] regulator: qcom_spmi: enable linear range info Jeffrey Hugo 2019-06-13 21:25 ` [PATCH v4 3/7] dt-bindings: qcom_spmi: Document PM8005 regulators Jeffrey Hugo @ 2019-06-13 21:26 ` Jeffrey Hugo 2019-06-13 21:27 ` [PATCH v4 6/7] dt-bindings: qcom_spmi: Document pms405 support Jeffrey Hugo 2019-06-17 14:58 ` [PATCH v4 0/7] PM8005 and PMS405 regulator support Mark Brown 4 siblings, 0 replies; 22+ messages in thread From: Jeffrey Hugo @ 2019-06-13 21:26 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 related [flat|nested] 22+ messages in thread
* [PATCH v4 6/7] dt-bindings: qcom_spmi: Document pms405 support 2019-06-13 21:24 [PATCH v4 0/7] PM8005 and PMS405 regulator support Jeffrey Hugo ` (2 preceding siblings ...) 2019-06-13 21:26 ` [PATCH v4 5/7] arm64: dts: msm8998-mtp: Add pm8005_s1 regulator Jeffrey Hugo @ 2019-06-13 21:27 ` Jeffrey Hugo 2019-06-13 21:27 ` [PATCH v4 7/7] regulator: qcom_spmi: add PMS405 SPMI regulator Jeffrey Hugo 2019-06-13 21:37 ` [PATCH v4 6/7] dt-bindings: qcom_spmi: Document pms405 support Bjorn Andersson 2019-06-17 14:58 ` [PATCH v4 0/7] PM8005 and PMS405 regulator support Mark Brown 4 siblings, 2 replies; 22+ messages in thread From: Jeffrey Hugo @ 2019-06-13 21:27 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> --- .../bindings/regulator/qcom,spmi-regulator.txt | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt b/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt index ba94bc2d407a..430b8622bda1 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,23 @@ Qualcomm SPMI Regulators Definition: Reference to regulator supplying the input pin, as described in the data sheet. +- 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 + 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 related [flat|nested] 22+ messages in thread
* [PATCH v4 7/7] regulator: qcom_spmi: add PMS405 SPMI regulator 2019-06-13 21:27 ` [PATCH v4 6/7] dt-bindings: qcom_spmi: Document pms405 support Jeffrey Hugo @ 2019-06-13 21:27 ` Jeffrey Hugo 2019-06-13 21:37 ` Bjorn Andersson 2019-06-13 21:37 ` [PATCH v4 6/7] dt-bindings: qcom_spmi: Document pms405 support Bjorn Andersson 1 sibling, 1 reply; 22+ messages in thread From: Jeffrey Hugo @ 2019-06-13 21:27 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 | 43 +++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c index c8791b036c53..debe4dc74d27 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) @@ -1399,12 +1408,26 @@ static struct regulator_ops spmi_ftsmps426_ops = { .set_pull_down = spmi_regulator_common_set_pull_down, }; +static struct regulator_ops spmi_hfs430_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, +}; + /* 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), @@ -1572,7 +1595,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; @@ -1589,7 +1613,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; @@ -1741,7 +1765,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; @@ -1909,6 +1940,11 @@ static const struct spmi_regulator_data pm8005_regulators[] = { { } }; +static const struct spmi_regulator_data pms405_regulators[] = { + { "s3", 0x1a00, "vdd_s3"}, + { } +}; + static const struct of_device_id qcom_spmi_regulator_match[] = { { .compatible = "qcom,pm8005-regulators", .data = &pm8005_regulators }, { .compatible = "qcom,pm8841-regulators", .data = &pm8841_regulators }, @@ -1916,6 +1952,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 related [flat|nested] 22+ messages in thread
* Re: [PATCH v4 7/7] regulator: qcom_spmi: add PMS405 SPMI regulator 2019-06-13 21:27 ` [PATCH v4 7/7] regulator: qcom_spmi: add PMS405 SPMI regulator Jeffrey Hugo @ 2019-06-13 21:37 ` Bjorn Andersson 0 siblings, 0 replies; 22+ messages in thread From: Bjorn Andersson @ 2019-06-13 21:37 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 14:27 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> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > drivers/regulator/qcom_spmi-regulator.c | 43 +++++++++++++++++++++++-- > 1 file changed, 40 insertions(+), 3 deletions(-) > > diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c > index c8791b036c53..debe4dc74d27 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) > @@ -1399,12 +1408,26 @@ static struct regulator_ops spmi_ftsmps426_ops = { > .set_pull_down = spmi_regulator_common_set_pull_down, > }; > > +static struct regulator_ops spmi_hfs430_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, > +}; > + > /* 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), > @@ -1572,7 +1595,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; > @@ -1589,7 +1613,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; > @@ -1741,7 +1765,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; > @@ -1909,6 +1940,11 @@ static const struct spmi_regulator_data pm8005_regulators[] = { > { } > }; > > +static const struct spmi_regulator_data pms405_regulators[] = { > + { "s3", 0x1a00, "vdd_s3"}, > + { } > +}; > + > static const struct of_device_id qcom_spmi_regulator_match[] = { > { .compatible = "qcom,pm8005-regulators", .data = &pm8005_regulators }, > { .compatible = "qcom,pm8841-regulators", .data = &pm8841_regulators }, > @@ -1916,6 +1952,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] 22+ messages in thread
* Re: [PATCH v4 6/7] dt-bindings: qcom_spmi: Document pms405 support 2019-06-13 21:27 ` [PATCH v4 6/7] dt-bindings: qcom_spmi: Document pms405 support Jeffrey Hugo 2019-06-13 21:27 ` [PATCH v4 7/7] regulator: qcom_spmi: add PMS405 SPMI regulator Jeffrey Hugo @ 2019-06-13 21:37 ` Bjorn Andersson 1 sibling, 0 replies; 22+ messages in thread From: Bjorn Andersson @ 2019-06-13 21:37 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 14:27 PDT 2019, Jeffrey Hugo wrote: > From: Jorge Ramirez <jorge.ramirez-ortiz@linaro.org> > > The PMS405 supports 5 SMPS and 13 LDO regulators. > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > 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> > --- > .../bindings/regulator/qcom,spmi-regulator.txt | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt b/Documentation/devicetree/bindings/regulator/qcom,spmi-regulator.txt > index ba94bc2d407a..430b8622bda1 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,23 @@ Qualcomm SPMI Regulators > Definition: Reference to regulator supplying the input pin, as > described in the data sheet. > > +- 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 > + 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] 22+ messages in thread
* Re: [PATCH v4 0/7] PM8005 and PMS405 regulator support 2019-06-13 21:24 [PATCH v4 0/7] PM8005 and PMS405 regulator support Jeffrey Hugo ` (3 preceding siblings ...) 2019-06-13 21:27 ` [PATCH v4 6/7] dt-bindings: qcom_spmi: Document pms405 support Jeffrey Hugo @ 2019-06-17 14:58 ` Mark Brown 2019-06-17 15:04 ` Jeffrey Hugo 4 siblings, 1 reply; 22+ messages in thread From: Mark Brown @ 2019-06-17 14:58 UTC (permalink / raw) To: Jeffrey Hugo Cc: agross, bjorn.andersson, lgirdwood, robh+dt, mark.rutland, linux-arm-msm, linux-kernel, devicetree [-- Attachment #1: Type: text/plain, Size: 535 bytes --] On Thu, Jun 13, 2019 at 02:24:36PM -0700, Jeffrey Hugo wrote: > 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. There's something really weird with the threading in how you posted these, a few of the patches are in reply to the prior patch so indented a level down. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 0/7] PM8005 and PMS405 regulator support 2019-06-17 14:58 ` [PATCH v4 0/7] PM8005 and PMS405 regulator support Mark Brown @ 2019-06-17 15:04 ` Jeffrey Hugo 2019-06-17 15:12 ` Mark Brown 0 siblings, 1 reply; 22+ messages in thread From: Jeffrey Hugo @ 2019-06-17 15:04 UTC (permalink / raw) To: Mark Brown Cc: Andy Gross, Bjorn Andersson, lgirdwood, Rob Herring, Mark Rutland, MSM, lkml, devicetree On Mon, Jun 17, 2019 at 8:58 AM Mark Brown <broonie@kernel.org> wrote: > > On Thu, Jun 13, 2019 at 02:24:36PM -0700, Jeffrey Hugo wrote: > > 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. > > There's something really weird with the threading in how you posted > these, a few of the patches are in reply to the prior patch so indented > a level down. Sorry about that. Bjorn pointed it out to me, and I think I figured out the glitch on my end. Are you ok to proceed in the review, or do you want a repost? ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 0/7] PM8005 and PMS405 regulator support 2019-06-17 15:04 ` Jeffrey Hugo @ 2019-06-17 15:12 ` Mark Brown 0 siblings, 0 replies; 22+ messages in thread From: Mark Brown @ 2019-06-17 15:12 UTC (permalink / raw) To: Jeffrey Hugo Cc: Andy Gross, Bjorn Andersson, lgirdwood, Rob Herring, Mark Rutland, MSM, lkml, devicetree [-- Attachment #1: Type: text/plain, Size: 174 bytes --] On Mon, Jun 17, 2019 at 09:04:23AM -0600, Jeffrey Hugo wrote: > Are you ok to proceed in the review, or do you want a repost? You should already have some review comments. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2019-06-17 19:41 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-13 21:24 [PATCH v4 0/7] PM8005 and PMS405 regulator support Jeffrey Hugo 2019-06-13 21:25 ` [PATCH v4 1/7] regulator: qcom_spmi: enable linear range info Jeffrey Hugo 2019-06-13 21:25 ` [PATCH v4 2/7] regulator: qcom_spmi: Refactor get_mode/set_mode Jeffrey Hugo 2019-06-13 21:32 ` Bjorn Andersson 2019-06-17 15:24 ` Applied "regulator: qcom_spmi: Refactor get_mode/set_mode" to the regulator tree Mark Brown 2019-06-17 15:24 ` Applied "regulator: qcom_spmi: enable linear range info" " Mark Brown 2019-06-13 21:25 ` [PATCH v4 3/7] dt-bindings: qcom_spmi: Document PM8005 regulators Jeffrey Hugo 2019-06-13 21:25 ` [PATCH v4 4/7] regulator: qcom_spmi: Add support for PM8005 Jeffrey Hugo 2019-06-17 15:05 ` Mark Brown 2019-06-17 15:17 ` Jeffrey Hugo 2019-06-17 16:03 ` Mark Brown 2019-06-17 17:07 ` Jeffrey Hugo 2019-06-17 18:37 ` Mark Brown [not found] ` <CAOCk7NpbZwAreGpVCvF2yFBDJKbAxBZ23oncfF_SyEwoiC2+PQ@mail.gmail.com> [not found] ` <20190617192413.GI5316@sirena.org.uk> 2019-06-17 19:41 ` Jeffrey Hugo 2019-06-13 21:26 ` [PATCH v4 5/7] arm64: dts: msm8998-mtp: Add pm8005_s1 regulator Jeffrey Hugo 2019-06-13 21:27 ` [PATCH v4 6/7] dt-bindings: qcom_spmi: Document pms405 support Jeffrey Hugo 2019-06-13 21:27 ` [PATCH v4 7/7] regulator: qcom_spmi: add PMS405 SPMI regulator Jeffrey Hugo 2019-06-13 21:37 ` Bjorn Andersson 2019-06-13 21:37 ` [PATCH v4 6/7] dt-bindings: qcom_spmi: Document pms405 support Bjorn Andersson 2019-06-17 14:58 ` [PATCH v4 0/7] PM8005 and PMS405 regulator support Mark Brown 2019-06-17 15:04 ` Jeffrey Hugo 2019-06-17 15:12 ` Mark Brown
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).