Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756169AbdGLJrz (ORCPT ); Wed, 12 Jul 2017 05:47:55 -0400 Received: from foss.arm.com ([217.140.101.70]:53144 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755234AbdGLJry (ORCPT ); Wed, 12 Jul 2017 05:47:54 -0400 Date: Wed, 12 Jul 2017 10:46:55 +0100 From: Mark Rutland To: linux-kernel-dev@beckhoff.com Cc: robh+dt@kernel.org, shawnguo@kernel.org, kernel@pengutronix.de, linux@armlinux.org.uk, fabio.estevam@nxp.com, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Patrick =?utf-8?B?QnLDvG5u?= Subject: Re: [PATCH] ARM: dts: imx: add CX9020 Embedded PC device tree Message-ID: <20170712094655.GC7472@leverpostej> References: <20170712090408.12212-1-linux-kernel-dev@beckhoff.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170712090408.12212-1-linux-kernel-dev@beckhoff.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3227 Lines: 143 Hi, On Wed, Jul 12, 2017 at 11:04:08AM +0200, linux-kernel-dev@beckhoff.com wrote: > +/dts-v1/; > +#include "imx53.dtsi" > + > +#define MX53_PAD_EIM_D26__UART2_RXD_MUX 0x144 0x48c 0x880 0x2 0x0 > +#define MX53_PAD_EIM_D27__UART2_TXD_MUX 0x148 0x490 0x000 0x2 0x0 > +#define MX53_PAD_EIM_D28__UART2_RTS 0x14c 0x494 0x87c 0x2 0x0 > +#define MX53_PAD_EIM_D29__UART2_CTS 0x150 0x498 0x000 0x2 0x0 > + > +/ { > + model = "Freescale i.MX53 based Beckhoff CX9020"; > + compatible = "fsl,imx53-qsb", "fsl,imx53"; > + > + chosen { > + stdout-path = &uart2; No baud-rate or bits configuration? > + }; > + > + memory { > + reg = <0x70000000 0x20000000>, > + <0xb0000000 0x20000000>; > + }; > + > + ccat { > + compatible = "bhf,emi-ccat"; > + }; > + > + display0: display@di0 { This unit-address (the bit after the @) isn't valid, as that should match a reg or ranges, but this node has neither. Just call this display-0. > + #address-cells =<1>; > + #size-cells = <0>; > + compatible = "fsl,imx-parallel-display"; > + interface-pix-fmt = "rgb24"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_ipu_disp0>; > + status = "okay"; > + > + port@0 { > + reg = <0>; > + display0_in: endpoint { > + remote-endpoint = <&ipu_di0_disp0>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + display0_out: endpoint { > + remote-endpoint = <&panel_in>; > + }; > + }; > + }; > + > + dvi_panel: display@0 { Likewise you have no reg here, so the unit address isn't valid. Surely panel-0? > + #address-cells =<1>; > + #size-cells = <0>; > + compatible = "simple,ddc-only"; I don't see that compatible string in my Linux tree, and it doesn't make sense to me -- "simple" isn't a vendor-prefix. Where has this come from? > + ddc-i2c-bus = <&i2c2>; > + > + port { > + panel_in: endpoint { > + remote-endpoint = <&display0_out>; > + }; > + }; > + }; [...] > + regulators { > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <0>; > + > + reg_3p2v: regulator@0 { > + compatible = "regulator-fixed"; > + reg = <0>; Meaningless reg entry. > + regulator-name = "3P2V"; > + regulator-min-microvolt = <3200000>; > + regulator-max-microvolt = <3200000>; > + regulator-always-on; > + }; > + > + reg_usb_vbus: regulator@1 { > + compatible = "regulator-fixed"; > + reg = <1>; Likewise. > + regulator-name = "usb_vbus"; > + regulator-min-microvolt = <5000000>; > + regulator-max-microvolt = <5000000>; > + gpio = <&gpio7 8 0>; > + enable-active-high; > + }; > + }; There's no need for a simple-bus here. It doesn't represent HW, and you can nothing. You can put these directly under the root node, without a synthetic reg or unnecessary container: reg_3p2v: regulator-3p2v { compatible = "regulator-fixed"; regulator-name = "3P2V"; regulator-min-microvolt = <3200000>; regulator-max-microvolt = <3200000>; regulator-always-on; }; reg_usb_vbus: regulator-usb-vbus { compatible = "regulator-fixed"; regulator-name = "usb_vbus"; regulator-min-microvolt = <5000000>; regulator-max-microvolt = <5000000>; gpio = <&gpio7 8 0>; enable-active-high; } Otherwise, looks fine to me. Thanks, Mark.