| Message ID | 20090212084356.7a921ce0@pedra.chehab.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
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. >> - [PATCH] Add support for Winfast Dongle Hybrid >> - [PATCH] Emtec S810 (1164:2edc) support >> - [PATCH] Add Elgato EyeTV Diversity to dibcom driver > > 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. >> - Wipe out an obsolete documentation about Flexcop refactoring >> - documentation fix for /Documentation/dvb/technisat.txt >> >> The most important one is >> >> - [PATCH] software IRQ watchdog for Flexcop B2C2 DVB PCI cards > > --- a/linux/drivers/media/dvb/b2c2/flexcop.c Wed Feb 11 11:30:08 2009 +0100 > +++ b/linux/drivers/media/dvb/b2c2/flexcop.c Wed Feb 11 11:45:09 2009 +0100 > @@ -212,8 +212,7 @@ void flexcop_reset_block_300(struct flex > v210.sw_reset_210.Block_reset_enable = 0xb2; > > fc->write_ibi_reg(fc,sw_reset_210,v210); > - msleep(1); > - > + udelay(1000); > fc->write_ibi_reg(fc,ctrl_208,v208_save); > } > > 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. > 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). > Ok, after pulling it, I'll add to 2.6.29 upstream series. Thanks, Patrick. -- Mail: patrick.boettcher@desy.de WWW: http://www.wi-bw.tfh-wildau.de/~pboettch/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 12 Feb 2009 12:04:29 +0100 (CET) Patrick Boettcher <patrick.boettcher@desy.de> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- a/linux/drivers/media/dvb/b2c2/flexcop.c Wed Feb 11 11:30:08 2009 +0100 +++ b/linux/drivers/media/dvb/b2c2/flexcop.c Wed Feb 11 11:45:09 2009 +0100 @@ -212,8 +212,7 @@ void flexcop_reset_block_300(struct flex v210.sw_reset_210.Block_reset_enable = 0xb2; fc->write_ibi_reg(fc,sw_reset_210,v210); - msleep(1); - + udelay(1000); fc->write_ibi_reg(fc,ctrl_208,v208_save); }