[linux-dvb] Re: refactoring

[Andrew de Quincey <adq_dvb@lidskialf.net>]

On Wednesday 06 Oct 2004 21:42, Holger Waechtler wrote:
> Andrew de Quincey wrote:
> >On Wednesday 06 Oct 2004 21:13, Holger Waechtler wrote:
> >>Andrew de Quincey wrote:
> >>>>>If so, thats quite neat - then we don't need to export masses of extra
> >>>>>functions from all the demodulator modules - just the attach(), that
> >>>>>structure, and possibly a detach().
> >>>>
> >>>>Do we still need attach/detach()?
> >>>
> >>>You mean expose them in the structure as well? I think I'd just
> >>>automatically thought to have those two as "real" functions.. not sure
> >>>why - I just had a hunch there might be problems otherwise.
> >>
> >>mmh, no... I just don't remember off-hand if any driver implements real
> >>functionality beside probing in those functions...
> >
> >Several of them load the firmware in the attach() function. Its also a
> > good place to setup any internal state the demodulator might need.
>
> wouldn't be init() the better place for firmware uploads? Since this is
> called at wakeup after sleep it will also work for USB devices which get
> completely powered-down. It should always be possible to determine on
> the register/device state whether a firmware reload in init() is
> required, not? Is anything serious done in the detach() functions?

Now that I've removed the i2c_driver stuff, the only thing done in the 
_detach() functions is dvb_unregister_adapter(). The only reason this is 
hidden in a separate function for the moment is because you have to pass a 
pointer to the internal ioctl() function to it. When we move to a function 
pointer interface, it can be removed.

The attach() functions basically do the following:

* check if adapter is really there
* setup state (optionally)
* load firmware (optionally)
* register frontend.

As discussed, the load firmware/state setup can be moved to the init() 
function.

Since several cards have multiple possible demod/plls, and you just have to 
try one after the other to find out which, all we need now is a wee function, 
e.g. stv0299_test(). All this would do is: 

        /* check if the demod is there */
        stv0299_writereg(state, 0x02, 0x34); /* standby off */
        msleep(200);
        id = stv0299_readreg(state, 0x00);

        /* register 0x00 contains 0xa1 for STV0299 and STV0299B */
        /* register 0x00 might contain 0x80 when returning from standby */
        if (id != 0xa1 && id != 0x80) return -ENODEV;