Hi Sascha,
> Hi Lukasz,
>
> On Fri, Mar 02, 2018 at 01:17:50PM +0100, Lukasz Majewski wrote:
> > This commit adds device tree description of K+P's TPC board.
>
> Can we get a hint what this board is? I assume this one:
>
> Technologic Systems' Full i.MX6 Portfolio Including SBC, COM, and
> Touch Panel PCs
I just took other imx6q boards as an example - e.g.
420127e5a5b53ff2cb5effaa781aed93709b09bb
Generally, descriptions of DTSes are rather short and simple.
>
> Anyway, future developers are thankful if they have the information
> around when they have to work on that file or have to decide if it is
> to be removed.
IMHO, there is plenty of information around (iMX6 Quad SoC, with
components described in dts).
>
> >
> > Signed-off-by: Lukasz Majewski <lukma@xxxxxxx>
> > ---
> > arch/arm/boot/dts/Makefile | 1 +
> > arch/arm/boot/dts/imx6q-kp-tpc.dts | 84 +++++++
> > arch/arm/boot/dts/imx6q-kp.dtsi | 468
> > +++++++++++++++++++++++++++++++++++++ 3 files changed, 553
> > insertions(+) create mode 100644 arch/arm/boot/dts/imx6q-kp-tpc.dts
> > create mode 100644 arch/arm/boot/dts/imx6q-kp.dtsi
> >
> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> > index ade7a38543dc..c148c4cf28f2 100644
> > --- a/arch/arm/boot/dts/Makefile
> > +++ b/arch/arm/boot/dts/Makefile
> > @@ -459,6 +459,7 @@ dtb-$(CONFIG_SOC_IMX6Q) += \
> > imx6q-icore-ofcap10.dtb \
> > imx6q-icore-ofcap12.dtb \
> > imx6q-icore-rqs.dtb \
> > + imx6q-kp-tpc.dtb \
> > imx6q-marsboard.dtb \
> > imx6q-mccmon6.dtb \
> > imx6q-nitrogen6x.dtb \
> > diff --git a/arch/arm/boot/dts/imx6q-kp-tpc.dts
> > b/arch/arm/boot/dts/imx6q-kp-tpc.dts new file mode 100644
> > index 000000000000..955462e778c9
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/imx6q-kp-tpc.dts
> > +/dts-v1/;
> > +
> > +#include "imx6q-kp.dtsi"
> > +
> > +/ {
> > + model = "Freescale i.MX6 Quad K+P TPC Board";
> > + compatible = "fsl,imx6q-tpc", "fsl,imx6q";
>
> If it is what I think it is the vendor is not fsl.
Yes. It is not from fsl.
>
> > +};
> > +
> > +&lcd_display {
> > + display-timings {
> > + 800x480x60 {
> > + clock-frequency = <34209000>;
> > + hactive = <800>;
> > + vactive = <480>;
> > + hback-porch = <85>;
> > + hfront-porch = <15>;
> > + vback-porch = <34>;
> > + vfront-porch = <10>;
> > + hsync-len = <28>;
> > + vsync-len = <1>;
> > + hsync-active = <1>;
> > + vsync-active = <1>;
> > + de-active = <1>;
> > + };
> > + };
> > +};
> > +
> > +&ipu1_di0_disp0 {
> > + remote-endpoint = <&lcd_display_in>;
> > +};
> > +
> > +&can1 {
> > + status = "disabled";
> > +};
> > +
> > +&can2 {
> > + status = "disabled";
> > +};
>
> These are not enabled in your base dtsi, so no need to disabled it
> here.
But they can be enabled if needed.
>
> > +
> > +&uart1 {
> > + status = "okay";
> > +};
>
> This is already enabled in your base dtsi.
>
> > +
> > +&uart2 {
> > + status = "disabled";
> > +};
>
> This is still disabled, no need to enable.
The goal here is to group those buses in one "logical" item - to allow
easy enabling if needed.
>
> > diff --git a/arch/arm/boot/dts/imx6q-kp.dtsi
> > b/arch/arm/boot/dts/imx6q-kp.dtsi new file mode 100644
> > index 000000000000..47a10fb1d46b
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/imx6q-kp.dtsi
> > +
> > + memory: memory {
> > + reg = <0x10000000 0x40000000>;
> > + };
> > +
> > + pwm-buzzer {
> > + compatible = "pwm-backlight";
>
> What is it? A backlight or a buzzer?
It is a buzzer, which is controlled by PWM.
>
> > + pwms = <&pwm2 0 500000>; //2kHz
> > + brightness-levels = <
> > + 0 7 8 9
> > + 10 11 12 13 14 15 16 17 18 19
> > + 20 21 22 23 24 25 26 27 28 29
> > + 30 31 32 33 34 35 36 37 38 39
> > + 40 41 42 43 44 45 46 47 48 49
> > + 50 51 52 53 54 55 56 57 58 59
> > + 60 61 62 63 64 65 66 67 68 69
> > + 70 71 72 73 74 75 76 77 78 79
> > + 80 81 82 83 84 85 86 87 88 89
> > + 90 91 92 93 94 95 96 97 98 99
> > + 100
> > + >;
> > + default-brightness-level = <0>;
> > + };
> > +
> > + regulators {
>
> AFAIK regulators shall no longer be in a separate subnode.
>
> > + compatible = "simple-bus";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + reg_usb_h1_vbus: regulator@1 {
> > + compatible = "regulator-fixed";
> > + reg = <1>;
>
> drop the reg property and also the @1 in the name.
>
> > + regulator-name = "usb_h1_vbus";
> > + regulator-min-microvolt = <5000000>;
> > + regulator-max-microvolt = <5000000>;
> > + enable-active-high;
> > + };
> > +
> > + reg_audio: regulator@2 {
> > + compatible = "regulator-fixed";
> > + reg = <2>;
>
> ditto
Ok.
>
> > + regulator-name = "sgtl5000-supply";
> > + gpio = <&gpio6 31 GPIO_ACTIVE_HIGH>;
> > + enable-active-high;
> > + regulator-always-on;
> > + };
> > +
> > + reg_3p3v: regulator@3 {
> > + compatible = "regulator-fixed";
> > + reg = <4>;
>
> ditto.
>
> (You have to change the node names of course to make them unique
> again)
Yes. correct.
Thanks for your review.
>
>
> Sascha
>
>
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@xxxxxxx
Attachment:
pgpAYUFhNezTA.pgp
Description: OpenPGP digital signature
