From owner-freebsd-arm@FreeBSD.ORG Fri May 2 12:50:09 2014 Return-Path: Delivered-To: freebsd-arm@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 2B39BA99; Fri, 2 May 2014 12:50:09 +0000 (UTC) Received: from mail-wi0-x230.google.com (mail-wi0-x230.google.com [IPv6:2a00:1450:400c:c05::230]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 9662513B5; Fri, 2 May 2014 12:50:08 +0000 (UTC) Received: by mail-wi0-f176.google.com with SMTP id f8so2277912wiw.15 for ; Fri, 02 May 2014 05:50:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=FV4VRICq45GUjlZpj3/GHTfQaK5UKQJnS70n2KdRbFc=; b=Hdvr6BVtFP4csrbKszRjE00ZJsSV5wYI3hObvK61fx7K2d5YToJWxmor2mYI63PROZ NwcZPGNQVIEvo2z3uKJ4v3perKxUEA0vKD6RAnC2Yy5L/V/k9e27Jl4cQoyqWjWmt/9R 2XBdjfzXumsTuMswUn+JFgkcuUK6F7H7gjHOzdhvAfSGzoXhMHwhviThwckSTssyvpBu MEL9wew6VAlxuYqjv/uIsudpDd5R2hafWqLWndD/+KZTCF26iMywRCpIO4qCrVXt5u8c GO7SoxSsmxj4vmoXomvT8TeBaZTAQsr4w58g65VlzGenfWYTSMGGJ+vK7F48nfCW2s9p rwbw== MIME-Version: 1.0 X-Received: by 10.180.211.116 with SMTP id nb20mr2812163wic.5.1399035006797; Fri, 02 May 2014 05:50:06 -0700 (PDT) Received: by 10.217.10.195 with HTTP; Fri, 2 May 2014 05:50:06 -0700 (PDT) In-Reply-To: <1398993421.22079.160.camel@revolution.hippie.lan> References: <1398987601.22079.140.camel@revolution.hippie.lan> <1398993421.22079.160.camel@revolution.hippie.lan> Date: Fri, 2 May 2014 08:50:06 -0400 Message-ID: Subject: Re: BBB/I2C: Using ioctl(I2CRDWR) warns: interrupt storm detected on "intr70:" From: Winston Smith To: Ian Lepore Content-Type: text/plain; charset=UTF-8 Cc: FreeBSD ARM X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "Porting FreeBSD to ARM processors." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 02 May 2014 12:50:09 -0000 On Thu, May 1, 2014 at 9:17 PM, Ian Lepore wrote: > Typically an interrupt storm will happen when a device driver's > interrupt handler fails to clear the condition that triggers the > interrupt, so it re-interrupts immediately, continuously. > > After spending a few minutes looking at arm/ti/ti_i2c.c, there's > something that seems not-quite-right... the interrupt handler reads the > I2C_STAT register and clears any interrupt bits it finds there, but: > > #define I2C_REG_STAT 0x88 > > When I look in the AM335x reference manual that's listed as "I2C > interrupt status vector (legacy)." There's another register at 0x28 > described as "Per-event enabled interrupt status vector." > > I wonder if just changing the I2C_REG_STAT 0x28 (in ti_i2c.h) would fix > things? If not, I'd recommend doing a more thorough version of what I > started doing... compare what you see in the manual with the code in > terms of register offsets and which bits are being checked and see if > there are other lurking glitches. I spent some time looking over the documentation and didn't see anything obvious beyond what you pointed out (interesting, the latest version of the TRM doesn't even mention the legacy register at 0x88). I changed I2C_REG_STAT to 0x28, rebuilt and there's no change in behavior from from before, the `bbb_eeprom` tool continues to work and also shows the same "interrupt storm detected". This is the change I made: --- ti_i2c.h 2014-05-02 06:58:17.000000000 -0400 +++ /tmp/ti_i2c.h 2014-05-02 08:44:25.000000000 -0400 @@ -52,7 +52,11 @@ #define I2C_IE_ARDY (1UL << 2) /* Register Access Ready interrupt */ #define I2C_IE_NACK (1UL << 1) /* No Acknowledgment interrupt */ #define I2C_IE_AL (1UL << 0) /* Arbitration Lost interrupt */ +#if defined(SOC_TI_AM335X) +#define I2C_REG_STAT 0x28 +#else #define I2C_REG_STAT 0x88 +#endif #define I2C_STAT_XDR (1UL << 14) #define I2C_STAT_RDR (1UL << 13) #define I2C_STAT_BB (1UL << 12) It may be worth adding it simply to keep the AM335X code up to date (rather than using the legacy register). Interestingly, the ti_i2c code already uses the newer I2C_IRQENABLE_CLR and I2C_IRQENABLE_SET non legacy registers for enabling/disabling the IRQs (vs the old legacy I2C_REG_IE); see ti_i2c_set_intr_enable() in ti_i2c.c. -W