All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] dts: sun8i-h3: Add DTS for Mapleboard MP130
@ 2018-10-25 19:55 ` Jonathan McDowell
  0 siblings, 0 replies; 6+ messages in thread
From: Jonathan McDowell @ 2018-10-25 19:55 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, devicetree, Maxime Ripard, Chen-Yu Tsai


The Mapleboard MP130 is a single board computer based on the Allwinner
H3 SoC, with all schematics freely available. The Lite version includes
1GB main memory and 8GB eMMC.

https://www.mapleboard.org/en (still mostly in Chinese even when English
is selected)

This DTS is based upon the DTS shipped with the board which uses
mapleboard,mp130- prefixes instead of the allwinner,sun8i variants.

Signed-off-by: Jonathan McDowell <noodles@earth.li>

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index b5bd3de87c33..dcf1b9e7f71e 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -1035,6 +1035,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
 	sun8i-h3-bananapi-m2-plus.dtb \
 	sun8i-h3-beelink-x2.dtb \
 	sun8i-h3-libretech-all-h3-cc.dtb \
+	sun8i-h3-mapleboard-mp130.dtb \
 	sun8i-h3-nanopi-m1.dtb	\
 	sun8i-h3-nanopi-m1-plus.dtb \
 	sun8i-h3-nanopi-neo.dtb \
diff --git a/arch/arm/boot/dts/sun8i-h3-mapleboard-mp130.dts b/arch/arm/boot/dts/sun8i-h3-mapleboard-mp130.dts
new file mode 100644
index 000000000000..bf948128af94
--- /dev/null
+++ b/arch/arm/boot/dts/sun8i-h3-mapleboard-mp130.dts
@@ -0,0 +1,179 @@
+/*
+ * Copyright (C) 2017 Centrum Embedded Systems, Jia-Bin Huang <jb@ces.com.tw>
+ * Copyright (C) 2018 Jonathan McDowell <noodles@earth.li>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+/dts-v1/;
+#include "sun8i-h3.dtsi"
+#include "sunxi-common-regulators.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+
+/ {
+	model = "MapleBoard MP130 Board";
+	compatible = "mapleboard,mp130_board", "allwinner,sun8i-h3";
+
+	aliases {
+		ethernet0 = &emac;
+		serial0 = &uart0;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	leds {
+		compatible = "gpio-leds";
+		pinctrl-names = "default";
+		pinctrl-0 = <&leds_pio>, <&leds_r_pio>;
+
+		pwr_led {
+			label = "mp130_board:orange:pwr";
+			gpios = <&r_pio 0 10 GPIO_ACTIVE_HIGH>;
+			default-state = "on";
+		};
+		status_led {
+			label = "mp130_board:orange:status";
+			gpios = <&pio 0 10 GPIO_ACTIVE_HIGH>;
+		};
+	};
+
+	r_gpio_keys {
+		compatible = "gpio-keys";
+		pinctrl-names = "default";
+		pinctrl-0 = <&sw_r_pio>;
+
+		power {
+			label = "power";
+			linux,code = <KEY_POWER>;
+			gpios = <&r_pio 0 3 GPIO_ACTIVE_LOW>; /* PL3 */
+		};
+
+		user {
+			label = "user";
+			linux,code = <BTN_0>;
+			gpios = <&r_pio 0 4 GPIO_ACTIVE_LOW>;
+		};
+	};
+};
+
+&codec {
+	allwinner,audio-routing =
+		"Line Out", "LINEOUT",
+		"LINEIN", "Line In";
+	status = "okay";
+};
+
+&ehci1 {
+	status = "okay";
+};
+
+&ehci2 {
+	status = "okay";
+};
+
+&ehci3 {
+	status = "okay";
+};
+
+&ir {
+	pinctrl-names = "default";
+	pinctrl-0 = <&ir_pins_a>;
+	status = "okay";
+};
+
+&mmc0 {
+	vmmc-supply = <&reg_vcc3v3>;
+	bus-width = <4>;
+	cd-gpios = <&pio 5 6 GPIO_ACTIVE_HIGH>; /* PF6 */
+	cd-inverted;
+	status = "okay";
+};
+
+&mmc2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&mmc2_8bit_pins>;
+	vmmc-supply = <&reg_vcc3v3>;
+	bus-width = <8>;
+	non-removable;
+	cap-mmc-hw-reset;
+	status = "okay";
+};
+
+&ohci1 {
+	status = "okay";
+};
+
+&ohci2 {
+	status = "okay";
+};
+
+&ohci3 {
+	status = "okay";
+};
+
+&pio {
+	leds_pio: led_pins {
+		pins = "PA15";
+		function = "gpio_out";
+	};
+};
+
+&r_pio {
+	leds_r_pio: led_pins {
+		pins = "PL10";
+		function = "gpio_out";
+	};
+
+	sw_r_pio: key_pins {
+		pins = "PL3", "PL4";
+		function = "gpio_in";
+	};
+};
+
+&uart0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart0_pins_a>;
+	status = "okay";
+};
+
+&uart1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart1_pins>;
+	status = "disabled";
+};
+
+&uart2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart2_pins>;
+	status = "disabled";
+};
+
+&uart3 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart3_pins>;
+	status = "disabled";
+};
+
+&usb_otg {
+	dr_mode = "peripheral";
+	status = "okay";
+};
+
+&usbphy {
+	/* USB VBUS is always on */
+	status = "okay";
+};
+
+&emac {
+	phy-handle = <&int_mii_phy>;
+	phy-mode = "mii";
+	allwinner,leds-active-low;
+	status = "okay";
+};
+

J.

-- 
Revd Jonathan McDowell, ULC | Do you believe in happy endings?

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] dts: sun8i-h3: Add DTS for Mapleboard MP130
@ 2018-10-25 19:55 ` Jonathan McDowell
  0 siblings, 0 replies; 6+ messages in thread
