All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] ARM: dts: pxa: add mioa701 board description
@ 2018-08-30 19:59 Robert Jarzmik
  2018-08-31  7:45   ` kbuild test robot
  2018-08-31  9:46 ` Marcel Ziswiler
  0 siblings, 2 replies; 7+ messages in thread
From: Robert Jarzmik @ 2018-08-30 19:59 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Kees Cook, Anton Vorontsov,
	Colin Cross, Tony Luck
  Cc: devicetree, linux-kernel, Robert Jarzmik

Add device-tree description of the Mitac MIO A701 board.
This is aimed at replacing mioa701.c board file, and once stabilized,
the leftover, such as the suspend resume mechanics will rely on a new
IPL, and not the legacy Windows CE one.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
Since v1: fix lcd_supply and lcd_backlight
          These were depending on my internal tree changes, fit it.
Since v2: take into account Rob's comments

This patch deserves some special "love review". As it will probably
serve for a more broad pxa conversion to devicetree of the other boards,
and because it touches almost all domains for a pxa platform (camera,
video, audio, i2c, ...), it should be as clean as possible so that
mistakes are not carried on ...

Therefore I expect the review of this one to be long (ie. it won't land
for v4.19), until it looks good enough.
---
 arch/arm/boot/dts/Makefile    |   2 +
 arch/arm/boot/dts/mioa701.dts | 558 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 560 insertions(+)
 create mode 100644 arch/arm/boot/dts/mioa701.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index b5bd3de87c33..8809f4e2244d 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -775,6 +775,8 @@ dtb-$(CONFIG_ARCH_PRIMA2) += \
 dtb-$(CONFIG_ARCH_OXNAS) += \
 	ox810se-wd-mbwe.dtb \
 	ox820-cloudengines-pogoplug-series-3.dtb
+dtb-$(CONFIG_ARCH_PXA) += \
+	mioa701.dtb
 dtb-$(CONFIG_ARCH_QCOM) += \
 	qcom-apq8060-dragonboard.dtb \
 	qcom-apq8064-arrow-sd-600eval.dtb \
