From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755453AbcCNIwo (ORCPT ); Mon, 14 Mar 2016 04:52:44 -0400 Received: from mail-by2on0054.outbound.protection.outlook.com ([207.46.100.54]:34435 "EHLO na01-by2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1755329AbcCNIwf (ORCPT ); Mon, 14 Mar 2016 04:52:35 -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: Mon, 14 Mar 2016 09:52:19 +0100 From: Jan Glauber To: Wolfram Sang CC: , , David Daney Subject: Re: [PATCH v3 02/14] i2c-octeon: Cleanup i2c-octeon driver Message-ID: <20160314085219.GA2503@hardcore> References: <20160312153459.GD1661@katana> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20160312153459.GD1661@katana> User-Agent: Mutt/1.5.21 (2010-09-15) X-Originating-IP: [109.193.40.177] X-ClientProxiedBy: DB5PR03CA0005.eurprd03.prod.outlook.com (25.162.150.15) To DM2PR0701MB762.namprd07.prod.outlook.com (10.242.126.19) X-MS-Office365-Filtering-Correlation-Id: edbed15b-b103-4613-8495-08d34be5fb70 X-Microsoft-Exchange-Diagnostics: 1;DM2PR0701MB762;2:8lTtv3b0zSOc9z/wkQrv2dUqYnkyfwBZUXF/tEtP9v7cwgTgnhZU+UAozYgcu2IHYCdk2XrcbSnhjKZtbBZZbJXp7YVDUq/c3UPPq69wg/zFAKKX8pscc9groeoonV+goOEAxU/DEmC2kMWpB8kOTuv0A8KTgwodH9I/p5ooATGD3YA83BZYdm7z1TXLzmsv;3:ZS4ekd99AMNiocK7UQVghzmxshiWT6osRLiE5seaQe4iOOJ3RGtaFo8DNVqZReeVLr4ltl+YgR0te6OH5B+I0fUX7mo4FWHAkItzLPA4em2eJTGyv8TrTI4Zi/P9kXMm;25:O84Gb09t+WDuDbJ/hY/eKryMsnDsqPTrhf1517gKv8Q3Mp8crlaWr7530C9HpiD8dfLNW4kQWKKR3KYR0PD9UvO7YN0FdOgcgGqWoQEJLkKkI5C2IFimtXfnJpjwAT7Z9Fbe5V10UsUdfcM2vGNo6r2+iDyLhgChDPIgr6GpYQDEgQ8Gl5+zvkGe+pSG41jTvRbICSe6+cfYZeWcn5w3ihfjVVV1NLKOE3ACQTlUSoeFGXxAmKJrCpISjYodAvHW/Uzv4JVnWunhsGNQgFK2XhfwCQtHxFDIiE1uIHSrOiKi6iZPlxVrt7XbN4D9vB9/zTZxCFNr/F9KnOqKlPgRAg== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DM2PR0701MB762; X-Microsoft-Exchange-Diagnostics: 1;DM2PR0701MB762;20:EXW3Nsl3yUj6GP0Olg/WIhuEtDfsnbjvnXD5qUAaxxOHN64ssPzmC0rHllKqfeGWoNERCkrXYGN3Zpb5XO2P+ARsNDqcftJrdYox4bwQXNo/0gRbb1lCHKKFGZ+lNx5V8FYZKB5ThE10fnvZ43tjHHPXVXDJXaBUBjeuvNyACqe11cKzY5lQF0dauP/0s2XqrYK4VwQ5SMPs7JN355sjfm0WNZi4is2X5QWjux8NbrqC77AFOyS/9BjhvNBDHGpVZoqmtJUzcYHsvuc6HY3xeh7o9w2CpjVb1AVptX1SFqQ4x/8dnYsoZ2TQDo3+DW93gk78k4NtijyTZkC7ePZB12zdqKsaGfwGPb3CJE2b5TEiG+FINJIC9rRY+2bU3Us9CuSgH4kN8QwxBVVftcyMHkfaJbNASoT/llcVf4G4IaHIzRoK+QO9tiTTKyu/DViM8dXxitIzzwqRcYouIoj/LxHtsYN6R4vfCKcqk1fQJv3fWYsiXxaldkbh25kGBVR8PQG7H9VbYNpH8U6B+X7TtLgDzbX2MqfSglVPfK5VsoQDQDqgalKkWqfoC4i/75BFaCoOXHpKAA6bMCcgSDGJ62FeIBagiQ7v9sFSUHRBt20= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001);SRVR:DM2PR0701MB762;BCL:0;PCL:0;RULEID:;SRVR:DM2PR0701MB762; X-Microsoft-Exchange-Diagnostics: 1;DM2PR0701MB762;4:QuRo2PbrdeHq8nV0agLDu9I1JNYVV4kupoSXaq6P3fPdoxwywk76ZnahII1rYm9CJt7Y+5b79Xt2kuE2uL83MGpCmN/NqrJiNzYeI8Pd4CY/C9uWZafQgAZ5Rkq6rMWH+j00C3NRlBYrfIhKXDJrmTwgXmCeon1JC+14Ez4g/PBGW/uqHEr8PcjdNI2wS3hoDzuu8AzV2kdIhndUDGeCcrCeCTE69rk4z5NqdFiGb1vpCUnOy1fItXCfo+TgKbPwKy4k33b2VrwuIHTcmifPOPf/mphtCf3b/FL2DpZ3FXbdJiYLlYFFAFzzOsVbSQj9EO3QmU8QWfysZXbXXfdog3D/cGxQSZv88a4WrhtNZ2PmZM2H02GYxUh2DK8Cy4cF X-Forefront-PRVS: 0881A7A935 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(4630300001)(6009001)(24454002)(164054003)(4326007)(50466002)(77096005)(586003)(47776003)(66066001)(46406003)(189998001)(42186005)(92566002)(1096002)(54356999)(5008740100001)(76176999)(33716001)(5004730100002)(4001430100002)(2906002)(2950100001)(83506001)(33656002)(97756001)(81166005)(86362001)(3846002)(50986999)(1076002)(110136002)(107886002)(6116002)(23726003);DIR:OUT;SFP:1101;SCL:1;SRVR:DM2PR0701MB762;H:hardcore;FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;DM2PR0701MB762;23:V/6J+eLBjTvXHpNMZiUpohF+PkK/sEkgsFEj/qlaGU9UjZfTJR88cnGIiI9SizZk04JEYrr68ob+QZmAUqeQhDHl9MMkGqWjgEq2tjYI0xQ539We8fFDxg4qm/axSFJGf9S5zXp+uHmAs/YHJcIQB1VSqaOcD8usoD8pSfbqV7+MZXjoQHsmG3S5e8Q7u2nFV6zqax8+y7GoZMxi8YlnklXyoGCAo/kdWZfFfyIUknuIv/vZIAghHX+MzzEdqjxKqb1vNlIBfYnG6+OgzaFpPi9CvMrpJDa86RE2OHv8U7mxk5ddVuKPOYzT+aJ1M3YyCBAQ0ZnQ0S7yQXqj6iAIiORvtU6O1VYnFCKn/XnJeBOCw6UjY+xtmwGFgZx7LEUigw2OdyGH9HGNXPXKOb4QIi3K5AEYkdc92agWgV1spBQNsxFZ8bbK9uzniRgT2KmDc3Ifw3XjC8XPdnticG/jJOsjxXHh4YEm/Y4bIhttI9VR0FHmZAvwdwHvVNqxJ5wl+ergVoKgCZpAnZSIUgxIHU1S3z5BnYh4WTyDtg8AxxhH7ZlErEo/ij1Kfe932z7QZuWXQd1a/h+0xo64+BI3ZuWDige02+MNCBWJyfjpGxaFlBAjnQH3RRqPl804Tf13MaZWsBmPUPysywlveOQdiMdacOd5rfG1bNVeAgXRi/woFgyTP9A+5GxvQRpVYAvt3TW1fte9NR/tZAevuUASTZNdVD3NLQP0Lm73TQgW9I6wczUl1lSNVj3XbugCTCbIXnVY46icw9+ic+bVWBGZmpzjOS5xtDbjvg7ZTxSjx6aPDTx7O6DTXxfOgb6pIttbd2Qj5/uxSqT8Ae1FJJAlnJ0/ZZkEu4HqAd+OHN4hmnPN+GbN6TCjWAVuZv3mtEDDhSkHe6+OxkPxil4d+Vfmmn2tRSRh8XoGYhlwrbBTQWzPg0QAIUHHJrhyfl1j/YKS X-Microsoft-Exchange-Diagnostics: 1;DM2PR0701MB762;5:PKPV5DQpXcQ9nE1NoiRUfcg5xBi82xwkL8YBufYGobTNKFFUdvbt71sUeNeE6Tfkq9phmF/2Q0ktUbqkhkCBBjtstZiLq1UcijKtn+2PJBGYOmD8xC6IWy/uo0UVkq/wNKB/JaLTZvtad0t4XLZ2Kw==;24:ZCghIa0ulP5rg7GINX1fJK3Vmk4LSD7V4abAkHDssLHtD2jlUC+EXqKO4gftJIrxPcecDLgYdrs89vL2sPRH8sIFgCUMPTEolVb+XaytK44= SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: caviumnetworks.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Mar 2016 08:52:31.3358 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM2PR0701MB762 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Mar 12, 2016 at 04:35:00PM +0100, Wolfram Sang wrote: > On Mon, Mar 07, 2016 at 04:10:45PM +0100, Jan Glauber wrote: > > Cleanup only without functional change. > > I like most of the changes, but there are still some functional changes > left. > > > -static int octeon_i2c_stop(struct octeon_i2c *i2c) > > +/* send STOP to the bus */ > > +static void octeon_i2c_stop(struct octeon_i2c *i2c) > > { > > u8 data; > > > > @@ -266,11 +259,8 @@ static int octeon_i2c_stop(struct octeon_i2c *i2c) > > > > data = octeon_i2c_read_sw(i2c, SW_TWSI_EOP_TWSI_STAT); > > > > - if (data != STAT_IDLE) { > > + if (data != STAT_IDLE) > > dev_err(i2c->dev, "%s: bad status(0x%x)\n", __func__, data); > > - return -EIO; > > - } > > - return 0; > > Why this change? I don't know what SW_TWSI_EOP_TWSI_STAT tells, but this > is surely not a cleanup. It is no functional change because the return value of octeon_i2c_stop() was ignored anyway. That said, the whole read-back of the status and the dev_err looks like debug code to me and is removed in a later patch anyway. I'll incoporate this in the cleanup, so octeon_i2c_stop() will only do the write. > > octeon_i2c_stop(i2c); > > > > - return (ret != 0) ? ret : num; > > + return ret ? -EAGAIN : num; > > This is also not a cleanup and looks wrong. -EAGAIN is for lost > arbitration only. I agree, this looks like an over-simplification and drops the EINVAL/ETIMEDOUT/EIO errors. I'll drop that completely. > > > > -static struct of_device_id octeon_i2c_match[] = { > > - { > > - .compatible = "cavium,octeon-3860-twsi", > > - }, > > +static const struct of_device_id octeon_i2c_match[] = { > > + { .compatible = "cavium,octeon-3860-twsi", }, > > Nit: I'd prefer no tabs within the curly braces. Agreed. thanks, Jan > Thanks, > > Wolfram >