Hi Marco, On Thu, Nov 14, 2019 at 4:56 PM Marco Antonio Franchi <marco.franchi@xxxxxxx> wrote: > > This patch adds the device tree to support Google Coral Edge TPU, > historicaly named as fsl-imx8mq-phanbell, a computer on module > which can be used for AI/ML propose. > > It introduces a minimal enablement support for this module and What are the features that have been tested? Also, is the schematics available? > was totally based on the NXP i.MX 8MQ EVK board and i.MX 8MQ Phanbell > Google Source Code for Coral Edge TPU Mendel release: > https://coral.googlesource.com/linux-imx/ > > This patch was tested using the U-Boot 2017-03-1-release-chef, > which is supported by the Coral Edge TPU Mendel release: > https://coral.googlesource.com/uboot-imx/ I would suggest removing this paragraph from the commit log as it is not relevant to the dts itself. > diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile > index 38e344a2f0ff..cc7e02a30ed1 100644 > --- a/arch/arm64/boot/dts/freescale/Makefile > +++ b/arch/arm64/boot/dts/freescale/Makefile > @@ -21,6 +21,7 @@ dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls2088a-rdb.dtb > dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-lx2160a-qds.dtb > dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-lx2160a-rdb.dtb > > +dtb-$(CONFIG_ARCH_MXC) += fsl-imx8mq-phanbell.dtb Please remove the fsl prefix and call it mx8mq-phanbell.dtb instead to align with the other imx8mq dtbs. > +&i2c1 { > + clock-frequency = <400000>; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_i2c1>; > + status = "okay"; > + > + pmic: pmic@4b { > + reg = <0x4b>; > + compatible = "rohm,bd71837"; > + pinctrl-0 = <&pinctrl_pmic>; > + gpio_intr = <&gpio1 3 GPIO_ACTIVE_LOW>; This property does not exist upstream. You should describe the interrupt like this instead: interrupt-parent = <&gpio1>; interrupts = <3 GPIO_ACTIVE_LOW>; > + > + gpo { > + rohm,drv = <0x0C>; This property does not exist upstream. > +&sai2 { > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_sai2>; > + assigned-clocks = > + <&clk IMX8MQ_AUDIO_PLL1_BYPASS>, <&clk IMX8MQ_CLK_SAI2>; Please don't split the lines as it gets harder to read. > + assigned-clock-parents = > + <&clk IMX8MQ_AUDIO_PLL1>, <&clk IMX8MQ_AUDIO_PLL1_OUT>; Same here. > + assigned-clock-rates = <0>, <24576000>; > + status = "okay"; > +}; > + > +&wdog1 { > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_wdog>; > + fsl,ext-reset-output; > + status = "okay"; > +}; > + > +&iomuxc { > + pinctrl-names = "default"; > + > + imx8mq-evk { No need for this imx8mq-evk container. > + pinctrl_pmic: pmicirq { > + fsl,pins = < > + MX8MQ_IOMUXC_GPIO1_IO03_GPIO1_IO3 0x41 /*0x17059*/ This comment looks confusing. I would suggest removing it. Regards, Fabio Estevam
