From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 83A2AC4332F for ; Wed, 1 Nov 2023 21:12:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1344921AbjKAVMX (ORCPT ); Wed, 1 Nov 2023 17:12:23 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37970 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345140AbjKAVMU (ORCPT ); Wed, 1 Nov 2023 17:12:20 -0400 Received: from cnc.isely.net (cnc.isely.net [192.69.181.175]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 514C6110 for ; Wed, 1 Nov 2023 14:12:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=isely.net; s=deb; t=1698873133; bh=LGvdfFN+4CI0ILsYYBUqVyLSwsn985/pmVLgko+fWgg=; h=Date:From:Reply-To:To:cc:Subject:In-Reply-To:References; b=CJ/o74tY7gVNPdAk9QyAn1daeydqENIFhNocIJQz5iTwKuzTxsmL7yCdpOcU6XJLN Anx5W6oTcySh61ZLX0wbuOEl3ww8jqkNjtybeGBlNfOStC8wtUabgZ3IhKGuk+2Sgg hNreVvd9mLJZl3+Qsyt267GuxjbazL90sW7ar3eKARWCewMofWk93mskjiSVq Original-Reply-To: Mike Isely at pobox Original-Subject: aborting i2c bcm2835 controller [was: [PATCH 2/2] [i2c-bcm2835] ALWAYS enable INTD] Author: Mike Isely Original-cc: Stefan Wahren , mike.isely@cobaltdigital.com, Andi Shyti , Florian Fainelli , Phil Elwell , Broadcom internal kernel review list , Ray Jui , Scott Branden , linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org, Linux Kernel Mailing List , Mike Isely at pobox Received: from ts3-dock1.isely.net (ts3-dock1.isely.net [::ffff:192.168.23.13]) (AUTH: PLAIN isely, TLS: TLS1.3,256bits,ECDHE_RSA_AES_256_GCM_SHA384) by cnc.isely.net with ESMTPSA id 0000000000080569.000000006542BF2D.00005394; Wed, 01 Nov 2023 16:12:13 -0500 Date: Wed, 1 Nov 2023 16:12:13 -0500 (CDT) From: Mike Isely Reply-To: Mike Isely at pobox To: Dave Stevenson cc: Stefan Wahren , mike.isely@cobaltdigital.com, Andi Shyti , Florian Fainelli , Phil Elwell , Broadcom internal kernel review list , Ray Jui , Scott Branden , linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org, Linux Kernel Mailing List , Mike Isely at pobox Subject: aborting i2c bcm2835 controller [was: [PATCH 2/2] [i2c-bcm2835] ALWAYS enable INTD] In-Reply-To: Message-ID: <2c8c3030-ede6-5283-a0a9-703df43245ac@isely.net> References: <20231030162114.3603829-1-mike.isely@cobaltdigital.com> <20231030162114.3603829-3-mike.isely@cobaltdigital.com> <750b1864-c271-fb64-dcca-e982f818e719@isely.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 1 Nov 2023, Mike Isely wrote: [...] > > > Given that the hardware can detect clock stretch problems then there > > > really should be no circumstance where the controller silicon can leave > > > the bus stuck. > > > > I can't find my debug notes from May, but it was aborting part way > > through a multi-byte read from the touchscreen without a STOP > > condition, leaving the touch controller still thinking it was still > > clocking out the current byte, so it messed up the next transaction. > > Hitting BCM2835_I2C_C with BCM2835_I2C_C_CLEAR whilst BCM2835_I2C_S_TA > > is set will abort immediately with no STOP :-( > > That's probably because the DLEN register still has a non-zero value and > now the hardware is waiting for you to push more bytes into the FIFO > (since you just cleared out what was already there). Thus it won't > release the bus until it has processed all expected bytes. Setting DLEN > to zero first then clearing the FIFO might be the way to fix that. I just re-read the datasheet and it does indeed state that clearing the FIFO would abort the transaction. But are you doing the clearing by just writing a word to the control register with CLEAR set or were you ensuring to preserve the state of I2CEN as well? Because if you were also unintentionally writing a zero to I2CEN as well I can see that freezing the RTL's state machine and thus jamming the bus. To actually do the abort still requires a few state transitions in order release the bus - and if the controller has been disabled then a jam would not be unexpected. In all other cases in the driver, you're only writing to the control register after the hardware has idled, so disabling the controller in this way would be working ok. I wonder if hitting CLEAR while also keeping I2CEN set and INTD on would then allow the hardware to sequence itself to release the bus then issue a DONE back to the driver once everything is idle. [...] -Mike