rcu NPTL fix for 1.3.34

Message ID 43A2D4A3.2020701@cadsoft.de
State New
Headers

Commit Message

Klaus Schmidinger Dec. 16, 2005, 2:52 p.m. UTC
  Andreas Share wrote:
>> [ ... rcu.c vs. NPTL ... ]
> no problem for me, and because i think most other user uses lirc or the 
> remote plugin to control vdr it is only a minor problem. So i still use 
> rcu without the locks and wait until you use a NPTL enabled linux;)

Ok, so I finally switched to kernel 2.6 and the dvb-kernel driver
for good, and now I get the problems with rcu.c, too.

I have modified rcu.c so that it now does all actual data
transmission in the Action() function, so no more locking
is necessary. Simply dropping the locking caused lots of
errors like

  ERROR (rcu.c,188): Invalid argument

when background and foreground thread tried to access the
interface at the same time.

Maybe you'd like to take a look at the attached patch.

Klaus
  

Comments

Andreas Share Dec. 17, 2005, 8:18 p.m. UTC | #1
> Andreas Share wrote:
>>> [ ... rcu.c vs. NPTL ... ]
>> no problem for me, and because i think most other user uses lirc or the
>> remote plugin to control vdr it is only a minor problem. So i still use
>> rcu without the locks and wait until you use a NPTL enabled linux;)
>
> Ok, so I finally switched to kernel 2.6 and the dvb-kernel driver
> for good, and now I get the problems with rcu.c, too.
>
> I have modified rcu.c so that it now does all actual data
> transmission in the Action() function, so no more locking
> is necessary. Simply dropping the locking caused lots of
> errors like
>
>  ERROR (rcu.c,188): Invalid argument
>
> when background and foreground thread tried to access the
> interface at the same time.
>
> Maybe you'd like to take a look at the attached patch.
>
> Klaus

I have had the same error messages here, but i have not seen any side effect 
the last month. But your solution is the better way, i have tested it with 
NPTL, including key-learnig, works fine:)

thank you

Andreas
  

Patch

--- rcu.h	2005/07/31 10:18:00	1.4
+++ rcu.h	2005/12/16 14:21:20
@@ -19,19 +19,19 @@ 
   enum { modeH = 'h', modeB = 'b', modeS = 's' };
   int f;
   unsigned char dp, code, mode;
-  int numberToSend;
-  int lastNumber;
+  int number;
+  unsigned int data;
   bool receivedCommand;
   bool SendCommand(unsigned char Cmd);
   int ReceiveByte(int TimeoutMs = 0);
   bool SendByteHandshake(unsigned char c);
   bool SendByte(unsigned char c);
-  bool Digit(int n, int v);
-  bool SetCode(unsigned char Code);
-  bool SetMode(unsigned char Mode);
-  bool Number(int n, bool Hex = false);
+  bool SendData(unsigned int n);
+  void SetCode(unsigned char Code);
+  void SetMode(unsigned char Mode);
+  void SetNumber(int n, bool Hex = false);
   void SetPoints(unsigned char Dp, bool On);
-  bool String(char *s);
+  void SetString(char *s);
   bool DetectCode(unsigned char *Code);
   virtual void Action(void);
   virtual void ChannelSwitch(const cDevice *Device, int ChannelNumber);
--- rcu.c	2005/08/15 12:30:21	1.10
+++ rcu.c	2005/12/16 14:43:37
@@ -23,8 +23,8 @@ 
   dp = 0;
   mode = modeB;
   code = 0;
-  numberToSend = -1;
-  lastNumber = 0;
+  number = 0;
+  data = 0;
   receivedCommand = false;
   if ((f = open(DeviceName, O_RDWR | O_NONBLOCK)) >= 0) {
      struct termios t;
@@ -32,7 +32,7 @@ 
         cfsetspeed(&t, B9600);
         cfmakeraw(&t);
         if (tcsetattr(f, TCSAFLUSH, &t) == 0) {
-           Number(0);//XXX 8888???
+           SetNumber(8888);
            const char *Setup = GetSetup();
            if (Setup) {
               code = *Setup;
@@ -95,13 +95,12 @@ 
 
   time_t LastCodeRefresh = 0;
   cTimeMs FirstTime;
+  unsigned char LastCode = 0, LastMode = 0;
   uint64 LastCommand = 0;
+  unsigned int LastData = 0;
   bool repeat = false;
 
   while (Running() && f >= 0) {
-
-        LOCK_THREAD;
-
         if (ReceiveByte(REPEATLIMIT) == 'X') {
            for (int i = 0; i < 6; i++) {
                int b = ReceiveByte();
@@ -140,11 +139,22 @@ 
            LastCommand = 0;
            }
         else {
-           LastCommand = 0;
-           if (numberToSend >= 0) {
-              Number(numberToSend);
-              numberToSend = -1;
+           unsigned int d = data;
+           if (d != LastData) {
+              SendData(d);
+              LastData = d;
               }
+           unsigned char c = code;
+           if (c != LastCode) {
+              SendCommand(c);
+              LastCode = c;
+              }
+           unsigned char m = mode;
+           if (m != LastMode) {
+              SendCommand(m);
+              LastMode = m;
+              }
+           LastCommand = 0;
            }
         if (code && time(NULL) - LastCodeRefresh > 60) {
            SendCommand(code); // in case the PIC listens to the wrong code
@@ -192,8 +202,6 @@ 
 
 bool cRcuRemote::SendByte(unsigned char c)
 {
-  LOCK_THREAD;
-
   for (int retry = 5; retry--;) {
       if (SendByteHandshake(c))
          return true;
@@ -201,16 +209,24 @@ 
   return false;
 }
 
-bool cRcuRemote::SetCode(unsigned char Code)
+bool cRcuRemote::SendData(unsigned int n)
+{
+  for (int i = 0; i < 4; i++) {
+      if (!SendByte(n & 0x7F))
+         return false;
+      n >>= 8;
+      }
+  return SendCommand(mode);
+}
+
+void cRcuRemote::SetCode(unsigned char Code)
 {
   code = Code;
-  return SendCommand(code);
 }
 
-bool cRcuRemote::SetMode(unsigned char Mode)
+void cRcuRemote::SetMode(unsigned char Mode)
 {
   mode = Mode;
-  return SendCommand(mode);
 }
 
 bool cRcuRemote::SendCommand(unsigned char Cmd)
@@ -218,15 +234,9 @@ 
   return SendByte(Cmd | 0x80);
 }
 
-bool cRcuRemote::Digit(int n, int v)
-{ 
-  return SendByte(((n & 0x03) << 5) | (v & 0x0F) | (((dp >> n) & 0x01) << 4));
-}
-
-bool cRcuRemote::Number(int n, bool Hex)
+void cRcuRemote::SetNumber(int n, bool Hex)
 {
-  LOCK_THREAD;
-
+  number = n;
   if (!Hex) {
      char buf[8];
      sprintf(buf, "%4d", n & 0xFFFF);
@@ -237,19 +247,17 @@ 
          n = (n << 4) | ((*d - '0') & 0x0F);
          }
      }
-  lastNumber = n;
+  unsigned int m = 0;
   for (int i = 0; i < 4; i++) {
-      if (!Digit(i, n))
-         return false;
+      m <<= 8;
+      m |= ((i & 0x03) << 5) | (n & 0x0F) | (((dp >> i) & 0x01) << 4);
       n >>= 4;
       }
-  return SendCommand(mode);
+  data = m;
 }
 
-bool cRcuRemote::String(char *s)
+void cRcuRemote::SetString(char *s)
 {
-  LOCK_THREAD;
-
   const char *chars = mode == modeH ? "0123456789ABCDEF" : "0123456789-EHLP ";
   int n = 0;
 
@@ -262,7 +270,7 @@ 
              }
           }
       }
-  return Number(n, true);
+  SetNumber(n, true);
 }
 
 void cRcuRemote::SetPoints(unsigned char Dp, bool On)
@@ -271,7 +279,7 @@ 
      dp |= Dp;
   else
      dp &= ~Dp;
-  Number(lastNumber, true);
+  SetNumber(number);
 }
 
 bool cRcuRemote::DetectCode(unsigned char *Code)
@@ -291,12 +299,12 @@ 
      SetMode(modeH);
      char buf[5];
      sprintf(buf, "C0D%c", *Code);
-     String(buf);
+     SetString(buf);
      SetCode(*Code);
      cCondWait::SleepMs(2 * REPEATDELAY);
      if (receivedCommand) {
         SetMode(modeB);
-        String("----");
+        SetString("----");
         return true;
         }
      if (*Code < 'D') {
@@ -310,10 +318,8 @@ 
 
 void cRcuRemote::ChannelSwitch(const cDevice *Device, int ChannelNumber)
 {
-  if (ChannelNumber && Device->IsPrimaryDevice()) {
-     LOCK_THREAD;
-     numberToSend = cDevice::CurrentChannel();
-     }
+  if (ChannelNumber && Device->IsPrimaryDevice())
+     SetNumber(cDevice::CurrentChannel());
 }
 
 void cRcuRemote::Recording(const cDevice *Device, const char *Name)