Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752321AbdFOHQT (ORCPT ); Thu, 15 Jun 2017 03:16:19 -0400 Received: from mail.kernel.org ([198.145.29.99]:48400 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750859AbdFOHQS (ORCPT ); Thu, 15 Jun 2017 03:16:18 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3F12022CB7 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=shawnguo@kernel.org Date: Thu, 15 Jun 2017 15:15:31 +0800 From: Shawn Guo To: Jagan Teki Cc: Jagan Teki , linux-arm-kernel , devicetree , linux-kernel , Sascha Hauer , Fabio Estevam , Matteo Lisi , Michael Trimarchi Subject: Re: [PATCH v3 1/9] ARM: dts: imx6ul-isiot: Add Sound card with codec node Message-ID: <20170615071529.GC5902@dragon> References: <1491501735-1649-1-git-send-email-jagan@openedev.com> <1491501735-1649-2-git-send-email-jagan@openedev.com> <20170407131649.GH16503@dragon> <20170615022024.GE30112@dragon> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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: 3503 Lines: 77 On Thu, Jun 15, 2017 at 10:21:43AM +0530, Jagan Teki wrote: > On Thu, Jun 15, 2017 at 7:50 AM, Shawn Guo wrote: > > On Wed, Jun 14, 2017 at 08:17:04PM +0530, Jagan Teki wrote: > >> On Fri, Apr 7, 2017 at 6:46 PM, Shawn Guo wrote: > >> > On Thu, Apr 06, 2017 at 11:32:07PM +0530, Jagan Teki wrote: > >> >> From: Jagan Teki > >> >> > >> >> Add support for Sound card and related codec(via i2c1) nodes > >> >> on Engicam Is.IoT MX6UL variant module boards. > >> >> > >> >> Cc: Shawn Guo > >> >> Cc: Matteo Lisi > >> >> Cc: Michael Trimarchi > >> >> Signed-off-by: Jagan Teki > >> >> --- > >> >> Changes for v3: > >> >> - Replace fsl,imx-audio-sgtl5000 and use simple-audio-card > >> >> Changes for v2: > >> >> - Use proper [label:] node-name[@unit-address] for codec > >> >> - Remove incorrect codec property 'wlf,shared-lrclk' > >> >> - Remove 'gpr' from sound card node > >> >> > >> >> arch/arm/boot/dts/imx6ul-isiot-common.dtsi | 10 +++++++ > >> >> arch/arm/boot/dts/imx6ul-isiot.dtsi | 44 ++++++++++++++++++++++++++++++ > >> > > >> > Can you help me understand how these two files are related? Why is > >> > sgtl5000 added into one and sound node added into the other? > >> > >> lcdif, ts and sound card which may differ based on the base-board > >> connected with SOM, So I moved these stuff which are related to > >> Starter kit supported once's and used with SOM dts files. if some > >> other board with same SOM can have different lcdif and etc so they can > >> define locally to dts. > > > > I do not follow how these stuff are organized. So far we have the > > following isiot dts files. > > > > - imx6ul-isiot-common.dtsi > > - imx6ul-isiot.dtsi > > - imx6ul-isiot-emmc.dts and imx6ul-isiot-nand.dts > > > > How are they mapping to SoM and base-board? > > isiot is a modules class, with that emmc and nand are two separate > SOM's. the current support is for mounting these SOM's on Development > base board[1]. So, for isiot module class we have imx6ul-isiot.dtsi > and emmc and nand SOM's have imx6ul-isiot-emmc.dts and > imx6ul-isiot-nand.dts. There are some Carrier boards[1] which were > used with different lcdif and other changes, So > imx6ul-isiot-common.dtsi have changes common across emmc and nand, > instead of adding them into individual dts files I moved in > -common.dtsi. So in future if isiot SOM mounted on carrier board > which should have a separate dts and which may or may not use > imx6ul-isiot-common.dtsi So you are not sure if imx6ul-isiot-common.dtsi will be used by carrier board. Then what's the point to have it now? You are saying imx6ul-isiot-common.dtsi is created to accommodate the common things across emmc and nand SoMs, but it actually contains LCD and Touch such base-board level of stuff. Confusing. I feel the abstraction is wrong from the beginning. Ideally, we should have something like below. - imx6ul-isiot.dtsi - imx6ul-isiot-kit.dts and imx6ul-isiot-carrier.dts The -isiot should have everything on SoM and common stuff between -kit and -carrier boards, while -kit and -carrier include -isiot and contains the base-board specific things. The -isiot can have both emmc and nand devices with "disabled" status, and let firmware turn device on per SoM it boots. In that case, the abstraction level can be less and clearer. Thoughts? Shawn