diff --git a/arch/arm/boot/dts/mioa701.dts b/arch/arm/boot/dts/mioa701.dts
new file mode 100644
index 000000000000..3791bc69e155
--- /dev/null
+++ b/arch/arm/boot/dts/mioa701.dts
@@ -0,0 +1,558 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  Copyright (C) 2018 Robert Jarzmik <robert.jarzmik@free.fr>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  publishhed by the Free Software Foundation.
+ */
+
+/dts-v1/;
+#include "pxa27x.dtsi"
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/clock/pxa-clock.h>
+
+/ {
+	model = "Mitac Mio A701 Board";
+	compatible = "marvell,pxa270";
+
+	chosen {
+		bootargs = "mtdparts=docg3.0:256k@3456k(barebox)ro,256k(barebox-logo),128k(barebox-env),4M(kernel),-(root) ubi.mtd=4 rootfstype=ubifs root=ubi0:linux_root ro";
+	};
+
+	memory {
+		reg = <0xa0000000 0x04000000>;
+
+		reserved-memory {
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			pstore_region:region@a2000000 {
+				compatible = "linux,contiguous-memory-region";
+				reg = <0xa2000000 0x100000>;
+			};
+		};
+	};
+
+	cpus {
+		cpu {
+			cpu-supply = <&vcc_core>;
+		};
+	};
+
+	pxabus {
+		pinctrl: pinctrl@40e00000 {
+			status = "okay";
+			pinctrl_ac97_default: ac97-default {
+				PMMUX(hpjack-detect, 12, gpio_in);
+				PMMUX(ac97-bitclk, 28, AC97_BITCLK);
+				PMMUX(ac97-sdata-in-0, 29, AC97_SDATA_IN_0);
+				PMMUX(ac97-sdata-out, 30, AC97_SDATA_OUT);
+				PMMUX(ac97-sync, 31, AC97_SYNC);
+				PMMUX(ac97-sysclk, 89, AC97_SYSCLK);
+			};
+			pinctrl_btuart_default: btuart-default {
+				PMMUX(btuart-nactivity, 14, gpio_in);
+				PMMUX(btuart-rxd, 42, BTRXD);
+				PMMUX(btuart-txd, 43, BTTXD);
+				PMMUX(btuart-cts, 44, BTCTS);
+				PMMUX(btuart-rts, 45, BTRTS);
+				PMMUX_LPM_LOW(bt-on, 83, gpio_out);
+				PMMUX_LPM_HIGH(bt-unknown, 77, gpio_out);
+				PMMUX_LPM_HIGH(bt-nreset, 86, gpio_out);
+			};
+			pinctrl_ffuart_default: ffuart-default {
+				PMMUX(ffuart-rxd, 34, FFRXD);
+				PMMUX(ffuart-cts, 35, FFCTS);
+				PMMUX(ffuart-dcd, 36, FFDCD);
+				PMMUX(ffuart-dsr, 37, FFDSR);
+				PMMUX(ffuart-txd, 39, FFTXD);
+				PMMUX(ffuart-dtr, 40, FFDTR);
+				PMMUX(ffuart-rts, 41, FFRTS);
+				PMMUX_LPM_LOW(gsm-reset, 24, gpio_out);
+				PMMUX(gsm-is-on, 25, gpio_in);
+				PMMUX_LPM_HIGH(gsm-nset-on, 88, gpio_out);
+				PMMUX_LPM_HIGH(gsm-nset-off, 90, gpio_out);
+				PMMUX(gsm-event-available, 113, gpio_in);
+				PMMUX_LPM_HIGH(gsm-dte-state, 114, gpio_out);
+			};
+			pinctrl_gpiokeys_default: gpiokeys-default {
+				PMMUX(key-power, 0, gpio_in);
+				PMMUX(key-volume_up, 93, gpio_in);
+				PMMUX(key-volume_down, 94, gpio_in);
+			};
+			pinctrl_keypad_default: keypad-default {
+				PMMUX(keypad-mkin0, 100, KP_MKIN<0>);
+				PMMUX(keypad-mkin1, 101, KP_MKIN<1>);
+				PMMUX(keypad-mkin2, 102, KP_MKIN<2>);
+				PMMUX(keypad-mkout0, 103, KP_MKOUT<0>);
+				PMMUX(keypad-mkout1, 104, KP_MKOUT<1>);
+				PMMUX(keypad-mkout2, 105, KP_MKOUT<2>);
+			};
+			pinctrl_i2c_default: i2c-default {
+				PMMUX(i2c-scl, 117, SCL);
+				PMMUX(i2c-sda, 118, SDA);
+			};
+			pinctrl_lcd_default: lcd-default {
+				PMMUX(ldd0, 58, LDD<0>);
+				PMMUX(ldd1, 59, LDD<1>);
+				PMMUX(ldd2, 60, LDD<2>);
+				PMMUX(ldd3, 61, LDD<3>);
+				PMMUX(ldd4, 62, LDD<4>);
+				PMMUX(ldd5, 63, LDD<5>);
+				PMMUX(ldd6, 64, LDD<6>);
+				PMMUX(ldd7, 65, LDD<7>);
+				PMMUX(ldd8, 66, LDD<8>);
+				PMMUX(ldd9, 67, LDD<9>);
+				PMMUX(ldd10, 68, LDD<10>);
+				PMMUX(ldd11, 69, LDD<11>);
+				PMMUX(ldd12, 70, LDD<12>);
+				PMMUX(ldd13, 71, LDD<13>);
+				PMMUX(ldd14, 72, LDD<14>);
+				PMMUX(ldd16, 73, LDD<15>);
+				PMMUX(lcd-fclk, 74, L_FCLK_RD);
+				PMMUX(lcd-lclk, 75, L_LCLK_A0);
+				PMMUX(lcd-pclk, 76, L_PCLK_WR);
+				PMMUX(lcd-power, 87, gpio_out);
+			};
+			pinctrl_leds_default: leds-default {
+				PMMUX_LPM_HIGH(led-charging, 10, gpio_out);
+				PMMUX_LPM_HIGH(led-vibra, 82, gpio_out);
+				PMMUX_LPM_HIGH(led-blue, 97, gpio_out);
+				PMMUX_LPM_HIGH(led-orange, 98, gpio_out);
+				PMMUX_LPM_HIGH(led-keyboard, 115, gpio_out);
+			};
+			pinctrl_pwm0_default: pwm0-defaut {
+				PMMUX(pwm0, 16, PWM_OUT<0>);
+			};
+			pinctrl_qci_default: qci-defaut {
+				PMMUX(cif-dd6, 17, CIF_DD<6>);
+				PMMUX(cif-dd3, 50, CIF_DD<3>);
+				PMMUX(cif-dd2, 51, CIF_DD<2>);
+				PMMUX(cif-dd4, 52, CIF_DD<4>);
+				PMMUX(cif-mclk, 53, CIF_MCLK);
+				PMMUX(cif-pclk, 54, CIF_PCLK);
+				PMMUX_LPM_HIGH(mt9m111-nOE, 56, gpio_out);
+				PMMUX(cif-dd1, 55, CIF_DD<1>);
+				PMMUX(cif-dd0, 81, CIF_DD<0>);
+				PMMUX(cif-dd5, 48, CIF_DD<5>);
+				PMMUX(cif-fv, 84, CIF_FV);
+				PMMUX(cif-lv, 85, CIF_LV);
+				PMMUX(cif-dd7, 108, CIF_DD<7>);
+			};
+			pinctrl_mmc_default: mmc-default {
+				PMMUX(sd-insert, 15, gpio_in);
+				PMMUX(mmclk, 32, MMCLK);
+				PMMUX(sd-ro, 78, gpio_in);
+				PMMUX_LPM_LOW(sd-enable, 91, gpio_out);
+				PMMUX(mmdat0, 92, MMDAT<0>);
+				PMMUX(mmdat1, 109, MMDAT<1>);
+				PMMUX(mmdat2, 110, MMDAT<2>);
+				PMMUX(mmdat3, 111, MMDAT<3>);
+				PMMUX(mmcmd, 112, MMCMD);
+			};
+			pinctrl_stuart_default: stuart-default {
+				PMMUX_LPM_LOW(gps-unknown1, 23, gpio_out);
+				PMMUX_LPM_LOW(gps-on, 26, gpio_out);
+				PMMUX_LPM_LOW(gps-nreset, 27, gpio_out);
+				PMMUX(stuart-rxd, 46, STRXD);
+				PMMUX(stuart-txd, 47, STTXD);
+				PMMUX_LPM_LOW(gps-unknown2, 106, gpio_out);
+				PMMUX_LPM_LOW(gps-unknown3, 107, gpio_out);
+			};
+			pinctrl_usb_default: usb-default {
+				PMMUX(n-usb-detect, 13, gpio_in);
+				PMMUX_LPM_LOW(n-dplus-pullup, 22, gpio_out);
+			};
+			pinctrl_power_default: power-default {
+				PMMUX_LPM_LOW(charge-enable, 9, gpio_out);
+				PMMUX(charge-vdrop, 80, gpio_out);
+				PMMUX(ac-detect, 96, gpio_in);
+			};
+		};
+
+		pwm0: pwm@40b00000 {
+		      status = "okay";
+		};
+
+		gpio: gpio@40e00000 {
+			status = "okay";
+		};
+
+		ffuart: serial@40100000 {
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_ffuart_default>;
+			status = "okay";
+		};
+
+		btuart: serial@40200000 {
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_btuart_default>;
+			status = "okay";
+		};
+
+		stuart: serial@40700000 {
+			status = "okay";
+		};
+
+		usb2phy: gpio-vbus@13 {
+			compatible = "usb-nop-xceiv";
+			#phy-cells = <1>;
+			vbus-detect-gpio = <&gpio 13 GPIO_ACTIVE_LOW>;
+			wakeup;
+		};
+
+		pxa27x_udc: udc@40600000 {
+			status = "okay";
+			gpios = <&gpio 22 0>;
+			phys = <&usb2phy>;
+			phys-names = "usb2phy";
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_usb_default>;
+		};
+
+		pwri2c: i2c@40f000180 {
+			status = "okay";
+
+			core_regulator@14 {
+				compatible = "maxim,max1586";
+				reg = <0x14>;
+				v3-gain = <1000000>;
+
+				regulators {
+					vcc_core: v3 {
+						regulator-name = "vcc_core";
+						regulator-compatible = "Output_V3";
+						regulator-min-microvolt = <1000000>;
+						regulator-max-microvolt = <1705000>;
+						regulator-always-on;
+					};
+				};
+			};
+		};
+
+		pxai2c1: i2c@40301680 {
+			mrvl,i2c-fast-mode;
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_i2c_default>;
+			status = "okay";
+
+			mt9m111: camera@5d {
+				compatible = "micron,mt9m111";
+				reg = <0x5d>;
+				gpios = <&gpio 56 GPIO_ACTIVE_HIGH>;
+
+				remote = <&pxa_camera>;
+				port {
+					mt9m111_1: endpoint {
+						bus-width = <8>;
+						remote-endpoint = <&pxa_camera>;
+					};
+				};
+			};
+		};
+
+		keypad: keypad@41500000 {
+			status = "okay";
+
+			keypad,num-rows = <3>;
+			keypad,num-columns = <3>;
+			linux,keymap = <
+				0x00000067	/* KEY_UP */
+				0x0001006a	/* KEY_RIGHT */
+				0x000200e2	/* KEY_MEDIA */
+				0x0100006c	/* KEY_DOWN */
+				0x0101001c	/* KEY_ENTER */
+				0x010200da	/* KEY_CONNECT */
+				0x02000069	/* KEY_LEFT */
+				0x020100a9	/* KEY_PHONE */
+				0x020200d4>;	/* KEY_CAMERA */
+			marvell,debounce-interval = <0>;
+
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_keypad_default>;
+		};
+
+		gpio-keys {
+			compatible = "gpio-keys";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			autorepeat;
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_gpiokeys_default>;
+			status = "okay";
+
+			power-button {
+				label = "GPIO Key Power";
+				linux,code = <174>;
+				gpios = <&gpio 0 0>;
+				gpio-key,wakeup;
+			};
+			hp-jack-detect {
+				label = "HP jack detect";
+				linux,code = <211>;
+				gpios = <&gpio 12 0>;
+			};
+			volume-up {
+				label = "Volume Up Key";
+				linux,code = <115>;
+				gpios = <&gpio 93 0>;
+			};
+			volume-down {
+				label = "Volume Down Key";
+				linux,code = <114>;
+				gpios = <&gpio 94 0>;
+			};
+		};
+
+		mmc0: mmc@41100000 {
+			vmmc-supply = <&reg_vmmc>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_mmc_default>;
+			bus-width = <4>;
+			cd-gpios = <&gpio 15 0>;
+			wp-gpios = <&gpio 78 0>;
+			status = "okay";
+		};
+
+		pxa_camera: imaging@50000000 {
+			status = "okay";
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_qci_default>;
+
+			port {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				/* Parallel bus endpoint */
+				qci: endpoint@0 {
+					reg = <0>;
+					remote-endpoint = <&mt9m111_1>;
+					bus-width = <8>;
+
+					hsync-active = <0>;
+					vsync-active = <0>;
+					pclk-sample = <1>;
+				};
+			};
+		};
+
+		rtc@40900000 {
+			status = "okay";
+		};
+
+		lcd-controller@40500000 {
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_lcd_default>;
+			status = "okay";
+			port {
+				lcdc_out: endpoint {
+					remote-endpoint = <&panel_in>;
+					bus-width = <16>;
+				};
+			};
+		};
+
+		ac97: sound@40500000 {
+			compatible = "marvell,pxa270-ac97";
+			reg = < 0x40500000 0x1000 >;
+			interrupts = <14>;
+			reset-gpios = <&gpio 95 GPIO_ACTIVE_HIGH>;
+			pinctrl-names = "default";
+			pinctrl-0 = < &pinctrl_ac97_default >;
+			clocks = <&clks CLK_AC97>, <&clks CLK_AC97CONF>;
+			clock-names = "AC97CLK", "AC97CONFCLK";
+			dmas = <&pdma 8 0
+				&pdma 9 0
+				&pdma 10 0
+				&pdma 11 0
+				&pdma 12 0>;
+			dma-names = "pcm_pcm_mic_mono", "pcm_pcm_aux_mono_in",
+				    "pcm_pcm_aux_mono_out", "pcm_pcm_stereo_in",
+				    "pcm_pcm_stereo_out";
+
+			#sound-dai-cells = <0>;
+
+			#address-cells = <1>;
+			#size-cells = <0>;
+			wm9713: audio-codec@0 {
+				reg = <0>;
+				compatible = "ac97,574d,4c13";
+				clocks = <&wm9713_bitclk>;
+				clock-names = "ac97_clk";
+				#sound-dai-cells = <0>;
+
+				wm9713_bitclk: ac97_bitclk {
+					compatible = "fixed-clock";
+					#clock-cells = <0>;
+					clock-frequency = <12285000>;
+					status = "okay";
+				};
+			};
+		};
+
+		pxa_pcm_audio: snd_soc_pxa_audio {
+			compatible = "mrvl,pxa-pcm-audio";
+			#sound-dai-cells = <0>;
+			status = "okay";
+		};
+
+		lcd-controller@40500000 {
+			lcd-supply = <&lcd_vmmc>;
+		};
+
+		docg3: flash@0 {
+			compatible = "m-systems,diskonchip-g3";
+			reg = <0x0 0x2000>;
+		};
+
+	};
+
+	reg_vmmc: regulator@0 {
+		compatible = "regulator-fixed";
+		regulator-name = "vmmc";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpio = <&gpio 91 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+	};
+
+	lcd_vmmc: regulator@1 {
+		compatible = "regulator-fixed";
+		regulator-name = "lcd-supply";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+
+		gpio = <&gpio 87 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+	};
+
+	lcd_backlight: backlight {
+		compatible = "pwm-backlight";
+		pwms = <&pwm0 4096000>;
+		pwm-names = "backlight";
+
+		brightness-levels = <0 4 8 16 32 64 128 255>;
+		default-brightness-level = <2>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_pwm0_default>;
+	};
+
+	panel {
+		compatible = "toshiba,ltm0305a776";
+		lcd-type = "color-tft";
+
+		backlight = <&lcd_backlight>;
+
+		port {
+			panel_in: endpoint {
+				remote-endpoint = <&lcdc_out>;
+			};
+		};
+
+		display-timings {
+			native-mode = <&timing0>;
+			timing0: 240p {
+				/* 240x320p24 */
+				clock-frequency = <4545000>;
+				hactive = <240>;
+				vactive = <320>;
+				hfront-porch = <4>;
+				hback-porch = <6>;
+				hsync-len = <4>;
+				vback-porch = <5>;
+				vfront-porch = <3>;
+				vsync-len = <2>;
+			};
+		};
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_leds_default>;
+
+		charger-led {
+			label = "mioa701:charging";
+			gpios = <&gpio 10 GPIO_ACTIVE_LOW>;
+			default-state = "off";
+		};
+
+		vibrator {
+			label = "mioa701:vibra";
+			gpios = <&gpio 82 GPIO_ACTIVE_LOW>;
+			default-state = "off";
+		};
+
+		bluetooth-led {
+			label = "mioa701:blue";
+			gpios = <&gpio 97 GPIO_ACTIVE_LOW>;
+			default-state = "off";
+		};
+
+		orange-led {
+			label = "mioa701:orange";
+			gpios = <&gpio 98 GPIO_ACTIVE_LOW>;
+			default-state = "off";
+		};
+
+		keyboard-led {
+			label = "mioa701:keyboard";
+			gpios = <&gpio 115 GPIO_ACTIVE_LOW>;
+			default-state = "off";
+		};
+	};
+
+	sound {
+		compatible = "simple-audio-card";
+		simple-audio-card,name = "MioA701";
+		simple-audio-card,widgets =
+		"Speaker", "Front Speaker",
+		"Speaker", "Rear Speaker",
+		"Microphone", "Headset",
+		"Microphone", "GSM Line Out",
+		"Line", "GSM Line In",
+		"Microphone", "Headset Mic",
+		"Microphone", "Front Mic";
+		simple-audio-card,routing =
+			/* Call Mic */
+			"Mic Bias", "Front Mic",
+			"MIC1", "Mic Bias",
+			/* Headset Mic */
+			"LINEL", "Headset Mic",
+			"LINER", "Headset Mic",
+			/* GSM Module */
+			"MONOIN", "GSM Line Out",
+			"PCBEEP", "GSM Line Out",
+			"GSM Line In", "MONO",
+			/* headphone connected to HPL, HPR */
+			"Headset", "HPL",
+			"Headset", "HPR",
+			/* front speaker connected to HPL, OUT3 */
+			"Front Speaker", "HPL",
+			"Front Speaker", "OUT3",
+			/* rear speaker connected to SPKL, SPKR */
+			"Rear Speaker", "SPKL",
+			"Rear Speaker", "SPKR";
+
+		simple-audio-card,cpu {
+			sound-dai = <&ac97>;
+		};
+		simple-audio-card,codec {
+			sound-dai = <&wm9713>;
+		};
+		simple-audio-card,plat {
+			sound-dai = <&pxa_pcm_audio>;
+		};
+	};
+
+	ac_charger: charger {
+		compatible = "gpio-charger";
+		charger-type = "usb-aca";
+		gpios = <&gpio 96 GPIO_ACTIVE_HIGH>;
+	};
+};
-- 
2.11.0


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

