On 10/01/2012 01:42 PM, Mauro Carvalho Chehab wrote:
Em Fri, 28 Sep 2012 21:31:07 +0300
Antti Palosaari <crope@xxxxxx> escreveu:
Hello,
Did not fix the issue. Problem remains same.
Ok, that's what I was afraid: there's likely something at drxk firmware that
it is needed for tda18271 to be visible - maybe gpio settings.
With the sleep + that patch
it works still.
Good, no regressions added.
Currently there is regression as you haven't committed that sleep patch.
IMO, we should add a defer job at dvb_attach, that will postpone the
tuner attach to happen after drxk firmware is loaded, or add there a
wait queue. Still, I think that this patch is needed anyway, in order
to avoid race conditions with CI and Remote Controller polls that may
affect devices with tda18271.
It should be easy to check if the firmware is loaded: all it is needed
is to, for example, call:
drxk_ops.get_tune_settings()
This function returns -ENODEV if firmware load fails; -EAGAIN if
firmware was not loaded yet and 0 or -EINVAL if firmware is OK.
So, instead of using sleep, you could do:
static bool is_drxk_firmware_loaded(struct dvb_frontend *fe) {
struct dvb_frontend_tune_settings sets;
int ret = fe->ops.get_tune_settings(fe, &sets);
if (ret == -ENODEV || ret == -EAGAIN)
return false;
else
return true;
};
and, at the place you coded the sleep(), replace it by:
ret = wait_event_interruptible(waitq, is_drxk_firmware_loaded(dev->dvb->fe[0]));
if (ret < 0) {
dvb_frontend_detach(dev->dvb->fe[0]);
dev->dvb->fe[0] = NULL;
return -EINVAL;
}
It might have sense to add an special callback to return the tuner
state (firmware not loaded, firmware loaded, firmware load failed).
This is stupid approach. It does not change the original behavior which
was we are not allowed to block module init path. It blocks module init
just as long as earlier, even longer, with increased code complexity!
Why the hell you want add this kind of hacks every single chip driver
that downloads firmware? Instead, put it to the bridge and leave demod,
tuner, sec, etc, drivers free.
If you put that asyncronous stuff to em28xx (with possible dev
unregister if you wish to be elegant) then all the rest sub-drivers
could be hack free.
regards
Antti
--
http://palosaari.fi/
--
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
