[{"id":1769504,"web_url":"http://patchwork.ozlabs.org/comment/1769504/","msgid":"<9d3e3029-f775-a9aa-8025-badccb699ed8@ti.com>","list_archive_url":null,"date":"2017-09-15T23:31:51","subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","submitter":{"id":25084,"url":"http://patchwork.ozlabs.org/api/people/25084/","name":"Grygorii Strashko","email":"grygorii.strashko@ti.com"},"content":"On 09/14/2017 10:39 AM, Claudio Foellmi wrote:\n> A very conservative check for bus activity (to prevent interference\n> in multimaster setups) prevented the bus recovery methods from being\n> triggered in the case that SDA or SCL was stuck low.\n> This defeats the purpose of the recovery mechanism, which was introduced\n> for exactly this situation (a slave device keeping SDA pulled down).\n> \n> Note that bus lockups can persist across reboots. The only other options\n> are to reset or power cycle the offending slave device, and many i2c\n> slaves do not even have a reset pin.\n> \n> If we see that one of the lines is low for the entire timeout duration,\n> we can actually be sure that there is no other master driving the bus.\n> It is therefore save for us to attempt a bus recovery.\n> \n\nReviewed-by: Grygorii Strashko \n\n> Signed-off-by: Claudio Foellmi \n> ---\n> Caveat: It turns out I don't have the hardware to fully test the\n> recovery mechanism. My faulty i2c slave device actually pulls down SCL,\n> not SDA (so the recovery will not succeed in my case).\n> But by directly connecting SDA to ground, I could at least make sure\n> the recovery function gets called after applying this patch.\n> \n> drivers/i2c/busses/i2c-omap.c | 7 ++++++-\n> 1 file changed, 6 insertions(+), 1 deletion(-)\n> \n> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c\n> index 1ebb5e9..4b25fd1 100644\n> --- a/drivers/i2c/busses/i2c-omap.c\n> +++ b/drivers/i2c/busses/i2c-omap.c\n> @@ -563,8 +563,13 @@ static int omap_i2c_wait_for_bb_valid(struct omap_i2c_dev *omap)\n> \t\t}\n> \n> \t\tif (time_after(jiffies, timeout)) {\n> +\t\t\t/*\n> +\t\t\t * SDA or SCL were low for the entire timeout without\n> +\t\t\t * any activity detected. Most likely, a slave is\n> +\t\t\t * locking up the bus with no master driving the clock.\n> +\t\t\t */\n> \t\t\tdev_warn(omap->dev, \"timeout waiting for bus ready\\n\");\n> -\t\t\treturn -ETIMEDOUT;\n> +\t\t\treturn i2c_recover_bus(&omap->adapter);\n> \t\t}\n> \n> \t\tmsleep(1);\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=ti.com header.i=@ti.com header.b=\"BdegX+QV\";\n\tdkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xvBWg2qvZz9sPr\n\tfor ;\n\tSat, 16 Sep 2017 09:32:15 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751812AbdIOXcM (ORCPT );\n\tFri, 15 Sep 2017 19:32:12 -0400","from lelnx194.ext.ti.com ([198.47.27.80]:51664 \"EHLO\n\tlelnx194.ext.ti.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751807AbdIOXcL (ORCPT\n\t); Fri, 15 Sep 2017 19:32:11 -0400","from dflxv15.itg.ti.com ([128.247.5.124])\n\tby lelnx194.ext.ti.com (8.15.1/8.15.1) with ESMTP id v8FNVqIr024847; \n\tFri, 15 Sep 2017 18:31:52 -0500","from DLEE70.ent.ti.com (dlemailx.itg.ti.com [157.170.170.113])\n\tby dflxv15.itg.ti.com (8.14.3/8.13.8) with ESMTP id v8FNVqG3011994;\n\tFri, 15 Sep 2017 18:31:52 -0500","from [128.247.59.147] (128.247.59.147) by DLEE70.ent.ti.com\n\t(157.170.170.113) with Microsoft SMTP Server id 14.3.294.0;\n\tFri, 15 Sep 2017 18:31:51 -0500"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ti.com;\n\ts=ti-com-17Q1; t=1505518312;\n\tbh=Im9HqqpOVLnXoWUVKxkP5HDc/Rm5ZkU15WbZqx27usk=;\n\th=Subject:To:CC:References:From:Date:In-Reply-To;\n\tb=BdegX+QVoIF5zMNyKDPUu2mTkA0b4ydun8PgjOqeqTfuaEshMl7sABhzY53B5qtPz\n\tLxWZWyc2vme8P3DMbofxJGPdfFYDvSik0XC0Ogy3NPut6E4OoxJZescMYNQfwZSnbb\n\tEa1sfTzyxV7W8nJ3efVawY5V4xKw85xX1+RvgrxQ=","Subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","To":"Claudio Foellmi ,\n\tTony Lindgren , Sekhar Nori ,\n\tFranklin S Cooper Jr ","CC":"Aaro Koskinen ,\n\tWolfram Sang , ,\n\t","References":"<1505403549-12992-1-git-send-email-claudio.foellmi@ergon.ch>","From":"Grygorii Strashko ","Message-ID":"<9d3e3029-f775-a9aa-8025-badccb699ed8@ti.com>","Date":"Fri, 15 Sep 2017 18:31:51 -0500","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<1505403549-12992-1-git-send-email-claudio.foellmi@ergon.ch>","Content-Type":"text/plain; charset=\"utf-8\"; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Originating-IP":"[128.247.59.147]","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1769870,"web_url":"http://patchwork.ozlabs.org/comment/1769870/","msgid":"<48ddc9bc-2b83-97c9-fbd7-90d4da9f2687@ti.com>","list_archive_url":null,"date":"2017-09-18T05:24:06","subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","submitter":{"id":65039,"url":"http://patchwork.ozlabs.org/api/people/65039/","name":"Raghavendra, Vignesh","email":"vigneshr@ti.com"},"content":"On Saturday 16 September 2017 05:01 AM, Strashko, Grygorii wrote:\n> \n> \n> On 09/14/2017 10:39 AM, Claudio Foellmi wrote:\n>> A very conservative check for bus activity (to prevent interference\n>> in multimaster setups) prevented the bus recovery methods from being\n>> triggered in the case that SDA or SCL was stuck low.\n>> This defeats the purpose of the recovery mechanism, which was introduced\n>> for exactly this situation (a slave device keeping SDA pulled down).\n>>\n>> Note that bus lockups can persist across reboots. The only other options\n>> are to reset or power cycle the offending slave device, and many i2c\n>> slaves do not even have a reset pin.\n>>\n>> If we see that one of the lines is low for the entire timeout duration,\n>> we can actually be sure that there is no other master driving the bus.\n>> It is therefore save for us to attempt a bus recovery.\n>>\n> \n> Reviewed-by: Grygorii Strashko \n> \n>> Signed-off-by: Claudio Foellmi \n>> ---\n>> Caveat: It turns out I don't have the hardware to fully test the\n>> recovery mechanism. My faulty i2c slave device actually pulls down SCL,\n>> not SDA (so the recovery will not succeed in my case).\n\nMaybe, you could detect SCL stuck low case by reading status of SCL line\nfrom OMAP_I2C_SYSTEST_REG and then call IP reset (there is nothing much\nthat can be done) instead of bus recovery.\n\n>> But by directly connecting SDA to ground, I could at least make sure\n>> the recovery function gets called after applying this patch.\n>>\n\nI had seen flood of XRDY & RRDY interrupts as soon as TMODE is set to\n0x3 as part of omap_i2c_prepare_recovery() leading to unusable system.\nDid you observe this behavior on your system? Could you mention the\nplatform on which this experiment done?\n\n>> drivers/i2c/busses/i2c-omap.c | 7 ++++++-\n>> 1 file changed, 6 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c\n>> index 1ebb5e9..4b25fd1 100644\n>> --- a/drivers/i2c/busses/i2c-omap.c\n>> +++ b/drivers/i2c/busses/i2c-omap.c\n>> @@ -563,8 +563,13 @@ static int omap_i2c_wait_for_bb_valid(struct omap_i2c_dev *omap)\n>> \t\t}\n>> \n>> \t\tif (time_after(jiffies, timeout)) {\n>> +\t\t\t/*\n>> +\t\t\t * SDA or SCL were low for the entire timeout without\n>> +\t\t\t * any activity detected. Most likely, a slave is\n>> +\t\t\t * locking up the bus with no master driving the clock.\n>> +\t\t\t */\n>> \t\t\tdev_warn(omap->dev, \"timeout waiting for bus ready\\n\");\n>> -\t\t\treturn -ETIMEDOUT;\n>> +\t\t\treturn i2c_recover_bus(&omap->adapter);\n>> \t\t}\n>> \n>> \t\tmsleep(1);\n>>\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=ti.com header.i=@ti.com header.b=\"O3VTgnqi\";\n\tdkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xwZDr4kz3z9s4s\n\tfor ;\n\tMon, 18 Sep 2017 15:24:12 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1750828AbdIRFYL (ORCPT );\n\tMon, 18 Sep 2017 01:24:11 -0400","from lelnx193.ext.ti.com ([198.47.27.77]:59613 \"EHLO\n\tlelnx193.ext.ti.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750757AbdIRFYK (ORCPT\n\t); Mon, 18 Sep 2017 01:24:10 -0400","from dlelxv90.itg.ti.com ([172.17.2.17])\n\tby lelnx193.ext.ti.com (8.15.1/8.15.1) with ESMTP id v8I5Nlaf022144; \n\tMon, 18 Sep 2017 00:23:47 -0500","from DLEE108.ent.ti.com (dlee108.ent.ti.com [157.170.170.38])\n\tby dlelxv90.itg.ti.com (8.14.3/8.13.8) with ESMTP id v8I5NlBW005655; \n\tMon, 18 Sep 2017 00:23:47 -0500","from DLEE115.ent.ti.com (157.170.170.26) by DLEE108.ent.ti.com\n\t(157.170.170.38) with Microsoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.1.845.34;\n\tMon, 18 Sep 2017 00:23:47 -0500","from dlep33.itg.ti.com (157.170.170.75) by DLEE115.ent.ti.com\n\t(157.170.170.26) with Microsoft SMTP Server (version=TLS1_0,\n\tcipher=TLS_RSA_WITH_AES_256_CBC_SHA) id 15.1.845.34 via Frontend\n\tTransport; Mon, 18 Sep 2017 00:23:47 -0500","from [172.24.190.89] (ileax41-snat.itg.ti.com [10.172.224.153])\n\tby dlep33.itg.ti.com (8.14.3/8.13.8) with ESMTP id v8I5Nhd3030053;\n\tMon, 18 Sep 2017 00:23:44 -0500"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ti.com;\n\ts=ti-com-17Q1; t=1505712227;\n\tbh=u9l/PUtsfppkqJLKJDY4CRqFOmsbpInFSlJjAPu2A+Q=;\n\th=Subject:To:CC:References:From:Date:In-Reply-To;\n\tb=O3VTgnqiXDbAt4MXKkvfz6jiUNC6ExeUKw78dEg25ymt7/XYTYcc74jaOfd03BkUB\n\t3v5ARkNxlENl95AHARci8KTiXOt4IKKmUblt5DFDi2wYtZGVOrhkYh5d8EIKZJ1Rxo\n\tnQuuE09t5wRydcibQFRNAltfnFZ/wPH+TYhUjxPs=","Subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","To":"\"Strashko, Grygorii\" ,\n\tClaudio Foellmi ,\n\tTony Lindgren , \"Nori, Sekhar\" ,\n\t\"Cooper Jr., Franklin\" ","CC":"Aaro Koskinen , Wolfram Sang ,\n\t\"linux-omap@vger.kernel.org\" ,\n\t\"linux-i2c@vger.kernel.org\" ","References":"<1505403549-12992-1-git-send-email-claudio.foellmi@ergon.ch>\n\t<9d3e3029-f775-a9aa-8025-badccb699ed8@ti.com>","From":"Vignesh R ","Message-ID":"<48ddc9bc-2b83-97c9-fbd7-90d4da9f2687@ti.com>","Date":"Mon, 18 Sep 2017 10:54:06 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<9d3e3029-f775-a9aa-8025-badccb699ed8@ti.com>","Content-Type":"text/plain; charset=\"utf-8\"","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-EXCLAIMER-MD-CONFIG":"e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1770105,"web_url":"http://patchwork.ozlabs.org/comment/1770105/","msgid":"<0a506277-6c9b-1407-dea3-f9895fdc3e63@ergon.ch>","list_archive_url":null,"date":"2017-09-18T12:01:27","subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","submitter":{"id":72366,"url":"http://patchwork.ozlabs.org/api/people/72366/","name":"Claudio Foellmi","email":"claudio.foellmi@ergon.ch"},"content":"On 18.09.2017 07:24, Vignesh R wrote:\n> \n> \n> On Saturday 16 September 2017 05:01 AM, Strashko, Grygorii wrote:\n>>\n>>\n>> On 09/14/2017 10:39 AM, Claudio Foellmi wrote:\n>>> A very conservative check for bus activity (to prevent interference\n>>> in multimaster setups) prevented the bus recovery methods from being\n>>> triggered in the case that SDA or SCL was stuck low.\n>>> This defeats the purpose of the recovery mechanism, which was introduced\n>>> for exactly this situation (a slave device keeping SDA pulled down).\n>>>\n>>> Note that bus lockups can persist across reboots. The only other options\n>>> are to reset or power cycle the offending slave device, and many i2c\n>>> slaves do not even have a reset pin.\n>>>\n>>> If we see that one of the lines is low for the entire timeout duration,\n>>> we can actually be sure that there is no other master driving the bus.\n>>> It is therefore save for us to attempt a bus recovery.\n>>>\n>>\n>> Reviewed-by: Grygorii Strashko \n>>\n>>> Signed-off-by: Claudio Foellmi \n>>> ---\n>>> Caveat: It turns out I don't have the hardware to fully test the\n>>> recovery mechanism. My faulty i2c slave device actually pulls down SCL,\n>>> not SDA (so the recovery will not succeed in my case).\n> \n> Maybe, you could detect SCL stuck low case by reading status of SCL line\n> from OMAP_I2C_SYSTEST_REG and then call IP reset (there is nothing much\n> that can be done) instead of bus recovery.\n> \n\nI plan on posting a related patch soon, that will print better messages\nif the generic recovery fails. If SCL is stuck low, I think the best we\ncan do is make the problem visible in the kernel log.\n\n>>> But by directly connecting SDA to ground, I could at least make sure\n>>> the recovery function gets called after applying this patch.\n>>>\n> \n> I had seen flood of XRDY & RRDY interrupts as soon as TMODE is set to\n> 0x3 as part of omap_i2c_prepare_recovery() leading to unusable system.\n> Did you observe this behavior on your system? Could you mention the\n> platform on which this experiment done?\n> \n\nSo attempting bus recovery is dangerous on some platforms?\nI did not notice any obvious problems (assuming an 'unusable system'\nwould be hard to miss), but then again I only have one target to test on.\nI'm working with a TI AM3352, the slave is a NXP NT3H2211 on i2c-1.\n\n>>> drivers/i2c/busses/i2c-omap.c | 7 ++++++-\n>>> 1 file changed, 6 insertions(+), 1 deletion(-)\n>>>\n>>> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c\n>>> index 1ebb5e9..4b25fd1 100644\n>>> --- a/drivers/i2c/busses/i2c-omap.c\n>>> +++ b/drivers/i2c/busses/i2c-omap.c\n>>> @@ -563,8 +563,13 @@ static int omap_i2c_wait_for_bb_valid(struct omap_i2c_dev *omap)\n>>> \t\t}\n>>> \n>>> \t\tif (time_after(jiffies, timeout)) {\n>>> +\t\t\t/*\n>>> +\t\t\t * SDA or SCL were low for the entire timeout without\n>>> +\t\t\t * any activity detected. Most likely, a slave is\n>>> +\t\t\t * locking up the bus with no master driving the clock.\n>>> +\t\t\t */\n>>> \t\t\tdev_warn(omap->dev, \"timeout waiting for bus ready\\n\");\n>>> -\t\t\treturn -ETIMEDOUT;\n>>> +\t\t\treturn i2c_recover_bus(&omap->adapter);\n>>> \t\t}\n>>> \n>>> \t\tmsleep(1);\n>>>\n>>\n> \n\n--\nRegards\nClaudio","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xwl3b4ZQpz9s78\n\tfor ;\n\tMon, 18 Sep 2017 22:01:47 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1754130AbdIRMBp (ORCPT );\n\tMon, 18 Sep 2017 08:01:45 -0400","from mx1.ergon.ch ([87.239.214.68]:49211 \"EHLO mx1.ergon.ch\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1753887AbdIRMBp (ORCPT );\n\tMon, 18 Sep 2017 08:01:45 -0400","from mahler.ergon.ch ([87.239.208.18])\n\tby mx1.ergon.ch with ESMTP; 18 Sep 2017 14:01:29 +0200","from [87.239.211.231] (crespo.ergon.ch [87.239.211.231])\n\tby mahler.ergon.ch (Postfix) with ESMTP id 15A8DEBC5;\n\tMon, 18 Sep 2017 14:01:27 +0200 (MEST)"],"X-IronPort-AV":"E=Sophos;i=\"5.42,412,1500933600\"; d=\"scan'208\";a=\"2414693\"","Subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","To":"Vignesh R ,\n\t\"Strashko, Grygorii\" ,\n\tTony Lindgren , \"Nori, Sekhar\" ,\n\t\"Cooper Jr., Franklin\" ","Cc":"Aaro Koskinen , Wolfram Sang ,\n\t\"linux-omap@vger.kernel.org\" ,\n\t\"linux-i2c@vger.kernel.org\" ","References":"<1505403549-12992-1-git-send-email-claudio.foellmi@ergon.ch>\n\t<9d3e3029-f775-a9aa-8025-badccb699ed8@ti.com>\n\t<48ddc9bc-2b83-97c9-fbd7-90d4da9f2687@ti.com>","From":"Claudio Foellmi ","Message-ID":"<0a506277-6c9b-1407-dea3-f9895fdc3e63@ergon.ch>","Date":"Mon, 18 Sep 2017 14:01:27 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<48ddc9bc-2b83-97c9-fbd7-90d4da9f2687@ti.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1770856,"web_url":"http://patchwork.ozlabs.org/comment/1770856/","msgid":"","list_archive_url":null,"date":"2017-09-19T10:50:49","subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","submitter":{"id":65039,"url":"http://patchwork.ozlabs.org/api/people/65039/","name":"Raghavendra, Vignesh","email":"vigneshr@ti.com"},"content":"On Monday 18 September 2017 05:31 PM, Claudio Foellmi wrote:\n> On 18.09.2017 07:24, Vignesh R wrote:\n>>\n>>\n>> On Saturday 16 September 2017 05:01 AM, Strashko, Grygorii wrote:\n>>>\n>>>\n>>> On 09/14/2017 10:39 AM, Claudio Foellmi wrote:\n>>>> A very conservative check for bus activity (to prevent interference\n>>>> in multimaster setups) prevented the bus recovery methods from being\n>>>> triggered in the case that SDA or SCL was stuck low.\n>>>> This defeats the purpose of the recovery mechanism, which was introduced\n>>>> for exactly this situation (a slave device keeping SDA pulled down).\n>>>>\n>>>> Note that bus lockups can persist across reboots. The only other options\n>>>> are to reset or power cycle the offending slave device, and many i2c\n>>>> slaves do not even have a reset pin.\n>>>>\n>>>> If we see that one of the lines is low for the entire timeout duration,\n>>>> we can actually be sure that there is no other master driving the bus.\n>>>> It is therefore save for us to attempt a bus recovery.\n>>>>\n>>>\n>>> Reviewed-by: Grygorii Strashko \n>>>\n>>>> Signed-off-by: Claudio Foellmi \n>>>> ---\n>>>> Caveat: It turns out I don't have the hardware to fully test the\n>>>> recovery mechanism. My faulty i2c slave device actually pulls down SCL,\n>>>> not SDA (so the recovery will not succeed in my case).\n>>\n>> Maybe, you could detect SCL stuck low case by reading status of SCL line\n>> from OMAP_I2C_SYSTEST_REG and then call IP reset (there is nothing much\n>> that can be done) instead of bus recovery.\n>>\n> \n> I plan on posting a related patch soon, that will print better messages\n> if the generic recovery fails. If SCL is stuck low, I think the best we\n> can do is make the problem visible in the kernel log.\n> \n>>>> But by directly connecting SDA to ground, I could at least make sure\n>>>> the recovery function gets called after applying this patch.\n>>>>\n>>\n>> I had seen flood of XRDY & RRDY interrupts as soon as TMODE is set to\n>> 0x3 as part of omap_i2c_prepare_recovery() leading to unusable system.\n>> Did you observe this behavior on your system? Could you mention the\n>> platform on which this experiment done?\n>>\n> \n> So attempting bus recovery is dangerous on some platforms?\n> I did not notice any obvious problems (assuming an 'unusable system'\n> would be hard to miss), but then again I only have one target to test on.\n> I'm working with a TI AM3352, the slave is a NXP NT3H2211 on i2c-1.\n> \n\nI hit a situation where when communicating with a faulty i2c device, the\nlast transaction on the bus does not end with proper STOP condition on\nthe i2c bus. Since, STOP condition was not detected by IP, Bus Busy will\nremain set even though both SCL and SDA are high. Thus,\nomap_i2c_wait_for_bb() function would end up calling bus recovery. And\nas soon as TMODE is set to 0x3 and ST_EN to 0x1, there is a flood of\nXRDY & RRDY interrupts.\n\nThis spurious irqs can be reproduced easily by setting TMODE to 0x3 and\nST_EN to 0x1 in OMAP_I2C_SYSTEST_REG when both SCL and SDA are high (bus\nis idle) even on AM335x.\n\nSo, if you are not seeing irq flood when SCL/SDA is stuck low, then\nmaybe its safe to enter TMODE 0x3 in such cases. Could you modify the\npatch to test whether or not SDA is stuck low before initiating bus\nrecovery?\n\n\n>>>> drivers/i2c/busses/i2c-omap.c | 7 ++++++-\n>>>> 1 file changed, 6 insertions(+), 1 deletion(-)\n>>>>\n>>>> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c\n>>>> index 1ebb5e9..4b25fd1 100644\n>>>> --- a/drivers/i2c/busses/i2c-omap.c\n>>>> +++ b/drivers/i2c/busses/i2c-omap.c\n>>>> @@ -563,8 +563,13 @@ static int omap_i2c_wait_for_bb_valid(struct omap_i2c_dev *omap)\n>>>> \t\t}\n>>>> \n>>>> \t\tif (time_after(jiffies, timeout)) {\n>>>> +\t\t\t/*\n>>>> +\t\t\t * SDA or SCL were low for the entire timeout without\n>>>> +\t\t\t * any activity detected. Most likely, a slave is\n>>>> +\t\t\t * locking up the bus with no master driving the clock.\n>>>> +\t\t\t */\n>>>> \t\t\tdev_warn(omap->dev, \"timeout waiting for bus ready\\n\");\n>>>> -\t\t\treturn -ETIMEDOUT;\n>>>> +\t\t\treturn i2c_recover_bus(&omap->adapter);\n>>>> \t\t}\n>>>> \n>>>> \t\tmsleep(1);\n>>>>\n>>>\n>>\n> \n> --\n> Regards\n> Claudio\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=ti.com header.i=@ti.com header.b=\"rBtrj5um\";\n\tdkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xxKSN35h5z9sNw\n\tfor ;\n\tTue, 19 Sep 2017 20:51:48 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751284AbdISKvr (ORCPT );\n\tTue, 19 Sep 2017 06:51:47 -0400","from fllnx209.ext.ti.com ([198.47.19.16]:27561 \"EHLO\n\tfllnx209.ext.ti.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751205AbdISKvq (ORCPT\n\t); Tue, 19 Sep 2017 06:51:46 -0400","from dflxv15.itg.ti.com ([128.247.5.124])\n\tby fllnx209.ext.ti.com (8.15.1/8.15.1) with ESMTP id v8JAoaRn001397; \n\tTue, 19 Sep 2017 05:50:36 -0500","from DFLE114.ent.ti.com (dfle114.ent.ti.com [10.64.6.35])\n\tby dflxv15.itg.ti.com (8.14.3/8.13.8) with ESMTP id v8JAoVmP000914;\n\tTue, 19 Sep 2017 05:50:31 -0500","from DFLE115.ent.ti.com (10.64.6.36) by DFLE114.ent.ti.com\n\t(10.64.6.35) with Microsoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.1.845.34;\n\tTue, 19 Sep 2017 05:50:31 -0500","from dflp32.itg.ti.com (10.64.6.15) by DFLE115.ent.ti.com\n\t(10.64.6.36) with Microsoft SMTP Server (version=TLS1_0,\n\tcipher=TLS_RSA_WITH_AES_256_CBC_SHA) id 15.1.845.34 via Frontend\n\tTransport; Tue, 19 Sep 2017 05:50:31 -0500","from [172.24.190.89] (ileax41-snat.itg.ti.com [10.172.224.153])\n\tby dflp32.itg.ti.com (8.14.3/8.13.8) with ESMTP id v8JAoRGf022960;\n\tTue, 19 Sep 2017 05:50:28 -0500"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ti.com;\n\ts=ti-com-17Q1; t=1505818236;\n\tbh=84OsbPskLdw+dH32UoQwJq35KUNa7CAX+uBlAmwNFAE=;\n\th=Subject:To:CC:References:From:Date:In-Reply-To;\n\tb=rBtrj5umeMPO7oHBfJs/UcPNEfLC4vXPR+8EVANPq6Y/rda1LppDrvtP7qP8w3Md3\n\t8pV/PBy5jpYXhYeSPcT4MVr37G+3DOldUqNY3Y0lxCzGZXZw8/Jv+tVKhjTdBl9kZk\n\tcGWkJVNuRrnYARLPiSJHzwYcplXPysbCj2A2SM+g=","Subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","To":"Claudio Foellmi ,\n\t\"Strashko, Grygorii\" ,\n\tTony Lindgren , \"Nori, Sekhar\" ,\n\t\"Cooper Jr., Franklin\" ","CC":"Aaro Koskinen , Wolfram Sang ,\n\t\"linux-omap@vger.kernel.org\" ,\n\t\"linux-i2c@vger.kernel.org\" ","References":"<1505403549-12992-1-git-send-email-claudio.foellmi@ergon.ch>\n\t<9d3e3029-f775-a9aa-8025-badccb699ed8@ti.com>\n\t<48ddc9bc-2b83-97c9-fbd7-90d4da9f2687@ti.com>\n\t<0a506277-6c9b-1407-dea3-f9895fdc3e63@ergon.ch>","From":"Vignesh R ","Message-ID":"","Date":"Tue, 19 Sep 2017 16:20:49 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<0a506277-6c9b-1407-dea3-f9895fdc3e63@ergon.ch>","Content-Type":"text/plain; charset=\"utf-8\"","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-EXCLAIMER-MD-CONFIG":"e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1771706,"web_url":"http://patchwork.ozlabs.org/comment/1771706/","msgid":"<7c68d7e7-6c10-dc51-4dd9-b11a0d2c1b62@ergon.ch>","list_archive_url":null,"date":"2017-09-20T09:24:44","subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","submitter":{"id":72366,"url":"http://patchwork.ozlabs.org/api/people/72366/","name":"Claudio Foellmi","email":"claudio.foellmi@ergon.ch"},"content":"On 19.09.2017 12:50, Vignesh R wrote:\n> \n> \n> On Monday 18 September 2017 05:31 PM, Claudio Foellmi wrote:\n>> On 18.09.2017 07:24, Vignesh R wrote:\n>>>\n>>>\n>>> On Saturday 16 September 2017 05:01 AM, Strashko, Grygorii wrote:\n>>>>\n>>>>\n>>>> On 09/14/2017 10:39 AM, Claudio Foellmi wrote:\n>>>>> A very conservative check for bus activity (to prevent interference\n>>>>> in multimaster setups) prevented the bus recovery methods from being\n>>>>> triggered in the case that SDA or SCL was stuck low.\n>>>>> This defeats the purpose of the recovery mechanism, which was introduced\n>>>>> for exactly this situation (a slave device keeping SDA pulled down).\n>>>>>\n>>>>> Note that bus lockups can persist across reboots. The only other options\n>>>>> are to reset or power cycle the offending slave device, and many i2c\n>>>>> slaves do not even have a reset pin.\n>>>>>\n>>>>> If we see that one of the lines is low for the entire timeout duration,\n>>>>> we can actually be sure that there is no other master driving the bus.\n>>>>> It is therefore save for us to attempt a bus recovery.\n>>>>>\n>>>>\n>>>> Reviewed-by: Grygorii Strashko \n>>>>\n>>>>> Signed-off-by: Claudio Foellmi \n>>>>> ---\n>>>>> Caveat: It turns out I don't have the hardware to fully test the\n>>>>> recovery mechanism. My faulty i2c slave device actually pulls down SCL,\n>>>>> not SDA (so the recovery will not succeed in my case).\n>>>\n>>> Maybe, you could detect SCL stuck low case by reading status of SCL line\n>>> from OMAP_I2C_SYSTEST_REG and then call IP reset (there is nothing much\n>>> that can be done) instead of bus recovery.\n>>>\n>>\n>> I plan on posting a related patch soon, that will print better messages\n>> if the generic recovery fails. If SCL is stuck low, I think the best we\n>> can do is make the problem visible in the kernel log.\n>>\n>>>>> But by directly connecting SDA to ground, I could at least make sure\n>>>>> the recovery function gets called after applying this patch.\n>>>>>\n>>>\n>>> I had seen flood of XRDY & RRDY interrupts as soon as TMODE is set to\n>>> 0x3 as part of omap_i2c_prepare_recovery() leading to unusable system.\n>>> Did you observe this behavior on your system? Could you mention the\n>>> platform on which this experiment done?\n>>>\n>>\n>> So attempting bus recovery is dangerous on some platforms?\n>> I did not notice any obvious problems (assuming an 'unusable system'\n>> would be hard to miss), but then again I only have one target to test on.\n>> I'm working with a TI AM3352, the slave is a NXP NT3H2211 on i2c-1.\n>>\n>\n> I hit a situation where when communicating with a faulty i2c device, the\n> last transaction on the bus does not end with proper STOP condition on\n> the i2c bus. Since, STOP condition was not detected by IP, Bus Busy will\n> remain set even though both SCL and SDA are high. Thus,\n> omap_i2c_wait_for_bb() function would end up calling bus recovery. And\n> as soon as TMODE is set to 0x3 and ST_EN to 0x1, there is a flood of\n> XRDY & RRDY interrupts.\n>\n\nIf I understand the code correctly, the problem is that omap_i2c_wait_for_bb_valid\ndoes not quite do what the name would suggest:\nIf SDA, SCL and BB are all 1 for a while, it correctly detects that\nthe bus is actually free, but then just sets bb_valid=1 and returns.\nSo now bb_valid is set, even though the value of BB is wrong.\nLater, omap_i2c_wait_for_bb runs into a timeout because it trusts that BB flag,\nand triggers the recovery.\n\nIt would be nice if we could manually set BB to 0 in that case.\nBut from what I've read in the manual, the only way to reset BB is to\nreset the device.\n\n> This spurious irqs can be reproduced easily by setting TMODE to 0x3 and\n> ST_EN to 0x1 in OMAP_I2C_SYSTEST_REG when both SCL and SDA are high (bus\n> is idle) even on AM335x.\n>\n> So, if you are not seeing irq flood when SCL/SDA is stuck low, then\n> maybe its safe to enter TMODE 0x3 in such cases. Could you modify the\n> patch to test whether or not SDA is stuck low before initiating bus\n> recovery?\n>\n\nWhat happens if we try to transmit while BB is still 1?\nIf the transmission succeeds, adding such a check in omap_i2c_wait_for_bb\nshould be enough to deal with your case (as the message we then send\nwill contain the stop condition to correct BB).\n\nBtw, it also looks like omap_i2c_wait_for_bb currently assumes that if\nrecovery succeeds, the bus will then also be free. I'm not sure if that\nis actually correct.\n\n>>>>> drivers/i2c/busses/i2c-omap.c | 7 ++++++-\n>>>>> 1 file changed, 6 insertions(+), 1 deletion(-)\n>>>>>\n>>>>> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c\n>>>>> index 1ebb5e9..4b25fd1 100644\n>>>>> --- a/drivers/i2c/busses/i2c-omap.c\n>>>>> +++ b/drivers/i2c/busses/i2c-omap.c\n>>>>> @@ -563,8 +563,13 @@ static int omap_i2c_wait_for_bb_valid(struct omap_i2c_dev *omap)\n>>>>> \t\t}\n>>>>> \n>>>>> \t\tif (time_after(jiffies, timeout)) {\n>>>>> +\t\t\t/*\n>>>>> +\t\t\t * SDA or SCL were low for the entire timeout without\n>>>>> +\t\t\t * any activity detected. Most likely, a slave is\n>>>>> +\t\t\t * locking up the bus with no master driving the clock.\n>>>>> +\t\t\t */\n>>>>> \t\t\tdev_warn(omap->dev, \"timeout waiting for bus ready\\n\");\n>>>>> -\t\t\treturn -ETIMEDOUT;\n>>>>> +\t\t\treturn i2c_recover_bus(&omap->adapter);\n>>>>> \t\t}\n>>>>> \n>>>>> \t\tmsleep(1);\n>>>>>\n>>>>\n>>>\n>>\n>\n\n--\nRegards\nClaudio","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xxvTY4hXyz9s7B\n\tfor ;\n\tWed, 20 Sep 2017 19:24:49 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751391AbdITJYs (ORCPT );\n\tWed, 20 Sep 2017 05:24:48 -0400","from mx1.ergon.ch ([87.239.214.68]:31065 \"EHLO mx1.ergon.ch\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751378AbdITJYq (ORCPT );\n\tWed, 20 Sep 2017 05:24:46 -0400","from mahler.ergon.ch ([87.239.208.18])\n\tby mx1.ergon.ch with ESMTP; 20 Sep 2017 11:24:45 +0200","from [87.239.211.231] (crespo.ergon.ch [87.239.211.231])\n\tby mahler.ergon.ch (Postfix) with ESMTP id 91B9EEA02;\n\tWed, 20 Sep 2017 11:24:44 +0200 (MEST)"],"X-IronPort-AV":"E=Sophos;i=\"5.42,420,1500933600\"; d=\"scan'208\";a=\"2425421\"","Subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","To":"Vignesh R ,\n\t\"Strashko, Grygorii\" ,\n\tTony Lindgren , \"Nori, Sekhar\" ,\n\t\"Cooper Jr., Franklin\" ","Cc":"Aaro Koskinen , Wolfram Sang ,\n\t\"linux-omap@vger.kernel.org\" ,\n\t\"linux-i2c@vger.kernel.org\" ","References":"<1505403549-12992-1-git-send-email-claudio.foellmi@ergon.ch>\n\t<9d3e3029-f775-a9aa-8025-badccb699ed8@ti.com>\n\t<48ddc9bc-2b83-97c9-fbd7-90d4da9f2687@ti.com>\n\t<0a506277-6c9b-1407-dea3-f9895fdc3e63@ergon.ch>\n\t","From":"Claudio Foellmi ","Message-ID":"<7c68d7e7-6c10-dc51-4dd9-b11a0d2c1b62@ergon.ch>","Date":"Wed, 20 Sep 2017 11:24:44 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1771957,"web_url":"http://patchwork.ozlabs.org/comment/1771957/","msgid":"<20170920150215.fw7dcru6m234pp56@earth>","list_archive_url":null,"date":"2017-09-20T15:02:16","subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","submitter":{"id":62589,"url":"http://patchwork.ozlabs.org/api/people/62589/","name":"Sebastian Reichel","email":"sre@kernel.org"},"content":"Hi,\n\nOn Tue, Sep 19, 2017 at 04:20:49PM +0530, Vignesh R wrote:\n> I hit a situation where when communicating with a faulty i2c device, the\n> last transaction on the bus does not end with proper STOP condition on\n> the i2c bus.\n\nYeah, omap-i2c seems to have big issues with a device not acking a\ntransaction properly. It can be easily triggered on N950 (OMAP3 based):\n\nhttps://patchwork.kernel.org/patch/9914785/\n\nPlease Cc me when sending a patch, so that I can test it on N950.\n\n-- Sebastian","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","mail.kernel.org;\n\tdmarc=none (p=none dis=none) header.from=kernel.org","mail.kernel.org;\n\tspf=none smtp.mailfrom=sre@kernel.org"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xy2z16Vqwz9sNr\n\tfor ;\n\tThu, 21 Sep 2017 01:02:21 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751640AbdITPCU (ORCPT );\n\tWed, 20 Sep 2017 11:02:20 -0400","from mail.kernel.org ([198.145.29.99]:45820 \"EHLO mail.kernel.org\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751570AbdITPCT (ORCPT );\n\tWed, 20 Sep 2017 11:02:19 -0400","from mail.kernel.org (dyndsl-031-150-134-208.ewe-ip-backbone.de\n\t[31.150.134.208])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby mail.kernel.org (Postfix) with ESMTPSA id 93CE021A1D;\n\tWed, 20 Sep 2017 15:02:18 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mail.kernel.org 93CE021A1D","Date":"Wed, 20 Sep 2017 17:02:16 +0200","From":"Sebastian Reichel ","To":"Vignesh R ","Cc":"Claudio Foellmi ,\n\t\"Strashko, Grygorii\" ,\n\tTony Lindgren , \"Nori, Sekhar\" ,\n\t\"Cooper Jr., Franklin\" ,\n\tAaro Koskinen , Wolfram Sang ,\n\t\"linux-omap@vger.kernel.org\" ,\n\t\"linux-i2c@vger.kernel.org\" ","Subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","Message-ID":"<20170920150215.fw7dcru6m234pp56@earth>","References":"<1505403549-12992-1-git-send-email-claudio.foellmi@ergon.ch>\n\t<9d3e3029-f775-a9aa-8025-badccb699ed8@ti.com>\n\t<48ddc9bc-2b83-97c9-fbd7-90d4da9f2687@ti.com>\n\t<0a506277-6c9b-1407-dea3-f9895fdc3e63@ergon.ch>\n\t","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha512;\n\tprotocol=\"application/pgp-signature\"; boundary=\"5ipqkxngmbtuhqfc\"","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"NeoMutt/20170609 (1.8.3)","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1775421,"web_url":"http://patchwork.ozlabs.org/comment/1775421/","msgid":"<3778fa75-0327-0c36-1721-e89e23a782b6@ergon.ch>","list_archive_url":null,"date":"2017-09-26T12:24:59","subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","submitter":{"id":72366,"url":"http://patchwork.ozlabs.org/api/people/72366/","name":"Claudio Foellmi","email":"claudio.foellmi@ergon.ch"},"content":"On 19.09.2017 12:50, Vignesh R wrote:\n> \n> \n> On Monday 18 September 2017 05:31 PM, Claudio Foellmi wrote:\n>> On 18.09.2017 07:24, Vignesh R wrote:\n>>>\n>>>\n>>> On Saturday 16 September 2017 05:01 AM, Strashko, Grygorii wrote:\n>>>>\n>>>>\n>>>> On 09/14/2017 10:39 AM, Claudio Foellmi wrote:\n>>>>> A very conservative check for bus activity (to prevent interference\n>>>>> in multimaster setups) prevented the bus recovery methods from being\n>>>>> triggered in the case that SDA or SCL was stuck low.\n>>>>> This defeats the purpose of the recovery mechanism, which was introduced\n>>>>> for exactly this situation (a slave device keeping SDA pulled down).\n>>>>>\n>>>>> Note that bus lockups can persist across reboots. The only other options\n>>>>> are to reset or power cycle the offending slave device, and many i2c\n>>>>> slaves do not even have a reset pin.\n>>>>>\n>>>>> If we see that one of the lines is low for the entire timeout duration,\n>>>>> we can actually be sure that there is no other master driving the bus.\n>>>>> It is therefore save for us to attempt a bus recovery.\n>>>>>\n>>>>\n>>>> Reviewed-by: Grygorii Strashko \n>>>>\n>>>>> Signed-off-by: Claudio Foellmi \n>>>>> ---\n>>>>> Caveat: It turns out I don't have the hardware to fully test the\n>>>>> recovery mechanism. My faulty i2c slave device actually pulls down SCL,\n>>>>> not SDA (so the recovery will not succeed in my case).\n>>>\n>>> Maybe, you could detect SCL stuck low case by reading status of SCL line\n>>> from OMAP_I2C_SYSTEST_REG and then call IP reset (there is nothing much\n>>> that can be done) instead of bus recovery.\n>>>\n>>\n>> I plan on posting a related patch soon, that will print better messages\n>> if the generic recovery fails. If SCL is stuck low, I think the best we\n>> can do is make the problem visible in the kernel log.\n>>\n>>>>> But by directly connecting SDA to ground, I could at least make sure\n>>>>> the recovery function gets called after applying this patch.\n>>>>>\n>>>\n>>> I had seen flood of XRDY & RRDY interrupts as soon as TMODE is set to\n>>> 0x3 as part of omap_i2c_prepare_recovery() leading to unusable system.\n>>> Did you observe this behavior on your system? Could you mention the\n>>> platform on which this experiment done?\n>>>\n>>\n>> So attempting bus recovery is dangerous on some platforms?\n>> I did not notice any obvious problems (assuming an 'unusable system'\n>> would be hard to miss), but then again I only have one target to test on.\n>> I'm working with a TI AM3352, the slave is a NXP NT3H2211 on i2c-1.\n>>\n> \n> I hit a situation where when communicating with a faulty i2c device, the\n> last transaction on the bus does not end with proper STOP condition on\n> the i2c bus. Since, STOP condition was not detected by IP, Bus Busy will\n> remain set even though both SCL and SDA are high. Thus,\n> omap_i2c_wait_for_bb() function would end up calling bus recovery. And\n> as soon as TMODE is set to 0x3 and ST_EN to 0x1, there is a flood of\n> XRDY & RRDY interrupts.\n> \n> This spurious irqs can be reproduced easily by setting TMODE to 0x3 and\n> ST_EN to 0x1 in OMAP_I2C_SYSTEST_REG when both SCL and SDA are high (bus\n> is idle) even on AM335x.\n> \n> So, if you are not seeing irq flood when SCL/SDA is stuck low, then\n> maybe its safe to enter TMODE 0x3 in such cases. Could you modify the\n> patch to test whether or not SDA is stuck low before initiating bus\n> recovery?\n> \n\nThis sounds more like a problem with the interrupt handler than with\nbus recovery, so I'm a bit hesitant to just add such a workaround.\nInstead, I spent a few hours looking through the interrupt handling\n(and poking my i2c bus with a wire to induce random faults), and\nI suspect to have found the underlying cause, or at least part of it:\n\nWe sometimes ignore some interrupts (such as RRDY if we think we are\nnot in receiving mode), but don't really deal with their cause.\nAs a result, the same interrupt will just be raised again as soon as\nwe leave the handler. It will then be ignored again, and raised again...\n\nI'm still not quite sure how we can reliably get into such situations in\nthe first place, but not sending a stop condition seems to be part of it.\nMaybe it is somehow connected to the automatic internal state change\nthat happens as part of AL or NACK interrupts.\n\n\nBelow is a small patch that should test my assumptions.\nIt clears the incoming fifo and acks the ignored RRDY interrupts.\n\nSebastian, can you please check if this helps with your problems on N950?\nIf it does, I'll turn it into a proper standalone patch.\n\n\nCc: Sebastian Reichel \nSigned-off-by: Claudio Foellmi \n---\n drivers/i2c/busses/i2c-omap.c | 17 ++++++++++++++---\n 1 file changed, 14 insertions(+), 3 deletions(-)\n\ndiff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c\nindex 1ebb5e9..1ad1b0c 100644\n--- a/drivers/i2c/busses/i2c-omap.c\n+++ b/drivers/i2c/busses/i2c-omap.c\n@@ -1009,6 +1009,7 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)\n \tstruct omap_i2c_dev *omap = dev_id;\n \tu16 bits;\n \tu16 stat;\n+\tu16 buf;\n \tint err = 0, count = 0;\n \n \tdo {\n@@ -1016,11 +1017,21 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)\n \t\tstat = omap_i2c_read_reg(omap, OMAP_I2C_STAT_REG);\n \t\tstat &= bits;\n \n-\t\t/* If we're in receiver mode, ignore XDR/XRDY */\n-\t\tif (omap->receiver)\n+\t\t/* If we're in receiver mode, ignore XDR/XRDY and vice versa */\n+\t\tif (omap->receiver && stat & (OMAP_I2C_STAT_XDR |\n+\t\t\t\tOMAP_I2C_STAT_XRDY)) {\n+\t\t\tdev_err(omap->dev, \"ignoring TX interrupts\\n\");\n \t\t\tstat &= ~(OMAP_I2C_STAT_XDR | OMAP_I2C_STAT_XRDY);\n-\t\telse\n+\t\t} else if (!omap->receiver && stat & (OMAP_I2C_STAT_RDR |\n+\t\t\t\tOMAP_I2C_STAT_RRDY)) {\n+\t\t\tdev_err(omap->dev, \"ignoring RX interrupts\\n\");\n \t\t\tstat &= ~(OMAP_I2C_STAT_RDR | OMAP_I2C_STAT_RRDY);\n+\t\t\tbuf = omap_i2c_read_reg(omap, OMAP_I2C_BUF_REG);\n+\t\t\tbuf |= OMAP_I2C_BUF_RXFIF_CLR;\n+\t\t\tomap_i2c_write_reg(omap, OMAP_I2C_BUF_REG, buf);\n+\t\t\tomap_i2c_ack_stat(omap,\n+\t\t\t\tOMAP_I2C_STAT_RRDY | OMAP_I2C_STAT_RDR);\n+\t\t}\n \n \t\tif (!stat) {\n \t\t\t/* my work here is done */","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y1gBs6ng1z9tXx\n\tfor ;\n\tTue, 26 Sep 2017 22:25:09 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S968694AbdIZMZG (ORCPT );\n\tTue, 26 Sep 2017 08:25:06 -0400","from mx1.ergon.ch ([87.239.214.68]:39147 \"EHLO mx1.ergon.ch\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S968651AbdIZMZE (ORCPT );\n\tTue, 26 Sep 2017 08:25:04 -0400","from mahler.ergon.ch ([87.239.208.18])\n\tby mx1.ergon.ch with ESMTP; 26 Sep 2017 14:24:59 +0200","from [87.239.211.231] (crespo.ergon.ch [87.239.211.231])\n\tby mahler.ergon.ch (Postfix) with ESMTP id 4F33AE779;\n\tTue, 26 Sep 2017 14:24:59 +0200 (MEST)"],"X-IronPort-AV":"E=Sophos;i=\"5.42,440,1500933600\"; d=\"scan'208\";a=\"2459364\"","Subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","To":"Vignesh R ,\n\t\"Strashko, Grygorii\" ,\n\tTony Lindgren , \"Nori, Sekhar\" ,\n\t\"Cooper Jr., Franklin\" ,\n\tSebastian Reichel ","Cc":"Aaro Koskinen , Wolfram Sang ,\n\t\"linux-omap@vger.kernel.org\" ,\n\t\"linux-i2c@vger.kernel.org\" ","References":"<1505403549-12992-1-git-send-email-claudio.foellmi@ergon.ch>\n\t<9d3e3029-f775-a9aa-8025-badccb699ed8@ti.com>\n\t<48ddc9bc-2b83-97c9-fbd7-90d4da9f2687@ti.com>\n\t<0a506277-6c9b-1407-dea3-f9895fdc3e63@ergon.ch>\n\t","From":"Claudio Foellmi ","Message-ID":"<3778fa75-0327-0c36-1721-e89e23a782b6@ergon.ch>","Date":"Tue, 26 Sep 2017 14:24:59 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1777523,"web_url":"http://patchwork.ozlabs.org/comment/1777523/","msgid":"<20170929125213.by5mwverw2a47lmo@earth>","list_archive_url":null,"date":"2017-09-29T12:52:13","subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","submitter":{"id":71508,"url":"http://patchwork.ozlabs.org/api/people/71508/","name":"Sebastian Reichel","email":"sebastian.reichel@collabora.co.uk"},"content":"Hi Claudio,\n\nOn Tue, Sep 26, 2017 at 02:24:59PM +0200, Claudio Foellmi wrote:\n> On 19.09.2017 12:50, Vignesh R wrote:\n> > \n> > \n> > On Monday 18 September 2017 05:31 PM, Claudio Foellmi wrote:\n> >> On 18.09.2017 07:24, Vignesh R wrote:\n> >>>\n> >>>\n> >>> On Saturday 16 September 2017 05:01 AM, Strashko, Grygorii wrote:\n> >>>>\n> >>>>\n> >>>> On 09/14/2017 10:39 AM, Claudio Foellmi wrote:\n> >>>>> A very conservative check for bus activity (to prevent interference\n> >>>>> in multimaster setups) prevented the bus recovery methods from being\n> >>>>> triggered in the case that SDA or SCL was stuck low.\n> >>>>> This defeats the purpose of the recovery mechanism, which was introduced\n> >>>>> for exactly this situation (a slave device keeping SDA pulled down).\n> >>>>>\n> >>>>> Note that bus lockups can persist across reboots. The only other options\n> >>>>> are to reset or power cycle the offending slave device, and many i2c\n> >>>>> slaves do not even have a reset pin.\n> >>>>>\n> >>>>> If we see that one of the lines is low for the entire timeout duration,\n> >>>>> we can actually be sure that there is no other master driving the bus.\n> >>>>> It is therefore save for us to attempt a bus recovery.\n> >>>>>\n> >>>>\n> >>>> Reviewed-by: Grygorii Strashko \n> >>>>\n> >>>>> Signed-off-by: Claudio Foellmi \n> >>>>> ---\n> >>>>> Caveat: It turns out I don't have the hardware to fully test the\n> >>>>> recovery mechanism. My faulty i2c slave device actually pulls down SCL,\n> >>>>> not SDA (so the recovery will not succeed in my case).\n> >>>\n> >>> Maybe, you could detect SCL stuck low case by reading status of SCL line\n> >>> from OMAP_I2C_SYSTEST_REG and then call IP reset (there is nothing much\n> >>> that can be done) instead of bus recovery.\n> >>>\n> >>\n> >> I plan on posting a related patch soon, that will print better messages\n> >> if the generic recovery fails. If SCL is stuck low, I think the best we\n> >> can do is make the problem visible in the kernel log.\n> >>\n> >>>>> But by directly connecting SDA to ground, I could at least make sure\n> >>>>> the recovery function gets called after applying this patch.\n> >>>>>\n> >>>\n> >>> I had seen flood of XRDY & RRDY interrupts as soon as TMODE is set to\n> >>> 0x3 as part of omap_i2c_prepare_recovery() leading to unusable system.\n> >>> Did you observe this behavior on your system? Could you mention the\n> >>> platform on which this experiment done?\n> >>>\n> >>\n> >> So attempting bus recovery is dangerous on some platforms?\n> >> I did not notice any obvious problems (assuming an 'unusable system'\n> >> would be hard to miss), but then again I only have one target to test on.\n> >> I'm working with a TI AM3352, the slave is a NXP NT3H2211 on i2c-1.\n> >>\n> > \n> > I hit a situation where when communicating with a faulty i2c device, the\n> > last transaction on the bus does not end with proper STOP condition on\n> > the i2c bus. Since, STOP condition was not detected by IP, Bus Busy will\n> > remain set even though both SCL and SDA are high. Thus,\n> > omap_i2c_wait_for_bb() function would end up calling bus recovery. And\n> > as soon as TMODE is set to 0x3 and ST_EN to 0x1, there is a flood of\n> > XRDY & RRDY interrupts.\n> > \n> > This spurious irqs can be reproduced easily by setting TMODE to 0x3 and\n> > ST_EN to 0x1 in OMAP_I2C_SYSTEST_REG when both SCL and SDA are high (bus\n> > is idle) even on AM335x.\n> > \n> > So, if you are not seeing irq flood when SCL/SDA is stuck low, then\n> > maybe its safe to enter TMODE 0x3 in such cases. Could you modify the\n> > patch to test whether or not SDA is stuck low before initiating bus\n> > recovery?\n> > \n> \n> This sounds more like a problem with the interrupt handler than with\n> bus recovery, so I'm a bit hesitant to just add such a workaround.\n> Instead, I spent a few hours looking through the interrupt handling\n> (and poking my i2c bus with a wire to induce random faults), and\n> I suspect to have found the underlying cause, or at least part of it:\n> \n> We sometimes ignore some interrupts (such as RRDY if we think we are\n> not in receiving mode), but don't really deal with their cause.\n> As a result, the same interrupt will just be raised again as soon as\n> we leave the handler. It will then be ignored again, and raised again...\n> \n> I'm still not quite sure how we can reliably get into such situations in\n> the first place, but not sending a stop condition seems to be part of it.\n> Maybe it is somehow connected to the automatic internal state change\n> that happens as part of AL or NACK interrupts.\n> \n> \n> Below is a small patch that should test my assumptions.\n> It clears the incoming fifo and acks the ignored RRDY interrupts.\n> \n> Sebastian, can you please check if this helps with your problems on N950?\n> If it does, I'll turn it into a proper standalone patch.\n\nNo, it does not. Also no interrupts ignoring messages appearing\nin dmesg:\n\nn950# dmesg | grep -E \"48072000.i2c|lp5523x\"\n[ 0.791046] omap_i2c 48072000.i2c: bus 1 rev4.4 at 400 kHz\n[ 4.934265] lp5523x 1-0032: reset command sent (no ACK)!\n[ 6.003875] omap_i2c 48072000.i2c: controller timed out\n[ 6.033874] lp5523x 1-0032: device detection err: -110\n[ 6.039154] lp5523x: probe of 1-0032 failed with error -110\n\n-- Sebastian","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y3Wfr0w4dz9t33\n\tfor ;\n\tFri, 29 Sep 2017 22:52:20 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751058AbdI2MwS (ORCPT );\n\tFri, 29 Sep 2017 08:52:18 -0400","from bhuna.collabora.co.uk ([46.235.227.227]:33084 \"EHLO\n\tbhuna.collabora.co.uk\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750954AbdI2MwR (ORCPT\n\t); Fri, 29 Sep 2017 08:52:17 -0400","from [127.0.0.1] (localhost [127.0.0.1])\n\t(Authenticated sender: sre) with ESMTPSA id 38EE926D276"],"Date":"Fri, 29 Sep 2017 14:52:13 +0200","From":"Sebastian Reichel ","To":"Claudio Foellmi ","Cc":"Vignesh R ,\n\t\"Strashko, Grygorii\" ,\n\tTony Lindgren , \"Nori, Sekhar\" ,\n\t\"Cooper Jr., Franklin\" ,\n\tAaro Koskinen , Wolfram Sang ,\n\t\"linux-omap@vger.kernel.org\" ,\n\t\"linux-i2c@vger.kernel.org\" ","Subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","Message-ID":"<20170929125213.by5mwverw2a47lmo@earth>","References":"<1505403549-12992-1-git-send-email-claudio.foellmi@ergon.ch>\n\t<9d3e3029-f775-a9aa-8025-badccb699ed8@ti.com>\n\t<48ddc9bc-2b83-97c9-fbd7-90d4da9f2687@ti.com>\n\t<0a506277-6c9b-1407-dea3-f9895fdc3e63@ergon.ch>\n\t\n\t<3778fa75-0327-0c36-1721-e89e23a782b6@ergon.ch>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha512;\n\tprotocol=\"application/pgp-signature\"; boundary=\"bru6povnofgkl62s\"","Content-Disposition":"inline","In-Reply-To":"<3778fa75-0327-0c36-1721-e89e23a782b6@ergon.ch>","User-Agent":"NeoMutt/20170609 (1.8.3)","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1777599,"web_url":"http://patchwork.ozlabs.org/comment/1777599/","msgid":"","list_archive_url":null,"date":"2017-09-29T15:17:47","subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","submitter":{"id":72366,"url":"http://patchwork.ozlabs.org/api/people/72366/","name":"Claudio Foellmi","email":"claudio.foellmi@ergon.ch"},"content":"On 29.09.2017 14:52, Sebastian Reichel wrote:\n> Hi Claudio,\n> \n> On Tue, Sep 26, 2017 at 02:24:59PM +0200, Claudio Foellmi wrote:\n>> On 19.09.2017 12:50, Vignesh R wrote:\n>>>\n>>>\n>>> On Monday 18 September 2017 05:31 PM, Claudio Foellmi wrote:\n>>>> On 18.09.2017 07:24, Vignesh R wrote:\n>>>>>\n>>>>>\n>>>>> On Saturday 16 September 2017 05:01 AM, Strashko, Grygorii wrote:\n>>>>>>\n>>>>>>\n>>>>>> On 09/14/2017 10:39 AM, Claudio Foellmi wrote:\n>>>>>>> A very conservative check for bus activity (to prevent interference\n>>>>>>> in multimaster setups) prevented the bus recovery methods from being\n>>>>>>> triggered in the case that SDA or SCL was stuck low.\n>>>>>>> This defeats the purpose of the recovery mechanism, which was introduced\n>>>>>>> for exactly this situation (a slave device keeping SDA pulled down).\n>>>>>>>\n>>>>>>> Note that bus lockups can persist across reboots. The only other options\n>>>>>>> are to reset or power cycle the offending slave device, and many i2c\n>>>>>>> slaves do not even have a reset pin.\n>>>>>>>\n>>>>>>> If we see that one of the lines is low for the entire timeout duration,\n>>>>>>> we can actually be sure that there is no other master driving the bus.\n>>>>>>> It is therefore save for us to attempt a bus recovery.\n>>>>>>>\n>>>>>>\n>>>>>> Reviewed-by: Grygorii Strashko \n>>>>>>\n>>>>>>> Signed-off-by: Claudio Foellmi \n>>>>>>> ---\n>>>>>>> Caveat: It turns out I don't have the hardware to fully test the\n>>>>>>> recovery mechanism. My faulty i2c slave device actually pulls down SCL,\n>>>>>>> not SDA (so the recovery will not succeed in my case).\n>>>>>\n>>>>> Maybe, you could detect SCL stuck low case by reading status of SCL line\n>>>>> from OMAP_I2C_SYSTEST_REG and then call IP reset (there is nothing much\n>>>>> that can be done) instead of bus recovery.\n>>>>>\n>>>>\n>>>> I plan on posting a related patch soon, that will print better messages\n>>>> if the generic recovery fails. If SCL is stuck low, I think the best we\n>>>> can do is make the problem visible in the kernel log.\n>>>>\n>>>>>>> But by directly connecting SDA to ground, I could at least make sure\n>>>>>>> the recovery function gets called after applying this patch.\n>>>>>>>\n>>>>>\n>>>>> I had seen flood of XRDY & RRDY interrupts as soon as TMODE is set to\n>>>>> 0x3 as part of omap_i2c_prepare_recovery() leading to unusable system.\n>>>>> Did you observe this behavior on your system? Could you mention the\n>>>>> platform on which this experiment done?\n>>>>>\n>>>>\n>>>> So attempting bus recovery is dangerous on some platforms?\n>>>> I did not notice any obvious problems (assuming an 'unusable system'\n>>>> would be hard to miss), but then again I only have one target to test on.\n>>>> I'm working with a TI AM3352, the slave is a NXP NT3H2211 on i2c-1.\n>>>>\n>>>\n>>> I hit a situation where when communicating with a faulty i2c device, the\n>>> last transaction on the bus does not end with proper STOP condition on\n>>> the i2c bus. Since, STOP condition was not detected by IP, Bus Busy will\n>>> remain set even though both SCL and SDA are high. Thus,\n>>> omap_i2c_wait_for_bb() function would end up calling bus recovery. And\n>>> as soon as TMODE is set to 0x3 and ST_EN to 0x1, there is a flood of\n>>> XRDY & RRDY interrupts.\n>>>\n>>> This spurious irqs can be reproduced easily by setting TMODE to 0x3 and\n>>> ST_EN to 0x1 in OMAP_I2C_SYSTEST_REG when both SCL and SDA are high (bus\n>>> is idle) even on AM335x.\n>>>\n>>> So, if you are not seeing irq flood when SCL/SDA is stuck low, then\n>>> maybe its safe to enter TMODE 0x3 in such cases. Could you modify the\n>>> patch to test whether or not SDA is stuck low before initiating bus\n>>> recovery?\n>>>\n>>\n>> This sounds more like a problem with the interrupt handler than with\n>> bus recovery, so I'm a bit hesitant to just add such a workaround.\n>> Instead, I spent a few hours looking through the interrupt handling\n>> (and poking my i2c bus with a wire to induce random faults), and\n>> I suspect to have found the underlying cause, or at least part of it:\n>>\n>> We sometimes ignore some interrupts (such as RRDY if we think we are\n>> not in receiving mode), but don't really deal with their cause.\n>> As a result, the same interrupt will just be raised again as soon as\n>> we leave the handler. It will then be ignored again, and raised again...\n>>\n>> I'm still not quite sure how we can reliably get into such situations in\n>> the first place, but not sending a stop condition seems to be part of it.\n>> Maybe it is somehow connected to the automatic internal state change\n>> that happens as part of AL or NACK interrupts.\n>>\n>>\n>> Below is a small patch that should test my assumptions.\n>> It clears the incoming fifo and acks the ignored RRDY interrupts.\n>>\n>> Sebastian, can you please check if this helps with your problems on N950?\n>> If it does, I'll turn it into a proper standalone patch.\n> \n> No, it does not. Also no interrupts ignoring messages appearing\n> in dmesg:\n> \n> n950# dmesg | grep -E \"48072000.i2c|lp5523x\"\n> [ 0.791046] omap_i2c 48072000.i2c: bus 1 rev4.4 at 400 kHz\n> [ 4.934265] lp5523x 1-0032: reset command sent (no ACK)!\n> [ 6.003875] omap_i2c 48072000.i2c: controller timed out\n> [ 6.033874] lp5523x 1-0032: device detection err: -110\n> [ 6.039154] lp5523x: probe of 1-0032 failed with error -110\n> \n\nHi Sebastian\n\nThank you for trying it out.\nIt seems that your symptoms are quite different from the ones that Vignesh\ndescribed earlier. He had never-ending storms of spurious interrupts\n(which that patch would have addressed), but you don't seem to get\nany interrupts at all. Not even the NACK one, which just looks wrong.\n\nIf you want to still dig deeper, you can enable debug logs for i2c-omap,\nso you can see every single interrupt. But if there are none, I don't see\nwhat we could possibly do to fix it.\n\n\nVignesh, do you still have access to any of those devices with interrupt\nfloods? If so, could you try the previous patch on one of them?\n\nAlso note that Sebastian's issue is definitely not caused (or helped)\nby bus recovery, the timeout he sees resets the adapter right away.\nSo he is not affected by my original patch either way.\n\n-- Claudio","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y3Ztq58gvz9t5P\n\tfor ;\n\tSat, 30 Sep 2017 01:17:55 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752351AbdI2PRw (ORCPT );\n\tFri, 29 Sep 2017 11:17:52 -0400","from mx1.ergon.ch ([87.239.214.68]:24150 \"EHLO mx1.ergon.ch\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1752294AbdI2PRu (ORCPT );\n\tFri, 29 Sep 2017 11:17:50 -0400","from mahler.ergon.ch ([87.239.208.18])\n\tby mx1.ergon.ch with ESMTP; 29 Sep 2017 17:17:49 +0200","from [87.239.211.231] (crespo.ergon.ch [87.239.211.231])\n\tby mahler.ergon.ch (Postfix) with ESMTP id E7B1AED8B;\n\tFri, 29 Sep 2017 17:17:47 +0200 (MEST)"],"X-IronPort-AV":"E=Sophos;i=\"5.42,452,1500933600\"; d=\"scan'208\";a=\"2479625\"","Subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","To":"Sebastian Reichel ,\n\tVignesh R ","Cc":"\"Strashko, Grygorii\" ,\n\tTony Lindgren , \"Nori, Sekhar\" ,\n\t\"Cooper Jr., Franklin\" ,\n\tAaro Koskinen , Wolfram Sang ,\n\t\"linux-omap@vger.kernel.org\" ,\n\t\"linux-i2c@vger.kernel.org\" ","References":"<1505403549-12992-1-git-send-email-claudio.foellmi@ergon.ch>\n\t<9d3e3029-f775-a9aa-8025-badccb699ed8@ti.com>\n\t<48ddc9bc-2b83-97c9-fbd7-90d4da9f2687@ti.com>\n\t<0a506277-6c9b-1407-dea3-f9895fdc3e63@ergon.ch>\n\t\n\t<3778fa75-0327-0c36-1721-e89e23a782b6@ergon.ch>\n\t<20170929125213.by5mwverw2a47lmo@earth>","From":"Claudio Foellmi ","Message-ID":"","Date":"Fri, 29 Sep 2017 17:17:47 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170929125213.by5mwverw2a47lmo@earth>","Content-Type":"text/plain; charset=UTF-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1777644,"web_url":"http://patchwork.ozlabs.org/comment/1777644/","msgid":"<20170929163716.nprxehj7eyld4io5@earth>","list_archive_url":null,"date":"2017-09-29T16:37:16","subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","submitter":{"id":71508,"url":"http://patchwork.ozlabs.org/api/people/71508/","name":"Sebastian Reichel","email":"sebastian.reichel@collabora.co.uk"},"content":"Hi,\n\nOn Fri, Sep 29, 2017 at 05:17:47PM +0200, Claudio Foellmi wrote:\n> >> Sebastian, can you please check if this helps with your problems on N950?\n> >> If it does, I'll turn it into a proper standalone patch.\n> > \n> > No, it does not. Also no interrupts ignoring messages appearing\n> > in dmesg:\n> > \n> > n950# dmesg | grep -E \"48072000.i2c|lp5523x\"\n> > [ 0.791046] omap_i2c 48072000.i2c: bus 1 rev4.4 at 400 kHz\n> > [ 4.934265] lp5523x 1-0032: reset command sent (no ACK)!\n> > [ 6.003875] omap_i2c 48072000.i2c: controller timed out\n> > [ 6.033874] lp5523x 1-0032: device detection err: -110\n> > [ 6.039154] lp5523x: probe of 1-0032 failed with error -110\n> > \n> \n> Hi Sebastian\n> \n> Thank you for trying it out.\n> It seems that your symptoms are quite different from the ones that Vignesh\n> described earlier. He had never-ending storms of spurious interrupts\n> (which that patch would have addressed), but you don't seem to get\n> any interrupts at all. Not even the NACK one, which just looks wrong.\n> \n> If you want to still dig deeper, you can enable debug logs for i2c-omap,\n> so you can see every single interrupt. But if there are none, I don't see\n> what we could possibly do to fix it.\n> \n> \n> Vignesh, do you still have access to any of those devices with interrupt\n> floods? If so, could you try the previous patch on one of them?\n> \n> Also note that Sebastian's issue is definitely not caused (or helped)\n> by bus recovery, the timeout he sees resets the adapter right away.\n> So he is not affected by my original patch either way.\n> \n> -- Claudio\n\nYeah, I don't see IRQ storm, so this might be a different issue.\nFWIW this is what I see on N950:\n\nn950# dmesg | grep -E \"48072000.i2c|lp55\"\n[ 2.136383] omap_i2c 48072000.i2c: bus 1 rev4.4 at 400 kHz\n[ 8.212951] omap_i2c 48072000.i2c: addr: 0x0032, len: 2, flags: 0x0, stop: 1\n[ 8.213287] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)\n[ 8.213592] omap_i2c 48072000.i2c: IRQ (ISR = 0x0002)\n[ 8.213897] lp5523x 1-0032: reset command sent (no ACK)!\n[ 8.242462] omap_i2c 48072000.i2c: addr: 0x0032, len: 2, flags: 0x0, stop: 1\n[ 8.243255] omap_i2c 48072000.i2c: IRQ (ISR = 0x0014)\n[ 8.243469] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)\n[ 8.253387] omap_i2c 48072000.i2c: addr: 0x0032, len: 1, flags: 0x0, stop: 0\n[ 8.253631] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)\n[ 9.272735] omap_i2c 48072000.i2c: controller timed out\n[ 9.278167] lp5523x 1-0032: device detection err: -110\n[ 9.283843] lp5523x: probe of 1-0032 failed with error -110\n[ 9.662780] omap_i2c 48072000.i2c: addr: 0x0060, len: 1, flags: 0x0, stop: 0\n[ 9.670166] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)\n[ 9.675659] omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)\n[ 9.682617] omap_i2c 48072000.i2c: addr: 0x0060, len: 1, flags: 0x1, stop: 1\n[ 9.689819] omap_i2c 48072000.i2c: IRQ (ISR = 0x0008)\n[ 9.695007] omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)\n\n-- Sebastian","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y3cfV1vKzz9t2Z\n\tfor ;\n\tSat, 30 Sep 2017 02:37:22 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752006AbdI2QhU (ORCPT );\n\tFri, 29 Sep 2017 12:37:20 -0400","from bhuna.collabora.co.uk ([46.235.227.227]:34088 \"EHLO\n\tbhuna.collabora.co.uk\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751899AbdI2QhU (ORCPT\n\t); Fri, 29 Sep 2017 12:37:20 -0400","from [127.0.0.1] (localhost [127.0.0.1])\n\t(Authenticated sender: sre) with ESMTPSA id A8B4A260713"],"Date":"Fri, 29 Sep 2017 18:37:16 +0200","From":"Sebastian Reichel ","To":"Claudio Foellmi ","Cc":"Vignesh R ,\n\t\"Strashko, Grygorii\" ,\n\tTony Lindgren , \"Nori, Sekhar\" ,\n\t\"Cooper Jr., Franklin\" ,\n\tAaro Koskinen , Wolfram Sang ,\n\t\"linux-omap@vger.kernel.org\" ,\n\t\"linux-i2c@vger.kernel.org\" ","Subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","Message-ID":"<20170929163716.nprxehj7eyld4io5@earth>","References":"<1505403549-12992-1-git-send-email-claudio.foellmi@ergon.ch>\n\t<9d3e3029-f775-a9aa-8025-badccb699ed8@ti.com>\n\t<48ddc9bc-2b83-97c9-fbd7-90d4da9f2687@ti.com>\n\t<0a506277-6c9b-1407-dea3-f9895fdc3e63@ergon.ch>\n\t\n\t<3778fa75-0327-0c36-1721-e89e23a782b6@ergon.ch>\n\t<20170929125213.by5mwverw2a47lmo@earth>\n\t","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha512;\n\tprotocol=\"application/pgp-signature\"; boundary=\"x773wkzvxrf5fd5b\"","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"NeoMutt/20170609 (1.8.3)","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1778652,"web_url":"http://patchwork.ozlabs.org/comment/1778652/","msgid":"<9520300c-4cbb-b905-a534-103ae7ca3f5f@ti.com>","list_archive_url":null,"date":"2017-10-02T23:01:19","subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","submitter":{"id":25084,"url":"http://patchwork.ozlabs.org/api/people/25084/","name":"Grygorii Strashko","email":"grygorii.strashko@ti.com"},"content":"On 09/29/2017 11:37 AM, Sebastian Reichel wrote:\n> Hi,\n> \n> On Fri, Sep 29, 2017 at 05:17:47PM +0200, Claudio Foellmi wrote:\n>>>> Sebastian, can you please check if this helps with your problems on N950?\n>>>> If it does, I'll turn it into a proper standalone patch.\n>>>\n>>> No, it does not. Also no interrupts ignoring messages appearing\n>>> in dmesg:\n>>>\n>>> n950# dmesg | grep -E \"48072000.i2c|lp5523x\"\n>>> [ 0.791046] omap_i2c 48072000.i2c: bus 1 rev4.4 at 400 kHz\n>>> [ 4.934265] lp5523x 1-0032: reset command sent (no ACK)!\n>>> [ 6.003875] omap_i2c 48072000.i2c: controller timed out\n>>> [ 6.033874] lp5523x 1-0032: device detection err: -110\n>>> [ 6.039154] lp5523x: probe of 1-0032 failed with error -110\n>>>\n>>\n>> Hi Sebastian\n>>\n>> Thank you for trying it out.\n>> It seems that your symptoms are quite different from the ones that Vignesh\n>> described earlier. He had never-ending storms of spurious interrupts\n>> (which that patch would have addressed), but you don't seem to get\n>> any interrupts at all. Not even the NACK one, which just looks wrong.\n\nregarding spurious interrupts during recovery. In my opinion,\nI2C IRQs should be disabled on entering recovery and re-enabled after, as\nbus state is unpredictable during recovery and OMAP I2C driver expect to\nreceive IRQs only and only when omap_i2c_xfer() is called.\n(omap->receiver will have correct value only in above case)\n\n>>\n>> If you want to still dig deeper, you can enable debug logs for i2c-omap,\n>> so you can see every single interrupt. But if there are none, I don't see\n>> what we could possibly do to fix it.\n>>\n>>\n>> Vignesh, do you still have access to any of those devices with interrupt\n>> floods? If so, could you try the previous patch on one of them?\n>>\n>> Also note that Sebastian's issue is definitely not caused (or helped)\n>> by bus recovery, the timeout he sees resets the adapter right away.\n>> So he is not affected by my original patch either way.\n>>\n>> -- Claudio\n> \n> Yeah, I don't see IRQ storm, so this might be a different issue.\n> FWIW this is what I see on N950:\n> \n> n950# dmesg | grep -E \"48072000.i2c|lp55\"\n> [ 2.136383] omap_i2c 48072000.i2c: bus 1 rev4.4 at 400 kHz\n> [ 8.212951] omap_i2c 48072000.i2c: addr: 0x0032, len: 2, flags: 0x0, stop: 1\n> [ 8.213287] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)\n> [ 8.213592] omap_i2c 48072000.i2c: IRQ (ISR = 0x0002)\n> [ 8.213897] lp5523x 1-0032: reset command sent (no ACK)!\n> [ 8.242462] omap_i2c 48072000.i2c: addr: 0x0032, len: 2, flags: 0x0, stop: 1\n> [ 8.243255] omap_i2c 48072000.i2c: IRQ (ISR = 0x0014)\n> [ 8.243469] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)\n\nthis second XRDY looks suspicious, as it's received after ARDY.\n\n> [ 8.253387] omap_i2c 48072000.i2c: addr: 0x0032, len: 1, flags: 0x0, stop: 0\n> [ 8.253631] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)\n> [ 9.272735] omap_i2c 48072000.i2c: controller timed out\n> [ 9.278167] lp5523x 1-0032: device detection err: -110\n> [ 9.283843] lp5523x: probe of 1-0032 failed with error -110\n> [ 9.662780] omap_i2c 48072000.i2c: addr: 0x0060, len: 1, flags: 0x0, stop: 0\n> [ 9.670166] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)\n> [ 9.675659] omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)\n> [ 9.682617] omap_i2c 48072000.i2c: addr: 0x0060, len: 1, flags: 0x1, stop: 1\n> [ 9.689819] omap_i2c 48072000.i2c: IRQ (ISR = 0x0008)\n> [ 9.695007] omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)\n\n \nWouldn't below diff help:\n\ndiff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c\nindex 1ebb5e9..e52bdae 100644\n--- a/drivers/i2c/busses/i2c-omap.c\n+++ b/drivers/i2c/busses/i2c-omap.c\n@@ -959,7 +959,7 @@ static int omap_i2c_transmit_data(struct omap_i2c_dev *omap, u8 num_bytes,\n {\n u16 w;\n \n- while (num_bytes--) {\n+ while (omap->buf_len && num_bytes--) {\n w = *omap->buf++;\n omap->buf_len--;","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=ti.com header.i=@ti.com header.b=\"LCk248bP\";\n\tdkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y5d411xdPz9sRm\n\tfor ;\n\tTue, 3 Oct 2017 10:02:57 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1750949AbdJBXCz (ORCPT );\n\tMon, 2 Oct 2017 19:02:55 -0400","from fllnx209.ext.ti.com ([198.47.19.16]:59896 \"EHLO\n\tfllnx209.ext.ti.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750770AbdJBXCy (ORCPT\n\t); Mon, 2 Oct 2017 19:02:54 -0400","from dflxv15.itg.ti.com ([128.247.5.124])\n\tby fllnx209.ext.ti.com (8.15.1/8.15.1) with ESMTP id v92N1Pi3020685; \n\tMon, 2 Oct 2017 18:01:25 -0500","from DLEE70.ent.ti.com (dlee70.ent.ti.com [157.170.170.113])\n\tby dflxv15.itg.ti.com (8.14.3/8.13.8) with ESMTP id v92N1KKb027253;\n\tMon, 2 Oct 2017 18:01:20 -0500","from [128.247.59.147] (128.247.59.147) by DLEE70.ent.ti.com\n\t(157.170.170.113) with Microsoft SMTP Server id 14.3.294.0;\n\tMon, 2 Oct 2017 18:01:19 -0500"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ti.com;\n\ts=ti-com-17Q1; t=1506985285;\n\tbh=QWrxOBMlV0XUc1mV8L6TP/lADaYBDByjMqiW2jHpqpY=;\n\th=Subject:To:CC:References:From:Date:In-Reply-To;\n\tb=LCk248bPd6mxKDFsZI1kqYiormEVcYBPfOXRYdDtOS3XK/bX7+MhGk8Rt8tH/uRaP\n\thjKl0ox4If/fgoALSh+hpuWiLCKwkGApk5QL2JEeTr6+VQbroJhpXGa7tiNXsz/2gL\n\tNC77Aetuv8w9Hbqw7kDdRAi0wv9X5Ua/yAX1CVEQ=","Subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","To":"Sebastian Reichel ,\n\tClaudio Foellmi ","CC":"Vignesh R , Tony Lindgren ,\n\t\"Nori, Sekhar\" , \"Cooper Jr., Franklin\" ,\n\tAaro Koskinen , Wolfram Sang ,\n\t\"linux-omap@vger.kernel.org\" ,\n\t\"linux-i2c@vger.kernel.org\" ","References":"<1505403549-12992-1-git-send-email-claudio.foellmi@ergon.ch>\n\t<9d3e3029-f775-a9aa-8025-badccb699ed8@ti.com>\n\t<48ddc9bc-2b83-97c9-fbd7-90d4da9f2687@ti.com>\n\t<0a506277-6c9b-1407-dea3-f9895fdc3e63@ergon.ch>\n\t\n\t<3778fa75-0327-0c36-1721-e89e23a782b6@ergon.ch>\n\t<20170929125213.by5mwverw2a47lmo@earth>\n\t\n\t<20170929163716.nprxehj7eyld4io5@earth>","From":"Grygorii Strashko ","Message-ID":"<9520300c-4cbb-b905-a534-103ae7ca3f5f@ti.com>","Date":"Mon, 2 Oct 2017 18:01:19 -0500","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170929163716.nprxehj7eyld4io5@earth>","Content-Type":"text/plain; charset=\"windows-1252\"","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Originating-IP":"[128.247.59.147]","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1778824,"web_url":"http://patchwork.ozlabs.org/comment/1778824/","msgid":"","list_archive_url":null,"date":"2017-10-03T10:32:15","subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","submitter":{"id":65039,"url":"http://patchwork.ozlabs.org/api/people/65039/","name":"Raghavendra, Vignesh","email":"vigneshr@ti.com"},"content":"On Friday 29 September 2017 08:47 PM, Claudio Foellmi wrote:\n[...]\n>>>> I hit a situation where when communicating with a faulty i2c device, the\n>>>> last transaction on the bus does not end with proper STOP condition on\n>>>> the i2c bus. Since, STOP condition was not detected by IP, Bus Busy will\n>>>> remain set even though both SCL and SDA are high. Thus,\n>>>> omap_i2c_wait_for_bb() function would end up calling bus recovery. And\n>>>> as soon as TMODE is set to 0x3 and ST_EN to 0x1, there is a flood of\n>>>> XRDY & RRDY interrupts.\n>>>>\n>>>> This spurious irqs can be reproduced easily by setting TMODE to 0x3 and\n>>>> ST_EN to 0x1 in OMAP_I2C_SYSTEST_REG when both SCL and SDA are high (bus\n>>>> is idle) even on AM335x.\n>>>>\n>>>> So, if you are not seeing irq flood when SCL/SDA is stuck low, then\n>>>> maybe its safe to enter TMODE 0x3 in such cases. Could you modify the\n>>>> patch to test whether or not SDA is stuck low before initiating bus\n>>>> recovery?\n>>>>\n>>>\n>>> This sounds more like a problem with the interrupt handler than with\n>>> bus recovery, so I'm a bit hesitant to just add such a workaround.\n\nI would not say its a workaround. As per I2C spec, bus recovery is to be\ntried only when SDA is stuck low. My suggestion is to check this\ncondition before requesting recovery.\n\n>>> Instead, I spent a few hours looking through the interrupt handling\n>>> (and poking my i2c bus with a wire to induce random faults), and\n>>> I suspect to have found the underlying cause, or at least part of it:\n>>>\n>>> We sometimes ignore some interrupts (such as RRDY if we think we are\n>>> not in receiving mode), but don't really deal with their cause.\n>>> As a result, the same interrupt will just be raised again as soon as\n>>> we leave the handler. It will then be ignored again, and raised again...\n>>>\n>>> I'm still not quite sure how we can reliably get into such situations in\n>>> the first place, but not sending a stop condition seems to be part of it.\n>>> Maybe it is somehow connected to the automatic internal state change\n>>> that happens as part of AL or NACK interrupts.\n>>>\n>>>\n>>> Below is a small patch that should test my assumptions.\n>>> It clears the incoming fifo and acks the ignored RRDY interrupts.\n>>>\n>>> Sebastian, can you please check if this helps with your problems on N950?\n>>> If it does, I'll turn it into a proper standalone patch.\n>>\n>> No, it does not. Also no interrupts ignoring messages appearing\n>> in dmesg:\n>>\n>> n950# dmesg | grep -E \"48072000.i2c|lp5523x\"\n>> [ 0.791046] omap_i2c 48072000.i2c: bus 1 rev4.4 at 400 kHz\n>> [ 4.934265] lp5523x 1-0032: reset command sent (no ACK)!\n>> [ 6.003875] omap_i2c 48072000.i2c: controller timed out\n>> [ 6.033874] lp5523x 1-0032: device detection err: -110\n>> [ 6.039154] lp5523x: probe of 1-0032 failed with error -110\n>>\n> \n> Hi Sebastian\n> \n> Thank you for trying it out.\n> It seems that your symptoms are quite different from the ones that Vignesh\n> described earlier. He had never-ending storms of spurious interrupts\n> (which that patch would have addressed), but you don't seem to get\n> any interrupts at all. Not even the NACK one, which just looks wrong.\n> \n> If you want to still dig deeper, you can enable debug logs for i2c-omap,\n> so you can see every single interrupt. But if there are none, I don't see\n> what we could possibly do to fix it.\n> \n> \n> Vignesh, do you still have access to any of those devices with interrupt\n> floods? If so, could you try the previous patch on one of them?\n\nIn past, I had tried to ACK all the IRQs instead of ignoring, but that\ndid not help. Anyway, I tried your patch, but unfortunately that does\nnot help either. I see interrupts being ignored, but the IRQ flood\ncontinues. Here is the log:\nhttp://pastebin.ubuntu.com/25666141/\n\n> \n> Also note that Sebastian's issue is definitely not caused (or helped)\n> by bus recovery, the timeout he sees resets the adapter right away.\n> So he is not affected by my original patch either way.\n> \n> -- Claudio\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=ti.com header.i=@ti.com header.b=\"Am9J+OvL\";\n\tdkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y5wMT3sZjz9t43\n\tfor ;\n\tTue, 3 Oct 2017 21:32:21 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751200AbdJCKcT (ORCPT );\n\tTue, 3 Oct 2017 06:32:19 -0400","from lelnx193.ext.ti.com ([198.47.27.77]:17857 \"EHLO\n\tlelnx193.ext.ti.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751001AbdJCKcS (ORCPT\n\t); Tue, 3 Oct 2017 06:32:18 -0400","from dlelxv90.itg.ti.com ([172.17.2.17])\n\tby lelnx193.ext.ti.com (8.15.1/8.15.1) with ESMTP id v93AVo7A019921; \n\tTue, 3 Oct 2017 05:31:50 -0500","from DFLE109.ent.ti.com (dfle109.ent.ti.com [10.64.6.30])\n\tby dlelxv90.itg.ti.com (8.14.3/8.13.8) with ESMTP id v93AVopf012976; \n\tTue, 3 Oct 2017 05:31:50 -0500","from DFLE107.ent.ti.com (10.64.6.28) by DFLE109.ent.ti.com\n\t(10.64.6.30) with Microsoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.1.845.34;\n\tTue, 3 Oct 2017 05:31:50 -0500","from dflp32.itg.ti.com (10.64.6.15) by DFLE107.ent.ti.com\n\t(10.64.6.28) with Microsoft SMTP Server (version=TLS1_0,\n\tcipher=TLS_RSA_WITH_AES_256_CBC_SHA) id 15.1.845.34 via Frontend\n\tTransport; Tue, 3 Oct 2017 05:31:50 -0500","from [172.24.190.89] (ileax41-snat.itg.ti.com [10.172.224.153])\n\tby dflp32.itg.ti.com (8.14.3/8.13.8) with ESMTP id v93AVlDZ006929;\n\tTue, 3 Oct 2017 05:31:47 -0500"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ti.com;\n\ts=ti-com-17Q1; t=1507026710;\n\tbh=/QxhKEJhwzVVf/X4WPxZ6es6ohuaEtqdTNEqkFdD2ks=;\n\th=Subject:To:CC:References:From:Date:In-Reply-To;\n\tb=Am9J+OvLWvmS51+Dm07CatRYzQ7gRY8XLdBX8jJhWSC9xe6zP0fqL7soF/ePTAJ8I\n\tnTS3yKybyo0gPwpCAfVpztE8uJZ8ovlTqhvPdDG4GRUW45frjg4EK23M7xCxTmm39h\n\tX1ycneJ3V2sWmnYTWwS9HZAvfUs65EBD2sb1zesA=","Subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","To":"Claudio Foellmi ,\n\tSebastian Reichel ","CC":"\"Strashko, Grygorii\" ,\n\tTony Lindgren , \"Nori, Sekhar\" ,\n\t\"Cooper Jr., Franklin\" ,\n\tAaro Koskinen , Wolfram Sang ,\n\t\"linux-omap@vger.kernel.org\" ,\n\t\"linux-i2c@vger.kernel.org\" ","References":"<1505403549-12992-1-git-send-email-claudio.foellmi@ergon.ch>\n\t<9d3e3029-f775-a9aa-8025-badccb699ed8@ti.com>\n\t<48ddc9bc-2b83-97c9-fbd7-90d4da9f2687@ti.com>\n\t<0a506277-6c9b-1407-dea3-f9895fdc3e63@ergon.ch>\n\t\n\t<3778fa75-0327-0c36-1721-e89e23a782b6@ergon.ch>\n\t<20170929125213.by5mwverw2a47lmo@earth>\n\t","From":"Vignesh R ","Message-ID":"","Date":"Tue, 3 Oct 2017 16:02:15 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=\"utf-8\"","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-EXCLAIMER-MD-CONFIG":"e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1781724,"web_url":"http://patchwork.ozlabs.org/comment/1781724/","msgid":"<20171006152215.wc255gkxnb2lmzvy@earth>","list_archive_url":null,"date":"2017-10-06T15:22:15","subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","submitter":{"id":71508,"url":"http://patchwork.ozlabs.org/api/people/71508/","name":"Sebastian Reichel","email":"sebastian.reichel@collabora.co.uk"},"content":"Hi,\n\nOn Mon, Oct 02, 2017 at 06:01:19PM -0500, Grygorii Strashko wrote:\n> \n> \n> On 09/29/2017 11:37 AM, Sebastian Reichel wrote:\n> > Hi,\n> > \n> > On Fri, Sep 29, 2017 at 05:17:47PM +0200, Claudio Foellmi wrote:\n> >>>> Sebastian, can you please check if this helps with your problems on N950?\n> >>>> If it does, I'll turn it into a proper standalone patch.\n> >>>\n> >>> No, it does not. Also no interrupts ignoring messages appearing\n> >>> in dmesg:\n> >>>\n> >>> n950# dmesg | grep -E \"48072000.i2c|lp5523x\"\n> >>> [ 0.791046] omap_i2c 48072000.i2c: bus 1 rev4.4 at 400 kHz\n> >>> [ 4.934265] lp5523x 1-0032: reset command sent (no ACK)!\n> >>> [ 6.003875] omap_i2c 48072000.i2c: controller timed out\n> >>> [ 6.033874] lp5523x 1-0032: device detection err: -110\n> >>> [ 6.039154] lp5523x: probe of 1-0032 failed with error -110\n> >>>\n> >>\n> >> Hi Sebastian\n> >>\n> >> Thank you for trying it out.\n> >> It seems that your symptoms are quite different from the ones that Vignesh\n> >> described earlier. He had never-ending storms of spurious interrupts\n> >> (which that patch would have addressed), but you don't seem to get\n> >> any interrupts at all. Not even the NACK one, which just looks wrong.\n> \n> regarding spurious interrupts during recovery. In my opinion,\n> I2C IRQs should be disabled on entering recovery and re-enabled after, as\n> bus state is unpredictable during recovery and OMAP I2C driver expect to\n> receive IRQs only and only when omap_i2c_xfer() is called.\n> (omap->receiver will have correct value only in above case)\n> \n> >>\n> >> If you want to still dig deeper, you can enable debug logs for i2c-omap,\n> >> so you can see every single interrupt. But if there are none, I don't see\n> >> what we could possibly do to fix it.\n> >>\n> >>\n> >> Vignesh, do you still have access to any of those devices with interrupt\n> >> floods? If so, could you try the previous patch on one of them?\n> >>\n> >> Also note that Sebastian's issue is definitely not caused (or helped)\n> >> by bus recovery, the timeout he sees resets the adapter right away.\n> >> So he is not affected by my original patch either way.\n> >>\n> >> -- Claudio\n> > \n> > Yeah, I don't see IRQ storm, so this might be a different issue.\n> > FWIW this is what I see on N950:\n> > \n> > n950# dmesg | grep -E \"48072000.i2c|lp55\"\n> > [ 2.136383] omap_i2c 48072000.i2c: bus 1 rev4.4 at 400 kHz\n> > [ 8.212951] omap_i2c 48072000.i2c: addr: 0x0032, len: 2, flags: 0x0, stop: 1\n> > [ 8.213287] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)\n> > [ 8.213592] omap_i2c 48072000.i2c: IRQ (ISR = 0x0002)\n> > [ 8.213897] lp5523x 1-0032: reset command sent (no ACK)!\n> > [ 8.242462] omap_i2c 48072000.i2c: addr: 0x0032, len: 2, flags: 0x0, stop: 1\n> > [ 8.243255] omap_i2c 48072000.i2c: IRQ (ISR = 0x0014)\n> > [ 8.243469] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)\n> \n> this second XRDY looks suspicious, as it's received after ARDY.\n> \n> > [ 8.253387] omap_i2c 48072000.i2c: addr: 0x0032, len: 1, flags: 0x0, stop: 0\n> > [ 8.253631] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)\n> > [ 9.272735] omap_i2c 48072000.i2c: controller timed out\n> > [ 9.278167] lp5523x 1-0032: device detection err: -110\n> > [ 9.283843] lp5523x: probe of 1-0032 failed with error -110\n> > [ 9.662780] omap_i2c 48072000.i2c: addr: 0x0060, len: 1, flags: 0x0, stop: 0\n> > [ 9.670166] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)\n> > [ 9.675659] omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)\n> > [ 9.682617] omap_i2c 48072000.i2c: addr: 0x0060, len: 1, flags: 0x1, stop: 1\n> > [ 9.689819] omap_i2c 48072000.i2c: IRQ (ISR = 0x0008)\n> > [ 9.695007] omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)\n> \n> \n> Wouldn't below diff help:\n> \n> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c\n> index 1ebb5e9..e52bdae 100644\n> --- a/drivers/i2c/busses/i2c-omap.c\n> +++ b/drivers/i2c/busses/i2c-omap.c\n> @@ -959,7 +959,7 @@ static int omap_i2c_transmit_data(struct omap_i2c_dev *omap, u8 num_bytes,\n> {\n> u16 w;\n> \n> - while (num_bytes--) {\n> + while (omap->buf_len && num_bytes--) {\n> w = *omap->buf++;\n> omap->buf_len--;\n\nthat did not change anything as far as I can see:\n\nn950# dmesg | grep -E \"48072000.i2c|lp5523x\"\n[ 2.139801] omap_i2c 48072000.i2c: bus 1 rev4.4 at 400 kHz\n[ 8.236968] omap_i2c 48072000.i2c: addr: 0x0032, len: 2, flags: 0x0, stop: 1\n[ 8.237457] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)\n[ 8.237792] omap_i2c 48072000.i2c: IRQ (ISR = 0x0002)\n[ 8.238189] lp5523x 1-0032: reset command sent (no ACK)!\n[ 8.256500] omap_i2c 48072000.i2c: addr: 0x0032, len: 2, flags: 0x0, stop: 1\n[ 8.256835] omap_i2c 48072000.i2c: IRQ (ISR = 0x0014)\n[ 8.257080] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)\n[ 8.258941] omap_i2c 48072000.i2c: addr: 0x0032, len: 1, flags: 0x0, stop: 0\n[ 8.259246] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)\n[ 9.276062] omap_i2c 48072000.i2c: controller timed out\n[ 9.306060] lp5523x 1-0032: device detection err: -110\n[ 9.311492] lp5523x: probe of 1-0032 failed with error -110\n[ 9.705993] omap_i2c 48072000.i2c: addr: 0x0060, len: 1, flags: 0x0, stop: 0\n[ 9.713256] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)\n[ 9.718536] omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)\n[ 9.723754] omap_i2c 48072000.i2c: addr: 0x0060, len: 1, flags: 0x1, stop: 1\n[ 9.731079] omap_i2c 48072000.i2c: IRQ (ISR = 0x0008)\n[ 9.736297] omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)\n\n-- Sebastian","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y7tfk5rhsz9t3m\n\tfor ;\n\tSat, 7 Oct 2017 02:22:22 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751875AbdJFPWV (ORCPT );\n\tFri, 6 Oct 2017 11:22:21 -0400","from bhuna.collabora.co.uk ([46.235.227.227]:36960 \"EHLO\n\tbhuna.collabora.co.uk\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751495AbdJFPWU (ORCPT\n\t); Fri, 6 Oct 2017 11:22:20 -0400","from [127.0.0.1] (localhost [127.0.0.1])\n\t(Authenticated sender: sre) with ESMTPSA id CBD56267FC6"],"Date":"Fri, 6 Oct 2017 17:22:15 +0200","From":"Sebastian Reichel ","To":"Grygorii Strashko ","Cc":"Claudio Foellmi ,\n\tVignesh R , Tony Lindgren ,\n\t\"Nori, Sekhar\" , \"Cooper Jr., Franklin\" ,\n\tAaro Koskinen , Wolfram Sang ,\n\t\"linux-omap@vger.kernel.org\" ,\n\t\"linux-i2c@vger.kernel.org\" ","Subject":"Re: [PATCH] i2c-omap: Trigger bus recovery in lockup case","Message-ID":"<20171006152215.wc255gkxnb2lmzvy@earth>","References":"<1505403549-12992-1-git-send-email-claudio.foellmi@ergon.ch>\n\t<9d3e3029-f775-a9aa-8025-badccb699ed8@ti.com>\n\t<48ddc9bc-2b83-97c9-fbd7-90d4da9f2687@ti.com>\n\t<0a506277-6c9b-1407-dea3-f9895fdc3e63@ergon.ch>\n\t\n\t<3778fa75-0327-0c36-1721-e89e23a782b6@ergon.ch>\n\t<20170929125213.by5mwverw2a47lmo@earth>\n\t\n\t<20170929163716.nprxehj7eyld4io5@earth>\n\t<9520300c-4cbb-b905-a534-103ae7ca3f5f@ti.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha512;\n\tprotocol=\"application/pgp-signature\"; boundary=\"3hmcjmvvnrmfowal\"","Content-Disposition":"inline","In-Reply-To":"<9520300c-4cbb-b905-a534-103ae7ca3f5f@ti.com>","User-Agent":"NeoMutt/20170609 (1.8.3)","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}}]