| Message ID | 1637314953-4215-3-git-send-email-quic_c_skakit@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add Qualcomm Technologies, Inc. PM8008 regulator driver | expand |
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.
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?
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.
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.
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.
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.
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.
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 +...
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