Re: idea on how to break the static dependencies on demodulator modules

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

 



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

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

  Powered by Linux