From: Jonathan McDowell @ 2018-10-25 19:55 UTC (permalink / raw)
  To: linux-arm-kernel


The Mapleboard MP130 is a single board computer based on the Allwinner
H3 SoC, with all schematics freely available. The Lite version includes
1GB main memory and 8GB eMMC.

https://www.mapleboard.org/en (still mostly in Chinese even when English
is selected)

This DTS is based upon the DTS shipped with the board which uses
mapleboard,mp130- prefixes instead of the allwinner,sun8i variants.

Signed-off-by: Jonathan McDowell <noodles@earth.li>

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index b5bd3de87c33..dcf1b9e7f71e 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -1035,6 +1035,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
 	sun8i-h3-bananapi-m2-plus.dtb \
 	sun8i-h3-beelink-x2.dtb \
 	sun8i-h3-libretech-all-h3-cc.dtb \
+	sun8i-h3-mapleboard-mp130.dtb \
 	sun8i-h3-nanopi-m1.dtb	\
 	sun8i-h3-nanopi-m1-plus.dtb \
 	sun8i-h3-nanopi-neo.dtb \
diff --git a/arch/arm/boot/dts/sun8i-h3-mapleboard-mp130.dts b/arch/arm/boot/dts/sun8i-h3-mapleboard-mp130.dts
new file mode 100644
index 000000000000..bf948128af94
--- /dev/null
+++ b/arch/arm/boot/dts/sun8i-h3-mapleboard-mp130.dts
@@ -0,0 +1,179 @@
+/*
+ * Copyright (C) 2017 Centrum Embedded Systems, Jia-Bin Huang <jb@ces.com.tw>
+ * Copyright (C) 2018 Jonathan McDowell <noodles@earth.li>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+/dts-v1/;
+#include "sun8i-h3.dtsi"
+#include "sunxi-common-regulators.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+
+/ {
+	model = "MapleBoard MP130 Board";
+	compatible = "mapleboard,mp130_board", "allwinner,sun8i-h3";
+
+	aliases {
+		ethernet0 = &emac;
+		serial0 = &uart0;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	leds {
+		compatible = "gpio-leds";
+		pinctrl-names = "default";
+		pinctrl-0 = <&leds_pio>, <&leds_r_pio>;
+
+		pwr_led {
+			label = "mp130_board:orange:pwr";
+			gpios = <&r_pio 0 10 GPIO_ACTIVE_HIGH>;
+			default-state = "on";
+		};
+		status_led {
+			label = "mp130_board:orange:status";
+			gpios = <&pio 0 10 GPIO_ACTIVE_HIGH>;
+		};
+	};
+
+	r_gpio_keys {
+		compatible = "gpio-keys";
+		pinctrl-names = "default";
+		pinctrl-0 = <&sw_r_pio>;
+
+		power {
+			label = "power";
+			linux,code = <KEY_POWER>;
+			gpios = <&r_pio 0 3 GPIO_ACTIVE_LOW>; /* PL3 */
+		};
+
+		user {
+			label = "user";
+			linux,code = <BTN_0>;
+			gpios = <&r_pio 0 4 GPIO_ACTIVE_LOW>;
+		};
+	};
+};
+
+&codec {
+	allwinner,audio-routing =
+		"Line Out", "LINEOUT",
+		"LINEIN", "Line In";
+	status = "okay";
+};
+
+&ehci1 {
+	status = "okay";
+};
+
+&ehci2 {
+	status = "okay";
+};
+
+&ehci3 {
+	status = "okay";
+};
+
+&ir {
+	pinctrl-names = "default";
+	pinctrl-0 = <&ir_pins_a>;
+	status = "okay";
+};
+
+&mmc0 {
+	vmmc-supply = <&reg_vcc3v3>;
+	bus-width = <4>;
+	cd-gpios = <&pio 5 6 GPIO_ACTIVE_HIGH>; /* PF6 */
+	cd-inverted;
+	status = "okay";
+};
+
+&mmc2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&mmc2_8bit_pins>;
+	vmmc-supply = <&reg_vcc3v3>;
+	bus-width = <8>;
+	non-removable;
+	cap-mmc-hw-reset;
+	status = "okay";
+};
+
+&ohci1 {
+	status = "okay";
+};
+
+&ohci2 {
+	status = "okay";
+};
+
+&ohci3 {
+	status = "okay";
+};
+
+&pio {
+	leds_pio: led_pins {
+		pins = "PA15";
+		function = "gpio_out";
+	};
+};
+
+&r_pio {
+	leds_r_pio: led_pins {
+		pins = "PL10";
+		function = "gpio_out";
+	};
+
+	sw_r_pio: key_pins {
+		pins = "PL3", "PL4";
+		function = "gpio_in";
+	};
+};
+
+&uart0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart0_pins_a>;
+	status = "okay";
+};
+
+&uart1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart1_pins>;
+	status = "disabled";
+};
+
+&uart2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart2_pins>;
+	status = "disabled";
+};
+
+&uart3 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart3_pins>;
+	status = "disabled";
+};
+
+&usb_otg {
+	dr_mode = "peripheral";
+	status = "okay";
+};
+
+&usbphy {
+	/* USB VBUS is always on */
+	status = "okay";
+};
+
+&emac {
+	phy-handle = <&int_mii_phy>;
+	phy-mode = "mii";
+	allwinner,leds-active-low;
+	status = "okay";
+};
+