* Re: [PATCH v3] ARM: dts: pxa: add mioa701 board description
  2018-08-30 19:59 [PATCH v3] ARM: dts: pxa: add mioa701 board description Robert Jarzmik
@ 2018-08-31  7:45   ` kbuild test robot
  2018-08-31  9:46 ` Marcel Ziswiler
  1 sibling, 0 replies; 7+ messages in thread
From: kbuild test robot @ 2018-08-31  7:45 UTC (permalink / raw)
  To: Robert Jarzmik
  Cc: kbuild-all, Rob Herring, Mark Rutland, Kees Cook,
	Anton Vorontsov, Colin Cross, Tony Luck, devicetree,
	linux-kernel, Robert Jarzmik

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

Hi Robert,

I love your patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.19-rc1 next-20180831]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Robert-Jarzmik/ARM-dts-pxa-add-mioa701-board-description/20180831-134244
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-keystone_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> Error: arch/arm/boot/dts/mioa701.dts:47.10-11 syntax error
   FATAL ERROR: Unable to parse input tree

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 21678 bytes --]

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

* Re: [PATCH v3] ARM: dts: pxa: add mioa701 board description
@ 2018-08-31  7:45   ` kbuild test robot
  0 siblings, 0 replies; 7+ messages in thread
From: kbuild test robot @ 2018-08-31  7:45 UTC (permalink / raw)
  Cc: kbuild-all, Rob Herring, Mark Rutland, Kees Cook,
	Anton Vorontsov, Colin Cross, Tony Luck, devicetree,
	linux-kernel, Robert Jarzmik

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

Hi Robert,

I love your patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.19-rc1 next-20180831]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Robert-Jarzmik/ARM-dts-pxa-add-mioa701-board-description/20180831-134244
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-keystone_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> Error: arch/arm/boot/dts/mioa701.dts:47.10-11 syntax error
   FATAL ERROR: Unable to parse input tree

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 21678 bytes --]

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

* Re: [PATCH v3] ARM: dts: pxa: add mioa701 board description
  2018-08-30 19:59 [PATCH v3] ARM: dts: pxa: add mioa701 board description Robert Jarzmik
  2018-08-31  7:45   ` kbuild test robot
