Messages in this thread | | | From | Marc Dietrich <> | Subject | Re: [PATCH] staging: nvec: fixed few coding style warnings | Date | Thu, 15 Oct 2015 11:15:07 +0200 |
| |
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 [unhandled content-type:application/pgp-signature] | |