On Thursday 06 July 2006 01:57, Trent Piepho wrote: > 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. Cool will check up on these and get back to you. > 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? I thought about that, but remembered someone could do "make kernel-links" against a pre-2.6.17 kernel - and that will bypass versions.txt - so I left the version check in. > 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)) duh yes. I was trying NOT to do that; too many late nights this week :) > Is the removal of "depends on DVB_CORE" for DVB_STV0299 a mistake? yup. > 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? For static building, would that not require spreading #ifdefs everywhere throughout the code? I really don't want to do that. _______________________________________________ linux-dvb@xxxxxxxxxxx http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
