From patchwork Thu Mar 19 10:09:44 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wolfram Sang X-Patchwork-Id: 6048181 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 5B34FBF90F for ; Thu, 19 Mar 2015 10:16:03 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 7D5532051C for ; Thu, 19 Mar 2015 10:16:02 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7F1E020515 for ; Thu, 19 Mar 2015 10:16:01 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1YYXOp-0005qq-G1; Thu, 19 Mar 2015 10:09:59 +0000 Received: from sauhun.de ([89.238.76.85] helo=pokefinder.org) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1YYXOk-0005dO-Af for linux-arm-kernel@lists.infradead.org; Thu, 19 Mar 2015 10:09:55 +0000 Received: from p4fe24721.dip0.t-ipconnect.de ([79.226.71.33]:52133 helo=katana) by pokefinder.org with esmtpsa (TLS1.2:RSA_AES_128_CBC_SHA1:128) (Exim 4.80) (envelope-from ) id 1YYXOC-0003w3-Tu; Thu, 19 Mar 2015 11:09:21 +0100 Date: Thu, 19 Mar 2015 11:09:44 +0100 From: Wolfram Sang To: Sebastian Hesselbarth Subject: Re: [PATCH v3 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes Message-ID: <20150319100944.GA914@katana> References: <1425039885-5137-2-git-send-email-sebastian.hesselbarth@gmail.com> <1425903665-19343-1-git-send-email-sebastian.hesselbarth@gmail.com> <20150318123012.GA3580@katana> <55097C46.9010605@gmail.com> <20150318140037.GE3580@katana> <550A05E5.3050100@gmail.com> MIME-Version: 1.0 In-Reply-To: <550A05E5.3050100@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20150319_030954_567936_B9FEF820 X-CRM114-Status: GOOD ( 26.09 ) X-Spam-Score: -0.0 (/) Cc: Andrew Lunn , Jason Cooper , Stephen Warren , linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Gabriel Dobato , linux-i2c@vger.kernel.org, Gregory Clement , linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP > >>Possible. But this change just makes i2c-mux-pinctrl honor status > >>property at all. There is no functional change except it now allows > >>you to disable any of the sub-busses. > > > >Actually, this is the feature I like. However, I wonder if we shouldn't > >have that in the core, say in of_i2c_register_devices()? > > Hmm, looking at of_i2c_register_devices(): > > for_each_available_child_of_node(adap->dev.of_node, node) > of_i2c_register_device(adap, node); > > already honors status properties by using for_each_available_foo. > Therefore, i2c-core will also skip i2c device nodes disabled by > status property. Yes, but only child nodes, not the complete bus. Here is an RFC of what I mean: From: Wolfram Sang Subject: [RFC] i2c: of: always check if busses are enabled Allow all busses to have a "status" property which allows busses to not be probed when set to "disabled". Needed for DTS overlays with i2c mux scenarios. Signed-off-by: Wolfram Sang --- drivers/i2c/i2c-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) > I am not too deep into i2c-core, but AFAIKS i2c-mux-pinctrl is not an > i2c device but an i2c mux that is dealt with differently? It is not > probed with of_i2c_register_devices() but as a separate platform_device > with a reference to the parent i2c bus. Yes, but the busses have seperate nodes. Those nodes with the 'reg = <0>' properties. And those are matched with of_i2c_register_devices when the reg-property and the chan_id match. > About the memory allocation for the maximum potential number of muxes: > We would need some way to distinguish disabled from enabled muxes in > i2c-mux-pinctrl's platform_data. Do we? Can't we claim that every described bus needs a pinctrl entry. Still, the disabled busses won't be probed? So, we could also think about putting the above code snippet into i2c-mux when registering busses, so not only the childs will be skipped but also the whole bus will not be created. > i2c_mux_pinctrl_probe() is basically DT-agnostic and it should > definitely stay that way. Currently, each mux within pd->bus_count I agree. > requires a non-NULL pd->pinctrl_states[i] otherwise _probe() will bail > out for all sub-busses. And I think that makes sense. > (b) allow (and skip) muxes with pinctrl_states[i] == NULL for now and > let the "maniac" deal with storing/re-probing the corresponding > pinctrl_state name once it gets dynamically enabled. Why do you need the NULL? > I am still not too eager working on it but if you insist, I can see > what I can do as long as Stephen sticks with testing it on Tegra. ;) Please decide if you want to work on it. Remember, I am not short of patches to deal with. diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index fe80f85896e267..d9a3ad2149332e 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -1305,8 +1305,8 @@ static void of_i2c_register_devices(struct i2c_adapter *adap) { struct device_node *node; - /* Only register child devices if the adapter has a node pointer set */ - if (!adap->dev.of_node) + /* Only register childs if adapter has a node pointer with enabled status */ + if (!adap->dev.of_node || !of_device_is_available(adap->dev.of_node)) return; dev_dbg(&adap->dev, "of_i2c: walking child nodes\n");