Hi, Shawn Guo wrote: > On Thu, Jun 12, 2014 at 03:09:44PM +0200, Lothar Waßmann wrote: > > Add support for Ka-Ro electronics i.MX51 based TX51 modules > > > > Signed-off-by: Lothar Waßmann <LW@xxxxxxxxxxxxxxxxxxx> > > --- > > arch/arm/boot/dts/Makefile | 1 + > > arch/arm/boot/dts/imx51-tx51.dts | 620 ++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 621 insertions(+) > > create mode 100644 arch/arm/boot/dts/imx51-tx51.dts > > > > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile > > index 0f1e8be..8dd4dbc 100644 > > --- a/arch/arm/boot/dts/Makefile > > +++ b/arch/arm/boot/dts/Makefile > > @@ -177,6 +177,7 @@ dtb-$(CONFIG_ARCH_MXC) += \ > > imx51-babbage.dtb \ > > imx51-digi-connectcore-jsk.dtb \ > > imx51-eukrea-mbimxsd51-baseboard.dtb \ > > + imx51-tx51.dtb \ > > imx53-ard.dtb \ > > imx53-m53evk.dtb \ > > imx53-mba53.dtb \ > > diff --git a/arch/arm/boot/dts/imx51-tx51.dts b/arch/arm/boot/dts/imx51-tx51.dts > > new file mode 100644 > > index 0000000..9ae7758 > > --- /dev/null > > +++ b/arch/arm/boot/dts/imx51-tx51.dts > > @@ -0,0 +1,620 @@ > > +/* > > + * Copyright 2012-2014 Lothar Waßmann <LW@xxxxxxxxxxxxxxxxxxx> > > + * > > + * The code contained herein is licensed under the GNU General Public > > + * License. You may obtain a copy of the GNU General Public License > > + * Version 2 at the following locations: > > + * > > + * http://www.opensource.org/licenses/gpl-license.html > > + * http://www.gnu.org/copyleft/gpl.html > > + */ > > + > > +/dts-v1/; > > +#include "imx51.dtsi" > > +#include <dt-bindings/gpio/gpio.h> > > +#include <dt-bindings/input/input.h> > > imx51.dtsi already includes them. > OK. > > +#include <dt-bindings/pwm/pwm.h> > > + > > +/ { > > + model = "Ka-Ro electronics TX51 module"; > > + compatible = "karo,tx51", "fsl,imx51"; > > + > > + aliases { > > + backlight = &backlight; > > + display = &display; > > + i2c1 = &i2c_gpio; > > + usbotg = &usbotg; > > + }; > > + > > + chosen { > > + stdout-path = &uart1; > > + }; > > + > > + backlight: pwm-backlight { > > + compatible = "pwm-backlight"; > > + > > Drop this new line. > OK. > > + pwms = <&pwm1 0 500000 PWM_POLARITY_INVERTED>; > > + power-supply = <®_3v3>; > > + brightness-levels = < > > + 0 1 2 3 4 5 6 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 = <50>; > > + }; > > + > > + clocks { > > + ckih1 { > > + clock-frequency = <0>; > > + }; > > + > > + mclk: clock@0 { > > + compatible = "fixed-clock"; > > + reg = <0>; > > + #clock-cells = <0>; > > + clock-frequency = <27000000>; > > + }; > > + > > + clk_26M: clock@1 { > > When using generic name, you will need clock-output-names property. > Otherwise, the clocks will fail in registration except the first one, > because of clock name collision. > Didn't recognize that yet. [...] > > + usbphy { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + compatible = "simple-bus"; > > + > > + usbh1phy: usbh1phy@0 { > > The node name should be something generic like usbphy? > OK. > > + compatible = "usb-nop-xceiv"; > > + reg = <0>; > > + clocks = <&clk_26M>; > > + clock-names = "main_clk"; > > + }; > > + }; > > +}; > > + > > +&audmux { > > + status = "okay"; > > +}; > > + > > +&fec { > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pinctrl_fec>; > > + phy-mode = "mii"; > > +// phy-reset-gpios = <&gpio2 14 GPIO_ACTIVE_LOW>; > > Drop it? > Leftover from debugging... > > + phy-handle = <&phy0>; > > + mac-address = [000000000000]; /* will be set by U-Boot */ > > Shouldn't it be local-mac-address? > probably yes, but both 'mac-address' and 'local-mac-address' are being set up by U-Boot anyway. [...] > > +&ipu_di0_disp0 { > > + remote-endpoint = <&display0_in>; > > +}; > > + > > +&kpp { > > + status = "okay"; > > Put 'status' at the bottom of property list. > OK. > > + > > + linux,keymap = < /* sample keymap */ > > + MATRIX_KEY(0, 0, KEY_POWER) > > + MATRIX_KEY(0, 1, KEY_KP0) > > + MATRIX_KEY(0, 2, KEY_KP1) > > + MATRIX_KEY(0, 3, KEY_KP2) > > + MATRIX_KEY(1, 0, KEY_KP3) > > + MATRIX_KEY(1, 1, KEY_KP4) > > + MATRIX_KEY(1, 2, KEY_KP5) > > + MATRIX_KEY(1, 3, KEY_KP6) > > + MATRIX_KEY(2, 0, KEY_KP7) > > + MATRIX_KEY(2, 1, KEY_KP8) > > + MATRIX_KEY(2, 2, KEY_KP9) > > + >; > > +}; > > + > > +&esdhc1 { > > + cd-gpios = <&gpio3 8 GPIO_ACTIVE_LOW>; > > + fsl,wp-controller; > > Does it work for you, since the driver does not support it as of today? > What driver doesn't support what? AFAICT the sdhci-esdhc-imx.c driver supports both of these properties. > > + status = "okay"; > > +}; > > + > > +&esdhc2 { > > + cd-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>; > > + fsl,wp-controller; > > + status = "okay"; > > +}; > > + > > +&pwm1 { > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pinctrl_pwm1>; > > +}; > > + > > +&ecspi1 { > > + fsl,spi-num-chipselects = <2>; > > + cs-gpios = <&gpio4 24 GPIO_ACTIVE_LOW &gpio4 25 GPIO_ACTIVE_LOW>; > > More readable to write it like below? > > cs-gpios = <&gpio4 24 GPIO_ACTIVE_LOW>, > <&gpio4 25 GPIO_ACTIVE_LOW>; > OK. > > + status = "okay"; > > + > > + spidev0: spi@0 { > > + compatible = "spidev"; > > + reg = <0>; > > + spi-max-frequency = <54000000>; > > + }; > > + > > + spidev1: spi@1 { > > + compatible = "spidev"; > > + reg = <1>; > > + spi-max-frequency = <54000000>; > > + }; > > I'm not sure we should have these two devices. > Why not? With this the SPI bus can readily be used with the spidev driver from Documentation/spi/spidev_test.c (which is what most of our customers are asking for)! > > +}; > > + > > +&ssi1 { > > + fsl,mode = "i2s-slave"; > > + codec-handle = <&sgtl5000>; > > + status = "okay"; > > +}; > > + > > +&ssi2 { > > + status = "disabled"; > > +}; > > Why is this needed, since the device is disabled in imx51.dtsi? > That's included here as a placeholder in case someone wants to enable the second SSI interface so that they... > > +&nfc { > > Please sort the nodes alphabetically. > ...won't have this problem. ;) Lothar Waßmann -- ___________________________________________________________ Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10 Geschäftsführer: Matthias Kaussen Handelsregistereintrag: Amtsgericht Aachen, HRB 4996 www.karo-electronics.de | info@xxxxxxxxxxxxxxxxxxx ___________________________________________________________ -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
