On Mon, Aug 6, 2012 at 3:13 PM, Antti Palosaari <crope@xxxxxx> wrote: > On 08/06/2012 09:57 PM, Michael Krufky wrote: >> >> 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. > > > OK, maybe it is safer then. I have no any hybrid hardware to test... > > Anyhow, I wonder how many years it will take to resolve that V4L2/DVB API > hybrid usage pŕoblem. I ran thinking that recently when looked how to > implement DVB SDR for V4L2 API... I could guess problem will not disappear > near future even analog TV disappears, because there is surely coming new > nasty features which spreads over both V4L2 and DVB APIs. Guys, Please take another look at this branch and test if possible -- I pushed an additional patch that takes Devin's concerns into account. After applying these patches, the tda18271 driver will behave as it always has, but it will sleep the tuner after attaching the first instance. If there is only one instance, then this works exactly as Antti desires. If there are more instances, then the tuner will only be woken up again during attach if the tda18271_need_cal_on_startup() returns non-zero. The driver does not attempt to re-sleep the hardware again during successive attach() calls. I have not tested this yet myself, but I believe it resolves the matter -- please comment. Regards, Mike Krufky -- 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
