[v2,1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings

Introduce bindings for RPMh regulator devices found on some
Qualcomm Technlogies, Inc. SoCs.  These devices allow a given
processor within the SoC to make PMIC regulator requests which
are aggregated within the RPMh hardware block along with requests
from other processors in the SoC to determine the final PMIC
regulator hardware state.

Signed-off-by: David Collins <collinsd@codeaurora.org>
---
 .../bindings/regulator/qcom,rpmh-regulator.txt     | 207 +++++++++++++++++++++
 .../dt-bindings/regulator/qcom,rpmh-regulator.h    |  36 ++++
 2 files changed, 243 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
 create mode 100644 include/dt-bindings/regulator/qcom,rpmh-regulator.h

On Fri, Apr 13, 2018 at 07:50:34PM -0700, David Collins wrote:
> Introduce bindings for RPMh regulator devices found on some
> Qualcomm Technlogies, Inc. SoCs.  These devices allow a given
> processor within the SoC to make PMIC regulator requests which
> are aggregated within the RPMh hardware block along with requests
> from other processors in the SoC to determine the final PMIC
> regulator hardware state.
> 
> Signed-off-by: David Collins <collinsd@codeaurora.org>
> ---
>  .../bindings/regulator/qcom,rpmh-regulator.txt     | 207 +++++++++++++++++++++
>  .../dt-bindings/regulator/qcom,rpmh-regulator.h    |  36 ++++
>  2 files changed, 243 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
>  create mode 100644 include/dt-bindings/regulator/qcom,rpmh-regulator.h
> 
> diff --git a/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
> new file mode 100644
> index 0000000..69748ea
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
> @@ -0,0 +1,207 @@
> +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 four
> +parameters for a given regulator: enable state, output voltage, operating mode,
> +and minimum headroom voltage.  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:
> +	PM8998:		smps1 - smps13, ldo1 - ldo28, lvs1 - lvs2
> +	PMI8998:	bob
> +	PM8005:		smps1 - smps4
> +
> +========================
> +First Level Nodes - PMIC
> +========================
> +
> +- compatible
> +	Usage:      required
> +	Value type: <string>
> +	Definition: Must be one of: "qcom,pm8998-rpmh-regulators",
> +		    "qcom,pmi8998-rpmh-regulators" or
> +		    "qcom,pm8005-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

Use '-' rather than '_' on all these.

> +- vdd_s2-supply
> +- vdd_s3-supply
> +- vdd_s4-supply
> +- 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
> +- vdd_lvs1_lvs2-supply
> +- vdd_lvs1_lvs2-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
> +
> +- vdd_s1-supply
> +- vdd_s2-supply
> +- vdd_s3-supply
> +- vdd_s4-supply

Listed twice?

> +	Usage:      optional (PM8005 only)
> +	Value type: <phandle>
> +	Definition: phandle of the parent supply regulator of one or more of the
> +		    regulators for this PMIC.
> +
> +===============================
> +Second Level Nodes - Regulators
> +===============================
> +
> +- qcom,regulator-initial-voltage
> +	Usage:      optional; VRM regulators only
> +	Value type: <u32>
> +	Definition: Specifies the initial voltage in microvolts to request for a
> +		    VRM regulator.
> +
> +- regulator-initial-mode

Vendor prefix?

> +	Usage:      optional; VRM regulators only
> +	Value type: <u32>
> +	Definition: Specifies the initial mode to request for a VRM regulator.
> +		    Supported values are RPMH_REGULATOR_MODE_* which are defined
> +		    in [1] (i.e. 0 to 3).  This property may be specified even
> +		    if the regulator-allow-set-load property is not specified.
> +
> +- qcom,allowed-drms-modes
> +	Usage:      required if regulator-allow-set-load is specified;
> +		    VRM regulators only
> +	Value type: <prop-encoded-array>
> +	Definition: A list of integers specifying the PMIC regulator modes which
> +		    can be configured at runtime based upon consumer load needs.
> +		    Supported values are RPMH_REGULATOR_MODE_* which are defined
> +		    in [1] (i.e. 0 to 3).
> +
> +- qcom,drms-mode-threshold-currents
> +	Usage:      required if regulator-allow-set-load is specified;
> +		    VRM regulators only
> +	Value type: <prop-encoded-array>
> +	Definition: A list of integers specifying the maximum allowed load
> +		    current in microamps for each of the modes listed in
> +		    qcom,allowed-drms-modes (matched 1-to-1 in order).  Elements
> +		    must be specified in order from lowest to highest value.
> +
> +- qcom,headroom-voltage
> +	Usage:      optional; VRM regulators only
> +	Value type: <u32>
> +	Definition: Specifies the headroom voltage in microvolts to request for
> +		    a VRM regulator.  RPMh hardware automatically ensures that
> +		    the parent of this regulator outputs a voltage high enough
> +		    to satisfy the requested headroom.  Supported values are
> +		    0 to 511000.
> +
> +- 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.txt
> +may also be used.
> +
> +[1] include/dt-bindings/regulator/qcom,rpmh-regulator.h
> +
> +========
> +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>;
> +			qcom,regulator-initial-voltage = <1100000>;
> +		};
> +
> +		pm8998_s5: smps5 {
> +			regulator-min-microvolt = <1904000>;
> +			regulator-max-microvolt = <2040000>;
> +			qcom,regulator-initial-voltage = <1904000>;
> +		};
> +
> +		ldo7 {
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <1800000>;
> +			qcom,regulator-initial-voltage = <1800000>;
> +			qcom,headroom-voltage = <56000>;
> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_LPM>;
> +			regulator-allow-set-load;
> +			qcom,allowed-drms-modes =
> +				<RPMH_REGULATOR_MODE_LPM
> +				 RPMH_REGULATOR_MODE_HPM>;
> +			qcom,drms-mode-threshold-currents = <10000 1000000>;
> +		};
> +
> +		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>;
> +			qcom,regulator-initial-voltage = <3312000>;
> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_AUTO>;
> +		};
> +	};
> +};
> diff --git a/include/dt-bindings/regulator/qcom,rpmh-regulator.h b/include/dt-bindings/regulator/qcom,rpmh-regulator.h
> new file mode 100644
> index 0000000..4378c4b
> --- /dev/null
> +++ b/include/dt-bindings/regulator/qcom,rpmh-regulator.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2018, The Linux Foundation. All rights reserved. */
> +
> +#ifndef __QCOM_RPMH_REGULATOR_H
> +#define __QCOM_RPMH_REGULATOR_H
> +
> +/*
> + * These mode constants may be used for regulator-initial-mode and
> + * qcom,allowed-drms-modes properties of an RPMh regulator device tree node.
> + * Each type of regulator supports a subset of the possible modes.
> + *
> + * %RPMH_REGULATOR_MODE_RET:	Retention mode in which only an extremely small
> + *				load current is allowed.  This mode is supported
> + *				by LDO and SMPS type regulators.
> + * %RPMH_REGULATOR_MODE_LPM:	Low power mode in which a small load current is
> + *				allowed.  This mode corresponds to PFM for SMPS
> + *				and BOB type regulators.  This mode is supported
> + *				by LDO, HFSMPS, BOB, and PMIC4 FTSMPS type
> + *				regulators.
> + * %RPMH_REGULATOR_MODE_AUTO:	Auto mode in which the regulator hardware
> + *				automatically switches between LPM and HPM based
> + *				upon the real-time load current.  This mode is
> + *				supported by HFSMPS, BOB, and PMIC4 FTSMPS type
> + *				regulators.
> + * %RPMH_REGULATOR_MODE_HPM:	High power mode in which the full rated current
> + *				of the regulator is allowed.  This mode
> + *				corresponds to PWM for SMPS and BOB type
> + *				regulators.  This mode is supported by all types
> + *				of regulators.
> + */
> +#define RPMH_REGULATOR_MODE_RET		0
> +#define RPMH_REGULATOR_MODE_LPM		1
> +#define RPMH_REGULATOR_MODE_AUTO	2
> +#define RPMH_REGULATOR_MODE_HPM		3
> +
> +#endif
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hello Rob,

