| Message ID | 1465918259-11138-4-git-send-email-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/14, Linus Walleij wrote: > diff --git a/arch/arm/boot/dts/qcom-msm8660.dtsi b/arch/arm/boot/dts/qcom-msm8660.dtsi > index a5a38820554a..51d3c9e70617 100644 > --- a/arch/arm/boot/dts/qcom-msm8660.dtsi > +++ b/arch/arm/boot/dts/qcom-msm8660.dtsi > @@ -215,6 +215,95 @@ > + > + rpmcc: clock-controller { > + compatible = "qcom,rpmcc-apq8660", "qcom,rpmcc"; > + #clock-cells = <1>; > + }; This driver isn't merged yet, but ok. > + > + pm8901-regulators { I'd rather have regulators@8901 and regulators@8058. That way the node name is generic per ePAPR suggestions. > + compatible = "qcom,rpm-pm8901-regulators"; > + > + pm8901_l0: l0 {}; > + pm8901_l1: l1 {}; > + pm8901_l2: l2 {}; > + pm8901_l3: l3 {}; > + pm8901_l4: l4 {}; > + pm8901_l5: l5 {}; > + pm8901_l6: l6 {}; > + > + /* S0 and S1 Handled as SAW regulators by SPM */ > + pm8901_s2: s2 {}; > + pm8901_s3: s3 {}; > + pm8901_s4: s4 {}; > + > + pm8901_lvs0: lvs0 {}; > + pm8901_lvs1: lvs1 {}; > + pm8901_lvs2: lvs2 {}; > + pm8901_lvs3: lvs3 {}; > + > + pm8058_mvs: mvs {}; pm8901_mvs? > + }; > + > + pm8058-regulators { > + compatible = "qcom,rpm-pm8058-regulators"; > + > + pm8058_l0: l0 {}; > + pm8058_l1: l1 {}; > + pm8058_l2: l2 {}; > + pm8058_l3: l3 {}; > + pm8058_l4: l4 {}; > + pm8058_l5: l5 {}; > + pm8058_l6: l6 {}; > + pm8058_l7: l7 {}; > + pm8058_l8: l8 {}; > + pm8058_l9: l9 {}; > + pm8058_l10: l10 {}; > + pm8058_l11: l11 {}; > + pm8058_l12: l12 {}; > + pm8058_l13: l13 {}; > + pm8058_l14: l14 {}; > + pm8058_l15: l15 {}; > + pm8058_l16: l16 {}; > + pm8058_l17: l17 {}; > + pm8058_l18: l18 {}; > + pm8058_l19: l19 {}; > + pm8058_l20: l20 {}; > + pm8058_l21: l21 {}; > + pm8058_l22: l22 {}; > + pm8058_l23: l23 {}; > + pm8058_l24: l24 {}; > + pm8058_l25: l25 {}; > + > + pm8058_s0: s0 {}; > + pm8058_s1: s1 {}; Just leave these out? RPM shouldn't be touching s0 or s1. > + pm8058_s2: s2 {}; > + pm8058_s3: s3 {}; > + pm8058_s4: s4 {}; > + > + pm8058_lvs0: lvs0 {}; > + pm8058_lvs1: lvs1 {}; > + > + pm8058_ncp: ncp {}; > + }; > + }; > +
On Tue 14 Jun 11:55 PDT 2016, Stephen Boyd wrote: > On 06/14, Linus Walleij wrote: > > diff --git a/arch/arm/boot/dts/qcom-msm8660.dtsi b/arch/arm/boot/dts/qcom-msm8660.dtsi [..] > > + > > + pm8901-regulators { > > I'd rather have regulators@8901 and regulators@8058. That way the > node name is generic per ePAPR suggestions. > I agree, but unfortunately we're not allowed to do that without a matching "reg = <>" anymore. Regards, Bjorn
On 06/14, Bjorn Andersson wrote: > On Tue 14 Jun 11:55 PDT 2016, Stephen Boyd wrote: > > > On 06/14, Linus Walleij wrote: > > > diff --git a/arch/arm/boot/dts/qcom-msm8660.dtsi b/arch/arm/boot/dts/qcom-msm8660.dtsi > [..] > > > + > > > + pm8901-regulators { > > > > I'd rather have regulators@8901 and regulators@8058. That way the > > node name is generic per ePAPR suggestions. > > > > I agree, but unfortunately we're not allowed to do that without a > matching "reg = <>" anymore. Oh does dtc blow up now?
On Tue, Jun 14, 2016 at 11:55:17AM -0700, Stephen Boyd wrote: > On 06/14, Linus Walleij wrote: > > + > > + pm8901-regulators { > > I'd rather have regulators@8901 and regulators@8058. That way the > node name is generic per ePAPR suggestions. But ePAPR requires reg= if you specify a unit-address part of the node name.
On Tue, Jun 14, 2016 at 12:48:53PM -0700, Stephen Boyd wrote: > On 06/14, Bjorn Andersson wrote: > > On Tue 14 Jun 11:55 PDT 2016, Stephen Boyd wrote: > > > > > On 06/14, Linus Walleij wrote: > > > > diff --git a/arch/arm/boot/dts/qcom-msm8660.dtsi b/arch/arm/boot/dts/qcom-msm8660.dtsi > > [..] > > > > + > > > > + pm8901-regulators { > > > > > > I'd rather have regulators@8901 and regulators@8058. That way the > > > node name is generic per ePAPR suggestions. > > > > > > > I agree, but unfortunately we're not allowed to do that without a > > matching "reg = <>" anymore. > > Oh does dtc blow up now? I think latest dtc complains, maybe with some additional warning enables. However, the requirement has been there for a long time in ePAPR: "The unit-address must match the first address specified in the reg property of the node. If the node has no reg property, the @ and unit-address must be omitted and the node-name alone differentiates the node from other nodes at the same level in the tree."
On Tue, Jun 14, 2016 at 8:55 PM, Stephen Boyd <sboyd@codeaurora.org> wrote: > On 06/14, Linus Walleij wrote: >> + rpmcc: clock-controller { >> + compatible = "qcom,rpmcc-apq8660", "qcom,rpmcc"; >> + #clock-cells = <1>; >> + }; > > This driver isn't merged yet, but ok. It's there because it is there in arch/arm/boot/dts/qcom-apq8064.dtsi >> + >> + pm8901-regulators { > > I'd rather have regulators@8901 and regulators@8058. That way the > node name is generic per ePAPR suggestions. Seems impossible if I should conclude the other discussion... >> + pm8058_mvs: mvs {}; > > pm8901_mvs? Ooops fixing. >> + pm8058-regulators { >> + compatible = "qcom,rpm-pm8058-regulators"; >> + >> + pm8058_l0: l0 {}; >> + pm8058_l1: l1 {}; >> + pm8058_l2: l2 {}; >> + pm8058_l3: l3 {}; >> + pm8058_l4: l4 {}; >> + pm8058_l5: l5 {}; >> + pm8058_l6: l6 {}; >> + pm8058_l7: l7 {}; >> + pm8058_l8: l8 {}; >> + pm8058_l9: l9 {}; >> + pm8058_l10: l10 {}; >> + pm8058_l11: l11 {}; >> + pm8058_l12: l12 {}; >> + pm8058_l13: l13 {}; >> + pm8058_l14: l14 {}; >> + pm8058_l15: l15 {}; >> + pm8058_l16: l16 {}; >> + pm8058_l17: l17 {}; >> + pm8058_l18: l18 {}; >> + pm8058_l19: l19 {}; >> + pm8058_l20: l20 {}; >> + pm8058_l21: l21 {}; >> + pm8058_l22: l22 {}; >> + pm8058_l23: l23 {}; >> + pm8058_l24: l24 {}; >> + pm8058_l25: l25 {}; >> + >> + pm8058_s0: s0 {}; >> + pm8058_s1: s1 {}; > > Just leave these out? RPM shouldn't be touching s0 or s1. Really? The msm-3.4 tree does: /* RPM early regulator constraints */ static struct rpm_regulator_init_data rpm_regulator_early_init_data[] = { /* ID a_on pd ss min_uV max_uV init_ip freq */ RPM_SMPS(PM8058_S0, 0, 1, 1, 500000, 1325000, SMPS_HMIN, 1p60), RPM_SMPS(PM8058_S1, 0, 1, 1, 500000, 1250000, SMPS_HMIN, 1p60), Are you sure you don't mean s0+s1 on PM8901 (which I left out)? Yours, Linus Walleij
On 06/14, Russell King - ARM Linux wrote: > On Tue, Jun 14, 2016 at 11:55:17AM -0700, Stephen Boyd wrote: > > On 06/14, Linus Walleij wrote: > > > + > > > + pm8901-regulators { > > > > I'd rather have regulators@8901 and regulators@8058. That way the > > node name is generic per ePAPR suggestions. > > But ePAPR requires reg= if you specify a unit-address part of the > node name. > It seems the ePAPR also leaves this open to interpretation because it states: "The binding for a particular bus may specify additional, more specific requirements for the format of reg and unit-address." but we haven't done anything formal for the RPM "bus" so far. I'm not worried either way, just a fun fact!
On 06/15/2016 06:16 AM, Linus Walleij wrote: > On Tue, Jun 14, 2016 at 8:55 PM, Stephen Boyd <sboyd@codeaurora.org> wrote: > >> >> Just leave these out? RPM shouldn't be touching s0 or s1. > Really? The msm-3.4 tree does: > > /* RPM early regulator constraints */ > static struct rpm_regulator_init_data > rpm_regulator_early_init_data[] = { > /* ID a_on pd ss min_uV max_uV init_ip freq */ > RPM_SMPS(PM8058_S0, 0, 1, 1, 500000, 1325000, SMPS_HMIN, 1p60), > RPM_SMPS(PM8058_S1, 0, 1, 1, 500000, 1250000, SMPS_HMIN, 1p60), > > Are you sure you don't mean s0+s1 on PM8901 (which I left out)? > Ah yes, I meant pm8901.
On Wed, Jun 15, 2016 at 05:17:14PM -0700, Stephen Boyd wrote: > On 06/14, Russell King - ARM Linux wrote: > > On Tue, Jun 14, 2016 at 11:55:17AM -0700, Stephen Boyd wrote: > > > On 06/14, Linus Walleij wrote: > > > > + > > > > + pm8901-regulators { > > > > > > I'd rather have regulators@8901 and regulators@8058. That way the > > > node name is generic per ePAPR suggestions. > > > > But ePAPR requires reg= if you specify a unit-address part of the > > node name. > > > > It seems the ePAPR also leaves this open to interpretation > because it states: > > "The binding for a particular bus may specify additional, more > specific requirements for the format of reg and unit-address." > > but we haven't done anything formal for the RPM "bus" so far. I'm > not worried either way, just a fun fact! Latest dtc with warnings enabled (W=1) warns if you don't follow what I quoted.
diff --git a/arch/arm/boot/dts/qcom-msm8660.dtsi b/arch/arm/boot/dts/qcom-msm8660.dtsi index a5a38820554a..51d3c9e70617 100644 --- a/arch/arm/boot/dts/qcom-msm8660.dtsi +++ b/arch/arm/boot/dts/qcom-msm8660.dtsi @@ -215,6 +215,95 @@ }; }; + l2cc: clock-controller@2082000 { + compatible = "syscon"; + reg = <0x02082000 0x1000>; + }; + + rpm: rpm@104000 { + compatible = "qcom,rpm-msm8660"; + reg = <0x00104000 0x1000>; + qcom,ipc = <&l2cc 0x8 2>; + + interrupts = <GIC_SPI 19 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 21 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 22 IRQ_TYPE_EDGE_RISING>; + interrupt-names = "ack", "err", "wakeup"; + clocks = <&gcc RPM_MSG_RAM_H_CLK>; + clock-names = "ram"; + + rpmcc: clock-controller { + compatible = "qcom,rpmcc-apq8660", "qcom,rpmcc"; + #clock-cells = <1>; + }; + + pm8901-regulators { + compatible = "qcom,rpm-pm8901-regulators"; + + pm8901_l0: l0 {}; + pm8901_l1: l1 {}; + pm8901_l2: l2 {}; + pm8901_l3: l3 {}; + pm8901_l4: l4 {}; + pm8901_l5: l5 {}; + pm8901_l6: l6 {}; + + /* S0 and S1 Handled as SAW regulators by SPM */ + pm8901_s2: s2 {}; + pm8901_s3: s3 {}; + pm8901_s4: s4 {}; + + pm8901_lvs0: lvs0 {}; + pm8901_lvs1: lvs1 {}; + pm8901_lvs2: lvs2 {}; + pm8901_lvs3: lvs3 {}; + + pm8058_mvs: mvs {}; + }; + + pm8058-regulators { + compatible = "qcom,rpm-pm8058-regulators"; + + pm8058_l0: l0 {}; + pm8058_l1: l1 {}; + pm8058_l2: l2 {}; + pm8058_l3: l3 {}; + pm8058_l4: l4 {}; + pm8058_l5: l5 {}; + pm8058_l6: l6 {}; + pm8058_l7: l7 {}; + pm8058_l8: l8 {}; + pm8058_l9: l9 {}; + pm8058_l10: l10 {}; + pm8058_l11: l11 {}; + pm8058_l12: l12 {}; + pm8058_l13: l13 {}; + pm8058_l14: l14 {}; + pm8058_l15: l15 {}; + pm8058_l16: l16 {}; + pm8058_l17: l17 {}; + pm8058_l18: l18 {}; + pm8058_l19: l19 {}; + pm8058_l20: l20 {}; + pm8058_l21: l21 {}; + pm8058_l22: l22 {}; + pm8058_l23: l23 {}; + pm8058_l24: l24 {}; + pm8058_l25: l25 {}; + + pm8058_s0: s0 {}; + pm8058_s1: s1 {}; + pm8058_s2: s2 {}; + pm8058_s3: s3 {}; + pm8058_s4: s4 {}; + + pm8058_lvs0: lvs0 {}; + pm8058_lvs1: lvs1 {}; + + pm8058_ncp: ncp {}; + }; + }; + /* Temporary fixed regulator */ vsdcc_fixed: vsdcc-regulator { compatible = "regulator-fixed";
This adds the L2CC IPC resource and RPM devices plus the nodes for the PM8901 and PM8058 regulators to the MSM8660 device tree. This was tested on the APQ8060 Dragonboard. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- arch/arm/boot/dts/qcom-msm8660.dtsi | 89 +++++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+)