-------- Original-Nachricht --------
Datum: Thu, 03 May 2007 19:15:32 +0400
Von: Manu Abraham <[email protected]>
An: Mauro Carvalho Chehab <[email protected]>
CC: [email protected], Trent Piepho <[email protected]>, [email protected]
Betreff: Re: [linux-dvb] Re: DST/BT878 module customization (.. was: Critical points about ...)
> Mauro Carvalho Chehab wrote:
> > Em Qua, 2007-05-02 às 04:10 -0700, Trent Piepho escreveu:
> >> On Tue, 1 May 2007, Mauro Carvalho Chehab wrote:
> >
> >>> However, when dst is selected, I got those errors:
> >> When I made this patch I was basing it off a patch I made around 9
> months
> >> ago. I thought since that one was ok, this would be ok too, and in my
> >> hurry to get it done I've clearly put too little effort into checking
> it,
> >> and I'm sorry to have wasted your time.
> >>
> >> I promise, this time it's right!
> >> http://linuxtv.org/hg/~tap/dst-new
> >
> > Confirmed. Now the patch is properly working. My tests were done with a
> > board with DST. Those are the results:
> >
> > 1) when DST is unselected, on a board with DST, it will print the errors
> > indicating that the Kconfig items were not selected:
> >
> > DVB: registering new adapter (bttv0).
> > DVB: Unable to find symbol dst_attach()
> > frontend_init: Could not find a Twinhan DST.
> > dvb-bt8xx: A frontend driver was not found for device 109e/0878
> subsystem fbfb/f800
> >
> > The only issue is the wrong printk msg, stating that a "frontend driver"
> > were not found. As this issue also happens with the current driver, due
> > the usage of dvb_attach() macro, I don't see any regressions.
> >
> > It would be nice, however, to have a patch making dvb_attach more
> > generic, by e.g. having a variant that allows passing another message.
> >
> > Trying to run dvb programs like scan and kaffeine will properly fail.
> >
> > 2) With DST selected, the driver works properly.
> >
> > The changes also solved the issue of removing the dst drivers. Before
> > the patch, sometimes it is required to force removal of dst module with
> > the '-f' flag, since the module count were wrong.
> >
> > ---
> >
> > I'll risk to make a briefing of the technical points:
> >
> > <SUMMARY>
> > Current issues:
> > 1) allow deselecting DST/DST_CA support, since this is not needed by
> > some supported boards;
> > 2) sometimes, module removal don't work with dst, since usage count
> > becomes wrong;
> > 3) if dst or dst_ca is not properly loaded, the printk message wrongly
> > indicates that "A frontend driver was not found"
> >
> > The Trent's original patch were written on Aug, 2006 (*). The current
> > version fixes (1) and (2). It doesn't touch on (3). There aren't any
> > other patches properly fixing (1) and (2).
> >
> > There's an argument against the prototype changes on dst_attach and
> > dst_ca_attach since they aren't frontend.
> > </SUMMARY>
> >
> > (*) Notice: I can't remember why the patches were not applied on that
> > time. I think somebody nacked they, although I didn't found any record
> > about the patch on my mailbox.
> >
> > About the usage of frontend support for a non-frontend module, I really
> > can't see any issues at the current implementation, since even before
> > the patch, all DST initialization are done by a routine called
> > "frontend_init()".
>
> *Wrong*. Frontend Init sends a command to the whole system to
> initialize the frontend, Not initialize the DST
> Whatever frontend naming conventions are in there are a relic from
> Andrew's FE_REFACTORING changesets.
>
> Even dst not being a frontend, the initialization
> > gets the result of the attachment process, using it as a "frontend":
> >
> > state = kmalloc(sizeof (struct dst_state), GFP_KERNEL);
> >
> > state->config = &dst_config;
> > state->i2c = card->i2c_adapter;
> > state->bt = card->bt;
> > state->dst_ca = NULL;
> >
> > dvb_attach(dst_attach, state, &card->dvb_adapter);
> >
> > card->fe = &state->frontend;
> >
> > (comments and error checks removed to make code cleaner)
> >
> > The patch basically moved state initialization to dst_attach. The above
> > code, after the patch is:
> >
> > card->fe = dvb_attach(dst_attach, &dst_config, card->bt,
> card->i2c_adapter);
> >
> > So, I didn't noticed any regressions by running the modified driver nor
> > I couldn't identify any regressions by inspecting the source code.
> >
> > The end result seems also cleaner and easier to understand, preserving
> > all characteristics of the original code. Also, it uses dvb_attach the
> > same way as the other dvb helper modules.
> >
> > If later any changes will be needed to allow using DST on different
> > configurations, future patches probably will need to move dst
> > initialization to other places, and use different callbacks for it,
> > altering the above initialization. I can't see why those changes would
> > possibly avoid any further changes at the driver.
> >
> > That's said, I intend to commit the two patches, fixing the current
> > issues, at this weekend.
> >
>
> I did explain my stand in a previous mail.
>
> NACK
Technically may be OK, psychologically this decision is desasterous. Why don't you learn, for god's sake?
>
>
> _______________________________________________
> linux-dvb mailing list
> [email protected]
> http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
--
"Feel free" - 10 GB Mailbox, 100 FreeSMS/Monat ...
Jetzt GMX TopMail testen: http://www.gmx.net/de/go/topmail
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