J.

-- 
Revd Jonathan McDowell, ULC | Do you believe in happy endings?

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] dts: sun8i-h3: Add DTS for Mapleboard MP130
  2018-10-25 19:55 ` Jonathan McDowell
@ 2018-10-29 15:20   ` Maxime Ripard
  -1 siblings, 0 replies; 6+ messages in thread
From: Maxime Ripard @ 2018-10-29 15:20 UTC (permalink / raw)
  To: Jonathan McDowell
  Cc: linux-arm-kernel, linux-kernel, devicetree, Chen-Yu Tsai

[-- Attachment #1: Type: text/plain, Size: 5859 bytes --]

Hi Jonathan,

Thanks for your patch.

On Thu, Oct 25, 2018 at 08:55:19PM +0100, Jonathan McDowell wrote:
> 
> The Mapleboard MP130 is a single board computer based on the Allwinner
> H3 SoC, with all schematics freely available. The Lite version includes
> 1GB main memory and 8GB eMMC.
> 
> https://www.mapleboard.org/en (still mostly in Chinese even when English
> is selected)
> 
> This DTS is based upon the DTS shipped with the board which uses
> mapleboard,mp130- prefixes instead of the allwinner,sun8i variants.
> 
> Signed-off-by: Jonathan McDowell <noodles@earth.li>

The prefix of your patch should be "ARM: dts: sun8i-h3: ..."

> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index b5bd3de87c33..dcf1b9e7f71e 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -1035,6 +1035,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
>  	sun8i-h3-bananapi-m2-plus.dtb \
>  	sun8i-h3-beelink-x2.dtb \
>  	sun8i-h3-libretech-all-h3-cc.dtb \
> +	sun8i-h3-mapleboard-mp130.dtb \
>  	sun8i-h3-nanopi-m1.dtb	\
>  	sun8i-h3-nanopi-m1-plus.dtb \
>  	sun8i-h3-nanopi-neo.dtb \
> diff --git a/arch/arm/boot/dts/sun8i-h3-mapleboard-mp130.dts b/arch/arm/boot/dts/sun8i-h3-mapleboard-mp130.dts
> new file mode 100644
> index 000000000000..bf948128af94
> --- /dev/null
> +++ b/arch/arm/boot/dts/sun8i-h3-mapleboard-mp130.dts
> @@ -0,0 +1,179 @@
> +/*
> + * Copyright (C) 2017 Centrum Embedded Systems, Jia-Bin Huang <jb@ces.com.tw>
> + * Copyright (C) 2018 Jonathan McDowell <noodles@earth.li>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.

You should add a SPDX tag instead of the license.

We're also trying to license the DT under a dual GPL/MIT license,
since they can be used by projects under a license that is not the
GPL. But that's your call, obviously.

> + */
> +
> +/dts-v1/;
> +#include "sun8i-h3.dtsi"
> +#include "sunxi-common-regulators.dtsi"
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +
> +/ {
> +	model = "MapleBoard MP130 Board";
> +	compatible = "mapleboard,mp130_board", "allwinner,sun8i-h3";

The "board" in both the model and compatible is redundant.

> +
> +	aliases {
> +		ethernet0 = &emac;
> +		serial0 = &uart0;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&leds_pio>, <&leds_r_pio>;

You can drop those pinctrl nodes

> +		pwr_led {
> +			label = "mp130_board:orange:pwr";
> +			gpios = <&r_pio 0 10 GPIO_ACTIVE_HIGH>;
> +			default-state = "on";
> +		};

A new line here would be nice

> +		status_led {
> +			label = "mp130_board:orange:status";
> +			gpios = <&pio 0 10 GPIO_ACTIVE_HIGH>;
> +		};
> +	};
> +
> +	r_gpio_keys {
> +		compatible = "gpio-keys";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&sw_r_pio>;

Samething the pinctrl reference can be dropped

> +		power {
> +			label = "power";
> +			linux,code = <KEY_POWER>;
> +			gpios = <&r_pio 0 3 GPIO_ACTIVE_LOW>; /* PL3 */
> +		};
> +
> +		user {
> +			label = "user";
> +			linux,code = <BTN_0>;
> +			gpios = <&r_pio 0 4 GPIO_ACTIVE_LOW>;
> +		};

The labels should be under the form "board:color:function"

> +	};
> +};
> +
> +&codec {
> +	allwinner,audio-routing =
> +		"Line Out", "LINEOUT",
> +		"LINEIN", "Line In";
> +	status = "okay";
> +};
> +
> +&ehci1 {
> +	status = "okay";
> +};
> +
> +&ehci2 {
> +	status = "okay";
> +};
> +
> +&ehci3 {
> +	status = "okay";
> +};
> +
> +&ir {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&ir_pins_a>;
> +	status = "okay";
> +};
> +
> +&mmc0 {
> +	vmmc-supply = <&reg_vcc3v3>;
> +	bus-width = <4>;
> +	cd-gpios = <&pio 5 6 GPIO_ACTIVE_HIGH>; /* PF6 */
> +	cd-inverted;
> +	status = "okay";
> +};
> +
> +&mmc2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&mmc2_8bit_pins>;
> +	vmmc-supply = <&reg_vcc3v3>;
> +	bus-width = <8>;
> +	non-removable;
> +	cap-mmc-hw-reset;
> +	status = "okay";
> +};
> +
> +&ohci1 {
> +	status = "okay";
> +};
> +
> +&ohci2 {
> +	status = "okay";
> +};
> +
> +&ohci3 {
> +	status = "okay";
> +};
> +
> +&pio {
> +	leds_pio: led_pins {
> +		pins = "PA15";
> +		function = "gpio_out";
> +	};
> +};
> +
> +&r_pio {
> +	leds_r_pio: led_pins {
> +		pins = "PL10";
> +		function = "gpio_out";
> +	};
> +
> +	sw_r_pio: key_pins {
> +		pins = "PL3", "PL4";
> +		function = "gpio_in";
> +	};
> +};
> +
> +&uart0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&uart0_pins_a>;
> +	status = "okay";
> +};
> +
> +&uart1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&uart1_pins>;
> +	status = "disabled";
> +};
> +
> +&uart2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&uart2_pins>;
> +	status = "disabled";
> +};
> +
> +&uart3 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&uart3_pins>;
> +	status = "disabled";
> +};
> +
> +&usb_otg {
> +	dr_mode = "peripheral";
> +	status = "okay";
> +};
> +
> +&usbphy {
> +	/* USB VBUS is always on */
> +	status = "okay";
> +};
> +
> +&emac {
> +	phy-handle = <&int_mii_phy>;
> +	phy-mode = "mii";
> +	allwinner,leds-active-low;
> +	status = "okay";
> +};
> +

