On Tue, 4 Jul 2006, Andrew de Quincey wrote: > I think that has changed since we first looked at this; pb changed the > frontend ops structure so that the ops functions are now always copied into > the struct dvb_frontend structure now, so they'll persist even if the module > unloads under it. _however_ this doesn't prevent the module from NULLing out The entire dvb_frontend structure is allocated and de-allocated by the frontend release and attach functions, so the ops functions still go away on release. On Thu, 6 Jul 2006, Andrew de Quincey wrote: > Great ideas guys, I've just updated the tree at: > > http://linuxtv.org/hg/~quincy/v4l-dvb-attach > > There are probably some bugs left in the patch to convert all drivers to > dvb_attach() - I've not done a final check over on that one yet. For the write() op patch, how about changing this: static inline int cx24110_pll_write(struct dvb_frontend *fe, u32 val) { int r = 0; u8 buf[] = {(u8) (val>>24), (u8) (val>>16), (u8) (val>>8)}; if (fe->ops.write) r = fe->ops.write(fe, buf, 3); return r; } to this: static inline int cx24110_pll_write(struct dvb_frontend *fe, u32 val) { u8 buf[] = {(u8)(val>>24), (u8)(val>>16), (u8)(val>>8)}; BUG_ON(!fe->ops.write); return fe->ops.write(fe, buf, 3); } It seems the only way ops.write could be NULL is if cx24110_pll_write() is called on a non-cx24110 frontend, which is surely a bug. The same thing goes for mt352_write(), stv0299_writereg(), and tda1004x_writereg(). Or, just change the one function in dvb-bt8xx.c that calls cx24110_pll_write() to use fe->ops.write() directly and get rid of the whole cx24110_pll_write() function. Is the bit that changes tda1004x_release() into two identical functions a mistake? It doesn't seem to have anything to do with the rest of the patch. For the tda1004x, the .write op isn't set to tda1004x_write. It looks like zl10353_write() could just be un-exported without putting it in .write, as nothing outside of zl10353.c ever calls it. In the Kconfig patch, I'd get rid of the extra checks for the kernel version. The requirement of kernel 2.6.17 in versions.txt for DVB_CORE_ATTACH will take of everything. What's the point of versions.txt if you're just going to stick a version check in the code anyway? If someone patches an older kernel so symbol_put_addr() works and then overrides versions.txt, dvb_attach should actually be on when they turn in on. I'm pretty sure that lines like this will screw up Mauro's script to make the git version of the sources: #if ((LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,17)) && defined(CONFIG_DVB_CORE_ATTACH)) Is the removal of "depends on DVB_CORE" for DVB_STV0299 a mistake? Only allowing FE customization when DVB_CORE_ATTACH is on seems backward. Customizing which frontends are built makes the most sense when you still have static dependencies or are not even using modules. There is no reason you can't customize both with and without dvb_attach, so why disable it? _______________________________________________ linux-dvb@xxxxxxxxxxx http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
