| 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:06PM +0530, satya priya wrote:
> Convert regulator bindings from .txt to .yaml format.
Please place any conversions of DT bindings to YAML at the end of patch
serieses, there is frequently a backlog on review of these conversions
which can hold everything else up.
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
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.
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. -- With best wishes Dmitry
On Wed, 24 Feb 2021 14:03:06 +0530, satya priya wrote: > Convert regulator bindings from .txt to .yaml format. > > Signed-off-by: satya priya <skakit@codeaurora.org> > --- > .../bindings/regulator/qcom,rpmh-regulator.txt | 180 --------------------- > .../bindings/regulator/qcom,rpmh-regulator.yaml | 147 +++++++++++++++++ > 2 files changed, 147 insertions(+), 180 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt > create mode 100644 Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: ./Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml:13:5: [warning] wrong indentation: expected 2 but found 4 (indentation) ./Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml:48:5: [warning] wrong indentation: expected 2 but found 4 (indentation) ./Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml:49:9: [warning] wrong indentation: expected 6 but found 8 (indentation) ./Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml:50:13: [warning] wrong indentation: expected 10 but found 12 (indentation) ./Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml:64:9: [warning] wrong indentation: expected 6 but found 8 (indentation) ./Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml:69:9: [warning] wrong indentation: expected 6 but found 8 (indentation) ./Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml:83:6: [warning] wrong indentation: expected 6 but found 5 (indentation) ./Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml:89:6: [warning] wrong indentation: expected 6 but found 5 (indentation) ./Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml:95:2: [warning] wrong indentation: expected 2 but found 1 (indentation) dtschema/dtc warnings/errors: See https://patchwork.ozlabs.org/patch/1443748 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
On Wed, Feb 24, 2021 at 2:34 AM satya priya <skakit@codeaurora.org> wrote: > > Convert regulator bindings from .txt to .yaml format. > > Signed-off-by: satya priya <skakit@codeaurora.org> > --- > .../bindings/regulator/qcom,rpmh-regulator.txt | 180 --------------------- > .../bindings/regulator/qcom,rpmh-regulator.yaml | 147 +++++++++++++++++ > 2 files changed, 147 insertions(+), 180 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt > create mode 100644 Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml > > diff --git a/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt > deleted file mode 100644 > index ce1e043..0000000 > --- a/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt > +++ /dev/null > @@ -1,180 +0,0 @@ > -Qualcomm Technologies, Inc. RPMh Regulators > - > -rpmh-regulator devices support PMIC regulator management via the Voltage > -Regulator Manager (VRM) and Oscillator Buffer (XOB) RPMh accelerators. The APPS > -processor communicates with these hardware blocks via a Resource State > -Coordinator (RSC) using command packets. The VRM allows changing three > -parameters for a given regulator: enable state, output voltage, and operating > -mode. The XOB allows changing only a single parameter for a given regulator: > -its enable state. Despite its name, the XOB is capable of controlling the > -enable state of any PMIC peripheral. It is used for clock buffers, low-voltage > -switches, and LDO/SMPS regulators which have a fixed voltage and mode. > - > -======================= > -Required Node Structure > -======================= > - > -RPMh regulators must be described in two levels of device nodes. The first > -level describes the PMIC containing the regulators and must reside within an > -RPMh device node. The second level describes each regulator within the PMIC > -which is to be used on the board. Each of these regulators maps to a single > -RPMh resource. > - > -The names used for regulator nodes must match those supported by a given PMIC. > -Supported regulator node names: > - PM8005: smps1 - smps4 > - PM8009: smps1 - smps2, ldo1 - ldo7 > - PM8150: smps1 - smps10, ldo1 - ldo18 > - PM8150L: smps1 - smps8, ldo1 - ldo11, bob, flash, rgb > - PM8350: smps1 - smps12, ldo1 - ldo10, > - PM8350C: smps1 - smps10, ldo1 - ldo13, bob > - PM8998: smps1 - smps13, ldo1 - ldo28, lvs1 - lvs2 > - PMI8998: bob > - PM6150: smps1 - smps5, ldo1 - ldo19 > - PM6150L: smps1 - smps8, ldo1 - ldo11, bob > - PMX55: smps1 - smps7, ldo1 - ldo16 > - > -======================== > -First Level Nodes - PMIC > -======================== > - > -- compatible > - Usage: required > - Value type: <string> > - Definition: Must be one of below: > - "qcom,pm8005-rpmh-regulators" > - "qcom,pm8009-rpmh-regulators" > - "qcom,pm8009-1-rpmh-regulators" > - "qcom,pm8150-rpmh-regulators" > - "qcom,pm8150l-rpmh-regulators" > - "qcom,pm8350-rpmh-regulators" > - "qcom,pm8350c-rpmh-regulators" > - "qcom,pm8998-rpmh-regulators" > - "qcom,pmc8180-rpmh-regulators" > - "qcom,pmc8180c-rpmh-regulators" > - "qcom,pmi8998-rpmh-regulators" > - "qcom,pm6150-rpmh-regulators" > - "qcom,pm6150l-rpmh-regulators" > - "qcom,pmx55-rpmh-regulators" > - > -- qcom,pmic-id > - Usage: required > - Value type: <string> > - Definition: RPMh resource name suffix used for the regulators found on > - this PMIC. Typical values: "a", "b", "c", "d", "e", "f". > - > -- vdd-s1-supply > -- vdd-s2-supply > -- vdd-s3-supply > -- vdd-s4-supply > - Usage: optional (PM8998 and PM8005 only) > - Value type: <phandle> > - Definition: phandle of the parent supply regulator of one or more of the > - regulators for this PMIC. > - > -- vdd-s5-supply > -- vdd-s6-supply > -- vdd-s7-supply > -- vdd-s8-supply > -- vdd-s9-supply > -- vdd-s10-supply > -- vdd-s11-supply > -- vdd-s12-supply > -- vdd-s13-supply > -- vdd-l1-l27-supply > -- vdd-l2-l8-l17-supply > -- vdd-l3-l11-supply > -- vdd-l4-l5-supply > -- vdd-l6-supply > -- vdd-l7-l12-l14-l15-supply > -- vdd-l9-supply > -- vdd-l10-l23-l25-supply > -- vdd-l13-l19-l21-supply > -- vdd-l16-l28-supply > -- vdd-l18-l22-supply > -- vdd-l20-l24-supply > -- vdd-l26-supply > -- vin-lvs-1-2-supply > - Usage: optional (PM8998 only) > - Value type: <phandle> > - Definition: phandle of the parent supply regulator of one or more of the > - regulators for this PMIC. > - > -- vdd-bob-supply > - Usage: optional (PMI8998 only) > - Value type: <phandle> > - Definition: BOB regulator parent supply phandle > - > -=============================== > -Second Level Nodes - Regulators > -=============================== > - > -- qcom,always-wait-for-ack > - Usage: optional > - Value type: <empty> > - Definition: Boolean flag which indicates that the application processor > - must wait for an ACK or a NACK from RPMh for every request > - sent for this regulator including those which are for a > - strictly lower power state. > - > -Other properties defined in Documentation/devicetree/bindings/regulator/regulator.txt > -may also be used. regulator-initial-mode and regulator-allowed-modes may be > -specified for VRM regulators using mode values from > -include/dt-bindings/regulator/qcom,rpmh-regulator.h. regulator-allow-bypass > -may be specified for BOB type regulators managed via VRM. > -regulator-allow-set-load may be specified for LDO type regulators managed via > -VRM. > - > -======== > -Examples > -======== > - > -#include <dt-bindings/regulator/qcom,rpmh-regulator.h> > - > -&apps_rsc { > - pm8998-rpmh-regulators { > - compatible = "qcom,pm8998-rpmh-regulators"; > - qcom,pmic-id = "a"; > - > - vdd-l7-l12-l14-l15-supply = <&pm8998_s5>; > - > - smps2 { > - regulator-min-microvolt = <1100000>; > - regulator-max-microvolt = <1100000>; > - }; > - > - pm8998_s5: smps5 { > - regulator-min-microvolt = <1904000>; > - regulator-max-microvolt = <2040000>; > - }; > - > - ldo7 { > - regulator-min-microvolt = <1800000>; > - regulator-max-microvolt = <1800000>; > - regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > - regulator-allowed-modes = > - <RPMH_REGULATOR_MODE_LPM > - RPMH_REGULATOR_MODE_HPM>; > - regulator-allow-set-load; > - }; > - > - lvs1 { > - regulator-min-microvolt = <1800000>; > - regulator-max-microvolt = <1800000>; > - }; > - }; > - > - pmi8998-rpmh-regulators { > - compatible = "qcom,pmi8998-rpmh-regulators"; > - qcom,pmic-id = "b"; > - > - bob { > - regulator-min-microvolt = <3312000>; > - regulator-max-microvolt = <3600000>; > - regulator-allowed-modes = > - <RPMH_REGULATOR_MODE_AUTO > - RPMH_REGULATOR_MODE_HPM>; > - regulator-initial-mode = <RPMH_REGULATOR_MODE_AUTO>; > - }; > - }; > -}; > diff --git a/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml > new file mode 100644 > index 0000000..c14baf8 > --- /dev/null > +++ b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml > @@ -0,0 +1,147 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/regulator/qcom,rpmh-regulator.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm Technologies, Inc. RPMh Regulators > + > +maintainers: > + - David Collins <collinsd@codeaurora.org> > + > +description: I assume you want the formatting here maintained, so you need a '|' at the end. > + rpmh-regulator devices support PMIC regulator management via the Voltage > + Regulator Manager (VRM) and Oscillator Buffer (XOB) RPMh accelerators. The APPS > + processor communicates with these hardware blocks via a Resource State > + Coordinator (RSC) using command packets. The VRM allows changing three > + parameters for a given regulator, enable state, output voltage, and operating > + mode. The XOB allows changing only a single parameter for a given regulator, > + its enable state. Despite its name, the XOB is capable of controlling the > + enable state of any PMIC peripheral. It is used for clock buffers, low-voltage > + switches, and LDO/SMPS regulators which have a fixed voltage and mode. > + > + ======================= > + Required Node Structure > + ======================= > + > + RPMh regulators must be described in two levels of device nodes. The first > + level describes the PMIC containing the regulators and must reside within an > + RPMh device node. The second level describes each regulator within the PMIC > + which is to be used on the board. Each of these regulators maps to a single > + RPMh resource. > + > + The names used for regulator nodes must match those supported by a given PMIC. > + Supported regulator node names are > + For PM8005, smps1 - smps4 > + For PM8009, smps1 - smps2, ldo1 - ldo7 > + For PM8150, smps1 - smps10, ldo1 - ldo18 > + For PM8150L, smps1 - smps8, ldo1 - ldo11, bob, flash, rgb flash and rgb aren't documented. > + For PM8350, smps1 - smps12, ldo1 - ldo10 > + For PM8350C, smps1 - smps10, ldo1 - ldo13, bob > + For PM8998, smps1 - smps13, ldo1 - ldo28, lvs1 - lvs2 > + For PMI8998, bob > + For PM6150, smps1 - smps5, ldo1 - ldo19 > + For PM6150L, smps1 - smps8, ldo1 - ldo11, bob > + For PMX55, smps1 - smps7, ldo1 - ldo16 > + > +properties: > + compatible: > + enum: > + - qcom,pm8005-rpmh-regulators > + - qcom,pm8009-rpmh-regulators > + - qcom,pm8009-1-rpmh-regulators > + - qcom,pm8150-rpmh-regulators > + - qcom,pm8150l-rpmh-regulators > + - qcom,pm8350-rpmh-regulators > + - qcom,pm8350c-rpmh-regulators > + - qcom,pm8998-rpmh-regulators > + - qcom,pmi8998-rpmh-regulators > + - qcom,pm6150-rpmh-regulators > + - qcom,pm6150l-rpmh-regulators > + - qcom,pmx55-rpmh-regulators > + > + qcom,pmic-id: > + description: RPMh resource name suffix used for the regulators found on > + this PMIC. Typical values are "a", "b", "c", "d", "e", "f". Sounds like constraints. Make the values a schema. > + $ref: /schemas/types.yaml#/definitions/string > + > + qcom,always-wait-for-ack: > + description: Boolean flag which indicates that the application processor > + must wait for an ACK or a NACK from RPMh for every request > + sent for this regulator including those which are for a > + strictly lower power state. > + $ref: /schemas/types.yaml#/definitions/string Boolean or string? > + > +patternProperties: > + ".*-supply$": You can drop '.*'. That's already the case without '^'. The supply names need to be defined. > + description: phandle of the parent supply regulator of one or more of the > + regulators for this PMIC. > + > + "^((smps|ldo|lvs)[0-9]*)$": s/*/+/ as 1 digit is always required, right? > + type: object > + allOf: Don't need allOf. > + - $ref: "regulator.yaml#" > + description: List of regulator parent supply phandles This is a node, not a list of phandles. > + > + "bob$": 'foobob' is okay as that would be allowed? If a fixed string, put under 'properties'. > + type: object > + allOf: > + - $ref: "regulator.yaml#" > + description: BOB regulator parent supply phandle > + > +additionalProperties: false > + > +required: > + - compatible > + - qcom,pmic-id > + > +examples: > + - | > + #include <dt-bindings/regulator/qcom,rpmh-regulator.h> > + > + pm8998-rpmh-regulators { > + compatible = "qcom,pm8998-rpmh-regulators"; > + qcom,pmic-id = "a"; > + > + vdd-l7-l12-l14-l15-supply = <&pm8998_s5>; > + > + smps2 { > + regulator-min-microvolt = <1100000>; > + regulator-max-microvolt = <1100000>; > + }; > + > + pm8998_s5: smps5 { Drop unused labels. > + regulator-min-microvolt = <1904000>; > + regulator-max-microvolt = <2040000>; > + }; > + > + ldo7 { > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; > + regulator-allowed-modes = > + <RPMH_REGULATOR_MODE_LPM > + RPMH_REGULATOR_MODE_HPM>; > + regulator-allow-set-load; > + }; > + > + lvs1 { > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + }; > + }; > + > + pmi8998-rpmh-regulators { > + compatible = "qcom,pmi8998-rpmh-regulators"; > + qcom,pmic-id = "b"; > + > + bob { > + regulator-min-microvolt = <3312000>; > + regulator-max-microvolt = <3600000>; > + regulator-allowed-modes = > + <RPMH_REGULATOR_MODE_AUTO > + RPMH_REGULATOR_MODE_HPM>; > + regulator-initial-mode = <RPMH_REGULATOR_MODE_AUTO>; > + }; > + }; > +... > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation >
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.
Hi Rob, Thanks for reviewing the patch! >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: >> http://devicetree.org/schemas/regulator/qcom,rpmh-regulator.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Qualcomm Technologies, Inc. RPMh Regulators >> + >> +maintainers: >> + - David Collins <collinsd@codeaurora.org> >> + >> +description: > > I assume you want the formatting here maintained, so you need a '|' at > the end. > Ok. >> + rpmh-regulator devices support PMIC regulator management via the >> Voltage >> + Regulator Manager (VRM) and Oscillator Buffer (XOB) RPMh >> accelerators. The APPS >> + processor communicates with these hardware blocks via a Resource >> State >> + Coordinator (RSC) using command packets. The VRM allows changing >> three >> + parameters for a given regulator, enable state, output voltage, >> and operating >> + mode. The XOB allows changing only a single parameter for a >> given regulator, >> + its enable state. Despite its name, the XOB is capable of >> controlling the >> + enable state of any PMIC peripheral. It is used for clock >> buffers, low-voltage >> + switches, and LDO/SMPS regulators which have a fixed voltage and >> mode. >> + >> + ======================= >> + Required Node Structure >> + ======================= >> + >> + RPMh regulators must be described in two levels of device nodes. >> The first >> + level describes the PMIC containing the regulators and must >> reside within an >> + RPMh device node. The second level describes each regulator >> within the PMIC >> + which is to be used on the board. Each of these regulators maps >> to a single >> + RPMh resource. >> + >> + The names used for regulator nodes must match those supported by >> a given PMIC. >> + Supported regulator node names are >> + For PM8005, smps1 - smps4 >> + For PM8009, smps1 - smps2, ldo1 - ldo7 >> + For PM8150, smps1 - smps10, ldo1 - ldo18 >> + For PM8150L, smps1 - smps8, ldo1 - ldo11, bob, flash, rgb > > flash and rgb aren't documented. > Ok will add them. >> + For PM8350, smps1 - smps12, ldo1 - ldo10 >> + For PM8350C, smps1 - smps10, ldo1 - ldo13, bob >> + For PM8998, smps1 - smps13, ldo1 - ldo28, lvs1 - lvs2 >> + For PMI8998, bob >> + For PM6150, smps1 - smps5, ldo1 - ldo19 >> + For PM6150L, smps1 - smps8, ldo1 - ldo11, bob >> + For PMX55, smps1 - smps7, ldo1 - ldo16 >> + >> +properties: >> + compatible: >> + enum: >> + - qcom,pm8005-rpmh-regulators >> + - qcom,pm8009-rpmh-regulators >> + - qcom,pm8009-1-rpmh-regulators >> + - qcom,pm8150-rpmh-regulators >> + - qcom,pm8150l-rpmh-regulators >> + - qcom,pm8350-rpmh-regulators >> + - qcom,pm8350c-rpmh-regulators >> + - qcom,pm8998-rpmh-regulators >> + - qcom,pmi8998-rpmh-regulators >> + - qcom,pm6150-rpmh-regulators >> + - qcom,pm6150l-rpmh-regulators >> + - qcom,pmx55-rpmh-regulators >> + >> + qcom,pmic-id: >> + description: RPMh resource name suffix used for the >> regulators found on >> + this PMIC. Typical values are "a", "b", "c", >> "d", "e", "f". > > Sounds like constraints. Make the values a schema. > Ok >> + $ref: /schemas/types.yaml#/definitions/string >> + >> + qcom,always-wait-for-ack: >> + description: Boolean flag which indicates that the >> application processor >> + must wait for an ACK or a NACK from RPMh for >> every request >> + sent for this regulator including those which >> are for a >> + strictly lower power state. >> + $ref: /schemas/types.yaml#/definitions/string > > Boolean or string? > Ok, will change it to /schemas/types.yaml#/definitions/flag >> + >> +patternProperties: >> + ".*-supply$": > > You can drop '.*'. That's already the case without '^'. > Ok. > The supply names need to be defined. > you mean I should define like this "^vdd-s|l([0-9]+)-supply$": ? >> + description: phandle of the parent supply regulator of one or >> more of the >> + regulators for this PMIC. >> + >> + "^((smps|ldo|lvs)[0-9]*)$": > > s/*/+/ as 1 digit is always required, right? > ok >> + type: object >> + allOf: > > Don't need allOf. > ok, will drop this. >> + - $ref: "regulator.yaml#" >> + description: List of regulator parent supply phandles > > This is a node, not a list of phandles. > Okay. >> + >> + "bob$": > > 'foobob' is okay as that would be allowed? If a fixed string, put > under 'properties'. > It is fixed string, will move it to properties. >> + type: object >> + allOf: >> + - $ref: "regulator.yaml#" >> + description: BOB regulator parent supply phandle >> + >> +additionalProperties: false >> + >> +required: >> + - compatible >> + - qcom,pmic-id >> + >> +examples: >> + - | >> + #include <dt-bindings/regulator/qcom,rpmh-regulator.h> >> + >> + pm8998-rpmh-regulators { >> + compatible = "qcom,pm8998-rpmh-regulators"; >> + qcom,pmic-id = "a"; >> + >> + vdd-l7-l12-l14-l15-supply = <&pm8998_s5>; >> + >> + smps2 { >> + regulator-min-microvolt = <1100000>; >> + regulator-max-microvolt = <1100000>; >> + }; >> + >> + pm8998_s5: smps5 { > > Drop unused labels. > Okay. >> + regulator-min-microvolt = <1904000>; >> + regulator-max-microvolt = <2040000>; >> + }; >> + >> + ldo7 { >> + regulator-min-microvolt = <1800000>; >> + regulator-max-microvolt = <1800000>; >> + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>; >> + regulator-allowed-modes = >> + <RPMH_REGULATOR_MODE_LPM >> + RPMH_REGULATOR_MODE_HPM>; >> + regulator-allow-set-load; >> + }; >> + >> + lvs1 { >> + regulator-min-microvolt = <1800000>; >> + regulator-max-microvolt = <1800000>; >> + }; >> + }; >> + >> + pmi8998-rpmh-regulators { >> + compatible = "qcom,pmi8998-rpmh-regulators"; >> + qcom,pmic-id = "b"; >> + >> + bob { >> + regulator-min-microvolt = <3312000>; >> + regulator-max-microvolt = <3600000>; >> + regulator-allowed-modes = >> + <RPMH_REGULATOR_MODE_AUTO >> + RPMH_REGULATOR_MODE_HPM>; >> + regulator-initial-mode = <RPMH_REGULATOR_MODE_AUTO>; >> + }; >> + }; >> +... >> -- >> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a >> member >> of Code Aurora Forum, hosted by The Linux Foundation >> Thanks, Satya Priya
On 2021-03-01 20:14, Rob Herring wrote: > On Wed, 24 Feb 2021 14:03:06 +0530, satya priya wrote: >> Convert regulator bindings from .txt to .yaml format. >> >> Signed-off-by: satya priya <skakit@codeaurora.org> >> --- >> .../bindings/regulator/qcom,rpmh-regulator.txt | 180 >> --------------------- >> .../bindings/regulator/qcom,rpmh-regulator.yaml | 147 >> +++++++++++++++++ >> 2 files changed, 147 insertions(+), 180 deletions(-) >> delete mode 100644 >> Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt >> create mode 100644 >> Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml >> > > My bot found errors running 'make dt_binding_check' on your patch: > > yamllint warnings/errors: > ./Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml:13:5: > [warning] wrong indentation: expected 2 but found 4 (indentation) > ./Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml:48:5: > [warning] wrong indentation: expected 2 but found 4 (indentation) > ./Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml:49:9: > [warning] wrong indentation: expected 6 but found 8 (indentation) > ./Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml:50:13: > [warning] wrong indentation: expected 10 but found 12 (indentation) > ./Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml:64:9: > [warning] wrong indentation: expected 6 but found 8 (indentation) > ./Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml:69:9: > [warning] wrong indentation: expected 6 but found 8 (indentation) > ./Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml:83:6: > [warning] wrong indentation: expected 6 but found 5 (indentation) > ./Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml:89:6: > [warning] wrong indentation: expected 6 but found 5 (indentation) > ./Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml:95:2: > [warning] wrong indentation: expected 2 but found 1 (indentation) > > dtschema/dtc warnings/errors: > > See https://patchwork.ozlabs.org/patch/1443748 > > This check can fail if there are any dependencies. The base for a patch > series is generally the most recent rc1. > > If you already ran 'make dt_binding_check' and didn't see the above > error(s), then make sure 'yamllint' is installed and dt-schema is up to > date: > > pip3 install dtschema --upgrade > > Please check and re-submit. I've updated dt-schema, and also installed yamllint but I am not seeing these errors, could you please let me know if I am missing anything here. Thanks, Satya Priya
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.