This should be ordered alphabetically

> J.
> 
> -- 
> Revd Jonathan McDowell, ULC | Do you believe in happy endings?

Please don't send patches through your mailer. git send-email can be
configured to send the patches directly, this should be your preferred
option.

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] dts: sun8i-h3: Add DTS for Mapleboard MP130
@ 2018-10-29 15:20   ` Maxime Ripard
  0 siblings, 0 replies; 6+ messages in thread
From: Maxime Ripard @ 2018-10-29 15:20 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Jonathan,

Thanks for your patch.

On Thu, Oct 25, 2018 at 08:55:19PM +0100, Jonathan McDowell wrote:
> 
> The Mapleboard MP130 is a single board computer based on the Allwinner
> H3 SoC, with all schematics freely available. The Lite version includes
> 1GB main memory and 8GB eMMC.
> 
> https://www.mapleboard.org/en (still mostly in Chinese even when English
> is selected)
> 
> This DTS is based upon the DTS shipped with the board which uses
> mapleboard,mp130- prefixes instead of the allwinner,sun8i variants.
> 
> Signed-off-by: Jonathan McDowell <noodles@earth.li>

The prefix of your patch should be "ARM: dts: sun8i-h3: ..."

> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index b5bd3de87c33..dcf1b9e7f71e 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -1035,6 +1035,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
>  	sun8i-h3-bananapi-m2-plus.dtb \
>  	sun8i-h3-beelink-x2.dtb \
>  	sun8i-h3-libretech-all-h3-cc.dtb \
> +	sun8i-h3-mapleboard-mp130.dtb \
>  	sun8i-h3-nanopi-m1.dtb	\
>  	sun8i-h3-nanopi-m1-plus.dtb \
>  	sun8i-h3-nanopi-neo.dtb \
> diff --git a/arch/arm/boot/dts/sun8i-h3-mapleboard-mp130.dts b/arch/arm/boot/dts/sun8i-h3-mapleboard-mp130.dts
> new file mode 100644
> index 000000000000..bf948128af94
> --- /dev/null
> +++ b/arch/arm/boot/dts/sun8i-h3-mapleboard-mp130.dts
> @@ -0,0 +1,179 @@
> +/*
> + * Copyright (C) 2017 Centrum Embedded Systems, Jia-Bin Huang <jb@ces.com.tw>
> + * Copyright (C) 2018 Jonathan McDowell <noodles@earth.li>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.

You should add a SPDX tag instead of the license.

We're also trying to license the DT under a dual GPL/MIT license,
since they can be used by projects under a license that is not the
GPL. But that's your call, obviously.

> + */
> +
> +/dts-v1/;
> +#include "sun8i-h3.dtsi"
> +#include "sunxi-common-regulators.dtsi"
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +
> +/ {
> +	model = "MapleBoard MP130 Board";
> +	compatible = "mapleboard,mp130_board", "allwinner,sun8i-h3";

The "board" in both the model and compatible is redundant.

> +
> +	aliases {
> +		ethernet0 = &emac;
> +		serial0 = &uart0;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&leds_pio>, <&leds_r_pio>;

You can drop those pinctrl nodes

> +		pwr_led {
> +			label = "mp130_board:orange:pwr";
> +			gpios = <&r_pio 0 10 GPIO_ACTIVE_HIGH>;
> +			default-state = "on";
> +		};

A new line here would be nice

> +		status_led {
> +			label = "mp130_board:orange:status";
> +			gpios = <&pio 0 10 GPIO_ACTIVE_HIGH>;
> +		};
> +	};
> +
> +	r_gpio_keys {
> +		compatible = "gpio-keys";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&sw_r_pio>;

Samething the pinctrl reference can be dropped

> +		power {
> +			label = "power";
> +			linux,code = <KEY_POWER>;
> +			gpios = <&r_pio 0 3 GPIO_ACTIVE_LOW>; /* PL3 */
> +		};
> +
> +		user {
> +			label = "user";
> +			linux,code = <BTN_0>;
> +			gpios = <&r_pio 0 4 GPIO_ACTIVE_LOW>;
> +		};

The labels should be under the form "board:color:function"

> +	};
> +};
> +
> +&codec {
> +	allwinner,audio-routing =
> +		"Line Out", "LINEOUT",
> +		"LINEIN", "Line In";
> +	status = "okay";
> +};
> +
> +&ehci1 {
> +	status = "okay";
> +};
> +
> +&ehci2 {
> +	status = "okay";
> +};
> +
> +&ehci3 {
> +	status = "okay";
> +};
> +
> +&ir {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&ir_pins_a>;
> +	status = "okay";
> +};
> +
> +&mmc0 {
> +	vmmc-supply = <&reg_vcc3v3>;
> +	bus-width = <4>;
> +	cd-gpios = <&pio 5 6 GPIO_ACTIVE_HIGH>; /* PF6 */
> +	cd-inverted;
> +	status = "okay";
> +};
> +
> +&mmc2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&mmc2_8bit_pins>;
> +	vmmc-supply = <&reg_vcc3v3>;
> +	bus-width = <8>;
> +	non-removable;
> +	cap-mmc-hw-reset;
> +	status = "okay";
> +};
> +
> +&ohci1 {
> +	status = "okay";
> +};
> +
> +&ohci2 {
> +	status = "okay";
> +};
> +
> +&ohci3 {
> +	status = "okay";
> +};
> +
> +&pio {
> +	leds_pio: led_pins {
> +		pins = "PA15";
> +		function = "gpio_out";
> +	};
> +};
> +
> +&r_pio {
> +	leds_r_pio: led_pins {
> +		pins = "PL10";
> +		function = "gpio_out";
> +	};
> +
> +	sw_r_pio: key_pins {
> +		pins = "PL3", "PL4";
> +		function = "gpio_in";
> +	};
> +};
> +
> +&uart0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&uart0_pins_a>;
> +	status = "okay";
> +};
> +
> +&uart1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&uart1_pins>;
> +	status = "disabled";
> +};
> +
> +&uart2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&uart2_pins>;
> +	status = "disabled";
> +};
> +
> +&uart3 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&uart3_pins>;
> +	status = "disabled";
> +};
> +
> +&usb_otg {
> +	dr_mode = "peripheral";
> +	status = "okay";
> +};
> +
> +&usbphy {
> +	/* USB VBUS is always on */
> +	status = "okay";
> +};
> +
> +&emac {
> +	phy-handle = <&int_mii_phy>;
> +	phy-mode = "mii";
> +	allwinner,leds-active-low;
> +	status = "okay";
> +};
> +

This should be ordered alphabetically

> J.
> 
> -- 
> Revd Jonathan McDowell, ULC | Do you believe in happy endings?

Please don't send patches through your mailer. git send-email can be
configured to send the patches directly, this should be your preferred
option.

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20181029/2d390263/attachment.sig>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] dts: sun8i-h3: Add DTS for Mapleboard MP130
  2018-10-29 15:20   ` Maxime Ripard
