From patchwork Mon Nov 17 16:41:57 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexander Kochetkov X-Patchwork-Id: 411714 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 088141401F6 for ; Tue, 18 Nov 2014 03:42:33 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751820AbaKQQmb (ORCPT ); Mon, 17 Nov 2014 11:42:31 -0500 Received: from mail-lb0-f172.google.com ([209.85.217.172]:56795 "EHLO mail-lb0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751712AbaKQQmb (ORCPT ); Mon, 17 Nov 2014 11:42:31 -0500 Received: by mail-lb0-f172.google.com with SMTP id u10so8826085lbd.3 for ; Mon, 17 Nov 2014 08:42:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id; bh=doNKv5tqMxtbr9P1wo14DXIqaV++2rFMNzaC9xByLDo=; b=zbMXW8ZYUFuVTEj+v+aEQ6NWknuAlILEBZKv3XVdbx/RiiEB2aUMlUIN6ZrmtiTJDv GEG2XIOYyoRLrTFSf/Sst2GVRAYvV+GqU1t2xunZ8qWYoinWLVeURVNnTapPxvcxqSl1 EIcp8GFj7LpCM7iX2VI/KdId0liJgHKMEMpD0n5YBEo1LvJTeg+xxP40jMBoUds34AQl kK4im4ssGaVnovgkI+ZVtnX2HS6tE+aZgE+2I3dex7lEwbIPmYHIx6xxitydUHGnnvGq A7e3LJXMg25cotD1Y4N4mqfGOmgjIIt+g9QPd1qpD4uLRm6uN8aeIDh2SDqQxdU4elm3 2SRw== X-Received: by 10.112.41.168 with SMTP id g8mr28570864lbl.59.1416242548751; Mon, 17 Nov 2014 08:42:28 -0800 (PST) Received: from ubuntu.lintech.ru ([188.35.135.73]) by mx.google.com with ESMTPSA id s5sm7185741laa.9.2014.11.17.08.42.27 for (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 17 Nov 2014 08:42:28 -0800 (PST) From: Alexander Kochetkov To: linux-omap@vger.kernel.org, linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org Cc: stable@vger.kernel.org, Wolfram Sang , Tony Lindgren , Felipe Balbi , Grygorii Strashko , Alexander Kochetkov Subject: [PATCH] Fixes: 1d7afc9 i2c: omap: ack IRQ in parts Date: Mon, 17 Nov 2014 20:41:57 +0400 Message-Id: <1416242517-29885-1-git-send-email-al.kochet@gmail.com> X-Mailer: git-send-email 1.7.9.5 Sender: linux-i2c-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-i2c@vger.kernel.org commit 1d7afc95946487945cc7f5019b41255b72224b70 (i2c: omap: ack IRQ in parts) changed the interrupt handler to complete transfers without clearing XRDY (AL case) and ARDY (NACK case) flags. XRDY or ARDY interrupts will be fired again. As a result, ISR keep processing transfer after it was already complete (from the driver code point of view). A didn't see real impacts of the 1d7afc9, but it is really bad idea to have ISR running on user data after transfer was complete. It looks, what 1d7afc9 violate TI specs in what how AL and NACK should be handled (see Note 1, sprugn4r, Figure 17-31 and Figure 17-32). According to specs (if I understood correctly), in case of NACK and AL driver must reset NACK, AL, ARDY, RDR, and RRDY (Master Receive Mode), and NACK, AL, ARDY, and XDR (Master Transmitter Mode). All that is done down the code under the if condition: if (stat & (OMAP_I2C_STAT_ARDY | OMAP_I2C_STAT_NACK | OMAP_I2C_STAT_AL)) ... The patch restore pre 1d7afc9 logic of handling NACK and AL interrupts, so no interrupts is fired after ISR informs the rest of driver what transfer complete. Note: instead of removing break under NACK case, we could just replace 'break' with 'continue' and allow NACK transfer to finish using ARDY event. I found that NACK and ARDY bits usually set together. That case confirm TI wiki: http://processors.wiki.ti.com/index.php/I2C_Tips#Detecting_and_handling_NACK In order if someone interested in the event traces for NACK and AL cases, I sent them to mailing list. Tested on Beagleboard XM C. Signed-off-by: Alexander Kochetkov Cc: # v3.7+ --- drivers/i2c/busses/i2c-omap.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 90dcc2e..9af7095 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -926,14 +926,12 @@ omap_i2c_isr_thread(int this_irq, void *dev_id) if (stat & OMAP_I2C_STAT_NACK) { err |= OMAP_I2C_STAT_NACK; omap_i2c_ack_stat(dev, OMAP_I2C_STAT_NACK); - break; } if (stat & OMAP_I2C_STAT_AL) { dev_err(dev->dev, "Arbitration lost\n"); err |= OMAP_I2C_STAT_AL; omap_i2c_ack_stat(dev, OMAP_I2C_STAT_AL); - break; } /*