@ 2018-08-31  9:46 ` Marcel Ziswiler
  2018-09-14  5:54   ` Robert Jarzmik
  1 sibling, 1 reply; 7+ messages in thread
From: Marcel Ziswiler @ 2018-08-31  9:46 UTC (permalink / raw)
  To: Robert Jarzmik, Rob Herring, Mark Rutland, Kees Cook,
	Anton Vorontsov, Colin Cross, Tony Luck
  Cc: devicetree, linux-kernel

Hi Robert

On Thu, 2018-08-30 at 21:59 +0200, Robert Jarzmik wrote:
> Add device-tree description of the Mitac MIO A701 board.
> This is aimed at replacing mioa701.c board file, and once stabilized,
> the leftover, such as the suspend resume mechanics will rely on a new
> IPL, and not the legacy Windows CE one.

Cool, I did work on a Colibri PXA270 device tree off and on just never
came around submitting it. Yours is now definitely a good start!

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> ---
> Since v1: fix lcd_supply and lcd_backlight
>           These were depending on my internal tree changes, fit it.
> Since v2: take into account Rob's comments
> 
> This patch deserves some special "love review". As it will probably
> serve for a more broad pxa conversion to devicetree of the other
> boards,
> and because it touches almost all domains for a pxa platform (camera,
> video, audio, i2c, ...), it should be as clean as possible so that
> mistakes are not carried on ...
> 
> Therefore I expect the review of this one to be long (ie. it won't
> land
> for v4.19), until it looks good enough.
> ---
>  arch/arm/boot/dts/Makefile    |   2 +
>  arch/arm/boot/dts/mioa701.dts | 558 

Isn't that one supposed to be prefixed by pxa270-?

> ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 560 insertions(+)
>  create mode 100644 arch/arm/boot/dts/mioa701.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index b5bd3de87c33..8809f4e2244d 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -775,6 +775,8 @@ dtb-$(CONFIG_ARCH_PRIMA2) += \
>  dtb-$(CONFIG_ARCH_OXNAS) += \
>  	ox810se-wd-mbwe.dtb \
>  	ox820-cloudengines-pogoplug-series-3.dtb
> +dtb-$(CONFIG_ARCH_PXA) += \

Wouldn't it rather make more sense to use CONFIG_MACH_PXA27X_DT
instead?

