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
-
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]
