[linux-dvb] [RFC | PATCHES] dibusb-mb / dvb-pll: TESTERS NEEDED!
Michael Krufky
mkrufky at linuxtv.org
Tue Jun 12 21:22:42 CEST 2007
Trent,
Trent Piepho wrote:
> On Tue, 12 Jun 2007, Michael Krufky wrote:
>
>> I have finalized the 'removal of static dependencies on the dvb-pll
>> module' changesets, but had to change some things inside the dvb-pll
>> framework in the process. See my "dvb-pll" tree for the topmost changesets:
>>
>> http://linuxtv.org/hg/~mkrufky/dvb-pll
>>
>>
> Looks good. I'd use a enum for the tuner id, but that's not a big deal.
>
Thank you for the review.
I'd like to stick with the integer #define, for debugging purposes....
I plan to add a patch in the future, that will allow forcing a
particular dvb_pll_desc_id as a module option, for the sake of testing
new card variations without the need for writing code /
recompiling..... This will be especially handy for cases where a vendor
may release a card using a different tuner, while not changing the
devices subsystem id, as we have seen numerous times before. When I do
this, I intend to include the text "(for debugging purposes only)" , as
users should not use this regularly -- it is only to handle debugging /
testing for the above situation.
> There is one thing that should be changed:
>
> +unsigned const int pll_count = ARRAY_SIZE(pll_list);
>
> This should at least be made static, otherwise there really will be a
> global variable in memory with pll_count in it. Some other source file
> could use it, so gcc can't delete it. I'd probably make it a macro, or
> just change:
>
I meant to mark this static -- my mistake. However, instead of marking
it static, I will apply the change that you've suggested below -- this
is much cleaner, IMHO.
> + BUG_ON(pll_desc_id < 1 || pll_desc_id > pll_count);
> to
> + BUG_ON(pll_desc_id < 1 || pll_desc_id > ARRAY_SIZE(pll_list));
>
> Also, is there a reason for DVB_PLL_UNDEFINED? It doesn't seem like there
> any way to make use of it.
>
I don't know if it is really needed, but I just wanted to reserve the
'0' as a default, uninitialized setting. I did this for the sake of
future-compatible cleanliness....... We have an undefined setting for
tuner.ko, and all the v4lfoo-cards.c arrays, I only thought it natural
to do the same here....... Should I remove it? I think it's safe to
leave as-is, but if it's really just a waste of space then it can go.......
Anyhow, I altered the topmost changeset as per your suggestion, above,
and pushed it here:
http://linuxtv.org/hg/~mkrufky/dvb-pll
I'd like to include your S-O-B on the changeset before requesting the
pull to Mauro. If you're OK with the changeset as-is, please respond to
this email with your sign-off.
Thanks again for the review.
Regards,
Mike
More information about the linux-dvb
mailing list