Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752538AbbHTMOx (ORCPT ); Thu, 20 Aug 2015 08:14:53 -0400 Received: from smtp34.i.mail.ru ([94.100.177.94]:48091 "EHLO smtp34.i.mail.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751576AbbHTMOv (ORCPT ); Thu, 20 Aug 2015 08:14:51 -0400 Subject: Re: [PATCH v3 1/4] i2c: tegra: implement slave mode To: Wolfram Sang References: <1437424546-30405-1-git-send-email-danindrey@mail.ru> <1437424546-30405-2-git-send-email-danindrey@mail.ru> <20150724092720.GA1597@katana> <55B210F4.6030700@mail.ru> <20150724105251.GA22787@katana> Cc: Stephen Warren , devicetree@vger.kernel.org, devel@linuxdriverproject.org, linux-i2c@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org, ac100@lists.launchpad.net, Laxman Dewangan , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Russell King , Thierry Reding , Alexandre Courbot , Greg Kroah-Hartman , Julian Andres Klode , Marc Dietrich From: Andrey Danin Message-ID: <55D5C4AA.2000307@mail.ru> Date: Thu, 20 Aug 2015 15:14:34 +0300 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <20150724105251.GA22787@katana> Content-Type: multipart/mixed; boundary="------------010603080407080002090603" X-Mras: Ok Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4960 Lines: 145 This is a multi-part message in MIME format. --------------010603080407080002090603 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit On 24.07.2015 13:52, Wolfram Sang wrote: > >> At the begin of my work on this patchset I even denied clock disable call if >> slave is registered (to minimize code that can affect transfer). > > I hacked something like this, but it seems it was not enough. > >> If only slave mode is used, then this logic is not needed. > > This is not sufficent. We shouldn't break being a master only because we > also listen to a slave address (as long as the HW supports that of > course). > >> tegra_i2c_init is called on probe and resume. Also it is called in case of >> xfer fail. If xfer is ok, then I think slave addr must be kept unchanged. > > This is fragile. Try scanning the bus with i2cdetect and slave setup > will be gone. > >> As far as I understand it is a loopback mode. Probably it will not work >> (Stephen Warren already mentioned this). > > Just to make clear: I am not saying we should support talking to our own > slave address. But it should still be possible to communicate with other > remote devices on the bus. Sorry for the long delay. I tried to analyze the issue. Attached patch works on AC100 (Misha Komarovsky helped me with testing). Wolfram could you please try the patch with your environment? Thanks. --------------010603080407080002090603 Content-Type: text/plain; charset=UTF-8; name="0001-i2c-tegra-don-t-reset-I2C-slave-address-on-init.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0001-i2c-tegra-don-t-reset-I2C-slave-address-on-init.patch" >From 0927b4007786b19e51415c4900863dd4e74fa034 Mon Sep 17 00:00:00 2001 From: Andrey Danin Date: Thu, 20 Aug 2015 00:41:39 +0300 Subject: [PATCH] i2c: tegra: don't reset I2C slave address on init Init function is called multuple times. If I2C controller works in slave mode, then driver must keep slave registers otherwise slave configuration will be reseted. Signed-off-by: Andrey Danin --- drivers/i2c/busses/i2c-tegra.c | 42 +++++++++++++++++++++++++-------------- 1 files changed, 27 insertions(+), 15 deletions(-) diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index 6467ce0..50250a1 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -402,6 +402,22 @@ static void tegra_dvc_init(struct tegra_i2c_dev *i2c_dev) dvc_writel(i2c_dev, val, DVC_CTRL_REG1); } +static int tegra_i2c_init_slave(struct tegra_i2c_dev *i2c_dev, u32 addr, u32 flags) +{ + int addr2 = 0; + + i2c_writel(i2c_dev, I2C_SL_CNFG_NEWSL, I2C_SL_CNFG); + i2c_writel(i2c_dev, I2C_SL_DELAY_COUNT_DEFAULT, I2C_SL_DELAY_COUNT); + + if (flags & I2C_CLIENT_TEN) + addr2 = (addr >> 7) | I2C_SL_ADDR2_TEN_BIT_MODE; + + i2c_writel(i2c_dev, addr, I2C_SL_ADDR1); + i2c_writel(i2c_dev, addr2, I2C_SL_ADDR2); + + return 0; +} + static inline int tegra_i2c_clock_enable(struct tegra_i2c_dev *i2c_dev) { int ret; @@ -461,12 +477,16 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev) i2c_writel(i2c_dev, clk_divisor, I2C_CLK_DIVISOR); if (!i2c_dev->is_dvc) { - u32 sl_cfg = i2c_readl(i2c_dev, I2C_SL_CNFG); - sl_cfg |= I2C_SL_CNFG_NACK | I2C_SL_CNFG_NEWSL; - i2c_writel(i2c_dev, sl_cfg, I2C_SL_CNFG); - i2c_writel(i2c_dev, 0xfc, I2C_SL_ADDR1); - i2c_writel(i2c_dev, 0x00, I2C_SL_ADDR2); - + if (i2c_dev->slave) { + tegra_i2c_init_slave(i2c_dev, i2c_dev->slave->addr, + i2c_dev->slave->flags); + } else { + u32 sl_cfg = i2c_readl(i2c_dev, I2C_SL_CNFG); + sl_cfg |= I2C_SL_CNFG_NACK | I2C_SL_CNFG_NEWSL; + i2c_writel(i2c_dev, sl_cfg, I2C_SL_CNFG); + i2c_writel(i2c_dev, 0xfc, I2C_SL_ADDR1); + i2c_writel(i2c_dev, 0x00, I2C_SL_ADDR2); + } } val = 7 << I2C_FIFO_CONTROL_TX_TRIG_SHIFT | @@ -767,7 +787,6 @@ static u32 tegra_i2c_func(struct i2c_adapter *adap) static int tegra_reg_slave(struct i2c_client *slave) { struct tegra_i2c_dev *i2c_dev = i2c_get_adapdata(slave->adapter); - int addr2 = 0; if (i2c_dev->slave) return -EBUSY; @@ -776,14 +795,7 @@ static int tegra_reg_slave(struct i2c_client *slave) tegra_i2c_clock_enable(i2c_dev); - i2c_writel(i2c_dev, I2C_SL_CNFG_NEWSL, I2C_SL_CNFG); - i2c_writel(i2c_dev, I2C_SL_DELAY_COUNT_DEFAULT, I2C_SL_DELAY_COUNT); - - if (slave->flags & I2C_CLIENT_TEN) - addr2 = (slave->addr >> 7) | I2C_SL_ADDR2_TEN_BIT_MODE; - - i2c_writel(i2c_dev, slave->addr, I2C_SL_ADDR1); - i2c_writel(i2c_dev, addr2, I2C_SL_ADDR2); + tegra_i2c_init_slave(i2c_dev, slave->addr, slave->flags); return 0; } -- 1.7.1 --------------010603080407080002090603-- -- 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/