Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752416AbcDUNlR (ORCPT ); Thu, 21 Apr 2016 09:41:17 -0400 Received: from mail-bn1bon0084.outbound.protection.outlook.com ([157.56.111.84]:64078 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751302AbcDUNlO (ORCPT ); Thu, 21 Apr 2016 09:41:14 -0400 Authentication-Results: caviumnetworks.com; dkim=none (message not signed) header.d=none;caviumnetworks.com; dmarc=none action=none header.from=caviumnetworks.com; Date: Thu, 21 Apr 2016 15:40:53 +0200 From: Jan Glauber To: David Daney CC: Wolfram Sang , , Subject: Re: [PATCH v6 08/19] i2c: octeon: Enable High-Level Controller Message-ID: <20160421134053.GB2623@hardcore> References: <34aac0bb7c0ae1c0ef7ca43d087059d04d9d8b09.1460387640.git.jglauber@cavium.com> <20160420214354.GD1546@katana> <5717FAC3.6010406@caviumnetworks.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <5717FAC3.6010406@caviumnetworks.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Originating-IP: [88.66.108.80] X-ClientProxiedBy: AM2PR09CA0074.eurprd09.prod.outlook.com (10.160.228.170) To SN2PR07MB2589.namprd07.prod.outlook.com (10.167.15.19) X-MS-Office365-Filtering-Correlation-Id: ded3902c-1118-4e90-eb97-08d369ea9820 X-Microsoft-Exchange-Diagnostics: 1;SN2PR07MB2589;2:jZ1kA6DPf7rsz19JjL0bJy0seJ28iQaTfbnrNncd/wLBihlnsfqRxdFKXgq8/kIkZRgKLia4nG1XpuQ1cUliZZMsdEeTVw/ZkEVW2Lix4O/HfUG87RTP6H0jGZ4v07vpprfVRjS1LngIA85YRha/yp4HPA847sdd+lUOCuVcUXpYm0buoCfU4hbBuKaJnJ1U;3:NnWz2SpC6cXIV4x/AzwQDLW8E8saEHe1zS/kvr8iq/9YUTvyDB0rAwZkbMKirYShpYhVBnrfCuS/3OCl/9XY9OxmpM5mOZLEei7spCmkfbwLRqDzZoceN8t+MHGIdgn/ X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:SN2PR07MB2589; X-Microsoft-Exchange-Diagnostics: 1;SN2PR07MB2589;25:GHy3zMhmU5JD0EvBtBRJMPWLGjYUyi39OxhGA9dCDCQSsgTVXLcTzc2mpM/JV5Uwb4oJbi+n77hUhW6jUgLzzo8ps95E079C7oiKmvqf8N0D0m9+RI0qnT0y+2sfvEJgfBqGDK4AV3bw4059bktuTcKVqAylWWe5bhUSWZ7RjOpFk9nJYPhUf8Vnb1wzmk/sQ2IsJ86J1v31GNdVQq7XW27Q4NayyoLyHNd1psyqXptnsBtf33+jtwctExxEzm5aJHggqXK2n1pSlHN4kwrXuLabQ2OmyPEjnLMx0wb4yNY9sDIhC2c7ra1f94gxTffQpTOFAuhXUokspi4ctqUyZH9T7umiDuV8u5/XF2xjk1FDFvwOSlKhHIWUDv8ayyCmS+Lvr3+9i2T4rRwHxkPcgYnClIm+0CSHBiGKvUq753qhC46gJhrIwd4VVDQw6n9xjk80rD0Ys1kvkHR5tqsJv2O6tWJwmaqvTv23b6WII75MkA4IsxZwlcGZYMBhaGyap6MwPNZy97Nj7RZPEWh8b4rkfskmPEF2znhSIWazQR7AcFG3+Jf3gUhLYDv81LC3W39U6U4zRBbNS83xIGZvbZ6+JrGyadgz+LILnPLNhN6gQ1/M100yNd4I9v0cMqo514bA/e7B4/UM/JqUFjfXlw== X-Microsoft-Exchange-Diagnostics: 1;SN2PR07MB2589;20:kwF52/3rLQwAwdbBXgR/aJD7UG2WDzvpLWUilRefBKiUv3pOtl3/iKXJtgAevA8KNKzmOMRXZJQqw+6Kqme9o7AOeY9vye2bVKqQ68pLN174Gz8b9EZAN0q8cXeNKW+/7z0iOOLO47Otn/eGgB8DUk2qsPCbxYThV0CPWWsa3sK9bXeCS1xJh+OvX7woEZA1G9MnWpLEH0ntyoJyzLuA22bzGSUj9mjCaJe+paFhmB0XPfoA3gRNM+ZMEOmBZQsTbWIQ2agU4vd7hQ/z895HsR893qAr87Tl3TwmlJU2iYYUPdVi+oqFqx716W5HG8MrW/Gyve2qaXYoW4obw4corOia4Y9UNBs38xGOvXgKYJVMMSC99ruh0vryEThcRfStDA+xYSLivwTKw70fVwq2ekV7q95BnkOJTSZwaLNCvk/8sHsRLdrL+gGGuQQVxYUeebtBs/6EbbMnp9buLXcRNDhCWEVg06UfNjQj1UxD1NFMC2SRc+PWXpnpfbgdRJC6I5Klu8X5ErOYksxQ6k6eMFi8dcx9fG1AV0kXy1rvgM/DyTzVAGQbARkmsaROytCB3YnyqSzn4nx4rCWZ0Ppr4iRbgB6FWk+5UcJvmfJS+JY= 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)(10201501046)(3002001);SRVR:SN2PR07MB2589;BCL:0;PCL:0;RULEID:;SRVR:SN2PR07MB2589; X-Microsoft-Exchange-Diagnostics: 1;SN2PR07MB2589;4:5X5Pr6IvRhvGLySjxLfsEs9tEfqivPC2zmMZUBZ9mByihcUTA2hiUGOv6PDpvVH0UCzXBvdS4NTK1YFvsXmpR5XUlRFKBj8GxGO/76y9L/5w8hpld6j2zZ/SOYKD6geYGszZXTfIwaNBuXBEmAI7qSD7TWVZ8ghpiWi7J9uqX6BNMpbVCCYKX1E+bfnYrpd/HGB38JmKFkSf1jSKYJGbPSqZ0Yg5Yr2UQxmWfprpS4LUpV0J7tFpobHt28NgyTWmL+hxqN0mPZISghwwOut08rTNqC6UV/+zrDyoDBVFOQDNsW5VAMWy9WGn87MLmYJfwdn5qx20g+bfbaRihckosP4YPIoX28+uyLbi+R6w65HNP88Sp+YEky/CXSDis1kI/6ZG6gN2TBFP5RztSPeFZw== X-Forefront-PRVS: 091949432C X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(4630300001)(6009001)(24454002)(377454003)(86362001)(33656002)(5008740100001)(1076002)(4001450100002)(42186005)(50466002)(110136002)(4001350100001)(66066001)(6116002)(3846002)(47776003)(77096005)(1096002)(83506001)(33716001)(586003)(2950100001)(2906002)(23726003)(4326007)(9686002)(97756001)(76176999)(54356999)(189998001)(50986999)(81166005)(5004730100002)(46406003)(92566002)(93886004);DIR:OUT;SFP:1101;SCL:1;SRVR:SN2PR07MB2589;H:hardcore;FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;SN2PR07MB2589;23:RporMa7LRUmU0Axo5AuOje92twfVew0UMlGWmHYNm?= =?us-ascii?Q?iLbRuawt6t4tY7cAIaXydQQOZzeYd6PhYG/+oTxy2vbDUu3me2EIYlsTqUCP?= =?us-ascii?Q?NvedJvNEPF/8yIlOkxY2Op9eNC+8frEiBdsWHE0AuYd4vG6CG5rrVwQqHSyX?= =?us-ascii?Q?ERjoQWxXnRv05eDD4UkC7dVYVI4OUGZN3p0JL2SQy24DqWOtVGF6dTHM9isD?= =?us-ascii?Q?s/gjIsDjVqIOedHjoccrRJOthw2z6avkz0Nr4wja+yFsFWvPzw8aHTQnH7Qd?= =?us-ascii?Q?4dDWhJQa71y9dIGlU5+Sf3eQMkIsatM6QftoVv8iH2+MITG8XPJxp8Y10d14?= =?us-ascii?Q?PHQpLGXCDd3GZ89PNrWt5Jut8B49vshgr+SkjC0qMHHdXqit7//pC9Y5YG19?= =?us-ascii?Q?GrZ+8f9o7VWmF4FrJ6MEYhES7rdwz9OeUbPYkYN/ySOFmMHd4kqWgq8wvLdc?= =?us-ascii?Q?lu8RNQNPHDg/EHOoinJuYf0dJYPvJTUG0VVFB9FghuR9IJXAruP+lDMeTdj9?= =?us-ascii?Q?augI1hVkFn87TZUMtRmnMHRPArBDXjFfFGureBsdrg5r+7r2HD8DMpRwExFm?= =?us-ascii?Q?KB3lPEesslymdckWIXG5D/Kop2ACT7P689agtEaz2JfchulovSYy163EAlNA?= =?us-ascii?Q?8vL9NpNgeLRmpRjqFar58ixoZIAVqojzj/dc3qlCLeEMq63S+xPKvfsDfEhC?= =?us-ascii?Q?v4QJrcRV262tp+VxttTtiJwlBvQn/iboXW/T6fqfQ2v92DdQiNbwaanb7s6l?= =?us-ascii?Q?DgiwUxOqYvyubDCLcT3JwdWPiq1ng7h5p+2E6vo77im2zHXbuBndsknPpJWG?= =?us-ascii?Q?/XpI0Sr/zgvZ4jCmoOGlwWxQcPH3nvmsPiLOh420mapeEdsO6ZbM3EBQSbs4?= =?us-ascii?Q?qmUzp3v1I8dk3d/gmuvG4YCnoRKHSgYNEVP+dLnDzvlh0HuIBurUY9Fnb+UQ?= =?us-ascii?Q?fN+5tv0+kWDZS0KNhL/LwmYrteMy+mwrk4VnoWMbsbWS8sLjiChKcAD+1Uhw?= =?us-ascii?Q?Rg=3D?= X-Microsoft-Exchange-Diagnostics: 1;SN2PR07MB2589;5:LBTSvp8apNJiuxuVdEAuv4RHM6Q7abftkNyj3tnUyf3m2wCCZib1rvFa9vpb8cKaAvr4AtumK93CmawoBTGxqMMhDzssoBtapfeNtlC9/vhp4iZA4J6ATDyg2/q30bd5NLVPY9ab8vYwLIk3FNg5ncGNrBvCn34BMVJvKcMjyxdwMULTRt8YqVRnRFPK+vCT;24:OcrUCjajVQRbUZgnWBFvYrvcmGO9QASH7HD8YVutDrn7MC22VmJvekAh8zhkWOmDt2j56SnNWnzRVi8GZumsBzDi/4kYOGguLnZ6i2Kfk+A=;7:TEJcE96qaIa1WWqb7zzLpSbrZ+MSRU1DMQ8AMJ7/hQcMNHywbO6pE32Dsip+TT4IzBE2aTUlAvYYHR1dohtGbzvH8qXLGfQ6Pcbh3ePRFhlPjJ2kJkv21rTnrcSx2Jzo1yNmtGkvtd89KA6LEDU2rmycfnCGCBXybJnau0SLdQzWNxuJ8dwarej2e+9lt8tVCVe9tcL3zX5atRikvv0VZGK97eTldEpCoh0q9dqja0Q= SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: caviumnetworks.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Apr 2016 13:41:07.5086 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN2PR07MB2589 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1502 Lines: 48 On Wed, Apr 20, 2016 at 02:55:15PM -0700, David Daney wrote: > On 04/20/2016 02:43 PM, Wolfram Sang wrote: > >On Mon, Apr 11, 2016 at 05:28:39PM +0200, Jan Glauber wrote: > [...] > >>+ */ > >>+static int octeon_i2c_hlc_wait(struct octeon_i2c *i2c) > >>+{ > >>+ int time_left; > >>+ > >>+ octeon_i2c_hlc_int_enable(i2c); > >>+ time_left = wait_event_interruptible_timeout(i2c->queue, > >>+ octeon_i2c_hlc_test_ready(i2c), > >>+ i2c->adap.timeout); > > > >Have you tested signal handling thoroughly? Most driver dropped the > >_interruptible after a while. Mostly they found out that the state > >machine of the interrupt handler couldn't gracefully deal with it and > >nobody really needed the interruptible. Just saying. > > Good point. We know that exiting with a signal leaves us in an > undefined state. > > We will have to think on this point. I think we should just drop the _interruptible_ and use wait_event_timeout. The same is already used in the octeon_i2c_wait(). The 2ms timeout should not hurt anyone. > > > >>+ octeon_i2c_int_disable(i2c); > >>+ if (!time_left) { > >>+ octeon_i2c_hlc_int_clear(i2c); > >>+ dev_dbg(i2c->dev, "%s: timeout\n", __func__); > >>+ return -ETIMEDOUT; > >>+ } > >>+ > >>+ if (time_left < 0) { > >>+ dev_dbg(i2c->dev, "%s: wait interrupted\n", __func__); > >>+ return time_left; > >>+ } > >>+ return 0; > >>+} > > > >Drop the debug messages? > > > >I can't say much about the HW details, of course. Didn't spot anything > >suspicious there. > >