On 04/16/2018 01:57 PM, Rob Herring wrote:
> On Fri, Apr 13, 2018 at 07:50:34PM -0700, David Collins wrote:
>> Introduce bindings for RPMh regulator devices found on some
>> Qualcomm Technlogies, Inc. SoCs.  These devices allow a given
>> processor within the SoC to make PMIC regulator requests which
>> are aggregated within the RPMh hardware block along with requests
>> from other processors in the SoC to determine the final PMIC
>> regulator hardware state.
>>
>> Signed-off-by: David Collins <collinsd@codeaurora.org>
>> ---
>>  .../bindings/regulator/qcom,rpmh-regulator.txt     | 207 +++++++++++++++++++++
>>  .../dt-bindings/regulator/qcom,rpmh-regulator.h    |  36 ++++
>> [...]
>> +++ b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
>> [...]
>> +- vdd_s1-supply
> 
> Use '-' rather than '_' on all these.

I will change this on the next patch set.


>> +- vdd_s2-supply
>> +- vdd_s3-supply
>> +- vdd_s4-supply
>> +- 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
>> +- vdd_lvs1_lvs2-supply
>> +- vdd_lvs1_lvs2-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
>> +
>> +- vdd_s1-supply
>> +- vdd_s2-supply
>> +- vdd_s3-supply
>> +- vdd_s4-supply
> 
> Listed twice?
> 
>> +	Usage:      optional (PM8005 only)
>> +	Value type: <phandle>
>> +	Definition: phandle of the parent supply regulator of one or more of the
>> +		    regulators for this PMIC.