> +	mioa701.dtb
>  dtb-$(CONFIG_ARCH_QCOM) += \
>  	qcom-apq8060-dragonboard.dtb \
>  	qcom-apq8064-arrow-sd-600eval.dtb \
> diff --git a/arch/arm/boot/dts/mioa701.dts
> b/arch/arm/boot/dts/mioa701.dts
> new file mode 100644
> index 000000000000..3791bc69e155
> --- /dev/null
> +++ b/arch/arm/boot/dts/mioa701.dts
> @@ -0,0 +1,558 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + *  Copyright (C) 2018 Robert Jarzmik <robert.jarzmik@free.fr>
> + *
> + *  This program is free software; you can redistribute it and/or
> modify
> + *  it under the terms of the GNU General Public License version 2
> as
> + *  publishhed by the Free Software Foundation.
> + */
> +
> +/dts-v1/;
> +#include "pxa27x.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/clock/pxa-clock.h>
> +
> +/ {
> +	model = "Mitac Mio A701 Board";
> +	compatible = "marvell,pxa270";

Usually, there is also a compatible for the particular board e.g.
"mitac,mioa701", not? You might have to check on the vendor prefix
though.

> +	chosen {
> +		bootargs = "mtdparts=docg3.0:256k@3456k(barebox)ro,2
> 56k(barebox-logo),128k(barebox-env),4M(kernel),-(root) ubi.mtd=4
> rootfstype=ubifs root=ubi0:linux_root ro";
> +	};
> +
> +	memory {
> +		reg = <0xa0000000 0x04000000>;
> +
> +		reserved-memory {
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			pstore_region:region@a2000000 {
> +				compatible = "linux,contiguous-
> memory-region";
> +				reg = <0xa2000000 0x100000>;
> +			};
> +		};
> +	};
> +
> +	cpus {
> +		cpu {
> +			cpu-supply = <&vcc_core>;
> +		};
> +	};
> +
> +	pxabus {
> +		pinctrl: pinctrl@40e00000 {
> +			status = "okay";
> +			pinctrl_ac97_default: ac97-default {
> +				PMMUX(hpjack-detect, 12, gpio_in);
> +				PMMUX(ac97-bitclk, 28, AC97_BITCLK);
> +				PMMUX(ac97-sdata-in-0, 29,
> AC97_SDATA_IN_0);
> +				PMMUX(ac97-sdata-out, 30,
> AC97_SDATA_OUT);
> +				PMMUX(ac97-sync, 31, AC97_SYNC);
> +				PMMUX(ac97-sysclk, 89, AC97_SYSCLK);
> +			};
> +			pinctrl_btuart_default: btuart-default {
> +				PMMUX(btuart-nactivity, 14,
> gpio_in);
> +				PMMUX(btuart-rxd, 42, BTRXD);
> +				PMMUX(btuart-txd, 43, BTTXD);
> +				PMMUX(btuart-cts, 44, BTCTS);
> +				PMMUX(btuart-rts, 45, BTRTS);
> +				PMMUX_LPM_LOW(bt-on, 83, gpio_out);
> +				PMMUX_LPM_HIGH(bt-unknown, 77,
> gpio_out);
> +				PMMUX_LPM_HIGH(bt-nreset, 86,
> gpio_out);
> +			};
> +			pinctrl_ffuart_default: ffuart-default {
> +				PMMUX(ffuart-rxd, 34, FFRXD);
> +				PMMUX(ffuart-cts, 35, FFCTS);
> +				PMMUX(ffuart-dcd, 36, FFDCD);
> +				PMMUX(ffuart-dsr, 37, FFDSR);
> +				PMMUX(ffuart-txd, 39, FFTXD);
> +				PMMUX(ffuart-dtr, 40, FFDTR);
> +				PMMUX(ffuart-rts, 41, FFRTS);
> +				PMMUX_LPM_LOW(gsm-reset, 24,
> gpio_out);
> +				PMMUX(gsm-is-on, 25, gpio_in);
> +				PMMUX_LPM_HIGH(gsm-nset-on, 88,
> gpio_out);
> +				PMMUX_LPM_HIGH(gsm-nset-off, 90,
> gpio_out);
> +				PMMUX(gsm-event-available, 113,
> gpio_in);
> +				PMMUX_LPM_HIGH(gsm-dte-state, 114,
> gpio_out);
> +			};
> +			pinctrl_gpiokeys_default: gpiokeys-default {
> +				PMMUX(key-power, 0, gpio_in);
> +				PMMUX(key-volume_up, 93, gpio_in);
> +				PMMUX(key-volume_down, 94, gpio_in);
> +			};
> +			pinctrl_keypad_default: keypad-default {
> +				PMMUX(keypad-mkin0, 100,
> KP_MKIN<0>);
> +				PMMUX(keypad-mkin1, 101,
> KP_MKIN<1>);
> +				PMMUX(keypad-mkin2, 102,
> KP_MKIN<2>);
> +				PMMUX(keypad-mkout0, 103,
> KP_MKOUT<0>);
> +				PMMUX(keypad-mkout1, 104,
> KP_MKOUT<1>);
> +				PMMUX(keypad-mkout2, 105,
> KP_MKOUT<2>);
> +			};
> +			pinctrl_i2c_default: i2c-default {
> +				PMMUX(i2c-scl, 117, SCL);
> +				PMMUX(i2c-sda, 118, SDA);
> +			};
> +			pinctrl_lcd_default: lcd-default {
> +				PMMUX(ldd0, 58, LDD<0>);
> +				PMMUX(ldd1, 59, LDD<1>);
> +				PMMUX(ldd2, 60, LDD<2>);
> +				PMMUX(ldd3, 61, LDD<3>);
> +				PMMUX(ldd4, 62, LDD<4>);
> +				PMMUX(ldd5, 63, LDD<5>);
> +				PMMUX(ldd6, 64, LDD<6>);
> +				PMMUX(ldd7, 65, LDD<7>);
> +				PMMUX(ldd8, 66, LDD<8>);
> +				PMMUX(ldd9, 67, LDD<9>);
> +				PMMUX(ldd10, 68, LDD<10>);
> +				PMMUX(ldd11, 69, LDD<11>);
> +				PMMUX(ldd12, 70, LDD<12>);
> +				PMMUX(ldd13, 71, LDD<13>);
> +				PMMUX(ldd14, 72, LDD<14>);
> +				PMMUX(ldd16, 73, LDD<15>);
> +				PMMUX(lcd-fclk, 74, L_FCLK_RD);
> +				PMMUX(lcd-lclk, 75, L_LCLK_A0);
> +				PMMUX(lcd-pclk, 76, L_PCLK_WR);
> +				PMMUX(lcd-power, 87, gpio_out);
> +			};
> +			pinctrl_leds_default: leds-default {
> +				PMMUX_LPM_HIGH(led-charging, 10,
> gpio_out);
> +				PMMUX_LPM_HIGH(led-vibra, 82,
> gpio_out);
> +				PMMUX_LPM_HIGH(led-blue, 97,
> gpio_out);
> +				PMMUX_LPM_HIGH(led-orange, 98,
> gpio_out);
> +				PMMUX_LPM_HIGH(led-keyboard, 115,
> gpio_out);
> +			};
> +			pinctrl_pwm0_default: pwm0-defaut {
> +				PMMUX(pwm0, 16, PWM_OUT<0>);
> +			};
> +			pinctrl_qci_default: qci-defaut {
> +				PMMUX(cif-dd6, 17, CIF_DD<6>);
> +				PMMUX(cif-dd3, 50, CIF_DD<3>);
> +				PMMUX(cif-dd2, 51, CIF_DD<2>);
> +				PMMUX(cif-dd4, 52, CIF_DD<4>);
> +				PMMUX(cif-mclk, 53, CIF_MCLK);
> +				PMMUX(cif-pclk, 54, CIF_PCLK);
> +				PMMUX_LPM_HIGH(mt9m111-nOE, 56,
> gpio_out);
> +				PMMUX(cif-dd1, 55, CIF_DD<1>);
> +				PMMUX(cif-dd0, 81, CIF_DD<0>);
> +				PMMUX(cif-dd5, 48, CIF_DD<5>);
> +				PMMUX(cif-fv, 84, CIF_FV);
> +				PMMUX(cif-lv, 85, CIF_LV);
> +				PMMUX(cif-dd7, 108, CIF_DD<7>);
> +			};
> +			pinctrl_mmc_default: mmc-default {
> +				PMMUX(sd-insert, 15, gpio_in);
> +				PMMUX(mmclk, 32, MMCLK);
> +				PMMUX(sd-ro, 78, gpio_in);
> +				PMMUX_LPM_LOW(sd-enable, 91,
> gpio_out);
> +				PMMUX(mmdat0, 92, MMDAT<0>);
> +				PMMUX(mmdat1, 109, MMDAT<1>);
> +				PMMUX(mmdat2, 110, MMDAT<2>);
> +				PMMUX(mmdat3, 111, MMDAT<3>);
> +				PMMUX(mmcmd, 112, MMCMD);
> +			};
> +			pinctrl_stuart_default: stuart-default {
> +				PMMUX_LPM_LOW(gps-unknown1, 23,
> gpio_out);
> +				PMMUX_LPM_LOW(gps-on, 26, gpio_out);
> +				PMMUX_LPM_LOW(gps-nreset, 27,
> gpio_out);
> +				PMMUX(stuart-rxd, 46, STRXD);
> +				PMMUX(stuart-txd, 47, STTXD);
> +				PMMUX_LPM_LOW(gps-unknown2, 106,
> gpio_out);
> +				PMMUX_LPM_LOW(gps-unknown3, 107,
> gpio_out);
> +			};
> +			pinctrl_usb_default: usb-default {
> +				PMMUX(n-usb-detect, 13, gpio_in);
> +				PMMUX_LPM_LOW(n-dplus-pullup, 22,
> gpio_out);
> +			};
> +			pinctrl_power_default: power-default {
> +				PMMUX_LPM_LOW(charge-enable, 9,
> gpio_out);
> +				PMMUX(charge-vdrop, 80, gpio_out);
> +				PMMUX(ac-detect, 96, gpio_in);
> +			};

I guess usually, one would add newlines in front and between those
pinctrls but its kind of a matter of taste.

> +		};
> +
> +		pwm0: pwm@40b00000 {
> +		      status = "okay";
> +		};
> +
> +		gpio: gpio@40e00000 {
> +			status = "okay";
> +		};
> +
> +		ffuart: serial@40100000 {
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_ffuart_default>;
> +			status = "okay";
> +		};
> +
> +		btuart: serial@40200000 {
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_btuart_default>;
> +			status = "okay";
> +		};
> +
> +		stuart: serial@40700000 {
> +			status = "okay";
> +		};

I believe pxa2xx.dtsi calls them uart rather than serial so unless you
plan to change this we will have to stick to using uart instead.

I also don't think those serial port labels buy us anything, so I would
get rid of them.

> +		usb2phy: gpio-vbus@13 {
> +			compatible = "usb-nop-xceiv";
> +			#phy-cells = <1>;
> +			vbus-detect-gpio = <&gpio 13
> GPIO_ACTIVE_LOW>;
> +			wakeup;
> +		};
> +
> +		pxa27x_udc: udc@40600000 {
> +			status = "okay";
> +			gpios = <&gpio 22 0>;
> +			phys = <&usb2phy>;
> +			phys-names = "usb2phy";
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_usb_default>;
> +		};

Same with that pxa27x_udc label.

> +		pwri2c: i2c@40f000180 {

Uups, I guess that address is wrong, not? I will send a patch to fix it
in pxa27x.dtsi as well.

> +			status = "okay";
> +
> +			core_regulator@14 {
> +				compatible = "maxim,max1586";
> +				reg = <0x14>;
> +				v3-gain = <1000000>;
> +
> +				regulators {
> +					vcc_core: v3 {
> +						regulator-name =
> "vcc_core";
> +						regulator-compatible 
> = "Output_V3";
> +						regulator-min-
> microvolt = <1000000>;
> +						regulator-max-
> microvolt = <1705000>;
> +						regulator-always-on;
> +					};
> +				};
> +			};

Haven't seen core_regulator before. Just regulator would do unless it
would be a more complex pmic.

> +		};

And the pwri2c.

> +		pxai2c1: i2c@40301680 {
> +			mrvl,i2c-fast-mode;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_i2c_default>;
> +			status = "okay";
> +
> +			mt9m111: camera@5d {
> +				compatible = "micron,mt9m111";
> +				reg = <0x5d>;
> +				gpios = <&gpio 56 GPIO_ACTIVE_HIGH>;
> +

Spurious newline.

> +				remote = <&pxa_camera>;

Missing newline.

> +				port {
> +					mt9m111_1: endpoint {
> +						bus-width = <8>;
> +						remote-endpoint =
> <&pxa_camera>;
> +					};
> +				};
> +			};
> +		};

Same with pxai2c1 and mt9m111.

> +		keypad: keypad@41500000 {
> +			status = "okay";
> +

Spurious newline.

> +			keypad,num-rows = <3>;
> +			keypad,num-columns = <3>;
> +			linux,keymap = <
> +				0x00000067	/* KEY_UP */
> +				0x0001006a	/* KEY_RIGHT */
> +				0x000200e2	/* KEY_MEDIA */
> +				0x0100006c	/* KEY_DOWN */
> +				0x0101001c	/* KEY_ENTER */
> +				0x010200da	/* KEY_CONNECT */
> +				0x02000069	/* KEY_LEFT */
> +				0x020100a9	/* KEY_PHONE */
> +				0x020200d4>;	/* KEY_CAMERA */
> +			marvell,debounce-interval = <0>;
> +

Spurious newline.

> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_keypad_default>;
> +		};

Same with keypad.

> +		gpio-keys {
> +			compatible = "gpio-keys";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			autorepeat;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_gpiokeys_default>;
> +			status = "okay";
> +
> +			power-button {
> +				label = "GPIO Key Power";
> +				linux,code = <174>;
> +				gpios = <&gpio 0 0>;
> +				gpio-key,wakeup;

I believe that got deprecated in favour of just wakeup-source.

> +			};

Missing newline.

> +			hp-jack-detect {
> +				label = "HP jack detect";
> +				linux,code = <211>;
> +				gpios = <&gpio 12 0>;
> +			};

Newline.

> +			volume-up {
> +				label = "Volume Up Key";
> +				linux,code = <115>;
> +				gpios = <&gpio 93 0>;
> +			};

Newline.

> +			volume-down {
> +				label = "Volume Down Key";
> +				linux,code = <114>;
> +				gpios = <&gpio 94 0>;
> +			};
> +		};
> +
> +		mmc0: mmc@41100000 {
> +			vmmc-supply = <&reg_vmmc>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_mmc_default>;
> +			bus-width = <4>;
> +			cd-gpios = <&gpio 15 0>;
> +			wp-gpios = <&gpio 78 0>;
> +			status = "okay";
> +		};

Another useless label being mmc0.

> +		pxa_camera: imaging@50000000 {
> +			status = "okay";
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_qci_default>;
> +
> +			port {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				/* Parallel bus endpoint */
> +				qci: endpoint@0 {
> +					reg = <0>;
> +					remote-endpoint =
> <&mt9m111_1>;
> +					bus-width = <8>;
> +

Spurious newline.

> +					hsync-active = <0>;
> +					vsync-active = <0>;
> +					pclk-sample = <1>;
> +				};

And qci.

> +			};
> +		};
> +
> +		rtc@40900000 {
> +			status = "okay";
> +		};
> +
> +		lcd-controller@40500000 {
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_lcd_default>;
> +			status = "okay";

Missing newline.

> +			port {
> +				lcdc_out: endpoint {
> +					remote-endpoint =
> <&panel_in>;
> +					bus-width = <16>;
> +				};
> +			};
> +		};
> +
> +		ac97: sound@40500000 {
> +			compatible = "marvell,pxa270-ac97";
> +			reg = < 0x40500000 0x1000 >;
> +			interrupts = <14>;
> +			reset-gpios = <&gpio 95 GPIO_ACTIVE_HIGH>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = < &pinctrl_ac97_default >;
> +			clocks = <&clks CLK_AC97>, <&clks
> CLK_AC97CONF>;
> +			clock-names = "AC97CLK", "AC97CONFCLK";
> +			dmas = <&pdma 8 0
> +				&pdma 9 0
> +				&pdma 10 0
> +				&pdma 11 0
> +				&pdma 12 0>;
> +			dma-names = "pcm_pcm_mic_mono",
> "pcm_pcm_aux_mono_in",
> +				    "pcm_pcm_aux_mono_out",
> "pcm_pcm_stereo_in",
> +				    "pcm_pcm_stereo_out";
> +

Spurious newline.

> +			#sound-dai-cells = <0>;
> +

Spurious newline.

> +			#address-cells = <1>;
> +			#size-cells = <0>;

Missing newline.

> +			wm9713: audio-codec@0 {
> +				reg = <0>;
> +				compatible = "ac97,574d,4c13";
> +				clocks = <&wm9713_bitclk>;
> +				clock-names = "ac97_clk";
> +				#sound-dai-cells = <0>;
> +
> +				wm9713_bitclk: ac97_bitclk {
> +					compatible = "fixed-clock";
> +					#clock-cells = <0>;
> +					clock-frequency =
> <12285000>;
> +					status = "okay";
> +				};
> +			};

While a few device trees seem to use audio-codec just codec would work
too.

> +		};
> +
> +		pxa_pcm_audio: snd_soc_pxa_audio {
> +			compatible = "mrvl,pxa-pcm-audio";
> +			#sound-dai-cells = <0>;
> +			status = "okay";
> +		};

I believe node names should not contain underscores therefore suggest
chaning snd_soc_pxa_audio to snd-soc-pxa-audio.

> +		lcd-controller@40500000 {
> +			lcd-supply = <&lcd_vmmc>;
> +		};
> +
> +		docg3: flash@0 {
> +			compatible = "m-systems,diskonchip-g3";
> +			reg = <0x0 0x2000>;
> +		};
> +

Spurious newline.

> +	};
> +
> +	reg_vmmc: regulator@0 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vmmc";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +

Spurious newline.

> +		gpio = <&gpio 91 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};