@ 2018-10-30 22:08     ` Jonathan McDowell
  -1 siblings, 0 replies; 6+ messages in thread
From: Jonathan McDowell @ 2018-10-30 22:08 UTC (permalink / raw)
  To: Maxime Ripard; +Cc: linux-arm-kernel, linux-kernel, devicetree, Chen-Yu Tsai

[-- Attachment #1: Type: text/plain, Size: 6534 bytes --]

On Mon, Oct 29, 2018 at 04:20:52PM +0100, Maxime Ripard wrote:
> Thanks for your patch.

Thanks for the comments.

> On Thu, Oct 25, 2018 at 08:55:19PM +0100, Jonathan McDowell wrote:
...
> The prefix of your patch should be "ARM: dts: sun8i-h3: ..."

Ok.

> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> > index b5bd3de87c33..dcf1b9e7f71e 100644
> > --- a/arch/arm/boot/dts/Makefile
> > +++ b/arch/arm/boot/dts/Makefile
> > @@ -1035,6 +1035,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
> >  	sun8i-h3-bananapi-m2-plus.dtb \
> >  	sun8i-h3-beelink-x2.dtb \
> >  	sun8i-h3-libretech-all-h3-cc.dtb \
> > +	sun8i-h3-mapleboard-mp130.dtb \
> >  	sun8i-h3-nanopi-m1.dtb	\
> >  	sun8i-h3-nanopi-m1-plus.dtb \
> >  	sun8i-h3-nanopi-neo.dtb \
> > diff --git a/arch/arm/boot/dts/sun8i-h3-mapleboard-mp130.dts b/arch/arm/boot/dts/sun8i-h3-mapleboard-mp130.dts
> > new file mode 100644
> > index 000000000000..bf948128af94
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/sun8i-h3-mapleboard-mp130.dts
> > @@ -0,0 +1,179 @@
> > +/*
> > + * Copyright (C) 2017 Centrum Embedded Systems, Jia-Bin Huang <jb@ces.com.tw>
> > + * Copyright (C) 2018 Jonathan McDowell <noodles@earth.li>
> > + *
> > + * This file is licensed under the terms of the GNU General Public
> > + * License version 2. This program is licensed "as is" without any
> > + * warranty of any kind, whether express or implied.
> 
> You should add a SPDX tag instead of the license.

Done.

> We're also trying to license the DT under a dual GPL/MIT license,
> since they can be used by projects under a license that is not the
> GPL. But that's your call, obviously.

I'm building on top of Jia-Bin Huang's work, which was GPL only, so I'll
stick with that for the moment.

> > + */
> > +
> > +/dts-v1/;
> > +#include "sun8i-h3.dtsi"
> > +#include "sunxi-common-regulators.dtsi"
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/input/input.h>
> > +
> > +/ {
> > +	model = "MapleBoard MP130 Board";
> > +	compatible = "mapleboard,mp130_board", "allwinner,sun8i-h3";
> 
> The "board" in both the model and compatible is redundant.

Ok.

> > +
> > +	aliases {
> > +		ethernet0 = &emac;
> > +		serial0 = &uart0;
> > +	};
> > +
> > +	chosen {
> > +		stdout-path = "serial0:115200n8";
> > +	};
> > +
> > +	leds {
> > +		compatible = "gpio-leds";
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&leds_pio>, <&leds_r_pio>;
> 
> You can drop those pinctrl nodes

Done.

> > +		pwr_led {
> > +			label = "mp130_board:orange:pwr";
> > +			gpios = <&r_pio 0 10 GPIO_ACTIVE_HIGH>;
> > +			default-state = "on";
> > +		};
> 
> A new line here would be nice

Ok.

> > +		status_led {
> > +			label = "mp130_board:orange:status";
> > +			gpios = <&pio 0 10 GPIO_ACTIVE_HIGH>;
> > +		};
> > +	};
> > +
> > +	r_gpio_keys {
> > +		compatible = "gpio-keys";
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&sw_r_pio>;
> 
> Samething the pinctrl reference can be dropped

Ok.

> > +		power {
> > +			label = "power";
> > +			linux,code = <KEY_POWER>;
> > +			gpios = <&r_pio 0 3 GPIO_ACTIVE_LOW>; /* PL3 */
> > +		};
> > +
> > +		user {
> > +			label = "user";
> > +			linux,code = <BTN_0>;
> > +			gpios = <&r_pio 0 4 GPIO_ACTIVE_LOW>;
> > +		};
> 
> The labels should be under the form "board:color:function"

These are GPIO buttons, not LEDs, so I think you mean "board:function".
Will change.

> > +	};
> > +};
> > +
> > +&codec {
> > +	allwinner,audio-routing =
> > +		"Line Out", "LINEOUT",
> > +		"LINEIN", "Line In";
> > +	status = "okay";
> > +};
> > +
> > +&ehci1 {
> > +	status = "okay";
> > +};
> > +
> > +&ehci2 {
> > +	status = "okay";
> > +};
> > +
> > +&ehci3 {
> > +	status = "okay";
> > +};
> > +
> > +&ir {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&ir_pins_a>;
> > +	status = "okay";
> > +};
> > +
> > +&mmc0 {
> > +	vmmc-supply = <&reg_vcc3v3>;
> > +	bus-width = <4>;
> > +	cd-gpios = <&pio 5 6 GPIO_ACTIVE_HIGH>; /* PF6 */
> > +	cd-inverted;
> > +	status = "okay";
> > +};
> > +
> > +&mmc2 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&mmc2_8bit_pins>;
> > +	vmmc-supply = <&reg_vcc3v3>;
> > +	bus-width = <8>;
> > +	non-removable;
> > +	cap-mmc-hw-reset;
> > +	status = "okay";
> > +};
> > +
> > +&ohci1 {
> > +	status = "okay";
> > +};
> > +
> > +&ohci2 {
> > +	status = "okay";
> > +};
> > +
> > +&ohci3 {
> > +	status = "okay";
> > +};
> > +
> > +&pio {
> > +	leds_pio: led_pins {
> > +		pins = "PA15";
> > +		function = "gpio_out";
> > +	};
> > +};
> > +
> > +&r_pio {
> > +	leds_r_pio: led_pins {
> > +		pins = "PL10";
> > +		function = "gpio_out";
> > +	};
> > +
> > +	sw_r_pio: key_pins {
> > +		pins = "PL3", "PL4";
> > +		function = "gpio_in";
> > +	};
> > +};
> > +
> > +&uart0 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&uart0_pins_a>;
> > +	status = "okay";
> > +};
> > +
> > +&uart1 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&uart1_pins>;
> > +	status = "disabled";
> > +};
> > +
> > +&uart2 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&uart2_pins>;
> > +	status = "disabled";
> > +};
> > +
> > +&uart3 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&uart3_pins>;
> > +	status = "disabled";
> > +};
> > +
> > +&usb_otg {
> > +	dr_mode = "peripheral";
> > +	status = "okay";
> > +};
> > +
> > +&usbphy {
> > +	/* USB VBUS is always on */
> > +	status = "okay";
> > +};
> > +
> > +&emac {
> > +	phy-handle = <&int_mii_phy>;
> > +	phy-mode = "mii";
> > +	allwinner,leds-active-low;
> > +	status = "okay";
> > +};
> > +
> 
> This should be ordered alphabetically

Ok.

> > J.
> > 
> > -- 
> > Revd Jonathan McDowell, ULC | Do you believe in happy endings?
> 
> Please don't send patches through your mailer. git send-email can be
> configured to send the patches directly, this should be your preferred
> option.

I haven't previously had problems with using mutt to send these things
out (and for a single patch I tend to trust it more and like to see
exactly what's being sent), but I'll look into it.

J.

-- 
   101 things you can't have too   |  .''`.  Debian GNU/Linux Developer
       much of : 53 - Space.       | : :' :  Happy to accept PGP signed
                                   | `. `'   or encrypted mail - RSA
                                   |   `-    key on the keyservers.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] dts: sun8i-h3: Add DTS for Mapleboard MP130
@ 2018-10-30 22:08     ` Jonathan McDowell
  0 siblings, 0 replies; 6+ messages in thread
