On Sun, 2002-07-14 at 00:27, Oded Arbel wrote:
> Got all that - but still I think the logic you imploy is not correct: 
> since CRTSCTS is already being set for the modem device, its kind of 
> silly to set it and then disable it again - instead, simply enable it if 
> needed (disable-crtscts not being set). I submit exhibit A - smsc_at2 
> disable-crtscts patch, the Oded way ;-)
> 
> -- 
> Oded Arbel
> m-Wise mobile solutions
> 
> 
> ----
> 

> --- gw/smsc_at2.c     2002-06-06 17:09:24.000000000 +0000
> +++ gw/smsc_at2.c     2002-07-13 14:22:47.000000000 +0000
> @@ -79,7 +79,8 @@
>      tios.c_oflag &= ~ONLCR; /* no NL to CR-NL mapping outgoing */
>      tios.c_iflag |= IGNPAR; /* ignore parity */
>      tios.c_iflag &= ~INPCK;
> -    tios.c_cflag |= CRTSCTS; /* enable hardware flow control */
> +    if (privdata->modem->use_crtscts)
> +     tios.c_cflag |= CRTSCTS; /* enable hardware flow control */
>      tios.c_cc[VSUSP] = 0; /* otherwhise we can not send CTRL Z */
>  
>      /*
> @@ -2053,6 +2131,9 @@
>          modem->message_storage = cfg_get(grp, octstr_imm("message-storage"));
>  
>          cfg_get_bool(&modem->enable_mms, grp, octstr_imm("enable-mms"));
> +     
> +     cfg_get_bool(&modem->use_crtscts, grp, octstr_imm("disable-crtscts"));
> +     modem->use_crtscts = !modem->use_crtscts;
>  
>          /*   
>          if (modem->message_storage == NULL)

On second thought, let me be more specific in pointing out the problem
with your code.

The function "at2_open_device" sets the terminal flags by first reading
the existing settings. Then it turns certain bits on or off depending on
what it requires. If I wish to turn CRTSCTS off, it may not work because
according to the logic of your code, you only turn it on if you require
it. What happens if CRTSCTS is on to start with? There is nothing to
turn it off when I want it off!

My original patch works. Perhaps the only change I could make is this:

*** smsc_at2.c  Sat Jul 13 22:01:58 2002
--- smsc_at2.c.new      Sat Jul 13 22:02:48 2002
***************
*** 79,90 ****
      tios.c_oflag &= ~ONLCR; /* no NL to CR-NL mapping outgoing */
      tios.c_iflag |= IGNPAR; /* ignore parity */
      tios.c_iflag &= ~INPCK;
-     tios.c_cflag |= CRTSCTS; /* enable hardware flow control */
      tios.c_cc[VSUSP] = 0; /* otherwhise we can not send CTRL Z */
  
      if (privdata->modem->disable_crtscts) {
          tios.c_cflag &= ~CRTSCTS; /* disable hardware flow control */
      }
  
      /*
      if ( ModemTypes[privdata->modemid].enable_parity )
--- 79,92 ----
      tios.c_oflag &= ~ONLCR; /* no NL to CR-NL mapping outgoing */
      tios.c_iflag |= IGNPAR; /* ignore parity */
      tios.c_iflag &= ~INPCK;
      tios.c_cc[VSUSP] = 0; /* otherwhise we can not send CTRL Z */
  
      if (privdata->modem->disable_crtscts) {
          tios.c_cflag &= ~CRTSCTS; /* disable hardware flow control */
      }
+     else {
+         tios.c_cflag |= CRTSCTS; /* enable hardware flow control */
+     }
  
      /*
      if ( ModemTypes[privdata->modemid].enable_parity )

But that is fairly minor and does not affect performance. This is really
nit-picking. I will say no more of this.

Regards,
Eric Yeo



Reply via email to