[asterisk-dev] DAHDI_CHECK_HOOKSTATE and setting rxisoffhook

[Shaun Ruffell]

On 09/13/2009 09:18 AM, Tzafrir Cohen wrote:
> The following has been a subject of a bug report or two, but sadly I
> still fail to understand the rationale of them.
>
> The following is code from available() in chan_dahdi.c . This function
> returns 1 if a channel can be used for an outgoing call or 0 otherwise.
>
> Code from 1.4.21 (chan_zap.c):
>
>                          if (!p->sig || (p->sig == SIG_FXSLS))
>                                  return 1;
>                          /* Check hook state */
>                          if (p->subs[SUB_REAL].zfd>  -1)
>                                  res = ioctl(p->subs[SUB_REAL].zfd, ZT_GET_PARAMS
>                          else {
>                                  /* Assume not off hook on CVRS */
>                                  res = 0;
>                                  par.rxisoffhook = 0;
>                          }
>                          if (res) {
>                                  ast_log(LOG_WARNING, "Unable to check hook state
>                          } else if ((p->sig == SIG_FXSKS) || (p->sig == SIG_FXSGS
>                                  /* When "onhook" that means no battery on the li
>                                    it is out of service..., if it's on a TDM card
>                                    bank, there is no telling... */
>                                  if (par.rxbits>  -1)
>                                          return 1;
>                                  if (par.rxisoffhook)
>                                          return 1;
>                                  else
> #ifdef ZAP_CHECK_HOOKSTATE
>                                          return 0;
> #else
>                                          return 1;
> #endif
>                          } else if (par.rxisoffhook) {
>                                  ast_log(LOG_DEBUG, "Channel %d off hook, can't u
>                                  /* Not available when the other end is off hook
>                                  return 0;
>                          }
>                  }
>                  return 1;
>
> As you can see, '0' may only be returned here if ZAP_CHECK_HOOKSTATE is
> defined or if the signalling is not fxs_ls, fxs_ks or fxs_gs (that is:
> not an FXO with LS, KS or GS)
>
> Code in recent 1.4 is very similar.
>
>
> The code in 1.6.0, however, is slightly different:
>
>                          if (!p->sig || (p->sig == SIG_FXSLS))
>                                  return 1;
>                          /* Check hook state */
>                          if (p->subs[SUB_REAL].dfd>  -1) {
>                                  memset(&par, 0, sizeof(par));
>                                  res = ioctl(p->subs[SUB_REAL].dfd, DAHDI_GET_PAR
>                          } else {
>                                  /* Assume not off hook on CVRS */
>                                  res = 0;
>                                  par.rxisoffhook = 0;
>                          }
>                          if (res) {
>                                  ast_log(LOG_WARNING, "Unable to check hook state
>                          } else if ((p->sig == SIG_FXSKS) || (p->sig == SIG_FXSGS
>                                  /* When "onhook" that means no battery on the li
>                                    it is out of service..., if it's on a TDM card
>                                    bank, there is no telling... */
>                                  if (par.rxbits>  -1)
>                                          return 1;
>                                  if (par.rxisoffhook)
>                                          return 1;
>                                  else
>                                          return 0;
>                          } else if (par.rxisoffhook) {
>                                  ast_debug(1, "Channel %d off hook, can't use\n",
>                                  /* Not available when the other end is off hook
> #ifdef DAHDI_CHECK_HOOKSTATE
>                                  return 0;
> #else
>                                  return 1;
> #endif
>                          }
>                  }
>                  return 1;
>
> As you can see, this code may also return 0 (fail) if signalling was
> fxs_ks or fxs_gs, if the rxisoffhook flag is not set. This flag gets
> set by asking the driver.
>
> So far I have failed to get any explanation as to why this test is
> needed. It is said to be some issue with channel bank, but when I asked
> how it can be reproduced, I failed to get more information.

I too am not sure why this change was made in user space (but I also 
have not looked up the 'svn blame' to see what the commit message was 
when it changed) and would be happy to see this issue resolved in user 
space without requiring an update to dahdi-linux.

I am of the opinion that this change still does reveal a bug in the 
drivers, which is that a spanconfig should cause the drivers to generate 
events for their detected hook states and this wasn't happening on the 
analog modules and not forget their current hookstate.

>
> The impact of this issue is that FXO interfaces fail to work with
> Asterisk>= 1.6.0 unless set to use LS signalling or unless there is an
> incoming call before.
>
> There is a related issue that rxisoffhook is not properly reported for
> various drivers. It gets reset to an "uninitialized" value after
> dahdi_cfg but the low-level driver has no decent way of setting it, as
> it is called right after the optional span startup hook.
>
> Sean Ruffel fixed the Digium analog drivers to properly set it. However
> as he could not use any hook, he had to check for an unintialized state
> at the code run at interrupt time. I don't think this is such a good
> idea to add extra code at this place, which is the most timing critical
> part of the driver. I would prefer to just avoid testing it in Asterisk
> if I don't have to.

I agree that an extra check in the interrupt handler is not ideal.  My 
intent is to move most of the polling of the module registers out of the 
interrupt handler anyway in a fashion similar to what was done in the 
wcte12xp driver in r6525[1] before the 2.3 release of DAHDI.  But I 
agree that some sort of way to register a callback that will allow 
dahdi-base to ask the board drivers for current hookstate might be cleaner.

[1] http://svn.asterisk.org/view/dahdi?view=revision&revision=6525.

-- 
Shaun Ruffell
Digium, Inc. | Linux Kernel Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at: www.digium.com & www.asterisk.org