[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