On Thu, Aug 31, 2017 at 5:03 PM, Maxime Ripard
[off-list ref] wrote:

Hi,

On Thu, Aug 31, 2017 at 02:52:43PM +0800, Chen-Yu Tsai wrote:

quoted

quoted

+       non-removable;
+       wakeup-source;
+       status = "okay";
+
+       brcmf: wifi at 1 {
+               reg = <1>;
+               compatible = "brcm,bcm4329-fmac";
+               interrupt-parent = <&r_pio>;
+               interrupts = <0 3 IRQ_TYPE_LEVEL_LOW>; /* PL3 WL_WAKE_UP */
+               interrupt-names = "host-wake";
+       };
+};
+
+&mmc2 {
+       pinctrl-0 = <&mmc2_8bit_emmc_pins>;
+       pinctrl-names = "default";
+       vmmc-supply = <&reg_vcc3v3>;

emmc has separate vddq lines. vqmmc-supply should be provided even though it
may be the same as vmmc-supply. The two refer to different things at the board
level.

My understanding was that vqmmc wasn't needed if it was the same
regulator than vmmc.

It doesn't say so in the binding, so that's probably only true for Linux.

ChenYu

quoted

quoted

+/* There's the BT part of the AP6210 connected to that UART */
+&uart1 {
+       pinctrl-names = "default";
+       pinctrl-0 = <&uart1_pg_pins &uart1_rtscts_pins>;

Splitting it as separate phandles ( <&...> ) would make it more readable.
Also the rx/tx pingroup name should be changed

Ack.

quoted

quoted

+       status = "okay";
+};
+
+&usbphy {

No VBUS?

Argh, I forgot them. I'll add them, obviously.

Thanks!
Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com