Re: [PATCH v2 05/11] mfd: devicetree: bindings: Add Qualcomm SMD based RPM DT binding

Lee Jones
2015-07-07T12:16:24Z

FAO Mark and DT chaps,

> From: Bjorn Andersson <bjorn.andersson@sonymobile.com>
>
> Add binding documentation for the Qualcomm Resource Power Manager (RPM)
> using shared memory (Qualcomm SMD) as transport mechanism. This is found
> in 8974 and newer based devices.
>
> The binding currently describes the rpm itself and the regulator
> subnodes.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> ---
> .../devicetree/bindings/mfd/qcom-rpm-smd.txt | 117 +++++++++++++++++++++
> include/dt-bindings/mfd/qcom-smd-rpm.h | 28 +++++
> 2 files changed, 145 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/qcom-rpm-smd.txt
> create mode 100644 include/dt-bindings/mfd/qcom-smd-rpm.h
>
> diff --git a/Documentation/devicetree/bindings/mfd/qcom-rpm-smd.txt b/Documentation/devicetree/bindings/mfd/qcom-rpm-smd.txt
> new file mode 100644
> index 0000000..e27f5c4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/qcom-rpm-smd.txt
> @@ -0,0 +1,117 @@
> +Qualcomm Resource Power Manager (RPM) over SMD
> +
> +This driver is used to interface with the Resource Power Manager (RPM) found in
> +various Qualcomm platforms. The RPM allows each component in the system to vote
> +for state of the system resources, such as clocks, regulators and bus
> +frequencies.
> +
> +- compatible:
> + Usage: required
> + Value type: <string>
> + Definition: must be one of:
> + "qcom,rpm-msm8974"
> +
> +- qcom,smd-channels:
> + Usage: required
> + Value type: <stringlist>
> + Definition: Shared Memory channel used for communication with the RPM

This is going to require a DT Ack.

Also, I don't see it being used anywhere.

> += SUBDEVICES
> +
> +The RPM exposes resources to its subnodes. The below bindings specify the set
> +of valid subnodes that can operate on these resources.
> +
> +== Regulators
> +
> +Regulator nodes are identified by their compatible:
> +
> +- compatible:
> + Usage: required
> + Value type: <string>
> + Definition: must be one of:
> + "qcom,rpm-pm8841-regulators"
> + "qcom,rpm-pm8941-regulators"
> +
> +- vdd_s1-supply:
> +- vdd_s2-supply:
> +- vdd_s3-supply:
> +- vdd_s4-supply:
> +- vdd_s5-supply:
> +- vdd_s6-supply:
> +- vdd_s7-supply:
> +- vdd_s8-supply:
> + Usage: optional (pm8841 only)
> + Value type: <phandle>
> + Definition: reference to regulator supplying the input pin, as
> + described in the data sheet
> +
> +- vdd_s1-supply:
> +- vdd_s2-supply:
> +- vdd_s3-supply:
> +- vdd_l1_l3-supply:
> +- vdd_l2_lvs1_2_3-supply:
> +- vdd_l4_l11-supply:
> +- vdd_l5_l7-supply:
> +- vdd_l6_l12_l14_l15-supply:
> +- vdd_l8_l16_l18_l19-supply:
> +- vdd_l9_l10_l17_l22-supply:
> +- vdd_l13_l20_l23_l24-supply:
> +- vdd_l21-supply:
> +- vin_5vs-supply:
> + Usage: optional (pm8941 only)
> + Value type: <phandle>
> + Definition: reference to regulator supplying the input pin, as
> + described in the data sheet
> +
> +The regulator node houses sub-nodes for each regulator within the device. Each
> +sub-node is identified using the node's name, with valid values listed for each
> +of the pmics below.
> +
> +pm8841:
> + s1, s2, s3, s4, s5, s6, s7, s8
> +
> +pm8941:
> + s1, s2, s3, s4, l1, l2, l3, l4, l5, l6, l7, l8, l9, l10, l11, l12, l13,
> + l14, l15, l16, l17, l18, l19, l20, l21, l22, l23, l24, lvs1, lvs2,
> + lvs3, 5vs1, 5vs2
> +
> +The content of each sub-node is defined by the standard binding for regulators -
> +see regulator.txt.

s-regulator.txt-../regulator/regulator.txt-

> +
> += EXAMPLE
> +
> + smd {
> + compatible = "qcom,smd";

Is an SMD (Shared Memory Device?) real hardware?

> + rpm {
> + interrupts = <0 168 1>;
> + qcom,ipc = <&apcs 8 0>;
> + qcom,smd-edge = <15>;

The child node won't probe without a compatible string. Shouldn't
"qcom,rpm-msm8974" be in here instead?

> + rpm_requests {

This node appears to be undocumented.

Does it represent real h/w?

> + compatible = "qcom,rpm-msm8974";
> + qcom,smd-channels = "rpm_requests";
> +
> + pm8941-regulators {
> + compatible = "qcom,rpm-pm8941-regulators";
> + vdd_l13_l20_l23_l24-supply = <&pm8941_boost>;

I'd like Mark to glance at this.

> + pm8941_s3: s3 {
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;

Aren't these fixed regulators?

> + };
> +
> + pm8941_boost: s4 {
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + };
> +
> + pm8941_l20: l20 {
> + regulator-min-microvolt = <2950000>;
> + regulator-max-microvolt = <2950000>;
> + };
> + };
> + };
> + };
> + };
> +
> diff --git a/include/dt-bindings/mfd/qcom-smd-rpm.h b/include/dt-bindings/mfd/qcom-smd-rpm.h
> new file mode 100644
> index 0000000..890ca52
> --- /dev/null
> +++ b/include/dt-bindings/mfd/qcom-smd-rpm.h
> @@ -0,0 +1,28 @@
> +/*
> + * This header provides constants for the Qualcomm RPM bindings.
> + */
> +
> +#ifndef _DT_BINDINGS_MFD_QCOM_SMD_RPM_H
> +#define _DT_BINDINGS_MFD_QCOM_SMD_RPM_H
> +
> +/*
> + * Constants used for addressing resources in the RPM.
> + */
> +#define QCOM_SMD_RPM_BUS_CLK 0x316b6c63
> +#define QCOM_SMD_RPM_BUS_MASTER 0x73616d62
> +#define QCOM_SMD_RPM_BUS_SLAVE 0x766c7362
> +#define QCOM_SMD_RPM_CLK_BUF_A 0x616B6C63
> +#define QCOM_SMD_RPM_LDOA 0x616f646c
> +#define QCOM_SMD_RPM_LDOB 0x626F646C
> +#define QCOM_SMD_RPM_MEM_CLK 0x326b6c63
> +#define QCOM_SMD_RPM_MISC_CLK 0x306b6c63
> +#define QCOM_SMD_RPM_NCPA 0x6170636E
> +#define QCOM_SMD_RPM_NCPB 0x6270636E
> +#define QCOM_SMD_RPM_OCMEM_PWR 0x706d636f
> +#define QCOM_SMD_RPM_QPIC_CLK 0x63697071
> +#define QCOM_SMD_RPM_SMPA 0x61706d73
> +#define QCOM_SMD_RPM_SMPB 0x62706d73
> +#define QCOM_SMD_RPM_SPDM 0x63707362
> +#define QCOM_SMD_RPM_VSA 0x00617376
> +
> +#endif

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog