[linux-dvb] Re: Skystar2 misc changes committed, testing wanted

[Niklas Peinecke]

Hi Roberto,

good work, indeed!
I attached a (trivial) patch that removes two empty if-clauses and 
speeds up the pid removal (from O(n) to O(1)).

Thanks,

Niklas

Roberto Ragusa wrote:
Hi all,

I just committed a not trivial patch to skystar2.c.
Changelog:
- pid addition/removal and open_whole_bandwidth() reworked:
  add_pid() and remove_pid() handle ref count tables only;
  add_hw_pid() and remove_hw_pid() set the hardware, enabling/disabling
  whole_bandwidth when necessary;
  pid==0x2000 is considered special by  *_hw_* functions only.
- write_reg_op() replaced by write_reg_bitfield(), simpler, cleaner
    instead of:
      write_reg_op(adapter,0x456,3,0xffffe000,0x1234);
    we have now:
      write_reg_bitfield(adapter,0x456,0x00001fff,0x1234);
- better usage of u8, u16, u32, int
- changed hexadecimal values to lowercase
- improved enable_hw_filters and related log messages
- lot of coding styling fixes
The first change is a simplification/reorganization of all the
pid handling stuff. Less duplication of code, better readability.
This code is moving towards final shape, but it's not there yet.
The second change touches a lot of parts. The ideas behind it are
that it's easier to spot errors if the mask is not negated and
that the op=1,2,3 for or/and/and+or was terribly ugly.
A careful examination of the diff would be useful to detect a
wrong conversion I may have introduced somewhere.
The other changes are less critical. Note that enable_hardware_filters
defaults now to 2 (i.e., try to activate all the 6+32 filters).
The diff is somewhat inflated because of the styling fixes.
Let me know if you see something wrong or if the code behaves
incorrectly.
Thank you.

Index: skystar2.c
===================================================================
RCS file: /cvs/linuxtv/dvb-kernel/linux/drivers/media/dvb/b2c2/skystar2.c,v
retrieving revision 1.22
diff -p -u -r1.22 skystar2.c
--- skystar2.c  10 Dec 2003 00:21:31 -0000      1.22
+++ skystar2.c  10 Dec 2003 07:27:31 -0000
@@ -1146,9 +1146,8 @@ static int add_hw_pid(struct adapter *ad
        if (pid <= 0x1f)
                return 1;
 
-       if (pid == 0x2000) {
-               /* we can't use a filter, so no search */
-       } else {
+       /* we can't use a filter for 0x2000, so no search */
+       if (pid != 0x2000) {
                /* find an unused hardware filter */
                for (i = 0; i < adapter->useable_hw_filters; i++) {
                        dprintk("%s: pid=%d searching slot=%d\n", __FUNCTION__, pid, 
i);
@@ -1177,9 +1176,8 @@ static int remove_hw_pid(struct adapter 
        if (pid <= 0x1f)
                return 1;
 
-       if (pid == 0x2000) {
-               /* we can't use a filter, so no search */
-       } else {
+       /* we can't use a filter for 0x2000, so no search */
+       if (pid != 0x2000) {
                for (i = 0; i < adapter->useable_hw_filters; i++) {
                        dprintk("%s: pid=%d searching slot=%d\n", __FUNCTION__, pid, 
i);
                        if (adapter->hw_pids[i] == pid) {       // find the pid slot
@@ -1245,10 +1243,7 @@ static int remove_pid(struct adapter *ad
                        if (adapter->pid_rc[i] <= 0) {
                                // remove from the list
                                adapter->pid_count--;
-                               for (j = i; j < adapter->pid_count; j++) {
-                                       adapter->pid_list[j] = adapter->pid_list[j + 
1];
-                                       adapter->pid_rc[j] = adapter->pid_rc[j + 1];
-                               }
+                               
adapter->pid_list[i]=adapter->pid_list[adapter->pid_count];
                                // hardware setting
                                remove_hw_pid(adapter, pid);
                        }