Re: never do symbol_put(tunerfoo_attach);

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Michel Ludwig wrote:
> On Fri 16 Nov 2007, you wrote:
>   
>> We like to make linuxtv drivers modular and forward compatible.  There
>> _are_ devices out there that use other tuners, and if you hardcode
>> xc3028 into this driver, it prevents future developers from adding
>> support for other devices without having to change existing code.
>>     
>
> Sure, but no one said that tm6000-dvb is ready to go into the main tree...
>   

I saw an issue and I felt it was appropriate for me to point it out 
immediately.

Better to fix a problem when it is noticed, no?

>   
>>> But anyway, how would dvb_frontend_detach(fe) release the xc3028_attach
>>> symbol, which is requested by dvb_attach?
>>>       
>> The answer is self-explanatory.  Take a look at the other drivers, and
>> take a look at dvb_frontend_detach.   (dvb_frontend.c , lines 1204 thru
>> 1221)
>>     
>
> Of course, I have done that, but it is still not clear to me. Here are the
two 
> macros:
>
> #define dvb_attach(FUNCTION, ARGS...) ({ \
> 	void *__r = NULL; \
> 	typeof(&FUNCTION) __a = symbol_request(FUNCTION); \
> 	if (__a) { \
> 		__r = (void *) __a(ARGS); \
> 		if (__r == NULL) \
> 			symbol_put(FUNCTION); \
> 	} else { \
> 		printk(KERN_ERR "DVB: Unable to find symbol
"#FUNCTION"()\n"); \
> 	} \
> 	__r; \
> })
>
> void dvb_frontend_detach(struct dvb_frontend* fe)
> {
> 	void *ptr;
>
> 	if (fe->ops.release_sec) {
> 		fe->ops.release_sec(fe);
> 		symbol_put_addr(fe->ops.release_sec);
> 	}
> 	if (fe->ops.tuner_ops.release) {
> 		fe->ops.tuner_ops.release(fe);
> 		symbol_put_addr(fe->ops.tuner_ops.release);
> 	}
> 	ptr = (void*)fe->ops.release;
> 	if (ptr) {
> 		fe->ops.release(fe);
> 		symbol_put_addr(ptr);
> 	}
> }
>
> dvb_attach is called with xc2028_attach, hence it will request
xc2028_attach. 
> But dvb_frontend_detach will release xc2028_dvb_release.
>   
It's the same module.
> I can try this out tomorrow, but if I have to reboot my machine after that
to 
> unload tuner-xc2028, then there is definitely a flaw in this approach...
:-)
>   
This is the accepted and proven method used across the entire dvb tree.  
If it doesnt work for you, then something is wrong in your code.  I 
don't think it will cause any problem.

Hey, I was just trying to point out a flaw in your patch -- Nobody is an 
expert, and there is always something to be learned.

-Mike

P.S. Why are you guys dropping cc's?  I cc'd linux-dvb mailing list 
because this is a development issue.  It is important for such 
discussions to be documented, because it is the only way for future 
developers to learn from our mistakes.  To conduct all emails in private 
will not lead to any progression.  This is a community, and we should 
all be willing to give and accept advice from each other.

_______________________________________________
linux-dvb mailing list
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

[Index of Archives]     [Linux Media]     [Video 4 Linux]     [Asterisk]     [Samba]     [Xorg]     [Xfree86]     [Linux USB]

  Powered by Linux