On Thu, 12 Feb 2009 12:04:29 +0100 (CET) Patrick Boettcher <patrick.boettcher@xxxxxxx> wrote: > Hi Mauro, > > thanks for your review. > > On Thu, 12 Feb 2009, Mauro Carvalho Chehab wrote: > > I'm assuming that you want me to pull from http://linuxtv.org/hg/~pb/v4l-dvb/, right? > > > > Please, next time specify the pull url, since people may have more than one tree there. > > Exactly. I only have one repository. I tried to avoid redundant > information :). > > *ahem* > > Not true: I forgot to mention it, sorry. :) No problem. > > Hmm... > > September, 2008. For sure this were already sent upstream. We should break this > > into two separate patches, and send the fix patch upstream. Could you please do > > it? > > Forget those patches for now. I'll check on it again. Ok. > > Hmm... is it really necessary to have a 1ms udelay here? As you know, udelay() > > will run a do-nothing loop blocking the CPU until it finishes, while msleep() > > calls schedule(), letting the processor to do something else while waiting. > > As this function is called from within a spin_lock-protected area (inside > a delayed_work) I have no choice, because calling schedule() is not > allowed when being (close to) IRQ-level. Ah, ok. In this case, you can't use a sleep function, so udelay() is fine. Yet, waiting for 1ms seems to much on my eyes, especially if this code is called to often. > > There are very few cases where udelay() should be used: when the time should be > > very precise. For most cases, msleep() do a better job. > > It might be, that his delay here is unnecessary, but I'm not sure. That's > why I'd like to keep the udelay here for now, for 2.6.29 and maybe remove > the delay entirely for 2.6.30 and see what the testers are reporting. > (locally I will remove it for personal testing, but it will takes some > weeks before I can be sure). Seems fine to me. Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
