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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 84551C4167B for ; Fri, 30 Dec 2022 19:11:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=cFjj0dLML9Mzpuck3LoJZrbOevtTxekPPRUN/D4anaU=; b=AJejwz7ew9gdIq 1BXJS1jyt0l3pA4oR1S1A1VcNaohLGwqJY5xIzRgST7GLHGI6O9C/2Ei61uIIakJJwgNlUeZUGdpR G5TljI5p4q3d8ffNPOr3t8jSIOvZmCZ9Pv016N2JRVJhSSdRY4g9ReFwWr9H+OqSg1zb5Jk4/A0bP ihEHn9q0coKhvfza6p5wX1JaX2yAIR6X/hiDlosT86JG1pYYeFppku1dj24Cn1opBRUOPQ5BXcS0R Nz47XXEFp+J3H2cj67Be61mNY8+sSUcjUZ/8RcGAoA8iN+L0mDAdKCjVXPpIYXrlwuXPL8sypH++3 ovPMe7wHGw1Sl85oAgGQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pBKly-00CVi5-BI; Fri, 30 Dec 2022 19:10:32 +0000 Received: from metis.ext.pengutronix.de ([2001:67c:670:201:290:27ff:fe1d:cc33]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1pBHzl-00AvN7-Ru for linux-arm-kernel@lists.infradead.org; Fri, 30 Dec 2022 16:12:36 +0000 Received: from ptx.hi.pengutronix.de ([2001:67c:670:100:1d::c0]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1pBHzS-0006uc-RL; Fri, 30 Dec 2022 17:12:14 +0100 Received: from ore by ptx.hi.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1pBHzN-0000y1-JX; Fri, 30 Dec 2022 17:12:09 +0100 Date: Fri, 30 Dec 2022 17:12:09 +0100 From: Oleksij Rempel To: Francesco Dolcini Cc: Primoz Fiser , Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= , linux-kernel@vger.kernel.org, Shawn Guo , Sascha Hauer , upstream@lists.phytec.de, Marco Felsch , Oleksij Rempel , NXP Linux Team , Pengutronix Kernel Team , Fabio Estevam , linux-arm-kernel@lists.infradead.org, linux-i2c@vger.kernel.org, francesco.dolcini@toradex.com, wsa@kernel.org Subject: Re: [PATCH] i2c: imx: increase retries on arbitration loss Message-ID: <20221230161209.GA14776@pengutronix.de> References: <20221216084511.2576786-1-primoz.fiser@norik.com> <20221216094518.bevkg5buzu7iybfh@pengutronix.de> <20221216110227.GA12327@pengutronix.de> <20221216111308.wckibotr5d3q6ree@pengutronix.de> <5c2e0531-e7c3-1b37-35ed-c8e9795a0d18@norik.com> <41991ce2-3e88-5afc-6def-6e718d624768@norik.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Sent-From: Pengutronix Hildesheim X-URL: http://www.pengutronix.de/ X-Accept-Language: de,en X-Accept-Content-Type: text/plain User-Agent: Mutt/1.10.1 (2018-07-13) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c0 X-SA-Exim-Mail-From: ore@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-arm-kernel@lists.infradead.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221230_081233_941272_3C38432E X-CRM114-Status: GOOD ( 34.50 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Dec 30, 2022 at 03:40:58PM +0100, Francesco Dolcini wrote: > +Wolfram > > On Wed, Dec 28, 2022 at 09:01:46AM +0100, Primoz Fiser wrote: > > On 16. 12. 22 13:51, Francesco Dolcini wrote: > > > On Fri, Dec 16, 2022 at 01:23:29PM +0100, Primoz Fiser wrote: > > > > The only solid point in the thread seems to be that in that case we are not > > > > covering up the potential i2c hardware issues? > > > > > > I believe that in this case we should just have a warning in the kernel. > > > The retry potentially work-around a transient issue and we do not hide any hardware > > > issue at the same time. It seems an easy win-win solution. > > > > I would agree about throwing a warning message in retry case. > > > > Not sure how would it affect other i2c bus drivers using retries > 0. > > Retries might be pretty rare with i2c-imx but some other drivers set this to > > 5 for example. At least using _ratelimited printk is a must using this > > approach. > > Wolfram, Uwe, Oleksij > > Would it be acceptable to have a warning when we have I2C retries, and > with that in place enabling retries on the imx driver? > > It exists hardware that requires this to work correctly, Well, this is persistent confusion in this monolog. It will not make it correctly. > and at a > minimum setting the retry count from user space is not going to solve > potential issues during initial driver probe. I assume it is not clear from programmer point of view. Lets try other way: - The I2C slave could not correctly interpret the data on SDA because the SDA high or low-level voltages do not reach its appropriate input thresholds. This means: You have this: /-\ /-\ ----- 2.5Vcc ___/ \__/ \___ Instead of this: /-\ /-\ ----- 3.3Vcc / \ / \ ___/ \__/ \___ This is bad, because master or slave will not be able to interpret the pick level correctly. It may see some times 0 instead of 1. This means, what ever we are writing we are to the slave or reading from the slave is potentially corrupt and only __sometimes__ the master was able to detect it. - The I2C slave missed an SCL cycle because the SCL high or low-level voltages do not reach its appropriate input thresholds. This means, the bus frequency is too high for current configured or physical PCB designed. So, you will have different kind of corruptions and some times they will be detected. - The I2C slave accidently interpreted a spike etc. as an SCL cycle. This means the noise level is to high. The driver strange should be increased or PCB redesign should be made. May be there are more options. If not done, data corruption can be expected. None of this issue can be "fixed" by retries or made more "robust". Doing more retries means: we do what ever we do until the system was not able to detect the error. > To me the only reasonable solution is to have the retry enabled with a > sensible number (3? 5?), however there is a concern that this might > hide real hardware issues. There is real hardware issue. Regards, Oleksij -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel