From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marc Dietrich Subject: Re: [PATCH] staging: nvec: fixed few coding style warnings Date: Thu, 15 Oct 2015 11:15:07 +0200 Message-ID: <3046503.lL92KFT0lm@fb07-iapwap2> References: <20151014140822.GA23114@localhost.localdomain> <1971511.Ec7FdKlGRN@fb07-iapwap2> <20151015084812.GP7289@mwanda> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2132116303015704388==" Return-path: In-Reply-To: <20151015084812.GP7289@mwanda> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: driverdev-devel-bounces@linuxdriverproject.org Sender: "devel" To: Dan Carpenter Cc: linux-tegra@vger.kernel.org, gregkh@linuxfoundation.org, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org, Sakshi Bansal List-Id: linux-tegra@vger.kernel.org --===============2132116303015704388== Content-Type: multipart/signed; boundary="nextPart4311297.pNsbEbgStr"; micalg="pgp-sha256"; protocol="application/pgp-signature" --nextPart4311297.pNsbEbgStr Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Am Donnerstag, 15. Oktober 2015, 11:48:12 schrieb Dan Carpenter: > On Thu, Oct 15, 2015 at 10:39:02AM +0200, Marc Dietrich wrote: > > > > - if (unlikely(nvec->rx == NULL)) { > > > > + if (!unlikely(nvec->rx)) { > > > > > > This isn't right. You intented to say: > > > if (unlikely(!nvec->rx)) { > > > > > > But even better to just remove the unlikely entirely. > > > > > > if (!nvec->rx) { > > > > why? the "unlikely" is there to optimize a critical interrupt path. > > The rule is that drivers should not use likely/unlikely() unless there > is a difference in benchmark numbers. well, we know that additional cpu cycles in this path break transfer for unknown reasons. However, the unlikely here may be overkill. On the other hand, I prefer not to change something here until these timing effects are better understood. > How critical can it be when it's > always followed by a udelay(100)??? yes, this delay shouldn't be there at all. This is one of the timing mysteries we still have to resolve. > There are more important optimizations needed here. sure. We are currently trying to move all this out into the tegra-i2c driver, so this code block will get a major review/rewrite in the near future anyway. Marc --nextPart4311297.pNsbEbgStr Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part. Content-Transfer-Encoding: 7Bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAABCAAGBQJWH26bAAoJEKyeR39HFBtoxq4H/j0PQF1sItKZqmXYIqNLnlPQ vgmm/dHRhOoLajIAcGjevat/bjuewz8U73LsM0BINLDJ9Mo0Z/GS6+MbbALVN7n/ ++vyWTCDoTahoxBvK9C+qOct6X0molcnHKrU9FEen4DDmN9m6bLgKztFJtoLeQxH IPSuQDVFTXU+Uj9nxSObBPlAsRiMe488JUu3v941zjtuDjYmJqtwBSH5cIcmazn0 RBcs80WZfvEt1eaSKH3sVNkR85lnMgA5fsGTOhH24CcZQGWA8B6Cj5BJNWFB8VP3 Kpwp17B5tGBTgltMhAX4Fw7lbGWlpUY9vMR47c0FZaZs4sUqeV4U0Cs4S/TBpxk= =n9CC -----END PGP SIGNATURE----- --nextPart4311297.pNsbEbgStr-- --===============2132116303015704388== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel --===============2132116303015704388==-- From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752057AbbJOJQG (ORCPT ); Thu, 15 Oct 2015 05:16:06 -0400 Received: from mout.gmx.net ([212.227.17.22]:65516 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751011AbbJOJQD (ORCPT ); Thu, 15 Oct 2015 05:16:03 -0400 From: Marc Dietrich To: Dan Carpenter Cc: gregkh@linuxfoundation.org, linux-tegra@vger.kernel.org, Sakshi Bansal , devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] staging: nvec: fixed few coding style warnings Date: Thu, 15 Oct 2015 11:15:07 +0200 Message-ID: <3046503.lL92KFT0lm@fb07-iapwap2> User-Agent: KMail/4.14.10 (Linux/4.2.1-1-desktop; KDE/4.14.10; x86_64; ; ) In-Reply-To: <20151015084812.GP7289@mwanda> References: <20151014140822.GA23114@localhost.localdomain> <1971511.Ec7FdKlGRN@fb07-iapwap2> <20151015084812.GP7289@mwanda> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart4311297.pNsbEbgStr"; micalg="pgp-sha256"; protocol="application/pgp-signature" X-Provags-ID: V03:K0:LY+hWj5iIMbXj3QxOt49UwaE88qlH28hIzwZ8w4HwgnuwQJ3+Oj upXPFdhfm0xj2lRr531D4YNU4cAuAGZ4TgoXYr56Zhy3eV/raUuzwaxUhsMVsPf4nVlRSeK yZ0JXkEKzChdo0bnm4MPJQ/r4s9pz4zIAzh+N5B7i3yZPLHDaTFofk82IlS9k3wdbr2PUDI 2KSvKtX8xzpInXdLTjD7Q== X-UI-Out-Filterresults: notjunk:1;V01:K0:UEdEQl3JJCo=:zaCrSriQdfpAT1D01IpSWk UVqvI70VX1vxGU+00RUqB+dmw4AdyCc3erD64o4abgzEV3RbGwbVr+Nfnzcs9t9id7LVhNIKv C6VIrZdR9Ghzvs1/OLLLV15oqCJ3THVBV8J4zj5q9zsjZxgjM+4aCD8rWjK5kSxvCZYXDx6Z/ pvhA6UsR2XpnzkhM62vxxecd2GD+VsUsTJRewBsxN7vUcvqzGCWo6/goXJmaD87XRU1NOaIT/ W06ACReeEkmSOdglcrgKDCBzTEWidAg44fPdzDABjeTMVpQY6AvM4+qID42XtRui4YRnldUDB k4KVHULTSFsEPemJ0szGSwH+ozHUAg0I2mpm3XnjQ2tGCCJiGAmA2EFXJFM2DrmwL/T/GqJ9k EmbYjbcfFdmHqEIjnAosrEr81W3S/8XWaG61J9zdgZrhZbxdn5aST5gRlxtGABgdFvvue7oeR CJkabodU9sOfx8XNwIrTGYJVT0oiX585neE+mamqMNX+863jPsCISCP2k9jDpuElUm6pEWDeU MDuy9waMvqu+2Mok8iL5Aa5+vwZuF+dVFjlKfy3CIG1udxTgmcJnMFBnnIpyj43lyF1PhydEq piy50cZAobr1PD3l+a7/L02XcEINiZ1fIpx7jTQ510dJ4DgaNFH7ZUxP9kgzz5sJIDVPrnqkb YdViT3oOyL1MogRbyG8XQLGp29fQ4e2mRgHh606BjIpPDV0v3Fyuxyq3ngNH1Fot/1LP0eLtp ODMBbcL+WRTu8pVFLeV9USTAblc27yLNdq9ElsvppoXDQdNKXqAv6ziPmtY= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --nextPart4311297.pNsbEbgStr Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Am Donnerstag, 15. Oktober 2015, 11:48:12 schrieb Dan Carpenter: > On Thu, Oct 15, 2015 at 10:39:02AM +0200, Marc Dietrich wrote: > > > > - if (unlikely(nvec->rx == NULL)) { > > > > + if (!unlikely(nvec->rx)) { > > > > > > This isn't right. You intented to say: > > > if (unlikely(!nvec->rx)) { > > > > > > But even better to just remove the unlikely entirely. > > > > > > if (!nvec->rx) { > > > > why? the "unlikely" is there to optimize a critical interrupt path. > > The rule is that drivers should not use likely/unlikely() unless there > is a difference in benchmark numbers. well, we know that additional cpu cycles in this path break transfer for unknown reasons. However, the unlikely here may be overkill. On the other hand, I prefer not to change something here until these timing effects are better understood. > How critical can it be when it's > always followed by a udelay(100)??? yes, this delay shouldn't be there at all. This is one of the timing mysteries we still have to resolve. > There are more important optimizations needed here. sure. We are currently trying to move all this out into the tegra-i2c driver, so this code block will get a major review/rewrite in the near future anyway. Marc --nextPart4311297.pNsbEbgStr Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part. Content-Transfer-Encoding: 7Bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAABCAAGBQJWH26bAAoJEKyeR39HFBtoxq4H/j0PQF1sItKZqmXYIqNLnlPQ vgmm/dHRhOoLajIAcGjevat/bjuewz8U73LsM0BINLDJ9Mo0Z/GS6+MbbALVN7n/ ++vyWTCDoTahoxBvK9C+qOct6X0molcnHKrU9FEen4DDmN9m6bLgKztFJtoLeQxH IPSuQDVFTXU+Uj9nxSObBPlAsRiMe488JUu3v941zjtuDjYmJqtwBSH5cIcmazn0 RBcs80WZfvEt1eaSKH3sVNkR85lnMgA5fsGTOhH24CcZQGWA8B6Cj5BJNWFB8VP3 Kpwp17B5tGBTgltMhAX4Fw7lbGWlpUY9vMR47c0FZaZs4sUqeV4U0Cs4S/TBpxk= =n9CC -----END PGP SIGNATURE----- --nextPart4311297.pNsbEbgStr--