2018-07-02 05:59:15

by Robert Jarzmik

[permalink] [raw]
Subject: [PATCH 1/2] ARM: dts: pxa: add pincontrol helpers

Add 3 helpers so that pincontrol definitions for pxa25x and pxa27x are
easier, and can be easily converted from old mfp mach-pxa code to
devicetree.

An example of such conversion would be :
static unsigned long mioa701_pin_config[] = {
GPIO32_MMC_CLK,
GPIO92_MMC_DAT_0,
GPIO109_MMC_DAT_1,
GPIO110_MMC_DAT_2,
GPIO111_MMC_DAT_3,
GPIO112_MMC_CMD,
MIO_CFG_IN(GPIO78_SDIO_RO, AF0),
MIO_CFG_IN(GPIO15_SDIO_INSERT, AF0),
MIO_CFG_OUT(GPIO91_SDIO_EN, AF0, DRIVE_LOW),
};
into:
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);
};

The third column of PMMUX*() helpers can be found in pincontrol muxing
functions, either in pinctrl-pxa27x.c (or pinctrl-pxa25x.c), or by
inspecting the pincontrol once booted in debugfs.

Signed-off-by: Robert Jarzmik <[email protected]>
---
arch/arm/boot/dts/pxa2xx.dtsi | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/pxa2xx.dtsi b/arch/arm/boot/dts/pxa2xx.dtsi
index e4ebcde17837..f609dbb86abf 100644
--- a/arch/arm/boot/dts/pxa2xx.dtsi
+++ b/arch/arm/boot/dts/pxa2xx.dtsi
@@ -9,6 +9,25 @@
#include "skeleton.dtsi"
#include "dt-bindings/clock/pxa-clock.h"

+#define PMGROUP(pin) #pin
+#define PMMUX(func, pin, af) \
+ mux- ## func { \
+ groups = PMGROUP(P ## pin); \
+ function = #af; \
+ }
+#define PMMUX_LPM_LOW(func, pin, af) \
+ mux- ## func { \
+ groups = PMGROUP(P ## pin); \
+ function = #af; \
+ low-power-disable; \
+ }
+#define PMMUX_LPM_HIGH(func, pin, af) \
+ mux- ## func { \
+ groups = PMGROUP(P ## pin); \
+ function = #af; \
+ low-power-enable; \
+ }
+
/ {
model = "Marvell PXA2xx family SoC";
compatible = "marvell,pxa2xx";
--
2.11.0



2018-07-02 06:00:13

by Robert Jarzmik

[permalink] [raw]
Subject: [PATCH 2/2] ARM: dts: pxa: add mioa701 board description

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 <[email protected]>
---
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 | 1 +
arch/arm/boot/dts/mioa701.dts | 565 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 566 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 37a3de760d40..6b12ab50c8d1 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -756,6 +756,7 @@ 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..680e0e44d526
--- /dev/null
+++ b/arch/arm/boot/dts/mioa701.dts
@@ -0,0 +1,565 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Robert Jarzmik <[email protected]>
+ *
+ * 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 = "mitac,mioa701"; */
+ 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@0xa2000000 {
+ compatible = "linux,contiguous-memory-region";
+ reg = <0xa2000000 1048576>;
+ };
+ };
+ };
+
+ 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: uart@40100000 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_ffuart_default>;
+ status = "okay";
+ };
+
+ btuart: uart@40200000 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_btuart_default>;
+ status = "okay";
+ };
+
+ stuart: uart@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";
+
+ button@0 {
+ label = "GPIO Key Power";
+ linux,code = <174>;
+ gpios = <&gpio 0 0>;
+ gpio-key,wakeup;
+ };
+ button@12 {
+ label = "HP jack detect";
+ linux,code = <211>;
+ gpios = <&gpio 12 0>;
+ };
+ button@93 {
+ label = "Volume Up Key";
+ linux,code = <115>;
+ gpios = <&gpio 93 0>;
+ };
+ button@94 {
+ 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>;
+ };
+ };
+
+ regulators {
+ compatible = "simple-bus";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ 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;
+ };
+ };
+
+ 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>;
+ };
+
+ docg3: flash@0 {
+ compatible = "m-systems,diskonchip-g3";
+ reg = <0x0 0x2000>;
+ };
+
+ panel {
+ compatible = "toshiba,ltm0305a776";
+ lcd-type = "color-tft";
+
+ power-supply = <&lcd_supply>;
+ 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@1 {
+ compatible = "gpio-charger";
+ charger-type = "usb-aca";
+ gpios = <&gpio 96 GPIO_ACTIVE_HIGH>;
+ };
+};
--
2.11.0


2018-07-03 02:41:20

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: dts: pxa: add mioa701 board description

Hi Robert,