I listed vdd_s1-supply to vdd_s4-supply here twice because I wanted to
group together the supplies supported by each PMIC.  PM8005 only supports
these 4 regulator supplies.  PM8998 supports these 4 along with many
others.  How else would suggest that I capture this per-PMIC support
information?


>> +===============================
>> +Second Level Nodes - Regulators
>> +===============================
>> +
>> +- qcom,regulator-initial-voltage
>> +	Usage:      optional; VRM regulators only
>> +	Value type: <u32>
>> +	Definition: Specifies the initial voltage in microvolts to request for a
>> +		    VRM regulator.
>> +
>> +- regulator-initial-mode
> 
> Vendor prefix?

No, this does not need a vendor prefix.  The regulator-initial-mode
property is already defined for all regulator devices [1]:

    regulator-initial-mode: initial operating mode. The set of possible
    operating modes depends on the capabilities of every hardware so each
    device binding documentation explains which values the regulator
    supports.

The values supported by the property are hardware specific and thus must
be listed in device specific binding files like this one.  Here is a
previously merged example: [2].

Thanks,
David

[1]:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/regulator/regulator.txt?h=v4.17-rc1#n59
[2]:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/regulator/twl-regulator.txt?h=v4.17-rc1#n60

Hi,

On Fri, Apr 13, 2018 at 7:50 PM, David Collins <collinsd@codeaurora.org> wrote:
> Introduce bindings for RPMh regulator devices found on some
> Qualcomm Technlogies, Inc. SoCs.  These devices allow a given
> processor within the SoC to make PMIC regulator requests which
> are aggregated within the RPMh hardware block along with requests
> from other processors in the SoC to determine the final PMIC
> regulator hardware state.
>
> Signed-off-by: David Collins <collinsd@codeaurora.org>
> ---
>  .../bindings/regulator/qcom,rpmh-regulator.txt     | 207 +++++++++++++++++++++
>  .../dt-bindings/regulator/qcom,rpmh-regulator.h    |  36 ++++
>  2 files changed, 243 insertions(+)

I noticed that "qcom,rpmh-resource-type" is now gone.  Not needed
anymore?  Oh, I see.  Stephen said to add it when it's needed.  OK,
fine.


> +===============================
> +Second Level Nodes - Regulators
> +===============================
> +
> +- qcom,regulator-initial-voltage

nit: regulator framework tends to include "microvolt" in the name to
make it really obvious in the device tree what the units are.  Can you
do that too?


> +- qcom,drms-mode-threshold-currents

Could use microamp in the name of the property too...


> +                       qcom,allowed-drms-modes =
> +                               <RPMH_REGULATOR_MODE_LPM
> +                                RPMH_REGULATOR_MODE_HPM>;
> +                       qcom,drms-mode-threshold-currents = <10000 1000000>;

optional nit: to make it match downstream drivers, does it make sense
to change this to:

  <9999 999999>

...so if a driver used to request exactly 10000 mA that it will end up
with the same mode (no idea if drivers actually do that).


