| Message ID | 20180702181005.18247-1-mka@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Mon, Jul 2, 2018 at 11:10 AM, Matthias Kaehlcke <mka@chromium.org> wrote: > This adds the spmi-temp-alarm node to pm8998 based on the examples in the > bindings. > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org> > --- > Changes in v2: > - none > > arch/arm64/boot/dts/qcom/pm8998.dtsi | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/pm8998.dtsi b/arch/arm64/boot/dts/qcom/pm8998.dtsi > index 92bed1e7d4bb..2f4989e7ef68 100644 > --- a/arch/arm64/boot/dts/qcom/pm8998.dtsi > +++ b/arch/arm64/boot/dts/qcom/pm8998.dtsi > @@ -11,6 +11,13 @@ > #address-cells = <1>; > #size-cells = <0>; > > + pm8998_temp: qcom,temp-alarm@2400 { Remove "qcom," from the node name (AKA please change to "temp-alarm@2400"). Someone internal in Qualcomm seems to have started this trend so you see it on all downstream kernels, but upstream device tree isn't supposed to have it. > + compatible = "qcom,spmi-temp-alarm"; > + reg = <0x2400 0x100>; Why are there two numbers for the "reg"? Should just be 0x2400. -Doug
On Mon, Jul 02, 2018 at 01:12:01PM -0700, Doug Anderson wrote: > Hi, > > On Mon, Jul 2, 2018 at 11:10 AM, Matthias Kaehlcke <mka@chromium.org> wrote: > > This adds the spmi-temp-alarm node to pm8998 based on the examples in the > > bindings. > > > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org> > > --- > > Changes in v2: > > - none > > > > arch/arm64/boot/dts/qcom/pm8998.dtsi | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/pm8998.dtsi b/arch/arm64/boot/dts/qcom/pm8998.dtsi > > index 92bed1e7d4bb..2f4989e7ef68 100644 > > --- a/arch/arm64/boot/dts/qcom/pm8998.dtsi > > +++ b/arch/arm64/boot/dts/qcom/pm8998.dtsi > > @@ -11,6 +11,13 @@ > > #address-cells = <1>; > > #size-cells = <0>; > > > > + pm8998_temp: qcom,temp-alarm@2400 { > > Remove "qcom," from the node name (AKA please change to > "temp-alarm@2400"). Someone internal in Qualcomm seems to have > started this trend so you see it on all downstream kernels, but > upstream device tree isn't supposed to have it. Ok, thanks > > + compatible = "qcom,spmi-temp-alarm"; > > + reg = <0x2400 0x100>; > > Why are there two numbers for the "reg"? Should just be 0x2400. From /Documentation/devicetree/bindings/thermal/qcom-spmi-temp-alarm.txt: Required properties: ... - reg: Specifies the SPMI address and length of the controller's registers.
Hi, On Mon, Jul 2, 2018 at 1:51 PM, Matthias Kaehlcke <mka@chromium.org> wrote: >> > + compatible = "qcom,spmi-temp-alarm"; >> > + reg = <0x2400 0x100>; >> >> Why are there two numbers for the "reg"? Should just be 0x2400. > > From /Documentation/devicetree/bindings/thermal/qcom-spmi-temp-alarm.txt: > > Required properties: > ... > - reg: Specifies the SPMI address and length of the controller's > registers. Hrm, something isn't adding up here. A) Do a "git grep" and you'll see that nobody else has the length. $ git grep -C2 -- qcom,spmi-temp-alarm | grep reg arch/arm/boot/dts/qcom-pm8841.dtsi- reg = <0x2400>; arch/arm/boot/dts/qcom-pm8941.dtsi- reg = <0x2400>; arch/arm/boot/dts/qcom-pma8084.dtsi- reg = <0x2400>; arch/arm64/boot/dts/qcom/pm8916.dtsi- reg = <0x2400>; --- B) The SPMI bus you're adding it to has "#size-cells = <0>;" ...as do all other SPMI busses: $ git grep -C5 qcom,spmi-pmic | grep size-cells arch/arm/boot/dts/qcom-pm8841.dtsi- #size-cells = <0>; arch/arm/boot/dts/qcom-pm8841.dtsi- #size-cells = <0>; arch/arm/boot/dts/qcom-pm8941.dtsi- #size-cells = <0>; arch/arm/boot/dts/qcom-pm8941.dtsi- #size-cells = <0>; arch/arm/boot/dts/qcom-pma8084.dtsi- #size-cells = <0>; arch/arm/boot/dts/qcom-pma8084.dtsi- #size-cells = <0>; arch/arm64/boot/dts/qcom/pm8004.dtsi- #size-cells = <0>; arch/arm64/boot/dts/qcom/pm8004.dtsi- #size-cells = <0>; arch/arm64/boot/dts/qcom/pm8005.dtsi- #size-cells = <0>; arch/arm64/boot/dts/qcom/pm8005.dtsi- #size-cells = <0>; arch/arm64/boot/dts/qcom/pm8916.dtsi- #size-cells = <0>; arch/arm64/boot/dts/qcom/pm8916.dtsi- #size-cells = <0>; arch/arm64/boot/dts/qcom/pm8994.dtsi- #size-cells = <0>; arch/arm64/boot/dts/qcom/pm8994.dtsi- #size-cells = <0>; arch/arm64/boot/dts/qcom/pm8998.dtsi- #size-cells = <0>; arch/arm64/boot/dts/qcom/pm8998.dtsi- #size-cells = <0>; arch/arm64/boot/dts/qcom/pmi8994.dtsi- #size-cells = <0>; arch/arm64/boot/dts/qcom/pmi8994.dtsi- #size-cells = <0>; ...I have no SPMI history, but my guess is that upstream dropped this because all SPMI devices have 256-byte regions (see qcom,spmi-pmic.txt)? So probably the right thing is to fix the qcom,spmi-temp-alarm not to have the size... -Doug
diff --git a/arch/arm64/boot/dts/qcom/pm8998.dtsi b/arch/arm64/boot/dts/qcom/pm8998.dtsi index 92bed1e7d4bb..2f4989e7ef68 100644 --- a/arch/arm64/boot/dts/qcom/pm8998.dtsi +++ b/arch/arm64/boot/dts/qcom/pm8998.dtsi @@ -11,6 +11,13 @@ #address-cells = <1>; #size-cells = <0>; + pm8998_temp: qcom,temp-alarm@2400 { + compatible = "qcom,spmi-temp-alarm"; + reg = <0x2400 0x100>; + interrupts = <0x0 0x24 0x0 IRQ_TYPE_EDGE_RISING>; + #thermal-sensor-cells = <0>; + }; + pm8998_gpio: gpios@c000 { compatible = "qcom,pm8998-gpio", "qcom,spmi-gpio"; reg = <0xc000>;
This adds the spmi-temp-alarm node to pm8998 based on the examples in the bindings. Signed-off-by: Matthias Kaehlcke <mka@chromium.org> --- Changes in v2: - none arch/arm64/boot/dts/qcom/pm8998.dtsi | 7 +++++++ 1 file changed, 7 insertions(+)