I love your patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.18-rc3 next-20180702]
[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-pincontrol-helpers/20180702-140342
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-u8500_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: Input tree has errors, aborting (use -f to force output)

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


Attachments:
(No filename) (1.07 kB)
.config.gz (22.13 kB)
Download all attachments

2018-07-09 18:02:55

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: dts: pxa: add mioa701 board description

On Sun, Jul 1, 2018 at 11:58 PM Robert Jarzmik <[email protected]> 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.
>
> Signed-off-by: Robert Jarzmik <[email protected]>
> ---
> 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 | 1 +
> arch/arm/boot/dts/mioa701.dts | 565 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 566 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 37a3de760d40..6b12ab50c8d1 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -756,6 +756,7 @@ 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..680e0e44d526
> --- /dev/null
> +++ b/arch/arm/boot/dts/mioa701.dts
> @@ -0,0 +1,565 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Robert Jarzmik <[email protected]>
> + *
> + * 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 = "mitac,mioa701"; */
> + compatible = "marvell,pxa270";

Why the comment?

> +
> + 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@0xa2000000 {

Drop the 0x

> + compatible = "linux,contiguous-memory-region";
> + reg = <0xa2000000 1048576>;

Use hex for the size.

> + };
> + };
> + };
> +
> + 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: uart@40100000 {

Should be "serial@...". You'll have to fix in the base dtsi.

> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_ffuart_default>;
> + status = "okay";
> + };
> +
> + btuart: uart@40200000 {

ditto

> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_btuart_default>;
> + status = "okay";
> + };
> +
> + stuart: uart@40700000 {

ditto

> + 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>;

Not needed with the graph:

> + 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";

Don't need status here? It shouldn't be defined in the base dtsi.

> +
> + button@0 {

If you have unit-address there should also be a reg property. Just
drop and name the node 'power' or 'power-button'. And similarly for
the rest.

> + label = "GPIO Key Power";
> + linux,code = <174>;
> + gpios = <&gpio 0 0>;
> + gpio-key,wakeup;
> + };
> + button@12 {
> + label = "HP jack detect";
> + linux,code = <211>;
> + gpios = <&gpio 12 0>;
> + };
> + button@93 {
> + label = "Volume Up Key";
> + linux,code = <115>;
> + gpios = <&gpio 93 0>;
> + };
> + button@94 {
> + 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>;
> + };
> + };
> +
> + regulators {
> + compatible = "simple-bus";

Drop this and move children to the top level. This is not an mmio bus.

> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + 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;
> + };
> + };
> +
> + 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>;
> + };
> +
> + docg3: flash@0 {

This probably should be a child of the bus controller.

> + compatible = "m-systems,diskonchip-g3";
> + reg = <0x0 0x2000>;
> + };
> +
> + panel {
> + compatible = "toshiba,ltm0305a776";
> + lcd-type = "color-tft";

This should be implied by the compatible.

> +
> + power-supply = <&lcd_supply>;
> + backlight = <&lcd_backlight>;
> +
> + port {
> + panel_in: endpoint {
> + remote-endpoint = <&lcdc_out>;
> + };
> + };
> +
> + display-timings {
> + native-mode = <&timing0>;

We generally imply this from the compatible (i.e. use simple-panel).

> + 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",

Could use another level of indentation.

> + "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@1 {

Drop the unit-address.

> + compatible = "gpio-charger";
> + charger-type = "usb-aca";
> + gpios = <&gpio 96 GPIO_ACTIVE_HIGH>;
> + };
> +};
> --
> 2.11.0
>

2018-07-14 20:16:53

by Robert Jarzmik

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: dts: pxa: add mioa701 board description

Rob Herring <[email protected]> writes:

Hi Rob,

>> + /* compatible = "mitac,mioa701"; */
>> + compatible = "marvell,pxa270";
>
> Why the comment?
Leftover, I'll remove it.

>> + pstore_region:region@0xa2000000 {
>
> Drop the 0x
Done.

>> + compatible = "linux,contiguous-memory-region";
>> + reg = <0xa2000000 1048576>;
>
> Use hex for the size.
Ok.

>> + ffuart: uart@40100000 {
>
> Should be "serial@...". You'll have to fix in the base dtsi.
Ok for all of these.
>> + 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>;
>
> Not needed with the graph:
I don't understand that, do you mean the port node is not needed ?

>
>> + port {
>> + mt9m111_1: endpoint {
>> + bus-width = <8>;
>> + remote-endpoint = <&pxa_camera>;
>> + };
>> + };
>> + };
>> + };
>> + gpio-keys {
>> + compatible = "gpio-keys";
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + autorepeat;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_gpiokeys_default>;
>> + status = "okay";
>
> Don't need status here? It shouldn't be defined in the base dtsi.
Heuh what ? The status is just above, and gpio-keys is not defined in any dtsi.

>> +
>> + button@0 {
>
> If you have unit-address there should also be a reg property. Just
> drop and name the node 'power' or 'power-button'. And similarly for
> the rest.
Ok.

>> +
>> + regulators {
>> + compatible = "simple-bus";
>
> Drop this and move children to the top level. This is not an mmio bus.
Ok.

>> + docg3: flash@0 {
>
> This probably should be a child of the bus controller.
Most certainly yes.

>
>> + compatible = "m-systems,diskonchip-g3";
>> + reg = <0x0 0x2000>;
>> + };
>> +
>> + panel {
>> + compatible = "toshiba,ltm0305a776";
>> + lcd-type = "color-tft";
>
> This should be implied by the compatible.
Mmm no. Actually, this is used by the framebuffer controller, ie. pxafb to set
it up. And I don't think adding a list of panel compatibles in pxafb is a good
idea.
The corresponding code is in : drivers/video/fbdev/pxafb.c, see
"static const char * const lcd_types[]".

>> + display-timings {
>> + native-mode = <&timing0>;
>
> We generally imply this from the compatible (i.e. use simple-panel).
But you suppose there is an actual driver for the panel, right ? But there is
none.

> Could use another level of indentation.
Sure.

>> + ac_charger: charger@1 {
>
> Drop the unit-address.
Ok.

Thanks for the review.

--
Robert