-Doug
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 04/17/2018 01:06 PM, Doug Anderson wrote:
> [...]
>> +- qcom,regulator-initial-voltage
> 
> nit: regulator framework tends to include "microvolt" in the name to
> make it really obvious in the device tree what the units are.  Can you
> do that too?

Sure, I'll change the name to be: qcom,regulator-initial-microvolt.


>> +- qcom,drms-mode-threshold-currents
> 
> Could use microamp in the name of the property too...

Ok, I'll change the name to be: qcom,drms-mode-max-microamps.


>> +                       qcom,allowed-drms-modes =
>> +                               <RPMH_REGULATOR_MODE_LPM
>> +                                RPMH_REGULATOR_MODE_HPM>;
>> +                       qcom,drms-mode-threshold-currents = <10000 1000000>;
> 
> optional nit: to make it match downstream drivers, does it make sense
> to change this to:
> 
>   <9999 999999>
> 
> ...so if a driver used to request exactly 10000 mA that it will end up
> with the same mode (no idea if drivers actually do that).

I'd prefer to leave the example with <10000 1000000> as it looks cleaner
to me and the example numbers are arbitrary.  It would also be good to use
<10000 1000000> in actual board DT files.  We can address consumers
expecting legacy behavior for 10000 uA requests as needed.

Take care,
David

Hi,

On Fri, Apr 13, 2018 at 7:50 PM, David Collins <collinsd@codeaurora.org> wrote:
> +- vdd_l26-supply
> +- vdd_lvs1_lvs2-supply
> +- vdd_lvs1_lvs2-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.

One small additional nit here is that "vdd_lvs1_lvs2-supply" is listed twice.

Also on the schematics (and in the PM8998 datasheet) I have this is
"VIN_LVS_1_2".  It seems like you should be consistent here and call
this "vin-lvs-1-2-supply".

-Doug
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 05/02/2018 09:37 AM, Doug Anderson wrote:
> On Fri, Apr 13, 2018 at 7:50 PM, David Collins <collinsd@codeaurora.org> wrote:
>> +- vdd_l26-supply
>> +- vdd_lvs1_lvs2-supply
>> +- vdd_lvs1_lvs2-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.
> 
> One small additional nit here is that "vdd_lvs1_lvs2-supply" is listed twice.

I'll remove the duplicate.


> Also on the schematics (and in the PM8998 datasheet) I have this is
> "VIN_LVS_1_2".  It seems like you should be consistent here and call
> this "vin-lvs-1-2-supply".

I was trying to keep the naming consistent within device tree binding
documentation for LVS vs LDO and SMPS (e.g. 'vdd' vs 'vin' prefix).  I
suppose that I can change this to match the hardware documentation pin
name.  I can also change '_' to '-' in the supply names if that is preferred.

Take care,
David

Hi,

On Wed, May 2, 2018 at 5:13 PM, David Collins <collinsd@codeaurora.org> wrote:
> On 05/02/2018 09:37 AM, Doug Anderson wrote:
>> On Fri, Apr 13, 2018 at 7:50 PM, David Collins <collinsd@codeaurora.org> wrote:
>>> +- vdd_l26-supply
>>> +- vdd_lvs1_lvs2-supply
>>> +- vdd_lvs1_lvs2-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.
>>
>> One small additional nit here is that "vdd_lvs1_lvs2-supply" is listed twice.
>
> I'll remove the duplicate.
>
>
>> Also on the schematics (and in the PM8998 datasheet) I have this is
>> "VIN_LVS_1_2".  It seems like you should be consistent here and call
>> this "vin-lvs-1-2-supply".
>
> I was trying to keep the naming consistent within device tree binding
> documentation for LVS vs LDO and SMPS (e.g. 'vdd' vs 'vin' prefix).  I
> suppose that I can change this to match the hardware documentation pin
> name.  I can also change '_' to '-' in the supply names if that is preferred.

I'd rather it match the docs.  I personally have no idea for why the
writer of the docs used "vdd" vs. "vin", but even if they had no good
reason matching the docs makes this searchable.

As far as the "_" to "-", Rob asked for that earlier in this thread
and you says "I will change this on the next patch set."  In general
the desire to convert "_" to "-" makes this less searchable (since the
docs use "_"), but that's the way device tree guys want it so so c'est
la vie.  Luckily it's easily to mentally change the "-"s back to "_"s
when searching...

-Doug
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html