* [PATCH V4 0/6] Add Qualcomm Technologies, Inc. PM8008 regulator driver @ 2021-11-19 9:42 Satya Priya 2021-11-19 9:42 ` [PATCH V4 1/6] dt-bindings: regulator: Add "regulator-min-dropout-voltage-microvolt" Satya Priya ` (5 more replies) 0 siblings, 6 replies; 27+ messages in thread From: Satya Priya @ 2021-11-19 9:42 UTC (permalink / raw) To: Bjorn Andersson, Rob Herring Cc: Liam Girdwood, Mark Brown, swboyd, collinsd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel, Satya Priya Satya Priya (6): dt-bindings: regulator: Add "regulator-min-dropout-voltage-microvolt" dt-bindings: regulator: Add pm8008 regulator bindings dt-bindings: mfd: pm8008: Add pm8008 regulator node regulator: Add a regulator driver for the PM8008 PMIC arm64: dts: qcom: pm8008: Add base dts file arm64: dts: qcom: sc7280: Add pm8008 regulators support for sc7280-idp .../devicetree/bindings/mfd/qcom,pm8008.yaml | 24 ++ .../bindings/regulator/qcom,pm8008-regulator.yaml | 68 ++++++ .../devicetree/bindings/regulator/regulator.yaml | 4 + arch/arm64/boot/dts/qcom/pm8008.dtsi | 57 +++++ arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 73 ++++++ drivers/regulator/Kconfig | 9 + drivers/regulator/Makefile | 1 + drivers/regulator/qcom-pm8008-regulator.c | 258 +++++++++++++++++++++ 8 files changed, 494 insertions(+) create mode 100644 Documentation/devicetree/bindings/regulator/qcom,pm8008-regulator.yaml create mode 100644 arch/arm64/boot/dts/qcom/pm8008.dtsi create mode 100644 drivers/regulator/qcom-pm8008-regulator.c -- 2.7.4 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH V4 1/6] dt-bindings: regulator: Add "regulator-min-dropout-voltage-microvolt" 2021-11-19 9:42 [PATCH V4 0/6] Add Qualcomm Technologies, Inc. PM8008 regulator driver Satya Priya @ 2021-11-19 9:42 ` Satya Priya 2021-11-25 15:17 ` Mark Brown 2021-11-19 9:42 ` [PATCH V4 2/6] dt-bindings: regulator: Add pm8008 regulator bindings Satya Priya ` (4 subsequent siblings) 5 siblings, 1 reply; 27+ messages in thread From: Satya Priya @ 2021-11-19 9:42 UTC (permalink / raw) To: Bjorn Andersson, Rob Herring Cc: Liam Girdwood, Mark Brown, swboyd, collinsd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel, Satya Priya Add "regulator-min-dropout-voltage-microvolt" property. Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com> --- Changes in V4: - This is newly added in V4. Documentation/devicetree/bindings/regulator/regulator.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/regulator/regulator.yaml b/Documentation/devicetree/bindings/regulator/regulator.yaml index a6ae9ec..fdb37d3 100644 --- a/Documentation/devicetree/bindings/regulator/regulator.yaml +++ b/Documentation/devicetree/bindings/regulator/regulator.yaml @@ -224,6 +224,10 @@ properties: description: Maximum difference between current and target voltages that can be changed safely in a single step. + regulator-min-dropout-voltage-microvolt: + description: Specifies the minimum voltage in microvolts that the + parent supply regulator must output, above the output of this regulator. + patternProperties: ".*-supply$": description: Input supply phandle(s) for this node -- 2.7.4 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH V4 1/6] dt-bindings: regulator: Add "regulator-min-dropout-voltage-microvolt" 2021-11-19 9:42 ` [PATCH V4 1/6] dt-bindings: regulator: Add "regulator-min-dropout-voltage-microvolt" Satya Priya @ 2021-11-25 15:17 ` Mark Brown 2021-12-06 13:03 ` Satya Priya Kakitapalli (Temp) 0 siblings, 1 reply; 27+ messages in thread From: Mark Brown @ 2021-11-25 15:17 UTC (permalink / raw) To: Satya Priya Cc: Bjorn Andersson, Rob Herring, Liam Girdwood, swboyd, collinsd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 715 bytes --] On Fri, Nov 19, 2021 at 03:12:28PM +0530, Satya Priya wrote: > + regulator-min-dropout-voltage-microvolt: > + description: Specifies the minimum voltage in microvolts that the > + parent supply regulator must output, above the output of this regulator. Usually this is a fixed property of the regulator rather than something that varies per board - why have a DT property? Please submit patches using subject lines reflecting the style for the subsystem, this makes it easier for people to identify relevant patches. Look at what existing commits in the area you're changing are doing and make sure your subject lines visually resemble what they're doing. There's no need to resubmit to fix this alone. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V4 1/6] dt-bindings: regulator: Add "regulator-min-dropout-voltage-microvolt" 2021-11-25 15:17 ` Mark Brown @ 2021-12-06 13:03 ` Satya Priya Kakitapalli (Temp) 2021-12-06 18:25 ` Mark Brown 0 siblings, 1 reply; 27+ messages in thread From: Satya Priya Kakitapalli (Temp) @ 2021-12-06 13:03 UTC (permalink / raw) To: Mark Brown Cc: Bjorn Andersson, Rob Herring, Liam Girdwood, swboyd, collinsd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel On 11/25/2021 8:47 PM, Mark Brown wrote: > On Fri, Nov 19, 2021 at 03:12:28PM +0530, Satya Priya wrote: > >> + regulator-min-dropout-voltage-microvolt: >> + description: Specifies the minimum voltage in microvolts that the >> + parent supply regulator must output, above the output of this regulator. > Usually this is a fixed property of the regulator rather than something > that varies per board - why have a DT property? The min-dropout value (headroom) varies with boards, that's why we have a DT property for it. We overwrite the default value in driver with actual value read from DT ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V4 1/6] dt-bindings: regulator: Add "regulator-min-dropout-voltage-microvolt" 2021-12-06 13:03 ` Satya Priya Kakitapalli (Temp) @ 2021-12-06 18:25 ` Mark Brown 2021-12-07 15:06 ` Satya Priya Kakitapalli (Temp) 0 siblings, 1 reply; 27+ messages in thread From: Mark Brown @ 2021-12-06 18:25 UTC (permalink / raw) To: Satya Priya Kakitapalli (Temp) Cc: Bjorn Andersson, Rob Herring, Liam Girdwood, swboyd, collinsd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1004 bytes --] On Mon, Dec 06, 2021 at 06:33:26PM +0530, Satya Priya Kakitapalli (Temp) wrote: > On 11/25/2021 8:47 PM, Mark Brown wrote: > > Usually this is a fixed property of the regulator rather than something > > that varies per board - why have a DT property? > The min-dropout value (headroom) varies with boards, that's why we have a DT > property for it. We overwrite the default value in driver with actual value > read from DT Interesting. How exactly does that end up happening - presumably other systems are going to run into it? If you do have board designs which somehow managed to introduce additional dropouts (seems pretty concerning TBH) then I think the best way to handle that is to add a generic property for it and have that either added on to or override the requirements of the regulator itself which should continue to be defined in the driver. That way only boards with issues need to do anything which will avoid bugs with the property being omitted in what should be the common case. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V4 1/6] dt-bindings: regulator: Add "regulator-min-dropout-voltage-microvolt" 2021-12-06 18:25 ` Mark Brown @ 2021-12-07 15:06 ` Satya Priya Kakitapalli (Temp) 2021-12-07 15:19 ` Mark Brown 0 siblings, 1 reply; 27+ messages in thread From: Satya Priya Kakitapalli (Temp) @ 2021-12-07 15:06 UTC (permalink / raw) To: Mark Brown Cc: Bjorn Andersson, Rob Herring, Liam Girdwood, swboyd, collinsd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel On 12/6/2021 11:55 PM, Mark Brown wrote: > On Mon, Dec 06, 2021 at 06:33:26PM +0530, Satya Priya Kakitapalli (Temp) wrote: >> On 11/25/2021 8:47 PM, Mark Brown wrote: >>> Usually this is a fixed property of the regulator rather than something >>> that varies per board - why have a DT property? >> The min-dropout value (headroom) varies with boards, that's why we have a DT >> property for it. We overwrite the default value in driver with actual value >> read from DT > Interesting. How exactly does that end up happening - presumably other > systems are going to run into it? The parent supplies such as "vdd-l1-l2" are coming from other pmic regulators, which are shared supplies with other subsystems like BT, Display etc, they vary between boards as per requirements, so we cannot expect these to be fixed and so are the headroom values. We get the headroom values from PMIC systems team for every target. > If you do have board designs which somehow managed to introduce > additional dropouts (seems pretty concerning TBH) then I think the best > way to handle that is to add a generic property for it and have that > either added on to or override the requirements of the regulator itself > which should continue to be defined in the driver. That way only boards > with issues need to do anything which will avoid bugs with the property > being omitted in what should be the common case. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V4 1/6] dt-bindings: regulator: Add "regulator-min-dropout-voltage-microvolt" 2021-12-07 15:06 ` Satya Priya Kakitapalli (Temp) @ 2021-12-07 15:19 ` Mark Brown 2021-12-09 0:56 ` David Collins 0 siblings, 1 reply; 27+ messages in thread From: Mark Brown @ 2021-12-07 15:19 UTC (permalink / raw) To: Satya Priya Kakitapalli (Temp) Cc: Bjorn Andersson, Rob Herring, Liam Girdwood, swboyd, collinsd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1657 bytes --] On Tue, Dec 07, 2021 at 08:36:11PM +0530, Satya Priya Kakitapalli (Temp) wrote: > On 12/6/2021 11:55 PM, Mark Brown wrote: > > On Mon, Dec 06, 2021 at 06:33:26PM +0530, Satya Priya Kakitapalli (Temp) wrote: > > > The min-dropout value (headroom) varies with boards, that's why we have a DT > > > property for it. We overwrite the default value in driver with actual value > > > read from DT > > Interesting. How exactly does that end up happening - presumably other > > systems are going to run into it? > The parent supplies such as "vdd-l1-l2" are coming from other pmic > regulators, which are shared supplies with other subsystems like BT, Display > etc, they vary between boards as per requirements, so we cannot expect these > to be fixed and so are the headroom values. We get the headroom values from > PMIC systems team for every target. I don't think you're talking about the thing the code is saying it's describing here. The regulator API is referring to the minimum droput voltage that individual regulators require, that is how much higher the input to a single regulator must be than the voltage being output by that regulator. We absolutely can and do expect this to be board independent, it's a function of the design of the regulator. Sharing the input supply has no impact on this, the input voltage that the regulator needs just get fed into the requiremnts on the supply voltage. If there is a board specific constraint on the minimum voltage that a given supply can have then that should be expressed using the normal constraint mechanism, that's nothing to do with the headroom that the regulators require to operate though. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V4 1/6] dt-bindings: regulator: Add "regulator-min-dropout-voltage-microvolt" 2021-12-07 15:19 ` Mark Brown @ 2021-12-09 0:56 ` David Collins 2021-12-10 21:11 ` Mark Brown 0 siblings, 1 reply; 27+ messages in thread From: David Collins @ 2021-12-09 0:56 UTC (permalink / raw) To: Mark Brown, Satya Priya Kakitapalli (Temp) Cc: Bjorn Andersson, Rob Herring, Liam Girdwood, swboyd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel On 12/7/21 7:19 AM, Mark Brown wrote: > On Tue, Dec 07, 2021 at 08:36:11PM +0530, Satya Priya Kakitapalli (Temp) wrote: >> On 12/6/2021 11:55 PM, Mark Brown wrote: >>> On Mon, Dec 06, 2021 at 06:33:26PM +0530, Satya Priya Kakitapalli (Temp) wrote: > >>>> The min-dropout value (headroom) varies with boards, that's why we have a DT >>>> property for it. We overwrite the default value in driver with actual value >>>> read from DT > >>> Interesting. How exactly does that end up happening - presumably other >>> systems are going to run into it? > >> The parent supplies such as "vdd-l1-l2" are coming from other pmic >> regulators, which are shared supplies with other subsystems like BT, Display >> etc, they vary between boards as per requirements, so we cannot expect these >> to be fixed and so are the headroom values. We get the headroom values from >> PMIC systems team for every target. > > I don't think you're talking about the thing the code is saying it's > describing here. The regulator API is referring to the minimum droput > voltage that individual regulators require, that is how much higher the > input to a single regulator must be than the voltage being output by > that regulator. We absolutely can and do expect this to be board > independent, it's a function of the design of the regulator. Sharing > the input supply has no impact on this, the input voltage that the > regulator needs just get fed into the requiremnts on the supply voltage. > > If there is a board specific constraint on the minimum voltage that a > given supply can have then that should be expressed using the normal > constraint mechanism, that's nothing to do with the headroom that the > regulators require to operate though. The PM8008 LDOs are low noise LDOs intended to supply noise sensitive camera sensor hardware. They can maintain output regulation with a fixed headroom voltage. However, in order to guarantee high PSRR, the headroom voltage must be scaled according to the peak load expected from the each LDO on a given board. Thus, we included support for a DT property to specify the headroom per LDO to meet noise requirements across boards. As a minor note the PM8008 chip package has a single pin to supply LDOs 1 and 2 along with a single pin for LDOs 3 and 4. That is why vdd_l1_l2-supply is specified instead of vdd_l1-supply and vdd_l2-supply. Take care, David ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V4 1/6] dt-bindings: regulator: Add "regulator-min-dropout-voltage-microvolt" 2021-12-09 0:56 ` David Collins @ 2021-12-10 21:11 ` Mark Brown 2022-01-03 14:35 ` Satya Priya Kakitapalli (Temp) 0 siblings, 1 reply; 27+ messages in thread From: Mark Brown @ 2021-12-10 21:11 UTC (permalink / raw) To: David Collins Cc: Satya Priya Kakitapalli (Temp), Bjorn Andersson, Rob Herring, Liam Girdwood, swboyd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1741 bytes --] On Wed, Dec 08, 2021 at 04:56:48PM -0800, David Collins wrote: > On 12/7/21 7:19 AM, Mark Brown wrote: > > On Tue, Dec 07, 2021 at 08:36:11PM +0530, Satya Priya Kakitapalli (Temp) wrote: > > that regulator. We absolutely can and do expect this to be board > > independent, it's a function of the design of the regulator. Sharing > > the input supply has no impact on this, the input voltage that the > > regulator needs just get fed into the requiremnts on the supply voltage. > The PM8008 LDOs are low noise LDOs intended to supply noise sensitive > camera sensor hardware. They can maintain output regulation with a > fixed headroom voltage. However, in order to guarantee high PSRR, the > headroom voltage must be scaled according to the peak load expected from > the each LDO on a given board. Thus, we included support for a DT > property to specify the headroom per LDO to meet noise requirements > across boards. Interesting... how much extra headroom are we talking about here? I'd be unsurprised to see this usually just quoted as part of the standard headroom requirement and this smells like the sort of thing that's going to be frequently misused. If the gains are something worth writing home about I'd think we should consider if it's better to support this dynamically at runtime based on load information and provide options for configuring the peak load information through DT instead for static configurations. That would fit in with the stuff we have for managing modes on DCDCs (which isn't really deployed but is there) and the API we have for allowing client drivers to indicate their load requirements at runtime that fits in with that. That'd allow us to only boost the headroom when it's really needed. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V4 1/6] dt-bindings: regulator: Add "regulator-min-dropout-voltage-microvolt" 2021-12-10 21:11 ` Mark Brown @ 2022-01-03 14:35 ` Satya Priya Kakitapalli (Temp) 2022-01-04 14:54 ` Mark Brown 0 siblings, 1 reply; 27+ messages in thread From: Satya Priya Kakitapalli (Temp) @ 2022-01-03 14:35 UTC (permalink / raw) To: Mark Brown, David Collins Cc: Bjorn Andersson, Rob Herring, Liam Girdwood, swboyd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel On 12/11/2021 2:41 AM, Mark Brown wrote: > On Wed, Dec 08, 2021 at 04:56:48PM -0800, David Collins wrote: >> On 12/7/21 7:19 AM, Mark Brown wrote: >>> On Tue, Dec 07, 2021 at 08:36:11PM +0530, Satya Priya Kakitapalli (Temp) wrote: >>> that regulator. We absolutely can and do expect this to be board >>> independent, it's a function of the design of the regulator. Sharing >>> the input supply has no impact on this, the input voltage that the >>> regulator needs just get fed into the requiremnts on the supply voltage. >> The PM8008 LDOs are low noise LDOs intended to supply noise sensitive >> camera sensor hardware. They can maintain output regulation with a >> fixed headroom voltage. However, in order to guarantee high PSRR, the >> headroom voltage must be scaled according to the peak load expected from >> the each LDO on a given board. Thus, we included support for a DT >> property to specify the headroom per LDO to meet noise requirements >> across boards. > Interesting... how much extra headroom are we talking about here? I'd > be unsurprised to see this usually just quoted as part of the standard > headroom requirement and this smells like the sort of thing that's going > to be frequently misused. If the gains are something worth writing home > about > I'd think we should consider if it's better to support this > dynamically at runtime based on load information and provide options for > configuring the peak load information through DT instead for static > configurations. That would fit in with the stuff we have for managing > modes on DCDCs (which isn't really deployed but is there) and the API we > have for allowing client drivers to indicate their load requirements at > runtime that fits in with that. That'd allow us to only boost the > headroom when it's really needed. This means Dynamic headroom control feature needs to be implemented. I need to explore more on this and gather info from team, Could we merge the present driver with "static headroom" for now? I'll do a follow up series to implement this feature. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V4 1/6] dt-bindings: regulator: Add "regulator-min-dropout-voltage-microvolt" 2022-01-03 14:35 ` Satya Priya Kakitapalli (Temp) @ 2022-01-04 14:54 ` Mark Brown 0 siblings, 0 replies; 27+ messages in thread From: Mark Brown @ 2022-01-04 14:54 UTC (permalink / raw) To: Satya Priya Kakitapalli (Temp) Cc: David Collins, Bjorn Andersson, Rob Herring, Liam Girdwood, swboyd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1110 bytes --] On Mon, Jan 03, 2022 at 08:05:40PM +0530, Satya Priya Kakitapalli (Temp) wrote: > On 12/11/2021 2:41 AM, Mark Brown wrote: > > I'd think we should consider if it's better to support this > > dynamically at runtime based on load information and provide options for > > configuring the peak load information through DT instead for static > > configurations. That would fit in with the stuff we have for managing > > modes on DCDCs (which isn't really deployed but is there) and the API we > > have for allowing client drivers to indicate their load requirements at > > runtime that fits in with that. That'd allow us to only boost the > > headroom when it's really needed. > This means Dynamic headroom control feature needs to be implemented. I need > to explore more on this and gather info from team, Could we merge the > present driver with "static headroom" for now? I'll do a follow up series to > implement this feature. I'd be happy to merge something with the headroom configured statically in the driver like we do for other devices - I guess if you set the highest headroom that should cover it. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH V4 2/6] dt-bindings: regulator: Add pm8008 regulator bindings 2021-11-19 9:42 [PATCH V4 0/6] Add Qualcomm Technologies, Inc. PM8008 regulator driver Satya Priya 2021-11-19 9:42 ` [PATCH V4 1/6] dt-bindings: regulator: Add "regulator-min-dropout-voltage-microvolt" Satya Priya @ 2021-11-19 9:42 ` Satya Priya 2021-11-25 15:24 ` Mark Brown 2021-11-19 9:42 ` [PATCH V4 3/6] dt-bindings: mfd: pm8008: Add pm8008 regulator node Satya Priya ` (3 subsequent siblings) 5 siblings, 1 reply; 27+ messages in thread From: Satya Priya @ 2021-11-19 9:42 UTC (permalink / raw) To: Bjorn Andersson, Rob Herring Cc: Liam Girdwood, Mark Brown, swboyd, collinsd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel, Satya Priya Add bindings for pm8008 pmic regulators. Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com> --- Changes in V2: - Moved this patch before "mfd: pm8008: Add pm8008 regulator node" to resolve dtschema errors. Removed regulator-min-microvolt and regulator-max-microvolt properties. Changes in V3: - As per Rob's comments added standard unit suffix for mindropout property, added blank lines where required and added description for reg property. Changes in V4: - Changed compatible string to "com,pm8008-regulators" - Moved "regulator-min-dropout-voltage-microvolt" to regulator.yaml as separate patch. .../bindings/regulator/qcom,pm8008-regulator.yaml | 68 ++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 Documentation/devicetree/bindings/regulator/qcom,pm8008-regulator.yaml diff --git a/Documentation/devicetree/bindings/regulator/qcom,pm8008-regulator.yaml b/Documentation/devicetree/bindings/regulator/qcom,pm8008-regulator.yaml new file mode 100644 index 0000000..38ee970 --- /dev/null +++ b/Documentation/devicetree/bindings/regulator/qcom,pm8008-regulator.yaml @@ -0,0 +1,68 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/regulator/qcom,pm8008-regulator.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm Technologies, Inc. PM8008 Regulator bindings + +maintainers: + - Satya Priya <skakit@codeaurora.org> + +description: + Qualcomm Technologies, Inc. PM8008 is an I2C controlled PMIC + containing 7 LDO regulators. + +properties: + compatible: + const: qcom,pm8008-regulators + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + + vdd_l1_l2-supply: + description: Input supply phandle of ldo1 and ldo2 regulators. + + vdd_l3_l4-supply: + description: Input supply phandle of ldo3 and ldo4 regulators. + + vdd_l5-supply: + description: Input supply phandle of ldo5 regulator. + + vdd_l6-supply: + description: Input supply phandle of ldo6 regulator. + + vdd_l7-supply: + description: Input supply phandle of ldo7 regulator. + +patternProperties: + "^l[1-7]@[0-9a-f]+$": + type: object + + $ref: "regulator.yaml#" + + description: PM8008 regulator peripherals of PM8008 regulator device + + properties: + reg: + maxItems: 1 + description: Base address of the regulator. + + regulator-name: true + + required: + - reg + - regulator-name + + unevaluatedProperties: false + +required: + - compatible + - "#address-cells" + - "#size-cells" + +additionalProperties: false +... -- 2.7.4 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH V4 2/6] dt-bindings: regulator: Add pm8008 regulator bindings 2021-11-19 9:42 ` [PATCH V4 2/6] dt-bindings: regulator: Add pm8008 regulator bindings Satya Priya @ 2021-11-25 15:24 ` Mark Brown 2021-12-06 13:43 ` Satya Priya Kakitapalli (Temp) 0 siblings, 1 reply; 27+ messages in thread From: Mark Brown @ 2021-11-25 15:24 UTC (permalink / raw) To: Satya Priya Cc: Bjorn Andersson, Rob Herring, Liam Girdwood, swboyd, collinsd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 581 bytes --] On Fri, Nov 19, 2021 at 03:12:29PM +0530, Satya Priya wrote: > +properties: > + compatible: > + const: qcom,pm8008-regulators Why are we adding a separate compatible for this when we already know that this is a pm8008 based on the parent? > + vdd_l1_l2-supply: > + description: Input supply phandle of ldo1 and ldo2 regulators. These supply nodes should be chip level, they're going into the chip and in general the expectation is that you should be able to describe the supplies going into a device without worrying about how or if any particular OS splits things up. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V4 2/6] dt-bindings: regulator: Add pm8008 regulator bindings 2021-11-25 15:24 ` Mark Brown @ 2021-12-06 13:43 ` Satya Priya Kakitapalli (Temp) 2021-12-06 13:47 ` Mark Brown 0 siblings, 1 reply; 27+ messages in thread From: Satya Priya Kakitapalli (Temp) @ 2021-12-06 13:43 UTC (permalink / raw) To: Mark Brown Cc: Bjorn Andersson, Rob Herring, Liam Girdwood, swboyd, collinsd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel On 11/25/2021 8:54 PM, Mark Brown wrote: > On Fri, Nov 19, 2021 at 03:12:29PM +0530, Satya Priya wrote: > >> +properties: >> + compatible: >> + const: qcom,pm8008-regulators > Why are we adding a separate compatible for this when we already know > that this is a pm8008 based on the parent? For the regulator driver to be probed we do need a separate compatible right? may be I didn't get your question.. My understanding is we should have a separate compatible for each peripheral under the parent mfd node.. like gpios, temp alarm, regulators etc.. >> + vdd_l1_l2-supply: >> + description: Input supply phandle of ldo1 and ldo2 regulators. > These supply nodes should be chip level, they're going into the chip and > in general the expectation is that you should be able to describe the > supplies going into a device without worrying about how or if any > particular OS splits things up. So, if i understand correctly, we don't have to mention these in the documentation as these are handled at framework level? ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V4 2/6] dt-bindings: regulator: Add pm8008 regulator bindings 2021-12-06 13:43 ` Satya Priya Kakitapalli (Temp) @ 2021-12-06 13:47 ` Mark Brown 2021-12-20 12:44 ` Satya Priya Kakitapalli (Temp) 0 siblings, 1 reply; 27+ messages in thread From: Mark Brown @ 2021-12-06 13:47 UTC (permalink / raw) To: Satya Priya Kakitapalli (Temp) Cc: Bjorn Andersson, Rob Herring, Liam Girdwood, swboyd, collinsd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1377 bytes --] On Mon, Dec 06, 2021 at 07:13:02PM +0530, Satya Priya Kakitapalli (Temp) wrote: > > On 11/25/2021 8:54 PM, Mark Brown wrote: > > On Fri, Nov 19, 2021 at 03:12:29PM +0530, Satya Priya wrote: > > > +properties: > > > + compatible: > > > + const: qcom,pm8008-regulators > > Why are we adding a separate compatible for this when we already know > > that this is a pm8008 based on the parent? > For the regulator driver to be probed we do need a separate compatible > right? may be I didn't get your question.. > My understanding is we should have a separate compatible for each peripheral > under the parent mfd node.. like gpios, temp alarm, regulators etc.. No, the MFD can register whatever children it likes without needing any help from the DT. > > > + vdd_l1_l2-supply: > > > + description: Input supply phandle of ldo1 and ldo2 regulators. > > These supply nodes should be chip level, they're going into the chip and > > in general the expectation is that you should be able to describe the > > supplies going into a device without worrying about how or if any > > particular OS splits things up. > So, if i understand correctly, we don't have to mention these in the > documentation as these are handled at framework level? No. I'm saying you should document these at the chip level, they do need to be documented though. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V4 2/6] dt-bindings: regulator: Add pm8008 regulator bindings 2021-12-06 13:47 ` Mark Brown @ 2021-12-20 12:44 ` Satya Priya Kakitapalli (Temp) [not found] ` <07dc5ba4-790b-0cb2-bc3e-2ce8d7e3e09d@quicinc.com> 0 siblings, 1 reply; 27+ messages in thread From: Satya Priya Kakitapalli (Temp) @ 2021-12-20 12:44 UTC (permalink / raw) To: Mark Brown Cc: Bjorn Andersson, Rob Herring, Liam Girdwood, swboyd, collinsd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel On 12/6/2021 7:17 PM, Mark Brown wrote: > On Mon, Dec 06, 2021 at 07:13:02PM +0530, Satya Priya Kakitapalli (Temp) wrote: >> On 11/25/2021 8:54 PM, Mark Brown wrote: >>> On Fri, Nov 19, 2021 at 03:12:29PM +0530, Satya Priya wrote: >>>> +properties: >>>> + compatible: >>>> + const: qcom,pm8008-regulators >>> Why are we adding a separate compatible for this when we already know >>> that this is a pm8008 based on the parent? >> For the regulator driver to be probed we do need a separate compatible >> right? may be I didn't get your question.. >> My understanding is we should have a separate compatible for each peripheral >> under the parent mfd node.. like gpios, temp alarm, regulators etc.. > No, the MFD can register whatever children it likes without needing any > help from the DT. I think this is possible by using of_platform_bus_probe() API. But, the mfd driver uses of_platform_populate() API, this needs all device nodes to have a 'compatible' property unlike the of_platform_bus_probe() API. All other MFD upstream drivers are also using the same API and registering the child regulators by using separate compatible strings. >>>> + vdd_l1_l2-supply: >>>> + description: Input supply phandle of ldo1 and ldo2 regulators. >>> These supply nodes should be chip level, they're going into the chip and >>> in general the expectation is that you should be able to describe the >>> supplies going into a device without worrying about how or if any >>> particular OS splits things up. >> So, if i understand correctly, we don't have to mention these in the >> documentation as these are handled at framework level? > No. I'm saying you should document these at the chip level, they do > need to be documented though. By chip level do you mean "pm8008.yaml" documentation? If so, yes, I can move these to pm8008.yaml and change DT accordingly. ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <07dc5ba4-790b-0cb2-bc3e-2ce8d7e3e09d@quicinc.com>]
* Re: [PATCH V4 2/6] dt-bindings: regulator: Add pm8008 regulator bindings [not found] ` <07dc5ba4-790b-0cb2-bc3e-2ce8d7e3e09d@quicinc.com> @ 2022-01-10 14:21 ` Mark Brown 2022-01-11 12:15 ` Satya Priya Kakitapalli (Temp) 0 siblings, 1 reply; 27+ messages in thread From: Mark Brown @ 2022-01-10 14:21 UTC (permalink / raw) To: Satya Priya Kakitapalli (Temp) Cc: Bjorn Andersson, Rob Herring, Liam Girdwood, swboyd, collinsd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 469 bytes --] On Mon, Jan 10, 2022 at 06:42:08PM +0530, Satya Priya Kakitapalli (Temp) wrote: > To understand how other upstream mfd drivers are handling this I've gone > through some of them. Taking one example, mfd/stpmic1.c is a pmic mfd > device which has a regulators sub-node with separate compatible, and has the > parent supplies listed under the regulators node. There are some devices that did get merged doing this, that doesn't mean it's a great idea though. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V4 2/6] dt-bindings: regulator: Add pm8008 regulator bindings 2022-01-10 14:21 ` Mark Brown @ 2022-01-11 12:15 ` Satya Priya Kakitapalli (Temp) 2022-01-11 13:59 ` Mark Brown 0 siblings, 1 reply; 27+ messages in thread From: Satya Priya Kakitapalli (Temp) @ 2022-01-11 12:15 UTC (permalink / raw) To: Mark Brown Cc: Bjorn Andersson, Rob Herring, Liam Girdwood, swboyd, collinsd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel On 1/10/2022 7:51 PM, Mark Brown wrote: > On Mon, Jan 10, 2022 at 06:42:08PM +0530, Satya Priya Kakitapalli (Temp) wrote: > >> To understand how other upstream mfd drivers are handling this I've gone >> through some of them. Taking one example, mfd/stpmic1.c is a pmic mfd >> device which has a regulators sub-node with separate compatible, and has the >> parent supplies listed under the regulators node. > There are some devices that did get merged doing this, that doesn't mean > it's a great idea though. In that case, it would be helpful if you could provide an example which has the design you suggested. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V4 2/6] dt-bindings: regulator: Add pm8008 regulator bindings 2022-01-11 12:15 ` Satya Priya Kakitapalli (Temp) @ 2022-01-11 13:59 ` Mark Brown 0 siblings, 0 replies; 27+ messages in thread From: Mark Brown @ 2022-01-11 13:59 UTC (permalink / raw) To: Satya Priya Kakitapalli (Temp) Cc: Bjorn Andersson, Rob Herring, Liam Girdwood, swboyd, collinsd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 450 bytes --] On Tue, Jan 11, 2022 at 05:45:16PM +0530, Satya Priya Kakitapalli (Temp) wrote: > On 1/10/2022 7:51 PM, Mark Brown wrote: > > There are some devices that did get merged doing this, that doesn't mean > > it's a great idea though. > In that case, it would be helpful if you could provide an example which has > the design you suggested. There's plenty of mfds that register child devices without using DT - off the top of my head wm8994 or arizona. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH V4 3/6] dt-bindings: mfd: pm8008: Add pm8008 regulator node 2021-11-19 9:42 [PATCH V4 0/6] Add Qualcomm Technologies, Inc. PM8008 regulator driver Satya Priya 2021-11-19 9:42 ` [PATCH V4 1/6] dt-bindings: regulator: Add "regulator-min-dropout-voltage-microvolt" Satya Priya 2021-11-19 9:42 ` [PATCH V4 2/6] dt-bindings: regulator: Add pm8008 regulator bindings Satya Priya @ 2021-11-19 9:42 ` Satya Priya 2021-11-19 9:42 ` [PATCH V4 4/6] regulator: Add a regulator driver for the PM8008 PMIC Satya Priya ` (2 subsequent siblings) 5 siblings, 0 replies; 27+ messages in thread From: Satya Priya @ 2021-11-19 9:42 UTC (permalink / raw) To: Bjorn Andersson, Rob Herring Cc: Liam Girdwood, Mark Brown, swboyd, collinsd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel, Satya Priya Add pm8008-regulator node and example. Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com> Reviewed-by: Stephen Boyd <swboyd@chromium.org> --- Changes in V2: - As per Rob's comments changed "pm8008[a-z]?-regulator" to "^pm8008[a-z]?-regulators". Changes in V3: - Fixed bot errors. - As per stephen's comments, changed "^pm8008[a-z]?-regulators$" to "regulators". Changes in V4: - Changed compatible string to "qcom,pm8008-regulators" .../devicetree/bindings/mfd/qcom,pm8008.yaml | 24 ++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml index ec3138c..ab6166d 100644 --- a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml +++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml @@ -44,6 +44,10 @@ properties: "#size-cells": const: 0 + regulators: + type: object + $ref: "../regulator/qcom,pm8008-regulator.yaml#" + patternProperties: "^gpio@[0-9a-f]+$": type: object @@ -122,6 +126,26 @@ examples: interrupt-controller; #interrupt-cells = <2>; }; + + regulators { + compatible = "qcom,pm8008-regulators"; + #address-cells = <1>; + #size-cells = <0>; + + vdd_l1_l2-supply = <&vreg_s8b_1p2>; + vdd_l3_l4-supply = <&vreg_s1b_1p8>; + vdd_l5-supply = <&vreg_bob>; + vdd_l6-supply = <&vreg_bob>; + vdd_l7-supply = <&vreg_bob>; + + pm8008_l1: l1@4000 { + reg = <0x4000>; + regulator-name = "pm8008_l1"; + regulator-min-microvolt = <950000>; + regulator-max-microvolt = <1300000>; + regulator-min-dropout-voltage-microvolt = <96000>; + }; + }; }; }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH V4 4/6] regulator: Add a regulator driver for the PM8008 PMIC 2021-11-19 9:42 [PATCH V4 0/6] Add Qualcomm Technologies, Inc. PM8008 regulator driver Satya Priya ` (2 preceding siblings ...) 2021-11-19 9:42 ` [PATCH V4 3/6] dt-bindings: mfd: pm8008: Add pm8008 regulator node Satya Priya @ 2021-11-19 9:42 ` Satya Priya 2021-11-25 15:45 ` Mark Brown 2021-11-19 9:42 ` [PATCH V4 5/6] arm64: dts: qcom: pm8008: Add base dts file Satya Priya 2021-11-19 9:42 ` [PATCH V4 6/6] arm64: dts: qcom: sc7280: Add pm8008 regulators support for sc7280-idp Satya Priya 5 siblings, 1 reply; 27+ messages in thread From: Satya Priya @ 2021-11-19 9:42 UTC (permalink / raw) To: Bjorn Andersson, Rob Herring Cc: Liam Girdwood, Mark Brown, swboyd, collinsd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel, Satya Priya Qualcomm Technologies, Inc. PM8008 is an I2C controlled PMIC containing 7 LDO regulators. Add a PM8008 regulator driver to support PMIC regulator management via the regulator framework. Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com> --- Changes in V2: - As per Mark's comments - Using regmap helpers for regulator enable/disable and is_enabled APIs - Changed pr_err to dev_err wherever possible. - Removed init_voltage property as it is not used. - Removed if check for registering LDOs - Other minor changes. Changes in V3: - As per Stephen's comments, - Removed unused includes - Removed PM8008_MAX_LDO macro. - Removed pm8008_read/write APIs, using regmap_bulk_read/write APIs - Using le16_to_cpu/cpu_to_le16 APIs in pm8008_regulator_get/set_voltage - Consolidated all probe related functions into single probe function. - Added of_parse_cb call back and removed regulator-name matching loop. - Fixed other minor nits. Changes in V4: - Removed unused members like rdev and of_node from pm8008_regulator struct. - Replaced set_voltage with set_voltage_sel - Removed init_data configuration as it is not needed. - Removed few other unused assignments from probe drivers/regulator/Kconfig | 9 ++ drivers/regulator/Makefile | 1 + drivers/regulator/qcom-pm8008-regulator.c | 258 ++++++++++++++++++++++++++++++ 3 files changed, 268 insertions(+) create mode 100644 drivers/regulator/qcom-pm8008-regulator.c diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig index 6be9b1c..61e3e98 100644 --- a/drivers/regulator/Kconfig +++ b/drivers/regulator/Kconfig @@ -916,6 +916,15 @@ config REGULATOR_PWM This driver supports PWM controlled voltage regulators. PWM duty cycle can increase or decrease the voltage. +config REGULATOR_QCOM_PM8008 + tristate "Qualcomm Technologies, Inc. PM8008 PMIC regulators" + depends on MFD_QCOM_PM8008 + help + Select this option to get support for the voltage regulators + of Qualcomm Technologies, Inc. PM8008 PMIC chip. PM8008 has 7 LDO + regulators. This driver provides support for basic operations like + set/get voltage and enable/disable. + config REGULATOR_QCOM_RPM tristate "Qualcomm RPM regulator driver" depends on MFD_QCOM_RPM diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile index b07d2a2..4fac16b6 100644 --- a/drivers/regulator/Makefile +++ b/drivers/regulator/Makefile @@ -100,6 +100,7 @@ obj-$(CONFIG_REGULATOR_MT6380) += mt6380-regulator.o obj-$(CONFIG_REGULATOR_MT6397) += mt6397-regulator.o obj-$(CONFIG_REGULATOR_MTK_DVFSRC) += mtk-dvfsrc-regulator.o obj-$(CONFIG_REGULATOR_QCOM_LABIBB) += qcom-labibb-regulator.o +obj-$(CONFIG_REGULATOR_QCOM_PM8008) += qcom-pm8008-regulator.o obj-$(CONFIG_REGULATOR_QCOM_RPM) += qcom_rpm-regulator.o obj-$(CONFIG_REGULATOR_QCOM_RPMH) += qcom-rpmh-regulator.o obj-$(CONFIG_REGULATOR_QCOM_SMD_RPM) += qcom_smd-regulator.o diff --git a/drivers/regulator/qcom-pm8008-regulator.c b/drivers/regulator/qcom-pm8008-regulator.c new file mode 100644 index 0000000..38d258e --- /dev/null +++ b/drivers/regulator/qcom-pm8008-regulator.c @@ -0,0 +1,258 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Copyright (c) 2021, The Linux Foundation. All rights reserved. */ + +#include <linux/device.h> +#include <linux/interrupt.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> +#include <linux/regulator/driver.h> +#include <linux/regulator/machine.h> + +#define STARTUP_DELAY_USEC 20 +#define VSET_STEP_MV 8 +#define VSET_STEP_UV (VSET_STEP_MV * 1000) + +#define LDO_ENABLE_REG(base) ((base) + 0x46) +#define ENABLE_BIT BIT(7) + +#define LDO_STATUS1_REG(base) ((base) + 0x08) +#define VREG_READY_BIT BIT(7) + +#define LDO_VSET_LB_REG(base) ((base) + 0x40) + +#define LDO_STEPPER_CTL_REG(base) ((base) + 0x3b) +#define DEFAULT_VOLTAGE_STEPPER_RATE 38400 +#define STEP_RATE_MASK GENMASK(1, 0) + +struct regulator_data { + const char *name; + const char *supply_name; + int min_uv; + int max_uv; + int min_dropout_uv; + const struct linear_range *voltage_range; +}; + +struct pm8008_regulator { + struct device *dev; + struct regmap *regmap; + struct regulator_desc rdesc; + u16 base; + int step_rate; +}; + +static const struct linear_range nldo_ranges[] = { + REGULATOR_LINEAR_RANGE(528000, 0, 122, 8000), +}; + +static const struct linear_range pldo_ranges[] = { + REGULATOR_LINEAR_RANGE(1504000, 0, 237, 8000), +}; + +static const struct regulator_data reg_data[] = { + /* name parent min_uv max_uv headroom_uv */ + { "l1", "vdd_l1_l2", 528000, 1504000, 225000, nldo_ranges }, + { "l2", "vdd_l1_l2", 528000, 1504000, 225000, nldo_ranges }, + { "l3", "vdd_l3_l4", 1504000, 3400000, 200000, pldo_ranges }, + { "l4", "vdd_l3_l4", 1504000, 3400000, 200000, pldo_ranges }, + { "l5", "vdd_l5", 1504000, 3400000, 300000, pldo_ranges }, + { "l6", "vdd_l6", 1504000, 3400000, 300000, pldo_ranges }, + { "l7", "vdd_l7", 1504000, 3400000, 300000, pldo_ranges }, + { } +}; + +static int pm8008_regulator_get_voltage(struct regulator_dev *rdev) +{ + struct pm8008_regulator *pm8008_reg = rdev_get_drvdata(rdev); + __le16 mV; + int rc; + + rc = regmap_bulk_read(pm8008_reg->regmap, + LDO_VSET_LB_REG(pm8008_reg->base), (void *)&mV, 2); + if (rc < 0) { + dev_err(&rdev->dev, "failed to read regulator voltage rc=%d\n", rc); + return rc; + } + + return le16_to_cpu(mV) * 1000; +} + +static inline int pm8008_write_voltage(struct pm8008_regulator *pm8008_reg, + int mV) +{ + int rc; + u16 vset_raw; + + vset_raw = cpu_to_le16(mV); + + rc = regmap_bulk_write(pm8008_reg->regmap, + LDO_VSET_LB_REG(pm8008_reg->base), + (const void *)&vset_raw, sizeof(vset_raw)); + if (rc < 0) { + dev_err(pm8008_reg->dev, "failed to write voltage rc=%d\n", rc); + return rc; + } + + return 0; +} + +static int pm8008_regulator_set_voltage_time(struct regulator_dev *rdev, + int old_uV, int new_uv) +{ + struct pm8008_regulator *pm8008_reg = rdev_get_drvdata(rdev); + + return DIV_ROUND_UP(abs(new_uv - old_uV), pm8008_reg->step_rate); +} + +static int pm8008_regulator_set_voltage(struct regulator_dev *rdev, + unsigned int selector) +{ + struct pm8008_regulator *pm8008_reg = rdev_get_drvdata(rdev); + int rc, mV; + + /* voltage control register is set with voltage in millivolts */ + mV = DIV_ROUND_UP(regulator_list_voltage_linear_range(rdev, selector), + 1000); + if (mV < 0) + return mV; + + rc = pm8008_write_voltage(pm8008_reg, mV); + if (rc < 0) + return rc; + + dev_dbg(&rdev->dev, "voltage set to %d\n", mV * 1000); + return 0; +} + +static const struct regulator_ops pm8008_regulator_ops = { + .enable = regulator_enable_regmap, + .disable = regulator_disable_regmap, + .is_enabled = regulator_is_enabled_regmap, + .set_voltage_sel = pm8008_regulator_set_voltage, + .get_voltage = pm8008_regulator_get_voltage, + .list_voltage = regulator_list_voltage_linear, + .set_voltage_time = pm8008_regulator_set_voltage_time, +}; + +static int pm8008_regulator_of_parse(struct device_node *node, + const struct regulator_desc *desc, + struct regulator_config *config) +{ + struct pm8008_regulator *pm8008_reg = config->driver_data; + struct device *dev = config->dev; + int rc; + unsigned int reg; + + rc = of_property_read_u32(node, "regulator-min-dropout-voltage-microvolt", + &pm8008_reg->rdesc.min_dropout_uV); + if (rc) { + dev_err(dev, "failed to read min-dropout voltage rc=%d\n", rc); + return rc; + } + + /* get slew rate */ + rc = regmap_bulk_read(pm8008_reg->regmap, + LDO_STEPPER_CTL_REG(pm8008_reg->base), ®, 1); + if (rc < 0) { + dev_err(dev, "%s: failed to read step rate configuration rc=%d\n", + pm8008_reg->rdesc.name, rc); + return rc; + } + reg &= STEP_RATE_MASK; + pm8008_reg->step_rate = DEFAULT_VOLTAGE_STEPPER_RATE >> reg; + + return 0; +} + +static int pm8008_regulator_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct device_node *parent_node = pdev->dev.of_node; + struct device_node *child_node; + struct regulator_dev *rdev; + struct pm8008_regulator *pm8008_reg; + struct regmap *regmap; + struct regulator_config reg_config = {}; + const struct regulator_data *reg; + int rc; + u32 base; + + regmap = dev_get_regmap(dev->parent, NULL); + if (!regmap) { + dev_err(dev, "parent regmap is missing\n"); + return -EINVAL; + } + + for (reg = ®_data[0]; reg->name; reg++) { + pm8008_reg = devm_kzalloc(dev, sizeof(*pm8008_reg), GFP_KERNEL); + + pm8008_reg->regmap = regmap; + + child_node = of_get_child_by_name(parent_node, reg->name); + if (!child_node) { + dev_err(dev, "child node %s not found\n", reg->name); + return -ENODEV; + } + + pm8008_reg->dev = dev; + + rc = of_property_read_u32(child_node, "reg", &base); + if (rc < 0) { + dev_err(dev, "%s: failed to get regulator base rc=%d\n", + reg->name, rc); + return rc; + } + pm8008_reg->base = base; + + pm8008_reg->rdesc.type = REGULATOR_VOLTAGE; + pm8008_reg->rdesc.ops = &pm8008_regulator_ops; + pm8008_reg->rdesc.name = reg->name; + pm8008_reg->rdesc.supply_name = reg->supply_name; + pm8008_reg->rdesc.of_match = reg->name; + pm8008_reg->rdesc.of_parse_cb = pm8008_regulator_of_parse; + pm8008_reg->rdesc.uV_step = VSET_STEP_UV; + pm8008_reg->rdesc.min_uV = reg->min_uv; + pm8008_reg->rdesc.n_voltages + = ((reg->max_uv - reg->min_uv) + / pm8008_reg->rdesc.uV_step) + 1; + pm8008_reg->rdesc.linear_ranges = reg->voltage_range; + pm8008_reg->rdesc.n_linear_ranges = 1; + pm8008_reg->rdesc.enable_reg = LDO_ENABLE_REG(base); + pm8008_reg->rdesc.enable_mask = ENABLE_BIT; + pm8008_reg->rdesc.min_dropout_uV = reg->min_dropout_uv; + + reg_config.dev = dev; + reg_config.driver_data = pm8008_reg; + + rdev = devm_regulator_register(dev, &pm8008_reg->rdesc, ®_config); + if (IS_ERR(rdev)) { + rc = PTR_ERR(rdev); + dev_err(dev, "%s: failed to register regulator rc=%d\n", reg->name, rc); + return rc; + } + } + return 0; +} + +static const struct of_device_id pm8008_regulator_match_table[] = { + { .compatible = "qcom,pm8008-regulators", }, + { } +}; +MODULE_DEVICE_TABLE(of, pm8008_regulator_match_table); + +static struct platform_driver pm8008_regulator_driver = { + .driver = { + .name = "qcom,pm8008-regulators", + .of_match_table = pm8008_regulator_match_table, + }, + .probe = pm8008_regulator_probe, +}; + +module_platform_driver(pm8008_regulator_driver); + +MODULE_DESCRIPTION("Qualcomm PM8008 PMIC Regulator Driver"); +MODULE_LICENSE("GPL v2"); -- 2.7.4 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH V4 4/6] regulator: Add a regulator driver for the PM8008 PMIC 2021-11-19 9:42 ` [PATCH V4 4/6] regulator: Add a regulator driver for the PM8008 PMIC Satya Priya @ 2021-11-25 15:45 ` Mark Brown 2021-12-06 14:43 ` Satya Priya Kakitapalli (Temp) 0 siblings, 1 reply; 27+ messages in thread From: Mark Brown @ 2021-11-25 15:45 UTC (permalink / raw) To: Satya Priya Cc: Bjorn Andersson, Rob Herring, Liam Girdwood, swboyd, collinsd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 902 bytes --] On Fri, Nov 19, 2021 at 03:12:31PM +0530, Satya Priya wrote: > + for (reg = ®_data[0]; reg->name; reg++) { Why is this not just iterating from 0 to ARRAY_SIZE() - that's the more normal way to write things and doesn't require a terminator on the array. > + child_node = of_get_child_by_name(parent_node, reg->name); > + if (!child_node) { > + dev_err(dev, "child node %s not found\n", reg->name); > + return -ENODEV; > + } This could be pulled out of the array. I think you're also missing an of_node_put() on the child_node. > + rc = of_property_read_u32(child_node, "reg", &base); > + if (rc < 0) { > + dev_err(dev, "%s: failed to get regulator base rc=%d\n", > + reg->name, rc); > + return rc; > + } It's not clear to me why this in particular is being read out of the DT binding, I'd expect this to be in the array describing the regulator the same as everything else? [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V4 4/6] regulator: Add a regulator driver for the PM8008 PMIC 2021-11-25 15:45 ` Mark Brown @ 2021-12-06 14:43 ` Satya Priya Kakitapalli (Temp) 2021-12-06 15:09 ` Mark Brown 0 siblings, 1 reply; 27+ messages in thread From: Satya Priya Kakitapalli (Temp) @ 2021-12-06 14:43 UTC (permalink / raw) To: Mark Brown Cc: Bjorn Andersson, Rob Herring, Liam Girdwood, swboyd, collinsd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel On 11/25/2021 9:15 PM, Mark Brown wrote: > On Fri, Nov 19, 2021 at 03:12:31PM +0530, Satya Priya wrote: > >> + for (reg = ®_data[0]; reg->name; reg++) { > Why is this not just iterating from 0 to ARRAY_SIZE() - that's the more > normal way to write things and doesn't require a terminator on the > array. While changing some other things in the probe I made this change, but I think this is not necessary, I'll change it to iterate from 0 to ARRAY_SIZE() >> + child_node = of_get_child_by_name(parent_node, reg->name); >> + if (!child_node) { >> + dev_err(dev, "child node %s not found\n", reg->name); >> + return -ENODEV; >> + } > This could be pulled out of the array. Not sure what you meant here. could you elaborate a bit? > I think you're also missing an > of_node_put() on the child_node. I'll add it in the next version. >> + rc = of_property_read_u32(child_node, "reg", &base); >> + if (rc < 0) { >> + dev_err(dev, "%s: failed to get regulator base rc=%d\n", >> + reg->name, rc); >> + return rc; >> + } > It's not clear to me why this in particular is being read out of the DT > binding, I'd expect this to be in the array describing the regulator the > same as everything else? I thought that the base address would change with boards, but seems it doesn't change. I'll add this in the array. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V4 4/6] regulator: Add a regulator driver for the PM8008 PMIC 2021-12-06 14:43 ` Satya Priya Kakitapalli (Temp) @ 2021-12-06 15:09 ` Mark Brown 2021-12-20 10:44 ` Satya Priya Kakitapalli (Temp) 0 siblings, 1 reply; 27+ messages in thread From: Mark Brown @ 2021-12-06 15:09 UTC (permalink / raw) To: Satya Priya Kakitapalli (Temp) Cc: Bjorn Andersson, Rob Herring, Liam Girdwood, swboyd, collinsd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 535 bytes --] On Mon, Dec 06, 2021 at 08:13:57PM +0530, Satya Priya Kakitapalli (Temp) wrote: > On 11/25/2021 9:15 PM, Mark Brown wrote: > > On Fri, Nov 19, 2021 at 03:12:31PM +0530, Satya Priya wrote: > > > + child_node = of_get_child_by_name(parent_node, reg->name); > > > + if (!child_node) { > > > + dev_err(dev, "child node %s not found\n", reg->name); > > > + return -ENODEV; > > > + } > > This could be pulled out of the array. > Not sure what you meant here. could you elaborate a bit? Why is this in every iteration of the loop? [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V4 4/6] regulator: Add a regulator driver for the PM8008 PMIC 2021-12-06 15:09 ` Mark Brown @ 2021-12-20 10:44 ` Satya Priya Kakitapalli (Temp) 0 siblings, 0 replies; 27+ messages in thread From: Satya Priya Kakitapalli (Temp) @ 2021-12-20 10:44 UTC (permalink / raw) To: Mark Brown Cc: Bjorn Andersson, Rob Herring, Liam Girdwood, swboyd, collinsd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel On 12/6/2021 8:39 PM, Mark Brown wrote: > On Mon, Dec 06, 2021 at 08:13:57PM +0530, Satya Priya Kakitapalli (Temp) wrote: >> On 11/25/2021 9:15 PM, Mark Brown wrote: >>> On Fri, Nov 19, 2021 at 03:12:31PM +0530, Satya Priya wrote: >>>> + child_node = of_get_child_by_name(parent_node, reg->name); >>>> + if (!child_node) { >>>> + dev_err(dev, "child node %s not found\n", reg->name); >>>> + return -ENODEV; >>>> + } >>> This could be pulled out of the array. >> Not sure what you meant here. could you elaborate a bit? > Why is this in every iteration of the loop? Getting the child node here is not required anymore. This got carried from previous versions, I'll remove this. ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH V4 5/6] arm64: dts: qcom: pm8008: Add base dts file 2021-11-19 9:42 [PATCH V4 0/6] Add Qualcomm Technologies, Inc. PM8008 regulator driver Satya Priya ` (3 preceding siblings ...) 2021-11-19 9:42 ` [PATCH V4 4/6] regulator: Add a regulator driver for the PM8008 PMIC Satya Priya @ 2021-11-19 9:42 ` Satya Priya 2021-11-19 9:42 ` [PATCH V4 6/6] arm64: dts: qcom: sc7280: Add pm8008 regulators support for sc7280-idp Satya Priya 5 siblings, 0 replies; 27+ messages in thread From: Satya Priya @ 2021-11-19 9:42 UTC (permalink / raw) To: Bjorn Andersson, Rob Herring Cc: Liam Girdwood, Mark Brown, swboyd, collinsd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel, Satya Priya Add base DTS file for pm8008 with chip and ldo nodes. Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com> --- Changes in V4: - This is newly added in V4, to add all the pm8008 common stuff. arch/arm64/boot/dts/qcom/pm8008.dtsi | 57 ++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 arch/arm64/boot/dts/qcom/pm8008.dtsi diff --git a/arch/arm64/boot/dts/qcom/pm8008.dtsi b/arch/arm64/boot/dts/qcom/pm8008.dtsi new file mode 100644 index 0000000..e23478c --- /dev/null +++ b/arch/arm64/boot/dts/qcom/pm8008.dtsi @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: BSD-3-Clause +// Copyright (c) 2021, The Linux Foundation. All rights reserved. + +pm8008_chip: pm8008@8 { + compatible = "qcom,pm8008"; + reg = <0x8>; + #address-cells = <1>; + #size-cells = <0>; +}; + +pm8008_ldo: pm8008@9 { + compatible = "qcom,pm8008"; + reg = <0x9>; + #address-cells = <1>; + #size-cells = <0>; + + pm8008_regulators: regulators { + compatible = "qcom,pm8008-regulators"; + #address-cells = <1>; + #size-cells = <0>; + + pm8008_l1: l1@4000 { + reg = <0x4000>; + regulator-name = "pm8008_l1"; + }; + + pm8008_l2: l2@4100 { + reg = <0x4100>; + regulator-name = "pm8008_l2"; + }; + + pm8008_l3: l3@4200 { + reg = <0x4200>; + regulator-name = "pm8008_l3"; + }; + + pm8008_l4: l4@4300 { + reg = <0x4300>; + regulator-name = "pm8008_l4"; + }; + + pm8008_l5: l5@4400 { + reg = <0x4400>; + regulator-name = "pm8008_l5"; + }; + + pm8008_l6: l6@4500 { + reg = <0x4500>; + regulator-name = "pm8008_l6"; + }; + + pm8008_l7: l7@4600 { + reg = <0x4600>; + regulator-name = "pm8008_l7"; + }; + }; +}; -- 2.7.4 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH V4 6/6] arm64: dts: qcom: sc7280: Add pm8008 regulators support for sc7280-idp 2021-11-19 9:42 [PATCH V4 0/6] Add Qualcomm Technologies, Inc. PM8008 regulator driver Satya Priya ` (4 preceding siblings ...) 2021-11-19 9:42 ` [PATCH V4 5/6] arm64: dts: qcom: pm8008: Add base dts file Satya Priya @ 2021-11-19 9:42 ` Satya Priya 5 siblings, 0 replies; 27+ messages in thread From: Satya Priya @ 2021-11-19 9:42 UTC (permalink / raw) To: Bjorn Andersson, Rob Herring Cc: Liam Girdwood, Mark Brown, swboyd, collinsd, subbaram, Das Srinagesh, linux-arm-msm, Lee Jones, devicetree, linux-kernel, Satya Priya Add pm8008 regulators support for sc7280 idp. Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com> --- Changes in V2: - As per Stephen's comments, replaced '_' with '-' for node names. Changes in V3: - Changed the regulator node names as l1, l2 etc - Changed "pm8008-regulators" to "regulators" - Changed "qcom,min-dropout-voltage" to "regulator-min-dropout-voltage-microvolt" Changes in V4: - Moved all common stuff to pm8008.dtsi and added board specific configurations here. arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 73 ++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi index d623d71..f86368d 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi @@ -309,6 +309,69 @@ }; }; +&i2c1 { + #address-cells = <1>; + #size-cells = <0>; + status = "okay"; + + #include "pm8008.dtsi" +}; + +&pm8008_chip { + pinctrl-names = "default"; + pinctrl-0 = <&pm8008_active>; +}; + +&pm8008_regulators { + vdd_l1_l2-supply = <&vreg_s8b_1p2>; + vdd_l3_l4-supply = <&vreg_s1b_1p8>; + vdd_l5-supply = <&vreg_bob>; + vdd_l6-supply = <&vreg_bob>; + vdd_l7-supply = <&vreg_bob>; +}; + +&pm8008_l1 { + regulator-min-microvolt = <950000>; + regulator-max-microvolt = <1300000>; + regulator-min-dropout-voltage-microvolt = <96000>; +}; + +&pm8008_l2 { + regulator-min-microvolt = <950000>; + regulator-max-microvolt = <1250000>; + regulator-min-dropout-voltage-microvolt = <24000>; +}; + +&pm8008_l3 { + regulator-min-microvolt = <1650000>; + regulator-max-microvolt = <3000000>; + regulator-min-dropout-voltage-microvolt = <224000>; +}; + +&pm8008_l4 { + regulator-min-microvolt = <1504000>; + regulator-max-microvolt = <1600000>; + regulator-min-dropout-voltage-microvolt = <0>; +}; + +&pm8008_l5 { + regulator-min-microvolt = <2600000>; + regulator-max-microvolt = <3000000>; + regulator-min-dropout-voltage-microvolt = <104000>; +}; + +&pm8008_l6 { + regulator-min-microvolt = <2600000>; + regulator-max-microvolt = <3000000>; + regulator-min-dropout-voltage-microvolt = <112000>; +}; + +&pm8008_l7 { + regulator-min-microvolt = <3000000>; + regulator-max-microvolt = <3544000>; + regulator-min-dropout-voltage-microvolt = <96000>; +}; + &qfprom { vcc-supply = <&vreg_l1c_1p8>; }; @@ -437,6 +500,16 @@ }; }; +&pm8350c_gpios { + pm8008_active: pm8008_active { + pins = "gpio4"; + function = "normal"; + bias-disable; + output-high; + power-source = <0>; + }; +}; + &qspi_cs0 { bias-disable; }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 27+ messages in thread
end of thread, other threads:[~2022-01-11 13:59 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-19 9:42 [PATCH V4 0/6] Add Qualcomm Technologies, Inc. PM8008 regulator driver Satya Priya 2021-11-19 9:42 ` [PATCH V4 1/6] dt-bindings: regulator: Add "regulator-min-dropout-voltage-microvolt" Satya Priya 2021-11-25 15:17 ` Mark Brown 2021-12-06 13:03 ` Satya Priya Kakitapalli (Temp) 2021-12-06 18:25 ` Mark Brown 2021-12-07 15:06 ` Satya Priya Kakitapalli (Temp) 2021-12-07 15:19 ` Mark Brown 2021-12-09 0:56 ` David Collins 2021-12-10 21:11 ` Mark Brown 2022-01-03 14:35 ` Satya Priya Kakitapalli (Temp) 2022-01-04 14:54 ` Mark Brown 2021-11-19 9:42 ` [PATCH V4 2/6] dt-bindings: regulator: Add pm8008 regulator bindings Satya Priya 2021-11-25 15:24 ` Mark Brown 2021-12-06 13:43 ` Satya Priya Kakitapalli (Temp) 2021-12-06 13:47 ` Mark Brown 2021-12-20 12:44 ` Satya Priya Kakitapalli (Temp) [not found] ` <07dc5ba4-790b-0cb2-bc3e-2ce8d7e3e09d@quicinc.com> 2022-01-10 14:21 ` Mark Brown 2022-01-11 12:15 ` Satya Priya Kakitapalli (Temp) 2022-01-11 13:59 ` Mark Brown 2021-11-19 9:42 ` [PATCH V4 3/6] dt-bindings: mfd: pm8008: Add pm8008 regulator node Satya Priya 2021-11-19 9:42 ` [PATCH V4 4/6] regulator: Add a regulator driver for the PM8008 PMIC Satya Priya 2021-11-25 15:45 ` Mark Brown 2021-12-06 14:43 ` Satya Priya Kakitapalli (Temp) 2021-12-06 15:09 ` Mark Brown 2021-12-20 10:44 ` Satya Priya Kakitapalli (Temp) 2021-11-19 9:42 ` [PATCH V4 5/6] arm64: dts: qcom: pm8008: Add base dts file Satya Priya 2021-11-19 9:42 ` [PATCH V4 6/6] arm64: dts: qcom: sc7280: Add pm8008 regulators support for sc7280-idp Satya Priya
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).