Re: [PATCH 5/6] arm64: dts: msm8994 SoC and Huawei Angler (Nexus 6P) support

On Thu 03 Nov 17:20 PDT 2016, Jeremy McNicoll wrote:

On Thu, Nov 03, 2016 at 04:04:29PM -0700, Bjorn Andersson wrote:

quoted

On Thu 03 Nov 15:42 PDT 2016, Stephen Boyd wrote:

quoted

On 11/03/2016 03:32 PM, Andy Gross wrote:

quoted

On Thu, Oct 27, 2016 at 05:54:01PM -0700, Jeremy McNicoll wrote:

quoted

On 2016-10-27 5:06 PM, Stephen Boyd wrote:

quoted

On 10/25, Jeremy McNicoll wrote:

quoted

diff --git a/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
new file mode 100644
index 0000000..2fc68c4
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts

@@ -0,0 +1,40 @@
+/* Copyright (c) 2015, Huawei Inc. All rights reserved.
+ * Copyright (c) 2016, The Linux Foundation. All rights reserved.
+ *
+ * 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 "msm8994-v2.0.dtsi"
+
+/ {
+	model = "HUAWEI MSM8994 ANGLER rev-1.01";
+	compatible = "qcom,msm8994";

Please replace this with something more specific for the actual
board. Preferably with the board manufacturer vendor prefix and
some sort of string for the board.

Something like?

"huawei,nexus-6P", "qcom,msm8994"

Sure, except completely remove qcom,msm8994 from there.

Why? So far we have always used the form <device>, <platform> as
compatible. This is even in line with ePAPR, what changed?

Also, the compatible should be "huawei,angler" (if I read the internet
correctly) and the model should contain the product name
(Huawei Nexus 6P).

What about 

  model = "Huawei Nexus 6P";
  compatible = "huawei, angler", "qcom,msm8994";

                         ^
			 |
                   unwanted space

Apart from that, I think it looks good and in line with how we've done
the other boards.

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 2016-11-03 7:57 PM, Bjorn Andersson wrote:

On Thu 03 Nov 17:20 PDT 2016, Jeremy McNicoll wrote:

quoted

On Thu, Nov 03, 2016 at 04:04:29PM -0700, Bjorn Andersson wrote:

quoted

On Thu 03 Nov 15:42 PDT 2016, Stephen Boyd wrote:

quoted

On 11/03/2016 03:32 PM, Andy Gross wrote:

quoted

On Thu, Oct 27, 2016 at 05:54:01PM -0700, Jeremy McNicoll wrote:

quoted

On 2016-10-27 5:06 PM, Stephen Boyd wrote:

quoted

On 10/25, Jeremy McNicoll wrote:

quoted

diff --git a/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
new file mode 100644
index 0000000..2fc68c4
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts

@@ -0,0 +1,40 @@
+/* Copyright (c) 2015, Huawei Inc. All rights reserved.
+ * Copyright (c) 2016, The Linux Foundation. All rights reserved.
+ *
+ * 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 "msm8994-v2.0.dtsi"
+
+/ {
+	model = "HUAWEI MSM8994 ANGLER rev-1.01";
+	compatible = "qcom,msm8994";

Please replace this with something more specific for the actual
board. Preferably with the board manufacturer vendor prefix and
some sort of string for the board.

Something like?

"huawei,nexus-6P", "qcom,msm8994"

Sure, except completely remove qcom,msm8994 from there.

Why? So far we have always used the form <device>, <platform> as
compatible. This is even in line with ePAPR, what changed?

Also, the compatible should be "huawei,angler" (if I read the internet
correctly) and the model should contain the product name
(Huawei Nexus 6P).

What about

  model = "Huawei Nexus 6P";
  compatible = "huawei, angler", "qcom,msm8994";

                         ^
			 |
                   unwanted space

Apart from that, I think it looks good and in line with how we've done
the other boards.

... just for completeness sake.  Here is what I plan on using for the 5X

model = "LG Nexus 5X";
compatible = "lg,bullhead", "qcom,msm8992";


-jeremy

Regards,
Bjorn

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html