All of lore.kernel.org
 help / color / mirror / Atom feed
From: shawnguo@kernel.org (Shawn Guo)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/2] ARM: dts: imx53: add support for USB armory board
Date: Tue, 28 Jun 2016 09:50:55 +0800	[thread overview]
Message-ID: <20160628015055.GA21328@tiger> (raw)
In-Reply-To: <20160621145053.7742-3-andrej@inversepath.com>

On Tue, Jun 21, 2016 at 04:50:53PM +0200, andrej at inversepath.com wrote:
> From: Andrej Rosano <andrej@inversepath.com>
> 
> Add support for Inverse Path USB armory board, an open source
> flash-drive sized computer based on NXP i.MX53 SoC.
> 
> https://inversepath.com/usbarmory
> 
> Signed-off-by: Andrej Rosano <andrej@inversepath.com>
> ---
>  arch/arm/boot/dts/Makefile            |   1 +
>  arch/arm/boot/dts/imx53-usbarmory.dts | 239 ++++++++++++++++++++++++++++++++++
>  2 files changed, 240 insertions(+)
>  create mode 100644 arch/arm/boot/dts/imx53-usbarmory.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 414b427..f8f85ab 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -303,6 +303,7 @@ dtb-$(CONFIG_SOC_IMX53) += \
>  	imx53-smd.dtb \
>  	imx53-tx53-x03x.dtb \
>  	imx53-tx53-x13x.dtb \
> +	imx53-usbarmory.dtb \
>  	imx53-voipac-bsb.dtb
>  dtb-$(CONFIG_SOC_IMX6Q) += \
>  	imx6dl-apf6dev.dtb \
> diff --git a/arch/arm/boot/dts/imx53-usbarmory.dts b/arch/arm/boot/dts/imx53-usbarmory.dts
> new file mode 100644
> index 0000000..9a172fd
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx53-usbarmory.dts
> @@ -0,0 +1,239 @@
> +/*
> + * USB armory MkI device tree include file
> + * https://inversepath.com/usbarmory
> + *
> + * Copyright (C) 2015, Inverse Path
> + * Andrej Rosano <andrej@inversepath.com>
> + *
> + * Licensed under GPLv2
> + */

Maybe GPL/X11 dual licence to consider non-Linux users.  There are
plenty of dual licence examples under arch/arm/boot/dts.