From: Jonathan McDowell @ 2018-10-30 22:08 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 29, 2018 at 04:20:52PM +0100, Maxime Ripard wrote:
> Thanks for your patch.

Thanks for the comments.

> On Thu, Oct 25, 2018 at 08:55:19PM +0100, Jonathan McDowell wrote:
...
> The prefix of your patch should be "ARM: dts: sun8i-h3: ..."

Ok.

> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> > index b5bd3de87c33..dcf1b9e7f71e 100644
> > --- a/arch/arm/boot/dts/Makefile
> > +++ b/arch/arm/boot/dts/Makefile
> > @@ -1035,6 +1035,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
> >  	sun8i-h3-bananapi-m2-plus.dtb \
> >  	sun8i-h3-beelink-x2.dtb \
> >  	sun8i-h3-libretech-all-h3-cc.dtb \
> > +	sun8i-h3-mapleboard-mp130.dtb \
> >  	sun8i-h3-nanopi-m1.dtb	\
> >  	sun8i-h3-nanopi-m1-plus.dtb \
> >  	sun8i-h3-nanopi-neo.dtb \
> > diff --git a/arch/arm/boot/dts/sun8i-h3-mapleboard-mp130.dts b/arch/arm/boot/dts/sun8i-h3-mapleboard-mp130.dts
> > new file mode 100644
> > index 000000000000..bf948128af94
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/sun8i-h3-mapleboard-mp130.dts
> > @@ -0,0 +1,179 @@
> > +/*
> > + * Copyright (C) 2017 Centrum Embedded Systems, Jia-Bin Huang <jb@ces.com.tw>
> > + * Copyright (C) 2018 Jonathan McDowell <noodles@earth.li>
> > + *
> > + * This file is licensed under the terms of the GNU General Public
> > + * License version 2. This program is licensed "as is" without any
> > + * warranty of any kind, whether express or implied.
> 
> You should add a SPDX tag instead of the license.

Done.

> We're also trying to license the DT under a dual GPL/MIT license,
> since they can be used by projects under a license that is not the
> GPL. But that's your call, obviously.

I'm building on top of Jia-Bin Huang's work, which was GPL only, so I'll
stick with that for the moment.

> > + */
> > +
> > +/dts-v1/;
> > +#include "sun8i-h3.dtsi"
> > +#include "sunxi-common-regulators.dtsi"
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/input/input.h>
> > +
> > +/ {
> > +	model = "MapleBoard MP130 Board";
> > +	compatible = "mapleboard,mp130_board", "allwinner,sun8i-h3";
> 
> The "board" in both the model and compatible is redundant.

Ok.

> > +
> > +	aliases {
> > +		ethernet0 = &emac;
> > +		serial0 = &uart0;
> > +	};
> > +
> > +	chosen {
> > +		stdout-path = "serial0:115200n8";
> > +	};
> > +
> > +	leds {
> > +		compatible = "gpio-leds";
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&leds_pio>, <&leds_r_pio>;
> 
> You can drop those pinctrl nodes

Done.

> > +		pwr_led {
> > +			label = "mp130_board:orange:pwr";
> > +			gpios = <&r_pio 0 10 GPIO_ACTIVE_HIGH>;
> > +			default-state = "on";
> > +		};
> 
> A new line here would be nice

Ok.

> > +		status_led {
> > +			label = "mp130_board:orange:status";
> > +			gpios = <&pio 0 10 GPIO_ACTIVE_HIGH>;
> > +		};
> > +	};
> > +
> > +	r_gpio_keys {
> > +		compatible = "gpio-keys";
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&sw_r_pio>;
> 
> Samething the pinctrl reference can be dropped

Ok.

> > +		power {
> > +			label = "power";
> > +			linux,code = <KEY_POWER>;
> > +			gpios = <&r_pio 0 3 GPIO_ACTIVE_LOW>; /* PL3 */
> > +		};
> > +
> > +		user {
> > +			label = "user";
> > +			linux,code = <BTN_0>;
> > +			gpios = <&r_pio 0 4 GPIO_ACTIVE_LOW>;
> > +		};
> 
> The labels should be under the form "board:color:function"

These are GPIO buttons, not LEDs, so I think you mean "board:function".
Will change.

> > +	};
> > +};
> > +
> > +&codec {
> > +	allwinner,audio-routing =
> > +		"Line Out", "LINEOUT",
> > +		"LINEIN", "Line In";
> > +	status = "okay";
> > +};
> > +
> > +&ehci1 {
> > +	status = "okay";
> > +};
> > +
> > +&ehci2 {
> > +	status = "okay";
> > +};
> > +
> > +&ehci3 {
> > +	status = "okay";
> > +};
> > +
> > +&ir {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&ir_pins_a>;
> > +	status = "okay";
> > +};
> > +
> > +&mmc0 {
> > +	vmmc-supply = <&reg_vcc3v3>;
> > +	bus-width = <4>;
> > +	cd-gpios = <&pio 5 6 GPIO_ACTIVE_HIGH>; /* PF6 */
> > +	cd-inverted;
> > +	status = "okay";
> > +};
> > +
> > +&mmc2 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&mmc2_8bit_pins>;
> > +	vmmc-supply = <&reg_vcc3v3>;
> > +	bus-width = <8>;
> > +	non-removable;
> > +	cap-mmc-hw-reset;
> > +	status = "okay";
> > +};
> > +
> > +&ohci1 {
> > +	status = "okay";
> > +};
> > +
> > +&ohci2 {
> > +	status = "okay";
> > +};
> > +
> > +&ohci3 {
> > +	status = "okay";
> > +};
> > +
> > +&pio {
> > +	leds_pio: led_pins {
> > +		pins = "PA15";
> > +		function = "gpio_out";
> > +	};
> > +};
> > +
> > +&r_pio {
> > +	leds_r_pio: led_pins {
> > +		pins = "PL10";
> > +		function = "gpio_out";
> > +	};
> > +
> > +	sw_r_pio: key_pins {
> > +		pins = "PL3", "PL4";
> > +		function = "gpio_in";
> > +	};
> > +};
> > +
> > +&uart0 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&uart0_pins_a>;
> > +	status = "okay";
> > +};
> > +
> > +&uart1 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&uart1_pins>;
> > +	status = "disabled";
> > +};
> > +
> > +&uart2 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&uart2_pins>;
> > +	status = "disabled";
> > +};
> > +
> > +&uart3 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&uart3_pins>;
> > +	status = "disabled";
> > +};
> > +
> > +&usb_otg {
> > +	dr_mode = "peripheral";
> > +	status = "okay";
> > +};
> > +
> > +&usbphy {
> > +	/* USB VBUS is always on */
> > +	status = "okay";
> > +};
> > +
> > +&emac {
> > +	phy-handle = <&int_mii_phy>;
> > +	phy-mode = "mii";
> > +	allwinner,leds-active-low;
> > +	status = "okay";
> > +};
> > +
> 
> This should be ordered alphabetically

Ok.

> > J.
> > 
> > -- 
> > Revd Jonathan McDowell, ULC | Do you believe in happy endings?
> 
> Please don't send patches through your mailer. git send-email can be
> configured to send the patches directly, this should be your preferred
> option.

I haven't previously had problems with using mutt to send these things
out (and for a single patch I tend to trust it more and like to see
exactly what's being sent), but I'll look into it.

J.

-- 
   101 things you can't have too   |  .''`.  Debian GNU/Linux Developer
       much of : 53 - Space.       | : :' :  Happy to accept PGP signed
                                   | `. `'   or encrypted mail - RSA
                                   |   `-    key on the keyservers.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20181030/842e4fdd/attachment.sig>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-10-30 22:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-25 19:55 [PATCH] dts: sun8i-h3: Add DTS for Mapleboard MP130 Jonathan McDowell
2018-10-25 19:55 ` Jonathan McDowell
2018-10-29 15:20 ` Maxime Ripard
2018-10-29 15:20   ` Maxime Ripard
2018-10-30 22:08   ` Jonathan McDowell
2018-10-30 22:08     ` Jonathan McDowell

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.