diff mbox

[v3,1/2] arm64: dts: qcom: pm8998: Add spmi-temp-alarm node

Message ID 20180709212522.187257-1-mka@chromium.org (mailing list archive)
State New, archived
Headers show

Commit Message

Matthias Kaehlcke July 9, 2018, 9:25 p.m. UTC
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 v3:
- changed node name from 'qcom,temp-alarm@2400' to 'temp-alarm@2400'
- removed controller register length value from 'reg'

Changes in v2:
- none
---
 arch/arm64/boot/dts/qcom/pm8998.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Doug Anderson July 12, 2018, 6:33 p.m. UTC | #1
Hi,

On Mon, Jul 9, 2018 at 2:25 PM, 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 v3:
> - changed node name from 'qcom,temp-alarm@2400' to 'temp-alarm@2400'
> - removed controller register length value from 'reg'
>
> Changes in v2:
> - none
> ---
>  arch/arm64/boot/dts/qcom/pm8998.dtsi | 7 +++++++
>  1 file changed, 7 insertions(+)

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Stephen Boyd July 13, 2018, 4 p.m. UTC | #2
Quoting Matthias Kaehlcke (2018-07-09 14:25:21)
> diff --git a/arch/arm64/boot/dts/qcom/pm8998.dtsi b/arch/arm64/boot/dts/qcom/pm8998.dtsi
> index 92bed1e7d4bb..7eea94701b23 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: temp-alarm@2400 {

Are we sorting this file based on node name or unit address? Otherwise
patch looks good.

> +                       compatible = "qcom,spmi-temp-alarm";
> +                       reg = <0x2400>;
> +                       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>;
Doug Anderson July 13, 2018, 4:08 p.m. UTC | #3
Hi,

On Fri, Jul 13, 2018 at 9:00 AM, Stephen Boyd <swboyd@chromium.org> wrote:
> Quoting Matthias Kaehlcke (2018-07-09 14:25:21)
>> diff --git a/arch/arm64/boot/dts/qcom/pm8998.dtsi b/arch/arm64/boot/dts/qcom/pm8998.dtsi
>> index 92bed1e7d4bb..7eea94701b23 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: temp-alarm@2400 {
>
> Are we sorting this file based on node name or unit address? Otherwise
> patch looks good.

My preference would be:

* Anything where we're defining a new node with an address: sort by the address.

* If we are defining nodes without addresses: sort by node name (not by label!)

* In general I'd prefer to see addressless things before things with addresses.

* If we are overriding nodes that were defined in a parent makefile:
sort by the name of the parent label we refer to.


...so I think Matthias's sort order in this case is correct / consistent.


-Doug
Matthias Kaehlcke July 13, 2018, 4:38 p.m. UTC | #4
On Fri, Jul 13, 2018 at 09:00:19AM -0700, Stephen Boyd wrote:
> Quoting Matthias Kaehlcke (2018-07-09 14:25:21)
> > diff --git a/arch/arm64/boot/dts/qcom/pm8998.dtsi b/arch/arm64/boot/dts/qcom/pm8998.dtsi
> > index 92bed1e7d4bb..7eea94701b23 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: temp-alarm@2400 {
> 
> Are we sorting this file based on node name or unit address? Otherwise
> patch looks good.

The idea was to sort by address, following the 'convention' of
sdm845.dtsi (https://patchwork.kernel.org/patch/10381745/).
diff mbox

Patch

diff --git a/arch/arm64/boot/dts/qcom/pm8998.dtsi b/arch/arm64/boot/dts/qcom/pm8998.dtsi
index 92bed1e7d4bb..7eea94701b23 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: temp-alarm@2400 {
+			compatible = "qcom,spmi-temp-alarm";
+			reg = <0x2400>;
+			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>;