[linux-dvb] idea on how to break the static dependencies on demodulator modules

Andrew de Quincey adq_dvb at lidskialf.net
Tue May 16 22:40:18 CEST 2006


On Tuesday 16 May 2006 14:54, Johannes Stezenbach wrote:
> On Tue, May 16, 2006, Andrew de Quincey wrote:
> > On Tuesday 16 May 2006 12:25, Johannes Stezenbach wrote:
> > > IOW, chaining ->release() is not the problem, but having
> > > ->release() decrement the refcount of some other module is.
> >
> > Hmm, sorry, not sure what you mean here - where is one module
> > decrementing the refcount of another module? Ah, do you mean the
> > symbol_put in the current dvb_attach() ?
>
> Sorry, you published your current idea as a patch against an
> ill-conceived, discarded idea. This makes it very hard to read.
> http://linuxtv.org/hg/~quincy/v4l-dvb-attach
>
> For discussion it would be *much* better to have a small patch
> against current mainline which just does the core changes plus
> one or two example demod driver changes.
>
> Anyway:
>
> dvb_unregister_frontend():
> ...
>
> + 	while(fe->ops->release) {
> + 		struct module *m = fe->ops->release(fe);
> + 		if (m)
> + 			module_put(m);
> + 	}
> +
>
> - this way of chaining release() looks just sick
>   (is it even legal C code? can the optimizer not optimize
>   away the fe->ops->release register reload and turn this
>   into an endless loop?)

Umm, why would it do that?

> - the module_put() in there is just as wrong as doing
>   the module_put() inside release()

Fair enough.

> Really, the more I think about it, this whole chaining idea
> is garbage. Better change lnbp21.

OK, so how should we go about doing this. The problem is the lnbp21 and 
suchlike need to override methods within frontend_ops, yet also need to have 
their own release() method.

What about adding a second release_sec() method pointer to frontend_ops. This 
permits your dvb_detach() idea using symbol_put_addr(), since we will have a 
pointer to an address within the LNBP21 module.

This also has the advantage it is a minimal change as possible, and doesn't 
touch anything but the LNBP21/ISL6421 modules... plus it doesn't need 
chaining.

WDYT?



More information about the linux-dvb mailing list