I guess that @0 is a legacy from when we used a fake regulator simple
bus and nowadays regulator-vmmc is more common.

> +	lcd_vmmc: regulator@1 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "lcd-supply";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-boot-on;
> +

Spurious newline.

> +		gpio = <&gpio 87 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};

Same for the @1 here and usually, for regulator labels the reg_ prefix
is used.

> +	lcd_backlight: backlight {
> +		compatible = "pwm-backlight";
> +		pwms = <&pwm0 4096000>;
> +		pwm-names = "backlight";
> +

Newline.

> +		brightness-levels = <0 4 8 16 32 64 128 255>;
> +		default-brightness-level = <2>;
> +

Newline.

> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_pwm0_default>;
> +	};
> +
> +	panel {
> +		compatible = "toshiba,ltm0305a776";
> +		lcd-type = "color-tft";
> +

Newline.

> +		backlight = <&lcd_backlight>;
> +
> +		port {
> +			panel_in: endpoint {
> +				remote-endpoint = <&lcdc_out>;
> +			};
> +		};
> +
> +		display-timings {
> +			native-mode = <&timing0>;
> +			timing0: 240p {
> +				/* 240x320p24 */
> +				clock-frequency = <4545000>;
> +				hactive = <240>;
> +				vactive = <320>;
> +				hfront-porch = <4>;
> +				hback-porch = <6>;
> +				hsync-len = <4>;
> +				vback-porch = <5>;
> +				vfront-porch = <3>;
> +				vsync-len = <2>;
> +			};
> +		};
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +

Newline.

> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_leds_default>;
> +
> +		charger-led {
> +			label = "mioa701:charging";
> +			gpios = <&gpio 10 GPIO_ACTIVE_LOW>;
> +			default-state = "off";
> +		};
> +
> +		vibrator {
> +			label = "mioa701:vibra";
> +			gpios = <&gpio 82 GPIO_ACTIVE_LOW>;
> +			default-state = "off";
> +		};
> +
> +		bluetooth-led {
> +			label = "mioa701:blue";
> +			gpios = <&gpio 97 GPIO_ACTIVE_LOW>;
> +			default-state = "off";
> +		};
> +
> +		orange-led {
> +			label = "mioa701:orange";
> +			gpios = <&gpio 98 GPIO_ACTIVE_LOW>;
> +			default-state = "off";
> +		};
> +
> +		keyboard-led {
> +			label = "mioa701:keyboard";
> +			gpios = <&gpio 115 GPIO_ACTIVE_LOW>;
> +			default-state = "off";
> +		};
> +	};
> +
> +	sound {
> +		compatible = "simple-audio-card";
> +		simple-audio-card,name = "MioA701";
> +		simple-audio-card,widgets =
> +		"Speaker", "Front Speaker",
> +		"Speaker", "Rear Speaker",
> +		"Microphone", "Headset",
> +		"Microphone", "GSM Line Out",
> +		"Line", "GSM Line In",
> +		"Microphone", "Headset Mic",
> +		"Microphone", "Front Mic";
> +		simple-audio-card,routing =
> +			/* Call Mic */
> +			"Mic Bias", "Front Mic",
> +			"MIC1", "Mic Bias",
> +			/* Headset Mic */
> +			"LINEL", "Headset Mic",
> +			"LINER", "Headset Mic",
> +			/* GSM Module */
> +			"MONOIN", "GSM Line Out",
> +			"PCBEEP", "GSM Line Out",
> +			"GSM Line In", "MONO",
> +			/* headphone connected to HPL, HPR */
> +			"Headset", "HPL",
> +			"Headset", "HPR",
> +			/* front speaker connected to HPL, OUT3 */
> +			"Front Speaker", "HPL",
> +			"Front Speaker", "OUT3",
> +			/* rear speaker connected to SPKL, SPKR */
> +			"Rear Speaker", "SPKL",
> +			"Rear Speaker", "SPKR";
> +
> +		simple-audio-card,cpu {
> +			sound-dai = <&ac97>;
> +		};

Missing newline.

> +		simple-audio-card,codec {
> +			sound-dai = <&wm9713>;
> +		};

Missing newline.

> +		simple-audio-card,plat {
> +			sound-dai = <&pxa_pcm_audio>;
> +		};
> +	};
> +
> +	ac_charger: charger {
> +		compatible = "gpio-charger";
> +		charger-type = "usb-aca";
> +		gpios = <&gpio 96 GPIO_ACTIVE_HIGH>;
> +	};
> +};

