Re: Flash support part 3: documentation

[Eli Zaretskii <eliz at gnu dot org>]

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Thu, 20 Jul 2006 13:43:32 +0400
> 
> This patch finishes the flash support work by adding documentation for all new
> packets, and by describing the format of XML memory map used in the remote 
> protocol.

Thanks.  My comments are below.

>              @value{GDBN} groups flash memory programming operations
> +together, and sends a @samp{vFlashDone} request after each group; the
> +to delay erase operation until the @samp{vFlashDone} packet is received.
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Something is wrong here.

> +Direct the stub to write data to flash address @var{addr}.  The data
> +is passed binary form using the same encoding as for the @samp{X}
      ^^^^^^^^^^^^^^^^^^
You meant "passed IN binary form", right?

>                                  The set of @samp{vFlashWrite} packets
> +preceding a @samp{vFlashDone} packet must not overlap

It is not clear what does ``must not overlap'' mean here.  Taken
verbatim, the sentence seems to say that the _packets_ must not
overlap, but is that really what you want to say?

>                                  Writes may fall outside the regions
> +given by the previously transmitted @samp{vFlashErase} packets, but
> +the results are unpredictable if a given area of flash is rewritten
> +without being erased.

I'd rephrase this as follows:

  If the series of packets write data outside the region erased by the
  preceding @samp{vFlashErase} packets, the results are unpredictable.

> +@item E.memtype
> +for vFlashWrite addressing non-flash memory

What is "E.memtype"? is it a literal string?

> +@table @samp
> +@item qXfer:memory-map:read::@var{offset},@var{length}
> +@anchor{qXfer memory map read}
> +Access the target's @dfn{memory-map}.  @xref{Memory map format}. The
                                                                  ^^^
Two spaces here, please.

> +@var{annex} must be empty.

There's no @var{annex} in the @item that gives the packet's syntax.

> +by suppling an appropriate @samp{qSupported} response (@pxref{qSupported}).
      ^^^^^^^^
A typo.

> +@node Memory map format
> +@section Memory map format
> +@cindex Memory map format

Please start @cindex entries with a lower-case letter.

> +lists memory regions. The top-level structure of the document is shown below:
                       ^^^
Need two spaces here.

> +@example

Please use @smallexample everywhere.

> +@itemize
> +
> +@item A region of RAM starting at @var{addr} and extending for @var{length}

An @item in an @itemize list should be alone on its line; the text
should start on the following line, like this:

  @item
  A region of RAM starting at @var{addr} and extending ...

> +<memory type="ram" start="addr" length="length"/>

Shouldn't this end with "/memory>"?

> +@example
> +<memory type="rom" start="addr" length="length"/>
> +@end example
> +
> +
> +@item A region of flash memory, with erasure blocks @var{blocksize}
> +bytes in length:
> +
> +@example
> +<memory type="flash" start="addr" length="length">
> +  <property name="blocksize">blocksize</property>
> +</memory>
> +@end example

The "addr", "blocksize" and "length" here refer to the values of
@var{addr}, @var{blocksize}, etc., right?  If so, you should use @var
in them.

> +Regions must not overlap. @value{GDBN} assumes that areas of memory not covered
                          ^^^^
Two spaces.

> +@example
> +<!-- ............................................................... -->
> +<!-- Memory Map XML DTD ............................................ -->
> +<!-- File: memory-map.dtd .......................................... -->
> +<!-- ............................................................... -->

Please make these lines shorter: they will overflow the page margins,
even with @smallexample, since TeX doesn't hyphenate inside @example.
In general, anything longer than 64 characters in a @smallexample is
bad.