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 EBE0BC433F5 for ; Mon, 29 Nov 2021 10:41:38 +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-Type:Cc: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: In-Reply-To:MIME-Version:References:Message-ID:Subject:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=lcgtds8coz8fv/gPeiinoi9U3tdz8fO2GhFc/xw9ZnU=; b=XQ6M2YRv6L+53It7zWwpAByQHW elNpL2XfIj0EJHRSpN2rw8s0+Q0QZ3NcNS3aq6Ioi/refUYakoLCV4/hhg6cRK3aNYtZAl9eXfuXo 1R3ZTvCu/ztFJ1TAXlYWtyEjYl33ZOS5I0dQY0YN7EgBaMyE+70yWQekH1S776G/sU4v/woAVwInG XSjVvZvjcx/tcGjgzLaMujokyEYp/XKvFQnk+mW9boEzMEaRZDVv6Z9nDy8EmCygoqeSqEtrtN6Uw N2MhJK6WVGgnyjCKKI5jrTqrbHfM1eIZyOm/kY96R0mfysKxGeYJEjZ2xvyqz++UQKyzJRkxPFaRh gLctTfcg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mre4s-000QON-Cx; Mon, 29 Nov 2021 10:40:06 +0000 Received: from ams.source.kernel.org ([2604:1380:4601:e00::1]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mre4o-000QMt-3m for linux-arm-kernel@lists.infradead.org; Mon, 29 Nov 2021 10:40:03 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 8550EB80E7C; Mon, 29 Nov 2021 10:40:00 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9AD8CC53FAD; Mon, 29 Nov 2021 10:39:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1638182399; bh=dEEOcQfEB95yM+DcjkZPRT2lrjVYea8Y1lBzvzrExkw=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=lqEICburaTXd7FBOJ8WP1mTsl3Lpk/QJjgwNPL6h4TTf/8aj3FQRIkWnzhSMwz99p VQftEDF43EG7hots65fgIwZeVSe8Wk31sszc+P5Au/1VWro6T10ftJ4Ow8ucvKpTT7 nqOm8ubVUh5TsjYs9V78G+H0Fw/n1SocH27DNOguHiYLuRUAbWZ/44HfFO/yepNTP+ wyAGK6IDY2viPPbTjJTLFz4EHh4TKDAY9sUFrIgcY5blD9tKz4ZkS47lx9Qy6i7rBC isYruR6GPBLAjxTfaBuPI1ImH38EIGALZ2jb1mR+gM4VfOrvVA2TOuJq44nmqAJrfO hoSmK05UEGQ/w== Date: Mon, 29 Nov 2021 11:39:56 +0100 From: Wolfram Sang To: Codrin Ciubotariu Subject: Re: [PATCH 1/3] i2c: at91: move i2c_recover_bus() outside of at91_do_twi_transfer() Message-ID: Mail-Followup-To: Wolfram Sang , Codrin Ciubotariu , linux-i2c@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, nicolas.ferre@microchip.com, alexandre.belloni@bootlin.com, ludovic.desroches@microchip.com, andrew@sanpeople.com, mhoffman@lightlink.com, khali@linux-fr.org References: <20210727111554.1338832-1-codrin.ciubotariu@microchip.com> <20210727111554.1338832-2-codrin.ciubotariu@microchip.com> MIME-Version: 1.0 In-Reply-To: <20210727111554.1338832-2-codrin.ciubotariu@microchip.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211129_024002_456983_2149CA5C X-CRM114-Status: GOOD ( 21.15 ) 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: , Cc: alexandre.belloni@bootlin.com, linux-kernel@vger.kernel.org, mhoffman@lightlink.com, ludovic.desroches@microchip.com, linux-i2c@vger.kernel.org, khali@linux-fr.org, andrew@sanpeople.com, linux-arm-kernel@lists.infradead.org Content-Type: multipart/mixed; boundary="===============0240449410348884394==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============0240449410348884394== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="3OthpyYLRMGfk6Jx" Content-Disposition: inline --3OthpyYLRMGfk6Jx Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi Codrin, sorry for the super-long delay. There is an issue here with regard to bus recovery which affetcs more drivers and I can't make up my mind how to handle it... > Fixes: d3d3fdcc4c90 ("i2c: at91: implement i2c bus recovery") Sidenote: I don't think this is a fix. > + if (ret < 0) { > + /* > + * some faulty I2C slave devices might hold SDA down; > + * we can send a bus clear command, hoping that the pins will be > + * released > + */ > + i2c_recover_bus(&dev->adapter); > + } else { > + ret = num; > + } So, one issue is more straightforward. Bus recovery is applied on all errors. It should only be called when SDA is stuck. The other issue is that bus recovery is applied after a transfer. The I2C specs mention bus recovery only at the beginning of a transfer when SDA is detected low. I think it also makes more sense because the bus may also be stuck because of a misbehaving bootloader etc. This will be caught when the check is done at the beginning. However, moving the detection to the beginning leaves room for a regression, because your driver already does it at the end of a transfer. However, I'd think all regressions coming up need seperate fixing anyhow. Unless I overlooked something, of course. So, I think it should be moved to the beginning of a transfer, but I am open for discussion, so we get the best possible bus recovery in Linux. Happy hacking, Wolfram --3OthpyYLRMGfk6Jx Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEOZGx6rniZ1Gk92RdFA3kzBSgKbYFAmGkrfgACgkQFA3kzBSg KbY+Fw//XBnb5n5/XOjcc6s8oJlvjKElkgzd8bDBmXWnRC8XyQPjiWNO4+oXLhOu UkkIaGzL7RcQEc0Bc1JswFqCK8PKdQQDGCJ+BIpk0e9Yfz0Srwg066PJWBuXtNlw L+11VKeIPG/18dwrk9HFxP/rQV6mu7hjTAFDUNvUrc7WKR0V/O1ll6ci8RUJ72Vq y86v+YF3eTOaBhy61AvttrTitCGyVb0m0TWt3i5nYY+BMKVLns96COqw/UE5GA7c jkhRaScyYy+QWhj3o5myWB3K6ynika+5kh1OhHFYsV9xkrdngAR1bLgroeV9PJ91 EcOZkFJ6fbbaDKBWNeGi4f3UDCpvSyQlPy8uAHXsmF2hGI5SUiW7mWXlezUbQzyk i3yFNtlsCQ6lY7qM+iA4JXtRSu1vB2wR69kK67RgpNl+K71mFYdutKVPZcDYHAza yOIB6Su6VYge1G2EaBAhdfopBUJyfDqMIBRgowQqd73urdHT4qcFLhRhnnhssvX6 W2OW7kY3uQvN5Kx1zoxbZGprdMGw/tMIosCesVWWOn+iGNYXiUxpDOr3yyLMvhz2 u1Rpq8Mn99tnA8TSwmuJnlvW7TTsKFY1y5kpbj3U6Uh49FAPyQMeS2jqT0L7fd58 rrWJ3M8BUS05ISoyfWvrqaOmW6YjIMCqYGOgmxLdEkyd9uppPTM= =dQkj -----END PGP SIGNATURE----- --3OthpyYLRMGfk6Jx-- --===============0240449410348884394== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --===============0240449410348884394==--