Cheers

Marcel

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

* Re: [PATCH v3] ARM: dts: pxa: add mioa701 board description
  2018-08-31  7:45   ` kbuild test robot
  (?)
@ 2018-08-31  9:48   ` Marcel Ziswiler
  -1 siblings, 0 replies; 7+ messages in thread
From: Marcel Ziswiler @ 2018-08-31  9:48 UTC (permalink / raw)
  To: kbuild test robot, Robert Jarzmik
  Cc: kbuild-all, Rob Herring, Mark Rutland, Kees Cook,
	Anton Vorontsov, Colin Cross, Tony Luck, devicetree,
	linux-kernel

On Fri, 2018-08-31 at 15:45 +0800, kbuild test robot wrote:
> Hi Robert,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on robh/for-next]
> [also build test ERROR on v4.19-rc1 next-20180831]
> [if your patch is applied to the wrong git tree, please drop us a
> note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Robert-Jarzmik/ARM-d
> ts-pxa-add-mioa701-board-description/20180831-134244
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.gi
> t for-next
> config: arm-keystone_defconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master
> /sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         GCC_VERSION=7.2.0 make.cross ARCH=arm 
> 
> All errors (new ones prefixed by >>):
> 
> > > Error: arch/arm/boot/dts/mioa701.dts:47.10-11 syntax error
> 
>    FATAL ERROR: Unable to parse input tree

I guess it just missed the dependency on the following:

https://lore.kernel.org/lkml/20180830195912.6025-1-robert.jarzmik@free.
fr/

> ---
> 0-DAY kernel test infrastructure                Open Source
> Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel
> Corporation

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

* Re: [PATCH v3] ARM: dts: pxa: add mioa701 board description
  2018-08-31  7:45   ` kbuild test robot
  (?)
  (?)
@ 2018-08-31 14:54   ` Robert Jarzmik
  -1 siblings, 0 replies; 7+ messages in thread
From: Robert Jarzmik @ 2018-08-31 14:54 UTC (permalink / raw)
  To: Rob Herring, kbuild test robot
  Cc: kbuild-all, Mark Rutland, Kees Cook, Anton Vorontsov,
	Colin Cross, Tony Luck, devicetree, linux-kernel

kbuild test robot <lkp@intel.com> writes:

> Hi Robert,
>
> I love your patch! Yet something to improve:
This is because you are missing at least this patch submitted earlier :
 - [PATCH] ARM: dts: pxa: add pincontrol helpers

You're also missing the previous patches in the pxa/dt tree, which are already
on the trampoline for linux-next :
 - git fetch github.com:rjarzmik/linux.git pxa/dt

Cheers.

--
Robert

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

* Re: [PATCH v3] ARM: dts: pxa: add mioa701 board description
  2018-08-31  9:46 ` Marcel Ziswiler
@ 2018-09-14  5:54   ` Robert Jarzmik
  0 siblings, 0 replies; 7+ messages in thread
From: Robert Jarzmik @ 2018-09-14  5:54 UTC (permalink / raw)
  To: Marcel Ziswiler
  Cc: Rob Herring, Mark Rutland, Kees Cook, Anton Vorontsov,
	Colin Cross, Tony Luck, devicetree, linux-kernel

Marcel Ziswiler <marcel@ziswiler.com> writes:

> Hi Robert
>>  arch/arm/boot/dts/Makefile    |   2 +
>>  arch/arm/boot/dts/mioa701.dts | 558 
>
> Isn't that one supposed to be prefixed by pxa270-?
Good point, for v4.

>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
...
>> +dtb-$(CONFIG_ARCH_PXA) += \
> Wouldn't it rather make more sense to use CONFIG_MACH_PXA27X_DT
> instead?
No, I'll have all the devicetree files under one config, for the PXA
architecture.

>> +	model = "Mitac Mio A701 Board";
>> +	compatible = "marvell,pxa270";
>
> Usually, there is also a compatible for the particular board e.g.
> "mitac,mioa701", not? You might have to check on the vendor prefix
> though.
Ok.

>> +			pinctrl_usb_default: usb-default {
>> +				PMMUX(n-usb-detect, 13, gpio_in);
>> +				PMMUX_LPM_LOW(n-dplus-pullup, 22,
>> gpio_out);
>> +			};
>> +			pinctrl_power_default: power-default {
>> +				PMMUX_LPM_LOW(charge-enable, 9,
>> gpio_out);
>> +				PMMUX(charge-vdrop, 80, gpio_out);
>> +				PMMUX(ac-detect, 96, gpio_in);
>> +			};
>
> I guess usually, one would add newlines in front and between those
> pinctrls but its kind of a matter of taste.
Ok, for this and all the following newlines.
>> +		ffuart: serial@40100000 {
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_ffuart_default>;
>> +			status = "okay";
>> +		};
>> +
>> +		btuart: serial@40200000 {
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_btuart_default>;
>> +			status = "okay";
>> +		};
>> +
>> +		stuart: serial@40700000 {
>> +			status = "okay";
>> +		};
>
> I believe pxa2xx.dtsi calls them uart rather than serial so unless you
> plan to change this we will have to stick to using uart instead.
There was a patch submitted for that earlier, at Rob's demand, to change these
names. That's another dependency on the pxa/dt tree.
>
> I also don't think those serial port labels buy us anything, so I would
> get rid of them.
As there are already in the .dtsi files, they don't hurt do they ?

>> +		pwri2c: i2c@40f000180 {
>
> Uups, I guess that address is wrong, not? I will send a patch to fix it
> in pxa27x.dtsi as well.
Fixed here as well.

>
>> +			status = "okay";
>> +
>> +			core_regulator@14 {
>> +				compatible = "maxim,max1586";
>> +				reg = <0x14>;
>> +				v3-gain = <1000000>;
>> +
>> +				regulators {
>> +					vcc_core: v3 {
>> +						regulator-name =
>> "vcc_core";
>> +						regulator-compatible 
>> = "Output_V3";
>> +						regulator-min-
>> microvolt = <1000000>;
>> +						regulator-max-
>> microvolt = <1705000>;
>> +						regulator-always-on;
>> +					};
>> +				};
>> +			};
>
> Haven't seen core_regulator before. Just regulator would do unless it
> would be a more complex pmic.
Ok.

>
>
>> +				port {
>> +					mt9m111_1: endpoint {
>> +						bus-width = <8>;
>> +						remote-endpoint =
>> <&pxa_camera>;
>> +					};
>> +				};
>> +			};
>> +		};
>
> Same with pxai2c1 and mt9m111.
Ok for mt9m111, same answer as before for pxai2c1.
>> +		gpio-keys {
>> +			compatible = "gpio-keys";
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +			autorepeat;
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_gpiokeys_default>;
>> +			status = "okay";
>> +
>> +			power-button {
>> +				label = "GPIO Key Power";
>> +				linux,code = <174>;
>> +				gpios = <&gpio 0 0>;
>> +				gpio-key,wakeup;
>
> I believe that got deprecated in favour of just wakeup-source.
Ok.

>> +		lcd-controller@40500000 {
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&pinctrl_lcd_default>;
>> +			status = "okay";
>
> Missing newline.
>
>> +			port {
>> +				lcdc_out: endpoint {
>> +					remote-endpoint =
>> <&panel_in>;
>> +					bus-width = <16>;
>> +				};
>> +			};
>> +		};
>> +
>> +		ac97: sound@40500000 {
>> +			compatible = "marvell,pxa270-ac97";
>> +			reg = < 0x40500000 0x1000 >;
>> +			interrupts = <14>;
>> +			reset-gpios = <&gpio 95 GPIO_ACTIVE_HIGH>;
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = < &pinctrl_ac97_default >;
>> +			clocks = <&clks CLK_AC97>, <&clks
>> CLK_AC97CONF>;
>> +			clock-names = "AC97CLK", "AC97CONFCLK";
>> +			dmas = <&pdma 8 0
>> +				&pdma 9 0
>> +				&pdma 10 0
>> +				&pdma 11 0
>> +				&pdma 12 0>;
>> +			dma-names = "pcm_pcm_mic_mono",
>> "pcm_pcm_aux_mono_in",
>> +				    "pcm_pcm_aux_mono_out",
>> "pcm_pcm_stereo_in",
>> +				    "pcm_pcm_stereo_out";
>> +
>
> Spurious newline.
>
>> +			#sound-dai-cells = <0>;
>> +
>
> Spurious newline.
>
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>
> Missing newline.
>
>> +			wm9713: audio-codec@0 {
>> +				reg = <0>;
>> +				compatible = "ac97,574d,4c13";
>> +				clocks = <&wm9713_bitclk>;
>> +				clock-names = "ac97_clk";
>> +				#sound-dai-cells = <0>;
>> +
>> +				wm9713_bitclk: ac97_bitclk {
>> +					compatible = "fixed-clock";
>> +					#clock-cells = <0>;
>> +					clock-frequency =
>> <12285000>;
>> +					status = "okay";
>> +				};
>> +			};
>
> While a few device trees seem to use audio-codec just codec would work
> too.
Ok.

>> +		};
>> +
>> +		pxa_pcm_audio: snd_soc_pxa_audio {
>> +			compatible = "mrvl,pxa-pcm-audio";
>> +			#sound-dai-cells = <0>;
>> +			status = "okay";
>> +		};
>
> I believe node names should not contain underscores therefore suggest
> chaning snd_soc_pxa_audio to snd-soc-pxa-audio.
Ok.
>
>> +		lcd-controller@40500000 {
>> +			lcd-supply = <&lcd_vmmc>;
>> +		};
>> +
>> +		docg3: flash@0 {
>> +			compatible = "m-systems,diskonchip-g3";
>> +			reg = <0x0 0x2000>;
>> +		};
>> +
>
> Spurious newline.
>
>> +	};
>> +
>> +	reg_vmmc: regulator@0 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "vmmc";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +
>
> Spurious newline.
>
>> +		gpio = <&gpio 91 GPIO_ACTIVE_HIGH>;
>> +		enable-active-high;
>> +	};
>
> I guess that @0 is a legacy from when we used a fake regulator simple
> bus and nowadays regulator-vmmc is more common.
Ok.

> Same for the @1 here and usually, for regulator labels the reg_ prefix
> is used.
Ok.

Cheers.

-- 
Robert

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

end of thread, other threads:[~2018-09-14  5:54 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-30 19:59 [PATCH v3] ARM: dts: pxa: add mioa701 board description Robert Jarzmik
2018-08-31  7:45 ` kbuild test robot
2018-08-31  7:45   ` kbuild test robot
2018-08-31  9:48   ` Marcel Ziswiler
2018-08-31 14:54   ` Robert Jarzmik
2018-08-31  9:46 ` Marcel Ziswiler
2018-09-14  5:54   ` Robert Jarzmik

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.