Re: [PATCH v5 2/4] arm64: dts: Add Qualcomm MSM8916 SoC and evaluation board dts

Kumar Gala
2015-03-17T17:07:15Z

On Mar 17, 2015, at 4:24 AM, Stanimir Varbanov <stanimir.varbanov@linaro.org> wrote:

> On 03/13/2015 06:06 PM, Kumar Gala wrote:
>> Add initial device tree support for Qualcomm MSM8916 SoC and MTP8916
>> evaluation board. At the current time we only boot up a single processor.
>>
>> Signed-off-by: Kumar Gala <galak@codeaurora.org>
>> ---
>> v5:
>> * killed use of skeleton, moved to addr/size cells both being 2
>> * Added serial alias in prep of being able to spec uart params
>> in chosen node
>>
>> v4:
>> * Added chosen node so we dont need to spec everything for earlycon
>> * fixed up timer node to armv8, dropped clock-frequency
>>
>> v3:
>> * Removed qcom,msm-id and qcom,board-id
>> * Added top level compat for "qcom,msm8916-mtp-smb1360"
>>
>> v2:
>> * Updated to dropping CONFIG_ARCH_QCOM_MSM8916
>> * Updated to use qcom-ids.h
>>
>> arch/arm64/boot/dts/Makefile | 1 +
>> arch/arm64/boot/dts/qcom/Makefile | 5 +
>> arch/arm64/boot/dts/qcom/msm8916-mtp.dts | 21 ++++
>> arch/arm64/boot/dts/qcom/msm8916-mtp.dtsi | 32 +++++
>> arch/arm64/boot/dts/qcom/msm8916.dtsi | 195 ++++++++++++++++++++++++++++++
>> 5 files changed, 254 insertions(+)
>> create mode 100644 arch/arm64/boot/dts/qcom/Makefile
>> create mode 100644 arch/arm64/boot/dts/qcom/msm8916-mtp.dts
>> create mode 100644 arch/arm64/boot/dts/qcom/msm8916-mtp.dtsi
>> create mode 100644 arch/arm64/boot/dts/qcom/msm8916.dtsi
>>
>> diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
>> index e0350ca..8517f15 100644
>> --- a/arch/arm64/boot/dts/Makefile
>> +++ b/arch/arm64/boot/dts/Makefile
>> @@ -5,5 +5,6 @@ dts-dirs += cavium
>> dts-dirs += exynos
>> dts-dirs += freescale
>> dts-dirs += mediatek
>> +dts-dirs += qcom
>>
>> subdir-y := $(dts-dirs)
>> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
>> new file mode 100644
>> index 0000000..360ec4c
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/Makefile
>> @@ -0,0 +1,5 @@
>> +dtb-$(CONFIG_ARCH_QCOM) += msm8916-mtp.dtb
>> +
>> +always := $(dtb-y)
>> +subdir-y := $(dts-dirs)
>> +clean-files := *.dtb
>> diff --git a/arch/arm64/boot/dts/qcom/msm8916-mtp.dts b/arch/arm64/boot/dts/qcom/msm8916-mtp.dts
>> new file mode 100644
>> index 0000000..784ad92
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/msm8916-mtp.dts
>> @@ -0,0 +1,21 @@
>> +/* Copyright (c) 2014-2015, The Linux Foundation. All rights reserved.
>
> multi line comment rules
>
>> +*
>> +* This program is free software; you can redistribute it and/or modify
>> +* it under the terms of the GNU General Public License version 2 and
>> +* only version 2 as published by the Free Software Foundation.
>> +*
>> +* This program is distributed in the hope that it will be useful,
>> +* but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> +* GNU General Public License for more details.
>> +*/
>> +
>> +/dts-v1/;
>> +
>> +#include "msm8916-mtp.dtsi"
>> +
>> +/ {
>> + model = "Qualcomm Technologies, Inc. MSM 8916 MTP";
>> + compatible = "qcom,msm8916-mtp", "qcom,msm8916-mtp-smb1360",
>> + "qcom,msm8916", "qcom,mtp";
>> +};
>> diff --git a/arch/arm64/boot/dts/qcom/msm8916-mtp.dtsi b/arch/arm64/boot/dts/qcom/msm8916-mtp.dtsi
>> new file mode 100644
>> index 0000000..8dbb77f
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/msm8916-mtp.dtsi
>> @@ -0,0 +1,32 @@
>> +/* Copyright (c) 2014-2014, The Linux Foundation. All rights reserved.
>
> multi line comment rules and 2014-2015
>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include "msm8916.dtsi"
>> +
>> +/ {
>> + aliases {
>> + serial0 = &blsp1_uart2;
>> + };
>> +
>> + chosen {
>> + stdout-path = "serial0";
>> + };
>> +
>> + soc {
>> + serial@78b0000 {
>> + status = "okay";
>> + pinctrl-names = "default", "sleep";
>> + pinctrl-0 = <&blsp1_uart2_default>;
>> + pinctrl-1 = <&blsp1_uart2_sleep>;
>> + };
>> + };
>
> Why don't describe this as:
>
> &blsp1_uart2 {
> status = "okay";
> pinctrl-names = xxxx;
> ....
> };
>
> it looks more readable and obvious. This is what the other boards doing
> as well.

I find the way it is more explicit and obvious, plus it works for cases that don’t have a label associated with them.

I’ll fixup the multiline comment issues.

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