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

[David Collins <collinsd@codeaurora.org>]

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

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project