Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752194AbcDUNXG (ORCPT ); Thu, 21 Apr 2016 09:23:06 -0400 Received: from mail-bn1bon0061.outbound.protection.outlook.com ([157.56.111.61]:30337 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751302AbcDUNXD (ORCPT ); Thu, 21 Apr 2016 09:23:03 -0400 Authentication-Results: the-dreams.de; dkim=none (message not signed) header.d=none;the-dreams.de; dmarc=none action=none header.from=caviumnetworks.com; Date: Thu, 21 Apr 2016 15:08:35 +0200 From: Jan Glauber To: Wolfram Sang CC: , , David Daney Subject: Re: [PATCH v6 07/19] i2c: octeon: Use i2c recovery framework Message-ID: <20160421130835.GA2623@hardcore> References: <20160420213121.GC1546@katana> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20160420213121.GC1546@katana> User-Agent: Mutt/1.5.21 (2010-09-15) X-Originating-IP: [88.66.108.80] X-ClientProxiedBy: VI1PR07CA0056.eurprd07.prod.outlook.com (10.164.94.152) To CO2PR07MB2584.namprd07.prod.outlook.com (10.166.201.23) X-MS-Office365-Filtering-Correlation-Id: ab891342-820e-4856-c7d8-08d369e61667 X-Microsoft-Exchange-Diagnostics: 1;CO2PR07MB2584;2:hLh6rwpbtkPGy7Co6hiVLcwsvd0GZGeoqvkIfZyrtjEo95mN+iky7hknAjPl3uSLslOdOeDMy/FMNpCFBbJUITTNDJ3eNv7uG1y5g5cOltoI8P+cti5ZV4trfq7yLWLd3N/Cakx05Emyjc3oSApEeoTsmIYwVRSTCh2bhj/bHSDmPKmJfI3OaTUTVl9nwSMp;3:z0ubKC1yxrjb7adfss+7v0U1VwNIiizArc3aO+rELqNojUci9zU2GyYOmCij6RU8d/IWhDVJj9YqZiDysfpRmTQpzPCLxxYEsudhcoWilIlg6idDIndjb7VPkouh41EF X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:CO2PR07MB2584; X-Microsoft-Exchange-Diagnostics: 1;CO2PR07MB2584;25:wbHlH4qfj+fgWPbOu7rb2V3wk7LEPtw/UL1dKxuCLSDHxWBUW6FRaHJSP6NCYXB/MG1R1f7HlPvbm8WCLqgdJkEU6a5xeoFGT6Lm3R5B9oNLaG1rDlfS82psA/IFmK22ZlvSxXNsBKS3z1Q6OrcOs0IKMweqRw2irwbgdS9d5aSlVU/SqlKXOJM5vdxCh+iqm++853Knh7p1mYzzQeQgizC/HSAcruMdr+7bR5fGTgdAKMMs/O/LYemIlEubdj95Es8cd9XxxCkplxnd7hBUBBjJ5AJCwZ+LyW3zs4JOtQh2L2Y7YSQt8jzLBlQyQpHMzxFdZAeAhIh4ByeuNQkZkC3TlSHPi+hubPBMqIQZUjyL1s5ceNzjqKFCZnMtCAE9Ug135rSH0ziTlkaQ4XL8fti34GSCPOOl+uGgd0SCxm+Ty07m9qkzgnPBBatMReHtVwGG0TyGFCSlSRrDmll2IX8s2adfqmKePN89mQQrRsUZqAjvvf3uDrH8+VsfH+qVMLhUWMGJRT04TOFXXsQ32pLhG5De+kHpkVxETFI32onw/LBEVMk4MmarSyqg2cBAaRzKB7VuLwB222IzDvNRWq3Y9+T1+qOkcRSuwwTqyEe7GCn4NZ6Kqm6+PXT7fhfBtti41eBGoQ8vaz6bHiMVUatPUehh5QZMfxEKfE0f1GI= X-Microsoft-Exchange-Diagnostics: 1;CO2PR07MB2584;20:8JNJ6YumFPwI4+KC3mrqRMhXT/tu3eKjLwjxayfVxgwHN0Fhw+xuFldK7kgGHiKsTKB/bXqLwmzCFFp27RPAFpX0naV5l6tKVi2kPs2VIuzbYBaPS5QZmqlv24+fJRc8wyVhIFJvvTDeSsmB01RcLr5Bj7q3SnsTGVfxHvWXmHe0XlL/R+eywJLgQHPcvZltjytKmfhsJTdPtXpg8cV2Snzkhmvw4vu4P1lD76sBk/8FDc/F+WU5Kt1gdQz4drEPjhoZvZ0oeDvloF+aSb+c0JBSfAuJx2577OS1p7VH4+QUIXKKgI1EQUYX4k5hc16M3Po1fHnhafXY0YwbmYnsU8ZQskXqkhNzqbIkZ3GWW56MprphoDxvxoiSmCg2t0lecEG8EBzknFyh6ohuopr+Of0HqpRtOFAgWQuDsHB4zMRBRafhKBh+GceRNWYFvtKRomWIU7W6CW733zz0zp9kVgZBVyofaBex/Q7FwepDN3VsT1XKncrM7501Cm/txEeGgFbVAcFdZJOuVKu6m/S89W3A0PxlHLtwxtmUwMtHOEvypvfXbZMVyjAja5YKZR60JgeS7NgFv7DUuI0A5/UVSGgW9QrUkDXcMBU9KV53nh4= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(9101521026)(601004)(2401047)(8121501046)(5005006)(3002001)(10201501046);SRVR:CO2PR07MB2584;BCL:0;PCL:0;RULEID:;SRVR:CO2PR07MB2584; X-Microsoft-Exchange-Diagnostics: 1;CO2PR07MB2584;4:tZDdj1dW7tGQBqpIg3Xg7L7gGSPbnpHVJmd0/+PPHWlpEXoLViygCDxJZ97pHtN7GMLHBwtz3t5zHK7MGj/xxJ6Ivl+dmgjkxdn4GAg45HC1QfX8nUPFDxtDY8L4eVycdnLnxei1ajmNj13ULteI5+l4umNBehzylc8lCVpclxG/D5Yi+glt5Gz6UxnQ2ROkCRl34wRTEcDAcxYlIevzXObUwkICVZ6Ak/Kafdo4/eVxGjypajAVLBCBjzOCO8UpP9w+FiNH+tGY8aqE9VjZSG6aklg5P5wsy0sAZ28G1broEa0iLQ9/5FTQWoTHHecB7EuSdgvKFOBun3AQdfwREvOUfnPgqF8E5efZnCKty1pRuVJTqZ1DxIRPqtlxUYceuqWBiBv7N2i+ppp+sNOlbg== X-Forefront-PRVS: 091949432C X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(4630300001)(6009001)(24454002)(4326007)(110136002)(46406003)(189998001)(81166005)(33716001)(86362001)(77096005)(50466002)(2950100001)(107886002)(3846002)(4001430100002)(5008740100001)(9686002)(2906002)(1096002)(6116002)(586003)(1076002)(23726003)(76176999)(54356999)(83506001)(19580405001)(97756001)(50986999)(5004730100002)(66066001)(4001350100001)(92566002)(42186005)(47776003)(33656002)(19580395003);DIR:OUT;SFP:1101;SCL:1;SRVR:CO2PR07MB2584;H:hardcore;FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;CO2PR07MB2584;23:Zy3x9dj1f8gBHpXTEJkReGrLZ0cLKtrAGJWYJ0jnR?= =?us-ascii?Q?sN4wVbxLhJfNEQZyOjgNCqQTAKISAx8mmdrrKk2KcigU/dQgkLLMQomwhqB/?= =?us-ascii?Q?vILg8FfeQfskqG6SOWWG+iz5Ltso0wSxZ22tcwEOp0Q5IeAO4ucifou46Z/O?= =?us-ascii?Q?BEfq/XqT59/cTeZKcU4UPVm2zb2meQtAtZhYrHGewGqQIZSdANVQdOxUCx0b?= =?us-ascii?Q?JdWNn4suw8t7Kwz80umQ98e7RbfWm3scAtlZOb65p8rZuWJ4m6lRwF4QEH5W?= =?us-ascii?Q?fG/SosaGaJ+jlLbWrvvFE5SpJTRMCJIBMkDbQtfE5F7JHJ1gjEluXdRTlm4S?= =?us-ascii?Q?+oiDwqH22HOR9VBmK6fNyZjzAGjQeYra0s/Ip9KbVDL1dHNtI6wF6eMDLe3C?= =?us-ascii?Q?AkHZC1dv5n/CLtQOX2WTrKUv1UVqHxx8vFNjGLW1uRzvIY8mlRFOBfERQxaS?= =?us-ascii?Q?xBW1LXzKtO/l+D8ZDU9QztshUYzpz2WQePm1HfLPNPbVsEO5dqyDjNZSgs2A?= =?us-ascii?Q?puI5fPcjLKuDEOLynCJgrq7mx1eIy4rPzrZIzMttwmFLGeWee4w8tWOPpUaf?= =?us-ascii?Q?BTSB7T1gi2P38IXgJK/A8HXXCqCE0lO810bPROnGPIi0zdj80doRCFdok2d6?= =?us-ascii?Q?7dWHB36YYl/ucHcj/ot8GayQJ8FqlbxePyr2OH4NTd8Uke34KRGrlBDgcitZ?= =?us-ascii?Q?BmXCIPsae6txpDwKZK2WvUtW6ZXDn3XGrLSIse2ca+EkMOoV0cicUGF+2DJc?= =?us-ascii?Q?+4Rx3sREJztoLQkjavMf2MNoCf3ZNgzDcjIZIhpiOJuNA2dRl62jp8MDda2k?= =?us-ascii?Q?KC0wxGAannCKCJiK6Jk6trluljZPJ3Av/s44EtXfNswq2biVaQuyM32b0I+l?= =?us-ascii?Q?75auJxARJsme8S0lg7eHcnxjUKKm7UuKvaPEc0sTqa2VefxoV3Ni0glRWhAA?= =?us-ascii?Q?h/yIfmYaysUqm4SbXxQGcE75EiREK0etGDiO/5IxVHTO0nV1aQ8ZS9wleS9d?= =?us-ascii?Q?uugKeXy320+J4AkLkel8be331TtxPXTejy8p4BCjTIehQ=3D=3D?= X-Microsoft-Exchange-Diagnostics: 1;CO2PR07MB2584;5:noPSHnSAmsO7co+kBviw51abB3wcH6yDBl7koeNNLf1yPiE2u3rMKl+YTRKnCUkIC3lUVM7Hkpe3Atr+HNGsZAXij85TXgPViXUqbyijGc/dJNX0Mcu9xbFj7s2sdVVF1lIIwNIJ2R7qKt1/7fK85X0cGm4NVEAIq9Kftee//bmOJeUOJIpdT8F4tq5Eiqxp;24:VXXbT3hno41VI5Ra2cPjg/KP6jpkQ8JtStQ7apUc8F6xWSuAKbuQGqX8RjUaCx4TDHDejT/te4kp7T0Hqr0Yx4Y5cMuUgEvhPlYWM6cNzGs=;7:lWRxdm7cwcB0leHbJGmSr+W6NvmUt1l69viiWAM5rXZut3lM172Aje4CRc5CFDZmNLMZY8ZByA6iS8osVwofIxWqx4zwdLMdtfuqHca55PIpRc0/dMUbyc/g3los2zK9yhpU0zvgspIfZo7hCk4Lg1G66UtO62304Vcvdj2WbPdeejtq0O8xRrMSwM8v0S7mlsi9RXCShhgGkNBwqdZEmt23/segdisItLITmLwr4tw= SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: caviumnetworks.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Apr 2016 13:08:51.3510 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO2PR07MB2584 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2173 Lines: 68 On Wed, Apr 20, 2016 at 11:31:21PM +0200, Wolfram Sang wrote: > On Mon, Apr 11, 2016 at 05:28:38PM +0200, Jan Glauber wrote: > > Switch to the i2c bus recovery framework using generic SCL recovery. > > If this fails try to reset the hardware. The recovery is triggered > > during START on timeout of the interrupt or failure to reach > > the START / repeated-START condition. > > > > The START function is moved to xfer and while at it: > > - removed xfer debug message (i2c core already provides debugging) > > - removed length is zero check > > > > Signed-off-by: Jan Glauber > > --- > > drivers/i2c/busses/i2c-octeon.c | 178 +++++++++++++++++++++++++--------------- > > 1 file changed, 111 insertions(+), 67 deletions(-) > > Interesting, it got larger... > > > > > +/** > > + * octeon_i2c_write_int - read the TWSI_INT register > > read_int OK > > + int ret, retries = 2; > > I don't think 'retries' makes sense here. On failure, you return > -EAGAIN, so the core will retry 'adapter->retries' times anyhow. OK. I'll remove the retry looping and call recovery once if the START condition is not reached. If recovery succeeds I'll return -EAGAIN, otherwise the error code. > > - if (length < 1) > > - return -EINVAL; > > So, the adapter support 0-length messages now? Or why was it there? I > have the feeling this is a seperate patch. I assumed this check was bogus and there are no valid 0-length messages... If 0-length messages can indeed happen I'll keep this check (but why would the i2c core not check for that in a central place and return an error before calling xfer() ?). > > +static void octeon_i2c_prepare_recovery(struct i2c_adapter *adap) > > +{ > > + struct octeon_i2c *i2c = i2c_get_adapdata(adap); > > + > > + /* > > + * The stop resets the state machine, does not _transmit_ STOP unless > > + * engine was active. > > + */ > > + octeon_i2c_stop(i2c); > > + > > + octeon_i2c_write_int(i2c, 0); > > Maybe a comment why the delay? Later patch "Flush TWSI writes with readback" adds synchronization after octeon_i2c_write_int(), so the delays in prepare/unprepare_recovery can go away. > > + udelay(5); > > +}