| Message ID | 1614155592-14060-1-git-send-email-skakit@codeaurora.org |
|---|---|
| Headers | show |
| Series | Add PM7325/PM8350C/PMR735A regulator support | expand |
On Wed, Feb 24, 2021 at 02:03:11PM +0530, satya priya wrote: > Use correct buck, that is, pmic5_hfsmps515 for S1C regulator > of PM8350C PMIC. Fixes like this and patch 3 should be at the start of the series, this ensures that they have no dependencies and can easily be sent to Linus as fixes.
On Wed, 24 Feb 2021 14:03:05 +0530, satya priya wrote: > This series is dependent on below series which adds DT files for SC7280 SoC > https://lore.kernel.org/patchwork/project/lkml/list/?series=484757 > > satya priya (7): > dt-bindings: regulator: Convert regulator bindings to YAML > dt-bindings: regulator: Add compatibles for PM7325/PMR735A > regulator: qcom-rpmh: Correct the pmic5_hfsmps515 buck > regulator: qcom-rpmh: Add pmic5_ftsmps520 buck > regulator: qcom-rpmh: Add PM7325/PMR735A regulator support > regulator: qcom-rpmh: Use correct buck for S1C regulator > arm64: dts: qcom: sc7280: Add RPMh regulators for sc7280-idp > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next Thanks! [3/7] regulator: qcom-rpmh: Correct the pmic5_hfsmps515 buck commit: 62861a478e06d87dbfbb0ed3684056ba19a9886e [6/7] regulator: qcom-rpmh: Use correct buck for S1C regulator commit: 8fb4acb880e9467adca913e51adf5c1f96fbbeb9 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
Hi, > + }; > + > + pm8350c-regulators { > + compatible = "qcom,pm8350c-rpmh-regulators"; > + qcom,pmic-id = "c"; > + vreg_s1c_2p2: smps1 { > + regulator-min-microvolt = <2190000>; The indentation on "compatible" and "qcom,pmic-id" is off. Konrad
On 24/02/2021 11:33, satya priya wrote: > Correct the REGULATOR_LINEAR_RANGE and n_voltges for > pmic5_hfsmps515 buck. > > Signed-off-by: satya priya <skakit@codeaurora.org> > --- > drivers/regulator/qcom-rpmh-regulator.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/regulator/qcom-rpmh-regulator.c b/drivers/regulator/qcom-rpmh-regulator.c > index 79a554f..36542c3 100644 > --- a/drivers/regulator/qcom-rpmh-regulator.c > +++ b/drivers/regulator/qcom-rpmh-regulator.c > @@ -726,8 +726,8 @@ static const struct rpmh_vreg_hw_data pmic5_ftsmps510 = { > static const struct rpmh_vreg_hw_data pmic5_hfsmps515 = { > .regulator_type = VRM, > .ops = &rpmh_regulator_vrm_ops, > - .voltage_range = REGULATOR_LINEAR_RANGE(2800000, 0, 4, 16000), > - .n_voltages = 5, > + .voltage_range = REGULATOR_LINEAR_RANGE(320000, 0, 235, 16000), > + .n_voltages = 236, I've checked the docs for pm8009, the chip which also uses hfsmps515 regulators. The pdf clearly states that the 'Output voltage operating range' is from 2.8 V to 2.85 V. So we'd probably need to define different versions of HFS515 regulator data (like I had to create for pm8009-1). > .pmic_mode_map = pmic_mode_map_pmic5_smps, > .of_map_mode = rpmh_regulator_pmic4_smps_of_map_mode, > }; >
On 2021-02-25 00:47, Konrad Dybcio wrote: > Hi, > > >> + }; >> + >> + pm8350c-regulators { >> + compatible = "qcom,pm8350c-rpmh-regulators"; >> + qcom,pmic-id = "c"; >> + vreg_s1c_2p2: smps1 { >> + regulator-min-microvolt = <2190000>; > > > The indentation on "compatible" and "qcom,pmic-id" is off. > Okay, will correct this. > > Konrad Thanks, Satya Priya
On 2021-02-24 22:01, Mark Brown wrote: > On Wed, Feb 24, 2021 at 02:03:11PM +0530, satya priya wrote: >> Use correct buck, that is, pmic5_hfsmps515 for S1C regulator >> of PM8350C PMIC. > > Fixes like this and patch 3 should be at the start of the series, this > ensures that they have no dependencies and can easily be sent to Linus > as fixes. Okay.
Hi, On 2021-02-25 16:39, Dmitry Baryshkov wrote: > On 24/02/2021 11:33, satya priya wrote: >> Correct the REGULATOR_LINEAR_RANGE and n_voltges for >> pmic5_hfsmps515 buck. >> >> Signed-off-by: satya priya <skakit@codeaurora.org> >> --- >> drivers/regulator/qcom-rpmh-regulator.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/regulator/qcom-rpmh-regulator.c >> b/drivers/regulator/qcom-rpmh-regulator.c >> index 79a554f..36542c3 100644 >> --- a/drivers/regulator/qcom-rpmh-regulator.c >> +++ b/drivers/regulator/qcom-rpmh-regulator.c >> @@ -726,8 +726,8 @@ static const struct rpmh_vreg_hw_data >> pmic5_ftsmps510 = { >> static const struct rpmh_vreg_hw_data pmic5_hfsmps515 = { >> .regulator_type = VRM, >> .ops = &rpmh_regulator_vrm_ops, >> - .voltage_range = REGULATOR_LINEAR_RANGE(2800000, 0, 4, 16000), >> - .n_voltages = 5, >> + .voltage_range = REGULATOR_LINEAR_RANGE(320000, 0, 235, 16000), >> + .n_voltages = 236, > > I've checked the docs for pm8009, the chip which also uses hfsmps515 > regulators. The pdf clearly states that the 'Output voltage operating > range' is from 2.8 V to 2.85 V. > > So we'd probably need to define different versions of HFS515 regulator > data (like I had to create for pm8009-1). > > The min-max voltages for S1C (PM8350c) regulator are 2190000-2210000uV for sc7280(kodiak), so we had to modify this buck to support this regulator. AFAIK, this struct defines the HW constraints of a regulator, but the platform specific min-max values can be controlled from DT files. So, can't we modify it like above instead of adding a new definition? the new min_uV value (32000) is anyway not exceeding the old value (2800000) right? please correct me if wrong. >> .pmic_mode_map = pmic_mode_map_pmic5_smps, >> .of_map_mode = rpmh_regulator_pmic4_smps_of_map_mode, >> }; >> Thanks, Satya Priya
On Fri, 26 Feb 2021 at 09:59, <skakit@codeaurora.org> wrote: > > Hi, > > On 2021-02-25 16:39, Dmitry Baryshkov wrote: > > On 24/02/2021 11:33, satya priya wrote: > >> Correct the REGULATOR_LINEAR_RANGE and n_voltges for > >> pmic5_hfsmps515 buck. > >> > >> Signed-off-by: satya priya <skakit@codeaurora.org> > >> --- > >> drivers/regulator/qcom-rpmh-regulator.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/regulator/qcom-rpmh-regulator.c > >> b/drivers/regulator/qcom-rpmh-regulator.c > >> index 79a554f..36542c3 100644 > >> --- a/drivers/regulator/qcom-rpmh-regulator.c > >> +++ b/drivers/regulator/qcom-rpmh-regulator.c > >> @@ -726,8 +726,8 @@ static const struct rpmh_vreg_hw_data > >> pmic5_ftsmps510 = { > >> static const struct rpmh_vreg_hw_data pmic5_hfsmps515 = { > >> .regulator_type = VRM, > >> .ops = &rpmh_regulator_vrm_ops, > >> - .voltage_range = REGULATOR_LINEAR_RANGE(2800000, 0, 4, 16000), > >> - .n_voltages = 5, > >> + .voltage_range = REGULATOR_LINEAR_RANGE(320000, 0, 235, 16000), > >> + .n_voltages = 236, > > > > I've checked the docs for pm8009, the chip which also uses hfsmps515 > > regulators. The pdf clearly states that the 'Output voltage operating > > range' is from 2.8 V to 2.85 V. > > > > So we'd probably need to define different versions of HFS515 regulator > > data (like I had to create for pm8009-1). > > > > > > The min-max voltages for S1C (PM8350c) regulator are 2190000-2210000uV > for sc7280(kodiak), so we had to modify this buck to support this > regulator. > > AFAIK, this struct defines the HW constraints of a regulator, but the > platform specific min-max values can be controlled from DT files. So, > can't we modify it like above instead of adding a new definition? the > new min_uV value (32000) is anyway not exceeding the old value (2800000) > right? please correct me if wrong. As far as I understand for other regulators we put 'output voltage limitations' from the docs into the regulator definition and further constrain it by the platform device tree. Please correct me if I'm wrong. For pm8009 the data from the datasheet matches the regulators defined in the source file. Unfortunately I don't have kodiak specs at hand.
On 2021-02-26 15:57, Dmitry Baryshkov wrote: > On Fri, 26 Feb 2021 at 09:59, <skakit@codeaurora.org> wrote: >> >> Hi, >> >> On 2021-02-25 16:39, Dmitry Baryshkov wrote: >> > On 24/02/2021 11:33, satya priya wrote: >> >> Correct the REGULATOR_LINEAR_RANGE and n_voltges for >> >> pmic5_hfsmps515 buck. >> >> >> >> Signed-off-by: satya priya <skakit@codeaurora.org> >> >> --- >> >> drivers/regulator/qcom-rpmh-regulator.c | 4 ++-- >> >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> >> >> diff --git a/drivers/regulator/qcom-rpmh-regulator.c >> >> b/drivers/regulator/qcom-rpmh-regulator.c >> >> index 79a554f..36542c3 100644 >> >> --- a/drivers/regulator/qcom-rpmh-regulator.c >> >> +++ b/drivers/regulator/qcom-rpmh-regulator.c >> >> @@ -726,8 +726,8 @@ static const struct rpmh_vreg_hw_data >> >> pmic5_ftsmps510 = { >> >> static const struct rpmh_vreg_hw_data pmic5_hfsmps515 = { >> >> .regulator_type = VRM, >> >> .ops = &rpmh_regulator_vrm_ops, >> >> - .voltage_range = REGULATOR_LINEAR_RANGE(2800000, 0, 4, 16000), >> >> - .n_voltages = 5, >> >> + .voltage_range = REGULATOR_LINEAR_RANGE(320000, 0, 235, 16000), >> >> + .n_voltages = 236, >> > >> > I've checked the docs for pm8009, the chip which also uses hfsmps515 >> > regulators. The pdf clearly states that the 'Output voltage operating >> > range' is from 2.8 V to 2.85 V. >> > >> > So we'd probably need to define different versions of HFS515 regulator >> > data (like I had to create for pm8009-1). >> > >> > >> >> The min-max voltages for S1C (PM8350c) regulator are 2190000-2210000uV >> for sc7280(kodiak), so we had to modify this buck to support this >> regulator. >> >> AFAIK, this struct defines the HW constraints of a regulator, but the >> platform specific min-max values can be controlled from DT files. So, >> can't we modify it like above instead of adding a new definition? the >> new min_uV value (32000) is anyway not exceeding the old value >> (2800000) >> right? please correct me if wrong. > > As far as I understand for other regulators we put 'output voltage > limitations' from the docs into the regulator definition and further > constrain it by the platform device tree. Please correct me if I'm > wrong. I see that for most of the regulators, these specifications are specific to regulator buck (like HFS515) but not chipset specific, we set the chipset specific(like pm8009/pm8350c) requirements from DT files. For example: pmic5_nldo regulator spec mentions LLIMIT= 0.32V and ULIMIT =1.304V with step 8mV .voltage_range = REGULATOR_LINEAR_RANGE(320000, 0, 123, 8000), max output voltage supported by this regulator is 123*8000 + 320000 = 1304000mV which is same as mentioned in the regulator spec. > For pm8009 the data from the datasheet matches the regulators defined > in the source file. Unfortunately I don't have kodiak specs at hand. From the HFS515 spec I got below info "HFS510 and lower max output voltage is limited to 2.04V max, and Yoda(pm8009) requirement was 2.4V for IOT PA and 2.85V for camera application. Hence, HFS515 added a new register and corresponding HW changes to support the higher voltage. Table 5‑24 shows the new FB_RANGE bit. When configured to 0 the buck works as earlier where Vout max = 2.04V in 8mV steps, but when configured to 1 the buck range doubles and can now support a Vout max = 4.08V in 16mV steps." As per above, the max output voltage supported by HFS515 buck is 4.08V (which is kodiak pm8350c pmic's requirement). So, we have modified the buck data to support pm8350c(palani) along with pm8009(yoda). Thanks, Satya Priya
Hello, On Mon, 1 Mar 2021 at 13:37, <skakit@codeaurora.org> wrote: > > On 2021-02-26 15:57, Dmitry Baryshkov wrote: > > On Fri, 26 Feb 2021 at 09:59, <skakit@codeaurora.org> wrote: > >> > >> Hi, > >> > >> On 2021-02-25 16:39, Dmitry Baryshkov wrote: > >> > On 24/02/2021 11:33, satya priya wrote: > >> >> Correct the REGULATOR_LINEAR_RANGE and n_voltges for > >> >> pmic5_hfsmps515 buck. > >> >> > >> >> Signed-off-by: satya priya <skakit@codeaurora.org> > >> >> --- > >> >> drivers/regulator/qcom-rpmh-regulator.c | 4 ++-- > >> >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> >> > >> >> diff --git a/drivers/regulator/qcom-rpmh-regulator.c > >> >> b/drivers/regulator/qcom-rpmh-regulator.c > >> >> index 79a554f..36542c3 100644 > >> >> --- a/drivers/regulator/qcom-rpmh-regulator.c > >> >> +++ b/drivers/regulator/qcom-rpmh-regulator.c > >> >> @@ -726,8 +726,8 @@ static const struct rpmh_vreg_hw_data > >> >> pmic5_ftsmps510 = { > >> >> static const struct rpmh_vreg_hw_data pmic5_hfsmps515 = { > >> >> .regulator_type = VRM, > >> >> .ops = &rpmh_regulator_vrm_ops, > >> >> - .voltage_range = REGULATOR_LINEAR_RANGE(2800000, 0, 4, 16000), > >> >> - .n_voltages = 5, > >> >> + .voltage_range = REGULATOR_LINEAR_RANGE(320000, 0, 235, 16000), > >> >> + .n_voltages = 236, > >> > > >> > I've checked the docs for pm8009, the chip which also uses hfsmps515 > >> > regulators. The pdf clearly states that the 'Output voltage operating > >> > range' is from 2.8 V to 2.85 V. > >> > > >> > So we'd probably need to define different versions of HFS515 regulator > >> > data (like I had to create for pm8009-1). > >> > > >> > > >> > >> The min-max voltages for S1C (PM8350c) regulator are 2190000-2210000uV > >> for sc7280(kodiak), so we had to modify this buck to support this > >> regulator. > >> > >> AFAIK, this struct defines the HW constraints of a regulator, but the > >> platform specific min-max values can be controlled from DT files. So, > >> can't we modify it like above instead of adding a new definition? the > >> new min_uV value (32000) is anyway not exceeding the old value > >> (2800000) > >> right? please correct me if wrong. > > > > As far as I understand for other regulators we put 'output voltage > > limitations' from the docs into the regulator definition and further > > constrain it by the platform device tree. Please correct me if I'm > > wrong. > > I see that for most of the regulators, these specifications are specific > to regulator buck (like HFS515) but not chipset specific, we set the > chipset specific(like pm8009/pm8350c) requirements from DT files. > > For example: > pmic5_nldo regulator spec mentions LLIMIT= 0.32V and ULIMIT =1.304V with > step 8mV > > .voltage_range = REGULATOR_LINEAR_RANGE(320000, 0, 123, 8000), > max output voltage supported by this regulator is 123*8000 + 320000 = > 1304000mV which is same as mentioned in the regulator spec. > > > For pm8009 the data from the datasheet matches the regulators defined > > in the source file. Unfortunately I don't have kodiak specs at hand. > > From the HFS515 spec I got below info > "HFS510 and lower max output voltage is limited to 2.04V max, and > Yoda(pm8009) requirement was 2.4V for IOT PA and 2.85V for camera > application. Hence, HFS515 added a new register and corresponding HW > changes to support the higher voltage. Table 5‑24 shows the new > FB_RANGE bit. When configured to 0 the buck works as earlier where Vout > max = 2.04V in 8mV steps, but when configured to 1 the buck range > doubles and can now support a Vout max = 4.08V in 16mV steps." > > As per above, the max output voltage supported by HFS515 buck is 4.08V > (which is kodiak pm8350c pmic's requirement). > So, we have modified the buck data to support pm8350c(palani) along with > pm8009(yoda). I'd still prefer to have two different regulator types (as we did for pm8009 P=0 and P=1 variants). However it's probably up to the maintainers to decide.
On 2021-03-02 19:51, Dmitry Baryshkov wrote: > Hello, > > On Mon, 1 Mar 2021 at 13:37, <skakit@codeaurora.org> wrote: >> >> On 2021-02-26 15:57, Dmitry Baryshkov wrote: >> > On Fri, 26 Feb 2021 at 09:59, <skakit@codeaurora.org> wrote: >> >> >> >> Hi, >> >> >> >> On 2021-02-25 16:39, Dmitry Baryshkov wrote: >> >> > On 24/02/2021 11:33, satya priya wrote: >> >> >> Correct the REGULATOR_LINEAR_RANGE and n_voltges for >> >> >> pmic5_hfsmps515 buck. >> >> >> >> >> >> Signed-off-by: satya priya <skakit@codeaurora.org> >> >> >> --- >> >> >> drivers/regulator/qcom-rpmh-regulator.c | 4 ++-- >> >> >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> >> >> >> >> diff --git a/drivers/regulator/qcom-rpmh-regulator.c >> >> >> b/drivers/regulator/qcom-rpmh-regulator.c >> >> >> index 79a554f..36542c3 100644 >> >> >> --- a/drivers/regulator/qcom-rpmh-regulator.c >> >> >> +++ b/drivers/regulator/qcom-rpmh-regulator.c >> >> >> @@ -726,8 +726,8 @@ static const struct rpmh_vreg_hw_data >> >> >> pmic5_ftsmps510 = { >> >> >> static const struct rpmh_vreg_hw_data pmic5_hfsmps515 = { >> >> >> .regulator_type = VRM, >> >> >> .ops = &rpmh_regulator_vrm_ops, >> >> >> - .voltage_range = REGULATOR_LINEAR_RANGE(2800000, 0, 4, 16000), >> >> >> - .n_voltages = 5, >> >> >> + .voltage_range = REGULATOR_LINEAR_RANGE(320000, 0, 235, 16000), >> >> >> + .n_voltages = 236, >> >> > >> >> > I've checked the docs for pm8009, the chip which also uses hfsmps515 >> >> > regulators. The pdf clearly states that the 'Output voltage operating >> >> > range' is from 2.8 V to 2.85 V. >> >> > >> >> > So we'd probably need to define different versions of HFS515 regulator >> >> > data (like I had to create for pm8009-1). >> >> > >> >> > >> >> >> >> The min-max voltages for S1C (PM8350c) regulator are 2190000-2210000uV >> >> for sc7280(kodiak), so we had to modify this buck to support this >> >> regulator. >> >> >> >> AFAIK, this struct defines the HW constraints of a regulator, but the >> >> platform specific min-max values can be controlled from DT files. So, >> >> can't we modify it like above instead of adding a new definition? the >> >> new min_uV value (32000) is anyway not exceeding the old value >> >> (2800000) >> >> right? please correct me if wrong. >> > >> > As far as I understand for other regulators we put 'output voltage >> > limitations' from the docs into the regulator definition and further >> > constrain it by the platform device tree. Please correct me if I'm >> > wrong. >> >> I see that for most of the regulators, these specifications are >> specific >> to regulator buck (like HFS515) but not chipset specific, we set the >> chipset specific(like pm8009/pm8350c) requirements from DT files. >> >> For example: >> pmic5_nldo regulator spec mentions LLIMIT= 0.32V and ULIMIT =1.304V >> with >> step 8mV >> >> .voltage_range = REGULATOR_LINEAR_RANGE(320000, 0, 123, 8000), >> max output voltage supported by this regulator is 123*8000 + 320000 = >> 1304000mV which is same as mentioned in the regulator spec. >> >> > For pm8009 the data from the datasheet matches the regulators defined >> > in the source file. Unfortunately I don't have kodiak specs at hand. >> >> From the HFS515 spec I got below info >> "HFS510 and lower max output voltage is limited to 2.04V max, and >> Yoda(pm8009) requirement was 2.4V for IOT PA and 2.85V for camera >> application. Hence, HFS515 added a new register and corresponding HW >> changes to support the higher voltage. Table 5‑24 shows the new >> FB_RANGE bit. When configured to 0 the buck works as earlier where >> Vout >> max = 2.04V in 8mV steps, but when configured to 1 the buck range >> doubles and can now support a Vout max = 4.08V in 16mV steps." >> >> As per above, the max output voltage supported by HFS515 buck is 4.08V >> (which is kodiak pm8350c pmic's requirement). >> So, we have modified the buck data to support pm8350c(palani) along >> with >> pm8009(yoda). > > I'd still prefer to have two different regulator types (as we did for > pm8009 P=0 and P=1 variants). However it's probably up to the > maintainers to decide. As Mark already picked this, I think we can leave it this way. Thanks, Satya Priya
On Thu, Mar 11, 2021 at 09:45:41AM +0530, skakit@codeaurora.org wrote: > On 2021-03-02 19:51, Dmitry Baryshkov wrote: > > I'd still prefer to have two different regulator types (as we did for > > pm8009 P=0 and P=1 variants). However it's probably up to the > > maintainers to decide. > As Mark already picked this, I think we can leave it this way. As far as I can tell this is a system configuration issue, the board constraints will ensure that we don't try to set a voltage that the system can't support so there should be no need for this to be handled as separate variants. That assumes that this P register field just extends the values available, it doesn't have to be tied to some board setup or anything. If it is a board configuration thing it probably makes more sense to add a boolean property for it, ideally something tied to whatever the board configuration is so that it's easier for people to discover. I had understood the pm8009 case as being two different parts with the same name rather than two different options for the same part.