Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754016AbbFOGN0 (ORCPT ); Mon, 15 Jun 2015 02:13:26 -0400 Received: from metis.ext.pengutronix.de ([92.198.50.35]:54666 "EHLO metis.ext.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753967AbbFOGNG (ORCPT ); Mon, 15 Jun 2015 02:13:06 -0400 Date: Mon, 15 Jun 2015 08:12:56 +0200 From: Sascha Hauer To: Daniel Kurtz Cc: Eddie Huang , Matthias Brugger , "open list:OPEN FIRMWARE AND..." , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , linux-mediatek@lists.infradead.org, Linus Walleij Subject: Re: [PATCH v3 2/2] arm64: dts: mt8173: Add I2C device node Message-ID: <20150615061256.GM6325@pengutronix.de> References: <1434101229-32695-1-git-send-email-eddie.huang@mediatek.com> <1434101229-32695-3-git-send-email-eddie.huang@mediatek.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Sent-From: Pengutronix Hildesheim X-URL: http://www.pengutronix.de/ X-IRC: #ptxdist @freenode X-Accept-Language: de,en X-Accept-Content-Type: text/plain X-Uptime: 07:46:02 up 90 days, 17:37, 99 users, load average: 0.14, 0.09, 0.09 User-Agent: Mutt/1.5.21 (2010-09-15) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c0 X-SA-Exim-Mail-From: sha@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3949 Lines: 87 On Fri, Jun 12, 2015 at 08:28:51PM +0800, Daniel Kurtz wrote: > Hi Eddie, > > On Fri, Jun 12, 2015 at 5:27 PM, Eddie Huang wrote: > > > > Add MT8173 I2C device nodes, include I2C controllers and pins. > > MT8173 has six I2C controllers, from i2c0 to i2c6, exclude i2c5. > > The 6th I2C controller register base doesn't next to 5th I2C, > > and there is a hardware between 5th and 6th I2C controller. So > > SoC designer name 6th controller as "i2c6", not "i2c5". > > > This is slightly misleading. There are in fact 7 I2C controllers, but > "i2c5" is dedicated for use by HDMI for ddc & hdcp. > Is there a reason why the HDMI I2C port cannot be controlled by the > generic i2c driver? > > Of course the hdmiddc / i2c5 node can always be added in a later > patch, so this is no reason to hold up this patch. > > > Signed-off-by: Eddie Huang > > --- > > arch/arm64/boot/dts/mediatek/mt8173-evb.dts | 50 ++++++++++++++++++++ > > arch/arm64/boot/dts/mediatek/mt8173.dtsi | 72 +++++++++++++++++++++++++++++ > > 2 files changed, 122 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts > > index 43d5401..2e01988 100644 > > --- a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts > > +++ b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts > > @@ -33,6 +33,56 @@ > > chosen { }; > > }; > > > > +&pio { > > I don't think we needed to move these i2c pinmux descriptions from > mt8173.dtsi to the board .dts file. > > AFAICT, the "i2cN_pins_a" nodes defined here are the pinctl > configuration that the corresponding enabled i2c nodes would choose. > Thus, they are generic to the MT8173 SoC, not specific to any board. > By themselves, these nodes do not actually select a pin configuration. On i.MX we started with i2cN_pins_[abcde] groups in the SoC dtsi file aswell. At some point we realized that this does not scale anymore. The problems were: - Whenever a new board came along which needed some pin setting which didn't already exist a new group was created in the SoC dtsi file using the next free letter from the alphabet. The ordering of the group names was rather arbitrary and often enough there were merge conflicts to resolve when during one merge window two new boards showed up which both added i2cN_pin_x with different content. - When a new board needs the same pins but with different drive strength settings a new group was required - With SD/MMC we had groups for 4bit data, groups adding the remaining pins for 8bit data - With UARTs we had groups for RX/TX and additionally groups adding RTS/CTS - For graphics we had groups for 16bit data and 24bit data - There is no way to remove the unused nodes from the binary dtb, so every board contained all existing pingroups for every other board, so the dtbs became quite big So with only a few existing boards with very similar pinmux groups it may work fine to put the groups into the SoC dtsi, but this way may also explode quite fast with more and different boards. I don't know how much variation there will be with Mediatek boards, so I'm fine with either way. You may want to look at: 5a2a7d5 ARM: dts: imx51: make pinctrl nodes board specific 7ac0f70 ARM: dts: imx53: make pinctrl nodes board specific 817c27a ARM: dts: imx6qdl: make pinctrl nodes board specific Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/