From patchwork Thu Jun 16 01:26:03 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Horman X-Patchwork-Id: 9179747 X-Patchwork-Delegate: horms@verge.net.au Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 23A6560573 for ; Thu, 16 Jun 2016 01:26:11 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 08E7627F3E for ; Thu, 16 Jun 2016 01:26:11 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id EF4E827F54; Thu, 16 Jun 2016 01:26:10 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 41B1527F3E for ; Thu, 16 Jun 2016 01:26:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933127AbcFPB0J (ORCPT ); Wed, 15 Jun 2016 21:26:09 -0400 Received: from kirsty.vergenet.net ([202.4.237.240]:33502 "EHLO kirsty.vergenet.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933096AbcFPB0I (ORCPT ); Wed, 15 Jun 2016 21:26:08 -0400 Received: from reginn.isobedori.kobe.vergenet.net (p6216-ipbfp1501kobeminato.hyogo.ocn.ne.jp [114.153.217.216]) by kirsty.vergenet.net (Postfix) with ESMTPA id 0036A25B77B; Thu, 16 Jun 2016 11:26:05 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=verge.net.au; s=mail; t=1466040365; bh=hY2O7wgQ1POOg+kc0Gte7I12rSgPcRNQK//mioTyPsg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=JW3t0srgRa6jS2rE5yTO73YIQleX/6TLMHlIHuP/3/tqV5PJClTKn7bLzesSHoup7 8GSYm4/OQIRtqb1I4L4/bBKtlI/O/q37VQ/V26feC/UqYPHfuXQ4OKLC3f64WFT3p9 nInn1IyPrsIc7c1O/aIzUxywKLkk+W4/UKonHdU0= Received: by reginn.isobedori.kobe.vergenet.net (Postfix, from userid 7100) id 57D2F9401DB; Thu, 16 Jun 2016 10:26:03 +0900 (JST) Date: Thu, 16 Jun 2016 10:26:03 +0900 From: Simon Horman To: Wolfram Sang Cc: linux-renesas-soc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Magnus Damm , Wolfram Sang Subject: Re: [PATCH v2 0/4] ARM: dts: r8a7790: lager: use demuxer for I2C Message-ID: <20160616012600.GA4745@verge.net.au> References: <1465370066-8450-1-git-send-email-horms+renesas@verge.net.au> <20160613143134.GB1603@katana> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20160613143134.GB1603@katana> Organisation: Horms Solutions Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-renesas-soc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-renesas-soc@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, Jun 13, 2016 at 04:31:34PM +0200, Wolfram Sang wrote: > > > # ./exercise-i2c-demux.sh > > I2C Demux: i2c-8 > > Master: 1:/i2c@e650800 > > [ 97.694487] i2c-rcar e6508000.i2c: probed 0 (1) > > Master: 0:/i2c@e650000 > > [ 102.706365] i2c-demux-pinctrl i2c-8: failed to setup demux-adapter 0 (-19) 0 (0) > > Confirmed :( Got today only this far that the ENODEV comes from the PFC > driver. CONFIG_DEBUG_PINCTRL is a good idea for further debugging > probably. I tried to track this down and my findings so far are a bit puzzling. It seems to me that the first and only item of the "pinctrl-names" property (a.k.a. statename) retrieved for i2c@e6508000 changes from i2c-exio0 to i2c-exio1. Perhaps it really is overwritten for some reason. Or perhaps there is a stale pointer. Or perhaps I am chasing the wrong thing. But in any case my observation is illustrated using the debug patch below. I am testing using only the first two patches of the series applied on top of renesas-devel-20160614-v4.7-rc3, that is the only demux devices are i2c-exio0 and i2c-exio1. I am using shmobile_defconfig, with CONFIG_DEBUG_PINCTRL enabled. I am examining only i2c-8, i2c-exio0. 1. At probe time the first parent, 0:/i2c@e6500000 i2c-demux-pinctrl i2c-8: i2c_demux_activate_master chan:0 bus_name:i2c-exio0 np->full_name/i2c@e6500000 np:ef7db76c statename:i2c-exio0 i2c-demux-pinctrl i2c-8: i2c_demux_activate_master chan:1 bus_name:i2c-exio0 np->full_name/i2c@e6508000 np:ef7dabfc statename:i2c-exio0 2. At run time I select the second parent, 1:/i2c@e6508000. This works. But the statename of i2c@e6500000 is already i2c-exio1. i2c-demux-pinctrl i2c-8: i2c_demux_deactivate_master chan:0 bus_name:i2c-exio0 np->full_name:/i2c@e6500000 np:ef7db76c statename:i2c-exio1 i2c-demux-pinctrl i2c-8: i2c_demux_deactivate_master chan:1 bus_name:i2c-exio0 np->full_name:/i2c@e6508000 np:ef7dabfc statename:i2c-exio0 i2c-demux-pinctrl i2c-8: i2c_demux_activate_master chan:0 bus_name:i2c-exio0 np->full_name:/i2c@e6500000 np:ef7db76c statename:i2c-exio1 i2c-demux-pinctrl i2c-8: i2c_demux_activate_master chan:1 bus_name:i2c-exio0 np->full_name:/i2c@e6508000 np:ef7dabfc statename:i2c-exio0 3. At run time I re-select the first parent, 0:/i2c@e6500000. This fails as described above. The new debug code shows. i2c-demux-pinctrl i2c-8: i2c_demux_deactivate_master chan:0 bus_name:i2c-exio0 np->full_name:/i2c@e6500000 np:ef7db76c statename:i2c-exio1 34.700444] i2c-demux-pinctrl i2c-8: i2c_demux_deactivate_master chan:1 bus_name:i2c-exio0 np->full_name:/i2c@e6508000 np:ef7dabfc statename:i2c-exio0 [i2c_demux_deactivate_master exits before reaching debug code, but I have previously observed that statename is i2c-exio1 for i2c@e6500000] > > 3.1 Unfortunately it fails for the I2C2 dmux if the GPIO fallback is > > present. > > Hopefully the same issue. Hopefully :) > > 3.2 The test also fails for the I2C3 demux but that appears to be due > > to a shortcoming in the voltage regulator code which does not > > appear to like being reinitialised. The kernel complains as follows: > > Yes, the demuxer trigger re-bind cycles which are not well considered > and/or tested. E.g. for soc-camera, this patch needed to go upstream: > > https://patchwork.linuxtv.org/patch/32473/ Understood. It looks like there is some more work to be done but from my point of view fixing individual drivers is less critical than resolving the switching issue(s) above. I think the order things are described is probably the order in which they should be addressed. diff --git a/drivers/i2c/muxes/i2c-demux-pinctrl.c b/drivers/i2c/muxes/i2c-demux-pinctrl.c index 8de073aed001..8a54380a9e0c 100644 --- a/drivers/i2c/muxes/i2c-demux-pinctrl.c +++ b/drivers/i2c/muxes/i2c-demux-pinctrl.c @@ -55,6 +55,27 @@ static u32 i2c_demux_functionality(struct i2c_adapter *adap) return parent->algo->functionality(parent); } +static int dbg_show(const char *prefix, struct i2c_demux_pinctrl_priv *priv) +{ + int i; + + for (i = 0; i < priv->num_chan; i++) { + struct device_node *np; + const char *statename; + int ret; + + np = priv->chan[i].parent_np; + + ret = of_property_read_string_index(np, "pinctrl-names", + 0, &statename); + if (ret) + statename = ""; + dev_err(priv->dev, "%s chan:%d bus_name:%s np->full_name:%s " + "np:%p statename:%s\n", prefix, i, priv->bus_name, + np->full_name, np, statename); + } +} + static int i2c_demux_activate_master(struct i2c_demux_pinctrl_priv *priv, u32 new_chan) { struct i2c_adapter *adap; @@ -99,6 +120,7 @@ static int i2c_demux_activate_master(struct i2c_demux_pinctrl_priv *priv, u32 ne if (ret < 0) goto err_with_put; + dbg_show(__func__, priv); return 0; err_with_put: @@ -115,6 +137,8 @@ static int i2c_demux_deactivate_master(struct i2c_demux_pinctrl_priv *priv) if (cur < 0) return 0; + dbg_show(__func__, priv); + i2c_del_adapter(&priv->cur_adap); i2c_put_adapter(priv->chan[cur].parent_adap);