> +
> +/dts-v1/;
> +#include "imx53.dtsi"
> +
> +/ {
> +	model = "Inverse Path USB armory";
> +	compatible = "inversepath,imx53-usbarmory", "fsl,imx53";
> +};
> +
> +/ {
> +	chosen {
> +		stdout-path = &uart1;
> +	};
> +
> +	memory {
> +		reg = <0x70000000 0x20000000>;
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&led_pin_gpio4_27>;
> +
> +		user {
> +			label = "LED";
> +			gpios = <&gpio4 27 0>;

Use the macro in include/dt-bindings/gpio/gpio.h for polarity.

> +			linux,default-trigger = "heartbeat";
> +		};
> +	};
> +};
> +
> +&cpu0 {
> +	operating-points = <
> +		/* kHz */
> +		166666  850000
> +		400000  900000
> +		800000 1050000
> +	>;

Can you put some comments in there explaining why you need a custom opp
table for the board?

> +};
> +
> +&esdhc1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_esdhc1>;
> +	status = "okay";
> +};
> +
> +&iomuxc {
> +	pinctrl-names = "default";

The property makes no sense where there is no pinctrl-0 property.

> +
> +	imx53-usbarmory {

This container node can be dropped to save an indentation level.

> +		led_pin_gpio4_27: led_gpio4_27 at 0 {

Can this node be named in a similar scheme as other pinctrl entries?

> +			fsl,pins = <
> +				MX53_PAD_DISP0_DAT6__GPIO4_27 0x0
> +			>;
> +		};
> +
> +		pinctrl_esdhc1: esdhc1grp {
> +			fsl,pins = <
> +				MX53_PAD_SD1_DATA0__ESDHC1_DAT0		0x1d5
> +				MX53_PAD_SD1_DATA1__ESDHC1_DAT1		0x1d5
> +				MX53_PAD_SD1_DATA2__ESDHC1_DAT2		0x1d5
> +				MX53_PAD_SD1_DATA3__ESDHC1_DAT3		0x1d5
> +				MX53_PAD_SD1_CMD__ESDHC1_CMD		0x1d5
> +				MX53_PAD_SD1_CLK__ESDHC1_CLK		0x1d5
> +			>;
> +		};
> +
> +		pinctrl_i2c1_pmic: i2c1grp_pmic {

Please try to sort these pinctrl entries alphabetically.

> +			fsl,pins = <
> +				MX53_PAD_EIM_D21__I2C1_SCL	0x0
> +				MX53_PAD_EIM_D28__I2C1_SDA	0x0
> +			>;
> +		};
> +
> +		/*
> +		 * UART mode pin header configration
> +		 * 3 - GPIO5[26], pull-down 100K
> +		 * 4 - GPIO5[27], pull-down 100K
> +		 * 5 - TX, pull-up 100K
> +		 * 6 - RX, pull-up 100K
> +		 * 7 - GPIO5[30], pull-down 100K
> +		 */
> +		pinctrl_uart1: uart1grp {
> +			fsl,pins = <
> +				MX53_PAD_CSI0_DAT8__GPIO5_26		0xc0
> +				MX53_PAD_CSI0_DAT9__GPIO5_27		0xc0
> +				MX53_PAD_CSI0_DAT10__UART1_TXD_MUX	0x1e4
> +				MX53_PAD_CSI0_DAT11__UART1_RXD_MUX	0x1e4
> +				MX53_PAD_CSI0_DAT12__GPIO5_30		0xc0
> +			>;
> +		};
> +
> +		/*
> +		 * GPIO mode pin header configuration
> +		 * 3 - GPIO5[26], pull-down 100K
> +		 * 4 - GPIO5[27], pull-down 100K
> +		 * 5 - GPIO5[28], pull-down 100K
> +		 * 6 - GPIO5[29], pull-down 100K
> +		 * 7 - GPIO5[30], pull-down 100K
> +		 */
> +		pinctrl_gpio5: gpio5grp {

The entry is referenced nowhere.

> +			fsl,pins = <
> +				MX53_PAD_CSI0_DAT8__GPIO5_26	0xc0
> +				MX53_PAD_CSI0_DAT9__GPIO5_27	0xc0
> +				MX53_PAD_CSI0_DAT10__GPIO5_28	0xc0
> +				MX53_PAD_CSI0_DAT11__GPIO5_29	0xc0
> +				MX53_PAD_CSI0_DAT12__GPIO5_30	0xc0
> +			>;
> +		};
> +
> +		/*
> +		 * SPI mode pin header configuration
> +		 * 3 - SCLK
> +		 * 4 - MOSI
> +		 * 5 - MISO
> +		 * 6 - /SS0
> +		 * 7 - /SS1
> +		 */
> +		pinctrl_ecspi2: ecspi2grp {
> +			fsl,pins = <
> +				MX53_PAD_CSI0_DAT8__ECSPI2_SCLK		0x0
> +				MX53_PAD_CSI0_DAT9__ECSPI2_MOSI		0x0
> +				MX53_PAD_CSI0_DAT10__ECSPI2_MISO	0x0
> +				MX53_PAD_CSI0_DAT11__GPIO5_29		0x0
> +				MX53_PAD_CSI0_DAT12__GPIO5_30		0x0
> +			>;
> +		};
> +
> +		/*
> +		 * I2C mode pin header configuration
> +		 * 3 - SDA
> +		 * 4 - SCL
> +		 * 5 - GPIO5[28], pull-down 100K
> +		 * 6 - GPIO5[29], pull-down 100K
> +		 * 7 - GPIO5[30], pull-down 100K
> +		 */
> +		pinctrl_i2c1_pinheader: i2c1grp_pinheader {

The entry is referenced nowhere.

> +			fsl,pins = <
> +				MX53_PAD_CSI0_DAT8__I2C1_SDA	0x0
> +				MX53_PAD_CSI0_DAT9__I2C1_SCL	0x0
> +				MX53_PAD_CSI0_DAT10__GPIO5_28	0xc0
> +				MX53_PAD_CSI0_DAT11__GPIO5_29	0xc0
> +				MX53_PAD_CSI0_DAT12__GPIO5_30	0xc0
> +			>;
> +		};
> +	};
> +};
> +
> +&uart1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart1>;
> +	status = "okay";
> +};
> +
> +&usbotg {
> +	dr_mode = "peripheral";
> +	status = "okay";
> +};
> +
> +&i2c1 {

Please sort these nodes alphabetically in label name.

> +	pinctrl-0 = <&pinctrl_i2c1_pmic>;
> +	status = "okay";

Have a newline between property list and sub-node.

> +	ltc3589: pmic at 34 {
> +		compatible = "lltc,ltc3589-2";
> +		reg = <0x34>;

Ditto

Shawn

> +		regulators {
> +			sw1_reg: sw1 {
> +				regulator-min-microvolt = <591930>;
> +				regulator-max-microvolt = <1224671>;
> +				lltc,fb-voltage-divider = <100000 158000>;
> +				regulator-ramp-delay = <7000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			sw2_reg: sw2 {
> +				regulator-min-microvolt = <704123>;
> +				regulator-max-microvolt = <1456803>;
> +				lltc,fb-voltage-divider = <180000 191000>;
> +				regulator-ramp-delay = <7000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			sw3_reg: sw3 {
> +				regulator-min-microvolt = <1341250>;
> +				regulator-max-microvolt = <2775000>;
> +				lltc,fb-voltage-divider = <270000 100000>;
> +				regulator-ramp-delay = <7000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			bb_out_reg: bb-out {
> +				regulator-min-microvolt = <3387341>;
> +				regulator-max-microvolt = <3387341>;
> +				lltc,fb-voltage-divider = <511000 158000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +			ldo1_reg: ldo1 {
> +				regulator-min-microvolt = <1306329>;
> +				regulator-max-microvolt = <1306329>;
> +				lltc,fb-voltage-divider = <100000 158000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo2_reg: ldo2 {
> +				regulator-min-microvolt = <704123>;
> +				regulator-max-microvolt = <1456806>;
> +				lltc,fb-voltage-divider = <180000 191000>;
> +				regulator-ramp-delay = <7000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo3_reg: ldo3 {
> +				regulator-min-microvolt = <2800000>;
> +				regulator-max-microvolt = <2800000>;
> +				regulator-boot-on;
> +			};
> +
> +			ldo4_reg: ldo4 {
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt = <3200000>;
> +			};
> +		};
> +	};
> +};
> -- 
> 2.9.0
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Shawn Guo <shawnguo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: andrej-6l9oX9VzF44FueaiHvvTSg@public.gmane.org
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Vagrant Cascadian
	<vagrant-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
	Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCH 2/2] ARM: dts: imx53: add support for USB armory board
Date: Tue, 28 Jun 2016 09:50:55 +0800	[thread overview]
Message-ID: <20160628015055.GA21328@tiger> (raw)
In-Reply-To: <20160621145053.7742-3-andrej-6l9oX9VzF44FueaiHvvTSg@public.gmane.org>

On Tue, Jun 21, 2016 at 04:50:53PM +0200, andrej-6l9oX9VzF44FueaiHvvTSg@public.gmane.org wrote:
> From: Andrej Rosano <andrej-6l9oX9VzF44FueaiHvvTSg@public.gmane.org>
> 
> Add support for Inverse Path USB armory board, an open source
> flash-drive sized computer based on NXP i.MX53 SoC.
> 
> https://inversepath.com/usbarmory
> 
> Signed-off-by: Andrej Rosano <andrej-6l9oX9VzF44FueaiHvvTSg@public.gmane.org>
> ---
>  arch/arm/boot/dts/Makefile            |   1 +
>  arch/arm/boot/dts/imx53-usbarmory.dts | 239 ++++++++++++++++++++++++++++++++++
>  2 files changed, 240 insertions(+)
>  create mode 100644 arch/arm/boot/dts/imx53-usbarmory.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 414b427..f8f85ab 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -303,6 +303,7 @@ dtb-$(CONFIG_SOC_IMX53) += \
>  	imx53-smd.dtb \
>  	imx53-tx53-x03x.dtb \
>  	imx53-tx53-x13x.dtb \
> +	imx53-usbarmory.dtb \
>  	imx53-voipac-bsb.dtb
>  dtb-$(CONFIG_SOC_IMX6Q) += \
>  	imx6dl-apf6dev.dtb \
> diff --git a/arch/arm/boot/dts/imx53-usbarmory.dts b/arch/arm/boot/dts/imx53-usbarmory.dts
> new file mode 100644
> index 0000000..9a172fd
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx53-usbarmory.dts
> @@ -0,0 +1,239 @@
> +/*
> + * USB armory MkI device tree include file
> + * https://inversepath.com/usbarmory
> + *
> + * Copyright (C) 2015, Inverse Path
> + * Andrej Rosano <andrej-6l9oX9VzF44FueaiHvvTSg@public.gmane.org>
> + *
> + * Licensed under GPLv2
> + */

Maybe GPL/X11 dual licence to consider non-Linux users.  There are
plenty of dual licence examples under arch/arm/boot/dts.

> +
> +/dts-v1/;
> +#include "imx53.dtsi"
> +
> +/ {
> +	model = "Inverse Path USB armory";
> +	compatible = "inversepath,imx53-usbarmory", "fsl,imx53";
> +};
> +
> +/ {
> +	chosen {
> +		stdout-path = &uart1;
> +	};
> +
> +	memory {
> +		reg = <0x70000000 0x20000000>;
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&led_pin_gpio4_27>;
> +
> +		user {
> +			label = "LED";
> +			gpios = <&gpio4 27 0>;

Use the macro in include/dt-bindings/gpio/gpio.h for polarity.

> +			linux,default-trigger = "heartbeat";
> +		};
> +	};
> +};
> +
> +&cpu0 {
> +	operating-points = <
> +		/* kHz */
> +		166666  850000
> +		400000  900000
> +		800000 1050000
> +	>;

Can you put some comments in there explaining why you need a custom opp
table for the board?

> +};
> +
> +&esdhc1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_esdhc1>;
> +	status = "okay";
> +};
> +
> +&iomuxc {
> +	pinctrl-names = "default";

The property makes no sense where there is no pinctrl-0 property.

> +
> +	imx53-usbarmory {

This container node can be dropped to save an indentation level.

> +		led_pin_gpio4_27: led_gpio4_27@0 {

Can this node be named in a similar scheme as other pinctrl entries?

> +			fsl,pins = <
> +				MX53_PAD_DISP0_DAT6__GPIO4_27 0x0
> +			>;
> +		};
> +
> +		pinctrl_esdhc1: esdhc1grp {
> +			fsl,pins = <
> +				MX53_PAD_SD1_DATA0__ESDHC1_DAT0		0x1d5
> +				MX53_PAD_SD1_DATA1__ESDHC1_DAT1		0x1d5
> +				MX53_PAD_SD1_DATA2__ESDHC1_DAT2		0x1d5
> +				MX53_PAD_SD1_DATA3__ESDHC1_DAT3		0x1d5
> +				MX53_PAD_SD1_CMD__ESDHC1_CMD		0x1d5
> +				MX53_PAD_SD1_CLK__ESDHC1_CLK		0x1d5
> +			>;
> +		};
> +
> +		pinctrl_i2c1_pmic: i2c1grp_pmic {

Please try to sort these pinctrl entries alphabetically.

> +			fsl,pins = <
> +				MX53_PAD_EIM_D21__I2C1_SCL	0x0
> +				MX53_PAD_EIM_D28__I2C1_SDA	0x0
> +			>;
> +		};
> +
> +		/*
> +		 * UART mode pin header configration
> +		 * 3 - GPIO5[26], pull-down 100K
> +		 * 4 - GPIO5[27], pull-down 100K
> +		 * 5 - TX, pull-up 100K
> +		 * 6 - RX, pull-up 100K
> +		 * 7 - GPIO5[30], pull-down 100K
> +		 */
> +		pinctrl_uart1: uart1grp {
> +			fsl,pins = <
> +				MX53_PAD_CSI0_DAT8__GPIO5_26		0xc0
> +				MX53_PAD_CSI0_DAT9__GPIO5_27		0xc0
> +				MX53_PAD_CSI0_DAT10__UART1_TXD_MUX	0x1e4
> +				MX53_PAD_CSI0_DAT11__UART1_RXD_MUX	0x1e4
> +				MX53_PAD_CSI0_DAT12__GPIO5_30		0xc0
> +			>;
> +		};
> +
> +		/*
> +		 * GPIO mode pin header configuration
> +		 * 3 - GPIO5[26], pull-down 100K
> +		 * 4 - GPIO5[27], pull-down 100K
> +		 * 5 - GPIO5[28], pull-down 100K
> +		 * 6 - GPIO5[29], pull-down 100K
> +		 * 7 - GPIO5[30], pull-down 100K
> +		 */
> +		pinctrl_gpio5: gpio5grp {

The entry is referenced nowhere.

> +			fsl,pins = <
> +				MX53_PAD_CSI0_DAT8__GPIO5_26	0xc0
> +				MX53_PAD_CSI0_DAT9__GPIO5_27	0xc0
> +				MX53_PAD_CSI0_DAT10__GPIO5_28	0xc0
> +				MX53_PAD_CSI0_DAT11__GPIO5_29	0xc0
> +				MX53_PAD_CSI0_DAT12__GPIO5_30	0xc0
> +			>;
> +		};
> +
> +		/*
> +		 * SPI mode pin header configuration
> +		 * 3 - SCLK
> +		 * 4 - MOSI
> +		 * 5 - MISO
> +		 * 6 - /SS0
> +		 * 7 - /SS1
> +		 */
> +		pinctrl_ecspi2: ecspi2grp {
> +			fsl,pins = <
> +				MX53_PAD_CSI0_DAT8__ECSPI2_SCLK		0x0
> +				MX53_PAD_CSI0_DAT9__ECSPI2_MOSI		0x0
> +				MX53_PAD_CSI0_DAT10__ECSPI2_MISO	0x0
> +				MX53_PAD_CSI0_DAT11__GPIO5_29		0x0
> +				MX53_PAD_CSI0_DAT12__GPIO5_30		0x0
> +			>;
> +		};
> +
> +		/*
> +		 * I2C mode pin header configuration
> +		 * 3 - SDA
> +		 * 4 - SCL
> +		 * 5 - GPIO5[28], pull-down 100K
> +		 * 6 - GPIO5[29], pull-down 100K
> +		 * 7 - GPIO5[30], pull-down 100K
> +		 */
> +		pinctrl_i2c1_pinheader: i2c1grp_pinheader {

The entry is referenced nowhere.

> +			fsl,pins = <
> +				MX53_PAD_CSI0_DAT8__I2C1_SDA	0x0
> +				MX53_PAD_CSI0_DAT9__I2C1_SCL	0x0
> +				MX53_PAD_CSI0_DAT10__GPIO5_28	0xc0
> +				MX53_PAD_CSI0_DAT11__GPIO5_29	0xc0
> +				MX53_PAD_CSI0_DAT12__GPIO5_30	0xc0
> +			>;
> +		};
> +	};
> +};
> +
> +&uart1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart1>;
> +	status = "okay";
> +};
> +
> +&usbotg {
> +	dr_mode = "peripheral";
> +	status = "okay";
> +};
> +
> +&i2c1 {

Please sort these nodes alphabetically in label name.

> +	pinctrl-0 = <&pinctrl_i2c1_pmic>;
> +	status = "okay";

Have a newline between property list and sub-node.

> +	ltc3589: pmic@34 {
> +		compatible = "lltc,ltc3589-2";
> +		reg = <0x34>;

Ditto

Shawn

> +		regulators {
> +			sw1_reg: sw1 {
> +				regulator-min-microvolt = <591930>;
> +				regulator-max-microvolt = <1224671>;
> +				lltc,fb-voltage-divider = <100000 158000>;
> +				regulator-ramp-delay = <7000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			sw2_reg: sw2 {
> +				regulator-min-microvolt = <704123>;
> +				regulator-max-microvolt = <1456803>;
> +				lltc,fb-voltage-divider = <180000 191000>;
> +				regulator-ramp-delay = <7000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			sw3_reg: sw3 {
> +				regulator-min-microvolt = <1341250>;
> +				regulator-max-microvolt = <2775000>;
> +				lltc,fb-voltage-divider = <270000 100000>;
> +				regulator-ramp-delay = <7000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			bb_out_reg: bb-out {
> +				regulator-min-microvolt = <3387341>;
> +				regulator-max-microvolt = <3387341>;
> +				lltc,fb-voltage-divider = <511000 158000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +			ldo1_reg: ldo1 {
> +				regulator-min-microvolt = <1306329>;
> +				regulator-max-microvolt = <1306329>;
> +				lltc,fb-voltage-divider = <100000 158000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo2_reg: ldo2 {
> +				regulator-min-microvolt = <704123>;
> +				regulator-max-microvolt = <1456806>;
> +				lltc,fb-voltage-divider = <180000 191000>;
> +				regulator-ramp-delay = <7000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo3_reg: ldo3 {
> +				regulator-min-microvolt = <2800000>;
> +				regulator-max-microvolt = <2800000>;
> +				regulator-boot-on;
> +			};
> +
> +			ldo4_reg: ldo4 {
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt = <3200000>;
> +			};
> +		};
> +	};
> +};
> -- 
> 2.9.0
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
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

WARNING: multiple messages have this Message-ID (diff)
From: Shawn Guo <shawnguo@kernel.org>
To: andrej@inversepath.com
Cc: devicetree@vger.kernel.org,
	Vagrant Cascadian <vagrant@debian.org>,
	linux-kernel@vger.kernel.org,
	Sascha Hauer <kernel@pengutronix.de>,
	Fabio Estevam <fabio.estevam@nxp.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 2/2] ARM: dts: imx53: add support for USB armory board
Date: Tue, 28 Jun 2016 09:50:55 +0800	[thread overview]
Message-ID: <20160628015055.GA21328@tiger> (raw)
In-Reply-To: <20160621145053.7742-3-andrej@inversepath.com>

On Tue, Jun 21, 2016 at 04:50:53PM +0200, andrej@inversepath.com wrote:
> From: Andrej Rosano <andrej@inversepath.com>
> 
> Add support for Inverse Path USB armory board, an open source
> flash-drive sized computer based on NXP i.MX53 SoC.
> 
> https://inversepath.com/usbarmory
> 
> Signed-off-by: Andrej Rosano <andrej@inversepath.com>
> ---
>  arch/arm/boot/dts/Makefile            |   1 +
>  arch/arm/boot/dts/imx53-usbarmory.dts | 239 ++++++++++++++++++++++++++++++++++
>  2 files changed, 240 insertions(+)
>  create mode 100644 arch/arm/boot/dts/imx53-usbarmory.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 414b427..f8f85ab 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -303,6 +303,7 @@ dtb-$(CONFIG_SOC_IMX53) += \
>  	imx53-smd.dtb \
>  	imx53-tx53-x03x.dtb \
>  	imx53-tx53-x13x.dtb \
> +	imx53-usbarmory.dtb \
>  	imx53-voipac-bsb.dtb
>  dtb-$(CONFIG_SOC_IMX6Q) += \
>  	imx6dl-apf6dev.dtb \
> diff --git a/arch/arm/boot/dts/imx53-usbarmory.dts b/arch/arm/boot/dts/imx53-usbarmory.dts
> new file mode 100644
> index 0000000..9a172fd
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx53-usbarmory.dts
> @@ -0,0 +1,239 @@
> +/*
> + * USB armory MkI device tree include file
> + * https://inversepath.com/usbarmory
> + *
> + * Copyright (C) 2015, Inverse Path
> + * Andrej Rosano <andrej@inversepath.com>
> + *
> + * Licensed under GPLv2
> + */

Maybe GPL/X11 dual licence to consider non-Linux users.  There are
plenty of dual licence examples under arch/arm/boot/dts.

> +
> +/dts-v1/;
> +#include "imx53.dtsi"
> +
> +/ {
> +	model = "Inverse Path USB armory";
> +	compatible = "inversepath,imx53-usbarmory", "fsl,imx53";
> +};
> +
> +/ {
> +	chosen {
> +		stdout-path = &uart1;
> +	};
> +
> +	memory {
> +		reg = <0x70000000 0x20000000>;
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&led_pin_gpio4_27>;
> +
> +		user {
> +			label = "LED";
> +			gpios = <&gpio4 27 0>;

Use the macro in include/dt-bindings/gpio/gpio.h for polarity.

> +			linux,default-trigger = "heartbeat";
> +		};
> +	};
> +};
> +
> +&cpu0 {
> +	operating-points = <
> +		/* kHz */
> +		166666  850000
> +		400000  900000
> +		800000 1050000
> +	>;

Can you put some comments in there explaining why you need a custom opp
table for the board?

> +};
> +
> +&esdhc1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_esdhc1>;
> +	status = "okay";
> +};
> +
> +&iomuxc {
> +	pinctrl-names = "default";

The property makes no sense where there is no pinctrl-0 property.

> +
> +	imx53-usbarmory {

This container node can be dropped to save an indentation level.

> +		led_pin_gpio4_27: led_gpio4_27@0 {

Can this node be named in a similar scheme as other pinctrl entries?

> +			fsl,pins = <
> +				MX53_PAD_DISP0_DAT6__GPIO4_27 0x0
> +			>;
> +		};
> +
> +		pinctrl_esdhc1: esdhc1grp {
> +			fsl,pins = <
> +				MX53_PAD_SD1_DATA0__ESDHC1_DAT0		0x1d5
> +				MX53_PAD_SD1_DATA1__ESDHC1_DAT1		0x1d5
> +				MX53_PAD_SD1_DATA2__ESDHC1_DAT2		0x1d5
> +				MX53_PAD_SD1_DATA3__ESDHC1_DAT3		0x1d5
> +				MX53_PAD_SD1_CMD__ESDHC1_CMD		0x1d5
> +				MX53_PAD_SD1_CLK__ESDHC1_CLK		0x1d5
> +			>;
> +		};
> +
> +		pinctrl_i2c1_pmic: i2c1grp_pmic {

Please try to sort these pinctrl entries alphabetically.

> +			fsl,pins = <
> +				MX53_PAD_EIM_D21__I2C1_SCL	0x0
> +				MX53_PAD_EIM_D28__I2C1_SDA	0x0
> +			>;
> +		};
> +
> +		/*
> +		 * UART mode pin header configration
> +		 * 3 - GPIO5[26], pull-down 100K
> +		 * 4 - GPIO5[27], pull-down 100K
> +		 * 5 - TX, pull-up 100K
> +		 * 6 - RX, pull-up 100K
> +		 * 7 - GPIO5[30], pull-down 100K
> +		 */
> +		pinctrl_uart1: uart1grp {
> +			fsl,pins = <
> +				MX53_PAD_CSI0_DAT8__GPIO5_26		0xc0
> +				MX53_PAD_CSI0_DAT9__GPIO5_27		0xc0
> +				MX53_PAD_CSI0_DAT10__UART1_TXD_MUX	0x1e4
> +				MX53_PAD_CSI0_DAT11__UART1_RXD_MUX	0x1e4
> +				MX53_PAD_CSI0_DAT12__GPIO5_30		0xc0
> +			>;
> +		};
> +
> +		/*
> +		 * GPIO mode pin header configuration
> +		 * 3 - GPIO5[26], pull-down 100K
> +		 * 4 - GPIO5[27], pull-down 100K
> +		 * 5 - GPIO5[28], pull-down 100K
> +		 * 6 - GPIO5[29], pull-down 100K
> +		 * 7 - GPIO5[30], pull-down 100K
> +		 */
> +		pinctrl_gpio5: gpio5grp {

The entry is referenced nowhere.

> +			fsl,pins = <
> +				MX53_PAD_CSI0_DAT8__GPIO5_26	0xc0
> +				MX53_PAD_CSI0_DAT9__GPIO5_27	0xc0
> +				MX53_PAD_CSI0_DAT10__GPIO5_28	0xc0
> +				MX53_PAD_CSI0_DAT11__GPIO5_29	0xc0
> +				MX53_PAD_CSI0_DAT12__GPIO5_30	0xc0
> +			>;
> +		};
> +
> +		/*
> +		 * SPI mode pin header configuration
> +		 * 3 - SCLK
> +		 * 4 - MOSI
> +		 * 5 - MISO
> +		 * 6 - /SS0
> +		 * 7 - /SS1
> +		 */
> +		pinctrl_ecspi2: ecspi2grp {
> +			fsl,pins = <
> +				MX53_PAD_CSI0_DAT8__ECSPI2_SCLK		0x0
> +				MX53_PAD_CSI0_DAT9__ECSPI2_MOSI		0x0
> +				MX53_PAD_CSI0_DAT10__ECSPI2_MISO	0x0
> +				MX53_PAD_CSI0_DAT11__GPIO5_29		0x0
> +				MX53_PAD_CSI0_DAT12__GPIO5_30		0x0
> +			>;
> +		};
> +
> +		/*
> +		 * I2C mode pin header configuration
> +		 * 3 - SDA
> +		 * 4 - SCL
> +		 * 5 - GPIO5[28], pull-down 100K
> +		 * 6 - GPIO5[29], pull-down 100K
> +		 * 7 - GPIO5[30], pull-down 100K
> +		 */
> +		pinctrl_i2c1_pinheader: i2c1grp_pinheader {

The entry is referenced nowhere.

> +			fsl,pins = <
> +				MX53_PAD_CSI0_DAT8__I2C1_SDA	0x0
> +				MX53_PAD_CSI0_DAT9__I2C1_SCL	0x0
> +				MX53_PAD_CSI0_DAT10__GPIO5_28	0xc0
> +				MX53_PAD_CSI0_DAT11__GPIO5_29	0xc0
> +				MX53_PAD_CSI0_DAT12__GPIO5_30	0xc0
> +			>;
> +		};
> +	};
> +};
> +
> +&uart1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart1>;
> +	status = "okay";
> +};
> +
> +&usbotg {
> +	dr_mode = "peripheral";
> +	status = "okay";
> +};
> +
> +&i2c1 {

Please sort these nodes alphabetically in label name.

> +	pinctrl-0 = <&pinctrl_i2c1_pmic>;
> +	status = "okay";

Have a newline between property list and sub-node.

> +	ltc3589: pmic@34 {
> +		compatible = "lltc,ltc3589-2";
> +		reg = <0x34>;

Ditto

Shawn

> +		regulators {
> +			sw1_reg: sw1 {
> +				regulator-min-microvolt = <591930>;
> +				regulator-max-microvolt = <1224671>;
> +				lltc,fb-voltage-divider = <100000 158000>;
> +				regulator-ramp-delay = <7000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			sw2_reg: sw2 {
> +				regulator-min-microvolt = <704123>;
> +				regulator-max-microvolt = <1456803>;
> +				lltc,fb-voltage-divider = <180000 191000>;
> +				regulator-ramp-delay = <7000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			sw3_reg: sw3 {
> +				regulator-min-microvolt = <1341250>;
> +				regulator-max-microvolt = <2775000>;
> +				lltc,fb-voltage-divider = <270000 100000>;
> +				regulator-ramp-delay = <7000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			bb_out_reg: bb-out {
> +				regulator-min-microvolt = <3387341>;
> +				regulator-max-microvolt = <3387341>;
> +				lltc,fb-voltage-divider = <511000 158000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +			ldo1_reg: ldo1 {
> +				regulator-min-microvolt = <1306329>;
> +				regulator-max-microvolt = <1306329>;
> +				lltc,fb-voltage-divider = <100000 158000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo2_reg: ldo2 {
> +				regulator-min-microvolt = <704123>;
> +				regulator-max-microvolt = <1456806>;
> +				lltc,fb-voltage-divider = <180000 191000>;
> +				regulator-ramp-delay = <7000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo3_reg: ldo3 {
> +				regulator-min-microvolt = <2800000>;
> +				regulator-max-microvolt = <2800000>;
> +				regulator-boot-on;
> +			};
> +
> +			ldo4_reg: ldo4 {
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt = <3200000>;
> +			};
> +		};
> +	};
> +};
> -- 
> 2.9.0
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2016-06-28  1:50 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-21 14:50 [PATCH 0/2] ARM: dts: imx53: add support for USB armory board andrej at inversepath.com
2016-06-21 14:50 ` andrej
2016-06-21 14:50 ` andrej
2016-06-21 14:50 ` [PATCH 1/2] devicetree: Add vendor prefix for Inverse Path andrej at inversepath.com
2016-06-21 14:50   ` andrej
2016-06-21 14:50   ` andrej
2016-06-21 21:59   ` Rob Herring
2016-06-21 21:59     ` Rob Herring
2016-06-21 14:50 ` [PATCH 2/2] ARM: dts: imx53: add support for USB armory board andrej at inversepath.com
2016-06-21 14:50   ` andrej
2016-06-21 14:50   ` andrej
2016-06-28  1:50   ` Shawn Guo [this message]
2016-06-28  1:50     ` Shawn Guo
2016-06-28  1:50     ` Shawn Guo
2016-06-28 13:24     ` Andrej Rosano
2016-06-28 13:24       ` Andrej Rosano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160628015055.GA21328@tiger \
    --to=shawnguo@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.