On Mon, Aug 6, 2012 at 2:35 PM, Devin Heitmueller <dheitmueller@xxxxxxxxxxxxxx> wrote: > On Mon, Aug 6, 2012 at 2:19 PM, Antti Palosaari <crope@xxxxxx> wrote: >> You should understand from DVB driver model: >> * attach() called only once when driver is loaded >> * init() called to wake-up device >> * sleep() called to sleep device >> >> What I would like to say is that there is very big risk to shoot own leg >> when doing some initialization on attach(). For example resuming from the >> suspend could cause device reset and if you rely some settings that are done >> during attach() you will likely fail as Kernel / USB-host controller has >> reset your device. >> >> See reset_resume from Kernel documentation: >> Documentation/usb/power-management.txt > > Be forewarned: there is a very high likelihood that this patch will > cause regressions on hybrid devices due to known race conditions > related to dvb_frontend sleeping the tuner asynchronously on close. > This is a hybrid tuner, and unless code is specifically added to the > bridge or tuner driver, going from digital to analog mode too quickly > will cause the tuner to be shutdown while it's actively in analog > mode. > > (I discovered this the hard way when working on problems MythTV users > reported against the HVR-950q). > > Description of race: > > 1. User opens DVB frontend tunes > 2. User closes DVB frontend > 3. User *immediately* opens V4L device using same tuner > 4. User performs tuning request for analog > 5. DVB frontend issues sleep() call to tuner, causing analog tuning to fail. > > This class of problem isn't seen on DVB only devices or those that > have dedicated digital tuners not shared for analog usage. And in > some cases it isn't noticed because a delay between closing the DVB > device and opening the analog devices causes the sleep() call to > happen before the analog tune (in which case you don't hit the race). > > I'm certainly not against improved power management, but it will > require the race conditions to be fixed first in order to avoid > regressions. > > Devin > > -- > Devin J. Heitmueller - Kernel Labs > http://www.kernellabs.com Devin's right. I'm sorry, please *don't* merge the patch, Mauro. Antti, you should just call sleep from your driver after attach(), as I had previously suggested. We can revisit this some time in the future after we can be sure that these race conditions wont occur. -Mike -- 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
