RFA: pause after sending S-records to ROM68K monitor

RFA: pause after sending S-records to ROM68K monitor

[Jim Blandy]

This patch creates another use of baud_rate (as set by `set baud').
I'm not sure that's such a hot idea.

But it does make `target rom68k' usable over an SSH tunnel to a
portmaster; without the patch, GDB transmits the entire program
immediately (I guess it gets absorbed by all the buffers between
itself and the real board), and then times out.

2001-08-08  Jim Blandy  <jimb@redhat.com>

	* rom68k-rom.c (init_rom68k_cmds): Request that monitor.c pause
	after sending each S-record.
	* monitor.h (MO_SREC_ACK_DELAY): New flag.
	* monitor.c: #include <sys/time.h>.
	(monitor_wait_srec_ack): If MO_SREC_ACK_DELAY is set, pause long
	enough for the record to be transmitted, according to baud_rate.
	* dsrec.c (load_srec): Change WAITACK function to take an
	argument: the number of bytes transmitted.
	* srec.h (load_srec): Update prototype.

Index: gdb/dsrec.c
===================================================================
RCS file: /cvs/src/src/gdb/dsrec.c,v
retrieving revision 1.11
diff -c -r1.11 dsrec.c
*** gdb/dsrec.c	2001/07/15 20:34:13	1.11
--- gdb/dsrec.c	2001/08/09 00:45:45
***************
*** 42,53 ****
     various random flags, and HASHMARK is non-zero to cause a `#' to be
     printed out for each record loaded.  WAITACK, if non-NULL, is a
     function that waits for an acknowledgement after each S-record, and
!    returns non-zero if the ack is read correctly.  */
  
  void
  load_srec (struct serial *desc, const char *file, bfd_vma load_offset,
  	   int maxrecsize,
! 	   int flags, int hashmark, int (*waitack) (void))
  {
    bfd *abfd;
    asection *s;
--- 42,55 ----
     various random flags, and HASHMARK is non-zero to cause a `#' to be
     printed out for each record loaded.  WAITACK, if non-NULL, is a
     function that waits for an acknowledgement after each S-record, and
!    returns non-zero if the ack is read correctly; its argument is the
!    number of characters sent to the monitor for this record, for use
!    in computing delays (if necessary).  */
  
  void
  load_srec (struct serial *desc, const char *file, bfd_vma load_offset,
  	   int maxrecsize,
! 	   int flags, int hashmark, int (*waitack) (int bytes_sent))
  {
    bfd *abfd;
    asection *s;
***************
*** 126,132 ****
  		  if (ui_load_progress_hook (section_name, (unsigned long) i))
  		    error ("Canceled the download");
  	      }
! 	    while (waitack != NULL && !waitack ());
  
  	    if (hashmark)
  	      {
--- 128,134 ----
  		  if (ui_load_progress_hook (section_name, (unsigned long) i))
  		    error ("Canceled the download");
  	      }
! 	    while (waitack != NULL && !waitack (reclen));
  
  	    if (hashmark)
  	      {
Index: gdb/monitor.c
===================================================================
RCS file: /cvs/src/src/gdb/monitor.c,v
retrieving revision 1.28
diff -c -r1.28 monitor.c
*** gdb/monitor.c	2001/07/15 20:34:13	1.28
--- gdb/monitor.c	2001/08/09 00:45:47
***************
*** 44,49 ****
--- 44,50 ----
  #include <signal.h>
  #include <ctype.h>
  #include "gdb_string.h"
+ #include <sys/time.h>
  #include <sys/types.h>
  #include "command.h"
  #include "serial.h"
***************
*** 2143,2151 ****
  
  /* monitor_wait_srec_ack -- wait for the target to send an acknowledgement for
     an S-record.  Return non-zero if the ACK is received properly.  */
- 
  static int
! monitor_wait_srec_ack (void)
  {
    int ch;
  
--- 2144,2151 ----
  
  /* monitor_wait_srec_ack -- wait for the target to send an acknowledgement for
     an S-record.  Return non-zero if the ACK is received properly.  */
  static int
! monitor_wait_srec_ack (int bytes_sent)
  {
    int ch;
  
***************
*** 2164,2169 ****
--- 2164,2197 ----
  	return 0;
        if ((ch = readchar (1)) < 0)
  	return 0;
+     }
+   else if (current_monitor->flags & MO_SREC_ACK_DELAY)
+     {
+       int delay_baud = baud_rate;
+       long milliseconds;
+       struct timeval tv;
+ 
+       /* If the user hasn't specified a baud rate, assume 9600.  */
+       if (delay_baud == -1)
+         delay_baud = 9600;
+ 
+       /* A baud rate is "symbols per second"; on a serial line, that's
+          the same as "bits per second".  Assume ten bits transmitted
+          per byte: eight data bits, one parity bit, and one stop bit.
+          (Usually people use 8N1, which I think means the parity bit
+          isn't transmitted, so we're actually at nine bits transmitted
+          per byte, but we need to be sure to allow enough time.)
+ 
+          So bytes_sent * 10 is at least the number of bits
+          transmitted; that divided by the bits-per-second rate yields
+          seconds.  That times 1000 yields milliseconds.  Do the
+          multiplication first, to minimize round-off errors.  */
+       milliseconds = (bytes_sent * 10 * 1000) / delay_baud;
+       tv.tv_usec = milliseconds * 1000;
+       tv.tv_sec = milliseconds / 1000;
+       if (select (0, 0, 0, 0, &tv) == -1)
+         error ("Error waiting for S-record transmission: %s",
+                strerror (errno));
      }
    return 1;
  }
Index: gdb/monitor.h
===================================================================
RCS file: /cvs/src/src/gdb/monitor.h,v
retrieving revision 1.7
diff -c -r1.7 monitor.h
*** gdb/monitor.h	2001/07/10 21:06:34	1.7
--- gdb/monitor.h	2001/08/09 00:45:47
***************
*** 231,236 ****
--- 231,241 ----
   */
  #define MO_HAS_BLOCKWRITES 0x800000
  
+ /* If set, assume that the target has no flow control, and we have to
+    sleep long enough (according to baud_rate) for the target to absorb
+    what we have sent.  */
+ #define MO_SREC_ACK_DELAY 0x1000000
+ 
  #define SREC_SIZE 160
  
  extern void monitor_open (char *args, struct monitor_ops *ops, int from_tty);
Index: gdb/rom68k-rom.c
===================================================================
RCS file: /cvs/src/src/gdb/rom68k-rom.c,v
retrieving revision 1.5
diff -c -r1.5 rom68k-rom.c
*** gdb/rom68k-rom.c	2001/03/06 08:21:16	1.5
--- gdb/rom68k-rom.c	2001/08/09 00:45:47
***************
*** 99,105 ****
  static void
  init_rom68k_cmds (void)
  {
!   rom68k_cmds.flags = 0;
    rom68k_cmds.init = rom68k_inits;	/* monitor init string */
    rom68k_cmds.cont = "go\r";
    rom68k_cmds.step = "st\r";
--- 99,105 ----
  static void
  init_rom68k_cmds (void)
  {
!   rom68k_cmds.flags = MO_SREC_ACK | MO_SREC_ACK_DELAY;
    rom68k_cmds.init = rom68k_inits;	/* monitor init string */
    rom68k_cmds.cont = "go\r";
    rom68k_cmds.step = "st\r";
Index: gdb/srec.h
===================================================================
RCS file: /cvs/src/src/gdb/srec.h,v
retrieving revision 1.4
diff -c -r1.4 srec.h
*** gdb/srec.h	2001/07/11 17:52:32	1.4
--- gdb/srec.h	2001/08/09 00:45:47
***************
*** 20,26 ****
  
  void load_srec (struct serial *desc, const char *file, bfd_vma load_offset,
  		int maxrecsize, int flags, int hashmark,
! 		int (*waitack) (void));
  
  /* S-record capability flags */
  
--- 20,26 ----
  
  void load_srec (struct serial *desc, const char *file, bfd_vma load_offset,
  		int maxrecsize, int flags, int hashmark,
! 		int (*waitack) (int chars_sent));
  
  /* S-record capability flags */
  
Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.48
diff -c -r1.48 gdb.texinfo
*** gdb/doc/gdb.texinfo	2001/08/02 10:52:07	1.48
--- gdb/doc/gdb.texinfo	2001/08/09 00:45:58
***************
*** 11794,11800 ****
  
  @kindex target rom68k
  @item target rom68k @var{dev}
! ROM 68K monitor, running on an M68K IDP board.
  
  @end table
  
--- 11794,11805 ----
  
  @kindex target rom68k
  @item target rom68k @var{dev}
! ROM 68K monitor, running on an M68K IDP board.  The IDP board's serial
! port doesn't support flow control, and the monitor doesn't provide any
! acknowledgement for each S-record received, so GDB's @code{load} command
! uses the current baud rate to pace its transmission of S-records.  See
! the description of @code{set baud} at @ref{Protocol,, Communication
! Protocol}.
  
  @end table
  

Re: RFA: pause after sending S-records to ROM68K monitor

[Eli Zaretskii]

On Wed, 8 Aug 2001, Jim Blandy wrote:

> This patch creates another use of baud_rate (as set by `set baud').
> I'm not sure that's such a hot idea.
[...]
> Index: gdb/doc/gdb.texinfo
> ===================================================================
> RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
> retrieving revision 1.48
> diff -c -r1.48 gdb.texinfo
> *** gdb/doc/gdb.texinfo	2001/08/02 10:52:07	1.48
> --- gdb/doc/gdb.texinfo	2001/08/09 00:45:58

The patch for the manual is approved, but please change "at" into "in"
in this:

> ! the description of @code{set baud} at @ref{Protocol,, Communication
> ! Protocol}.

(``at section "Communication Protocol"'', which is what TeX produces
from this, sounds incorrect English).

Re: RFA: pause after sending S-records to ROM68K monitor

[Jim Blandy]

Eli Zaretskii <eliz@is.elta.co.il> writes:
> The patch for the manual is approved, but please change "at" into "in"
> in this:
> 
> > ! the description of @code{set baud} at @ref{Protocol,, Communication
> > ! Protocol}.
> 
> (``at section "Communication Protocol"'', which is what TeX produces
> from this, sounds incorrect English).

Will do, thanks.

Re: RFA: pause after sending S-records to ROM68K monitor

[Andrew Cagney]

> This patch creates another use of baud_rate (as set by `set baud').
> I'm not sure that's such a hot idea.
> 
> But it does make `target rom68k' usable over an SSH tunnel to a
> portmaster; without the patch, GDB transmits the entire program
> immediately (I guess it gets absorbed by all the buffers between
> itself and the real board), and then times out.


Jim, while an inovative interpretation of baud, I don't know that the 
patch is either correct or robust.

 From memory, two things govern a serial connection:

	o	bandwidth
	o	latency

It sounds like you've got a latency problem while the patch you have is 
playing with the bandwidth knob.

	Andrew

--


To misquote AST: Never underestimate the bandwidth of a jumbo full of 
DDS4 tapes.

Re: RFA: pause after sending S-records to ROM68K monitor

[Jim Blandy]

Andrew Cagney <ac131313@cygnus.com> writes:
> Jim, while an inovative interpretation of baud, I don't know that the 
> patch is either correct or robust.

Well, okay.  Can you suggest a correct and robust implementation for
downloading S-records to a monitor which provides no hardware
handshaking on the serial line, and no handshaking in the protocol
(say, by transmitting a `+' after each record)?

> It sounds like you've got a latency problem while the patch you have is 
> playing with the bandwidth knob.

In the absence of any per-record feedback, I don't see any
alternative.

Re: RFA: pause after sending S-records to ROM68K monitor

[Andrew Cagney]

> Andrew Cagney <ac131313@cygnus.com> writes:
> 
>> Jim, while an inovative interpretation of baud, I don't know that the 
>> patch is either correct or robust.
> 
> 
> Well, okay.  Can you suggest a correct and robust implementation for
> downloading S-records to a monitor which provides no hardware
> handshaking on the serial line, and no handshaking in the protocol
> (say, by transmitting a `+' after each record)?
> 
> 
>> It sounds like you've got a latency problem while the patch you have is 
>> playing with the bandwidth knob.
> 
> 
> In the absence of any per-record feedback, I don't see any
> alternative.


I think you've got it half right  The time taken is something like:

	size-of-pipe * amount-of-data + round-trip

(I don't have my AST networks book handy to don't trust this). 
round-trip is loosely defined by remote_timeout.  As you note the baud 
loosely defines the transfer-rate.  This is if we assume 100% efficiency :-)

In your case, it is probably more exactly defined as:

	ABOVE - (current-time - start-time)

remember, if you are talking down a real serial port then the buffering 
will be zero and you only want to wait for round-trip time.

Using this, i'd expect something like

	if (remote_throughput (??better-name??) >= 0)
	  transfer_time = remote_throughput * sizeoftransfer;
	else
	  transfer_time = something made up from the baud-rate

	timeout = transfer_time - (current_time - start_time) + remote_timeout


Andrew

Re: RFA: pause after sending S-records to ROM68K monitor

[Jim Blandy]

I see what you're looking for.  That would certainly be better than
what I did --- under my system, if you had really high latency, you'd
have to lower the alleged baud rate some amount that varies based on
the size of your program... ugh...

I'll withdraw this patch for revisions.

Andrew Cagney <ac131313@cygnus.com> writes:
> I think you've got it half right  The time taken is something like:
> 
> 	size-of-pipe * amount-of-data + round-trip
> 
> (I don't have my AST networks book handy to don't trust this). 
> round-trip is loosely defined by remote_timeout.  As you note the baud 
> loosely defines the transfer-rate.  This is if we assume 100% efficiency :-)
> 
> In your case, it is probably more exactly defined as:
> 
> 	ABOVE - (current-time - start-time)
> 
> remember, if you are talking down a real serial port then the buffering 
> will be zero and you only want to wait for round-trip time.
> 
> Using this, i'd expect something like
> 
> 	if (remote_throughput (??better-name??) >= 0)
> 	  transfer_time = remote_throughput * sizeoftransfer;
> 	else
> 	  transfer_time = something made up from the baud-rate
> 
> 	timeout = transfer_time - (current_time - start_time) + remote_timeout

Re: RFA: pause after sending S-records to ROM68K monitor

[Andrew Cagney]

> I see what you're looking for.  That would certainly be better than
> what I did --- under my system, if you had really high latency, you'd
> have to lower the alleged baud rate some amount that varies based on
> the size of your program... ugh...


> I'll withdraw this patch for revisions.


Thanks!  If the magic equations are done using functions, I might steal 
them for remote.c.  Remote.c currently does the exact oposite of your 
patch - it assmes the there isn't a throughput problem and everything is 
due to latency :-/

	Andrew

Re: RFA: pause after sending S-records to ROM68K monitor

[Frank Ch. Eigler]

jimb wrote:

: [...]
: +       /* A baud rate is "symbols per second"; on a serial line, that's
: +          the same as "bits per second".  Assume ten bits transmitted
: +          per byte: eight data bits, one parity bit, and one stop bit.
: +          (Usually people use 8N1, which I think means the parity bit
: +          isn't transmitted, so we're actually at nine bits transmitted
: +          per byte, but we need to be sure to allow enough time.)
: [...]

The 10-vs-9 error is because you are forgetting the start bit: there's
always one.

(FWIW, I like the hack.)


- FChE

Re: RFA: pause after sending S-records to ROM68K monitor

[Jim Blandy]

fche@redhat.com (Frank Ch. Eigler) writes:
> : +       /* A baud rate is "symbols per second"; on a serial line, that's
> : +          the same as "bits per second".  Assume ten bits transmitted
> : +          per byte: eight data bits, one parity bit, and one stop bit.
> : +          (Usually people use 8N1, which I think means the parity bit
> : +          isn't transmitted, so we're actually at nine bits transmitted
> : +          per byte, but we need to be sure to allow enough time.)
> : [...]
> 
> The 10-vs-9 error is because you are forgetting the start bit: there's
> always one.

[light bulb]

> (FWIW, I like the hack.)

Oh, you mean my kludge?  :)