Re: [PATCH v3] arm, imx6, dts: add DT for aristainetos2 board

From: Heiko Schocher
Date: Wed May 20 2015 - 07:14:05 EST


Hello Philipp,

Am 20.05.2015 12:38, schrieb Philipp Zabel:
Hi Heiko,

Am Mittwoch, den 20.05.2015, 07:31 +0200 schrieb Heiko Schocher:
[...]
diff --git a/arch/arm/boot/dts/imx6dl-aristainetos2_4.dts b/arch/arm/boot/dts/imx6dl-aristainetos2_4.dts
new file mode 100644
index 0000000..780fc42
--- /dev/null
+++ b/arch/arm/boot/dts/imx6dl-aristainetos2_4.dts
@@ -0,0 +1,174 @@
+/*
+ * support fot the imx6 based aristainetos2 board

Typo, "support for".

Oh, thanks! Fixed (for all files)

+ *
+ * Copyright (C) 2015 Heiko Schocher <hs@xxxxxxx>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This file is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This file 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.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this file; if not, write to the Free
+ * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
+ * MA 02110-1301 USA

checkpatch warns that you should not include this paragraph because the
mail address might change in the future and it is contained in COPYING.

Hmm... Shawn mentioned to change it to the "GPL/X11 dual licence"
as in the arch/arm/boot/dts/imx6sl-warp.dts file ... so I copied this
from there ... is there a better option for this?

[...]
+/dts-v1/;
+#include "imx6dl.dtsi"
+#include "imx6qdl-aristainetos2.dtsi"
+
+/ {
+ model = "aristainetos2 i.MX6 Dual Lite Board 4";
+ compatible = "fsl,imx6dl";

Doesn't this board get its own compatible?

Hmm... why?

[...]
+ display0: display@di0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "fsl,imx-parallel-display";
+ interface-pix-fmt = "rgb24";

It's ok to keep this property for now, but in the future I'd like it to
be removed. The panel driver should provide all necessary information
using drm_display_info_set_bus_formats. The imx parallel-display driver
then needs to be made aware of the connector display_info->bus_formats.

Ok.

[...]
diff --git a/arch/arm/boot/dts/imx6dl-aristainetos2_7.dts b/arch/arm/boot/dts/imx6dl-aristainetos2_7.dts
new file mode 100644
index 0000000..14cf4e8
--- /dev/null
+++ b/arch/arm/boot/dts/imx6dl-aristainetos2_7.dts
@@ -0,0 +1,103 @@
+/*
+ * support fot the imx6 based aristainetos2 board

Typo, "support for".

fixed.

+ * Copyright (C) 2015 Heiko Schocher <hs@xxxxxxx>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This file is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This file 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.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this file; if not, write to the Free
+ * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
+ * MA 02110-1301 USA

Same comment as above.

[...]
+/ {
+ model = "aristainetos2 i.MX6 Dual Lite Board 7";
+ compatible = "fsl,imx6dl";

Same comment as above.

+&ldb {
+ status = "okay";
+
+ lvds-channel@0 {
+ fsl,data-mapping = "spwg";
+ fsl,data-width = <24>;
+ status = "okay";
+
+ display-timings {
+ native-mode = <&timing0>;
+ timing0: 800x480p60 {
+ clock-frequency = <33246000>;
+ hactive = <800>;
+ vactive = <480>;
+ hfront-porch = <88>;
+ hback-porch = <88>;
+ hsync-len = <80>;
+ vback-porch = <10>;
+ vfront-porch = <10>;
+ vsync-len = <25>;
+ };
+ };
+ };

What panel is this? I'd prefer if you used the panel-simple driver here
and connected it to the ldb channel using OF graph bindings same as the
parallel display above. That would allow to remove the fsl,data-mapping
and fsl,data-width properties and the display-timings node.

It is an Display from LG, MODEL LB070WV8, SUFFIX SL01
I try to add it to the panel-simple driver

+};
diff --git a/arch/arm/boot/dts/imx6qdl-aristainetos2.dtsi b/arch/arm/boot/dts/imx6qdl-aristainetos2.dtsi
new file mode 100644
index 0000000..eac7c3b
--- /dev/null
+++ b/arch/arm/boot/dts/imx6qdl-aristainetos2.dtsi
@@ -0,0 +1,634 @@
+/*
+ * support fot the imx6 based aristainetos2 board

Typo, "support for". Also, maybe s/board/boards/ as this contains the
common parts for both the 4 and 7 variant?

Yep, fixed.

+ *
+ * Copyright (C) 2015 Heiko Schocher <hs@xxxxxxx>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This file is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This file 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.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this file; if not, write to the Free
+ * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
+ * MA 02110-1301 USA

Same comment as above.

[...]
+&i2c1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_i2c1>;
+ status = "okay";
+
+ pmic@58 {
+ compatible = "dialog,da9063";

This should be "dlg,da9063" as per
Documentation/devicetree/bindings/mfd/da9063.txt.

fixed.

[...]
+&i2c4 {
+ clocks = <&clks IMX6DL_CLK_I2C4>;

i> The clocks property is already contained in imx6dl.dtsi

removed.

[...]
+&fec
+&gpmi
+&pcie
+&pwm1
+&uart1
+&uart2
+&uart3
+&uart4
+&usbh1
+&usbotg
+&usdhc1
+&usdhc2
+&iomuxc

Do all aristainetos2 boards have all of these ports routed out? (Should
the status = "okay" properties be set here, or in the per-board .dts
files?)

All aristainetos2 variants use this ports, yes.

Thanks for your review!

bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/