This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Flash support part 3: documentation


On Thursday 20 July 2006 23:57, Eli Zaretskii wrote:
> > 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.

Yes, this is the revised wording:

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

> > +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?

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?

No, I meant something more to this:

The memory ranges specified by
@samp{vFlashWrite} packets preceding a @samp{vFlashDone} packet must
not overlap, and must appear in order of increasing addresses.

> >                                  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.

I think this is slightly different. For example, target might have some extra 
fast method to erase entire flash, that gdb does not know about, but that 
user can invoke in some. Then, it's ok to send vFlashWrite without any 
vFlashErase packets at all. So the constraint is that area of flash is in 
erased state when vFlashWrite arrives, but not that vFlashErase was seen.

> > +@item E.memtype
> > +for vFlashWrite addressing non-flash memory
>
> What is "E.memtype"? is it a literal string?

Yes, it's a literal string, just like "OK" two lines above.

> > +@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.

Ok.

>
> > +@var{annex} must be empty.
>
> There's no @var{annex} in the @item that gives the packet's syntax.

I must admit that I've copied this sentence from description of 
qXfer:auxv:read. While you're right that @var{annex} is not present in 
description of qXfer:memory-map:read, it's present in description of generic 
syntax of qXfer packet. The point of this sentence is to emphasise that annex
part of generic qXfer packet should be empty for memory map.

>
> > +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.

What is the difference between them? Do you mean thatsmallexample should be 
used everywhere in gdb docs? 

> > +@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>"?

No, there's "/>" at the end of the element. That's abbreviated XML syntax for 
element without children.

>
> > +@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.

Ok.

>
> > +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.

I've trimmed this to 60 characters.

I attach the revised patch, is it better now?

Thanks,
Volodya





=== gdb.texinfo
==================================================================
--- gdb.texinfo	(revision 177)
+++ gdb.texinfo	(revision 296)
@@ -12195,6 +12195,9 @@
 specifies a fixed address.
 @c FIXME! This would be a good place for an xref to the GNU linker doc.
 
+Depending on the remote side capabilities, @value{GDBN} may be able to
+load programs into flash memory.
+
 @code{load} does not repeat if you press @key{RET} again after using it.
 @end table
 
@@ -22497,6 +22500,7 @@
 * Interrupts::
 * Examples::
 * File-I/O remote protocol extension::
+* Memory map format::
 @end menu
 
 @node Overview
@@ -22995,6 +22999,56 @@
 The @samp{vCont} packet is not supported.
 @end table
 
+@item vFlashErase:@var{addr},@var{length}
+@cindex @samp{vFlashErase} packet
+Direct the stub to erase @var{length} bytes of flash starting at
+@var{addr}.  The region may enclose any number of flash blocks, but
+its start and end must fall on block boundaries, as indicated by the
+flash block size appearing in the memory map (@pxref{Memory map
+format}).  @value{GDBN} groups flash memory programming operations
+together, and sends a @samp{vFlashDone} request after each group; the
+stub is allowed to delay erase operation until the @samp{vFlashDone}
+packet is received.
+
+Reply:
+@table @samp
+@item OK
+for success
+@item E @var{NN}
+for an error
+@end table
+
+@item vFlashWrite:@var{addr}:@var{XX@dots{}}
+@cindex @samp{vFlashWrite} packet
+Direct the stub to write data to flash address @var{addr}.  The data
+is passed in binary form using the same encoding as for the @samp{X}
+packet (@pxref{Binary Data}).  The memory ranges specified by
+@samp{vFlashWrite} packets preceding a @samp{vFlashDone} packet must
+not overlap, and must appear in order of increasing addresses.  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.
+
+
+Reply:
+@table @samp
+@item OK
+for success
+@item E.memtype
+for vFlashWrite addressing non-flash memory
+@item E @var{NN}
+for an error
+@end table
+
+@item vFlashDone
+@cindex @samp{vFlashDone} packet
+Indicate to the stub that flash programming operation is finished.
+The stub is permitted to delay or batch the effects of a group of
+@samp{vFlashErase} and @samp{vFlashWrite} packets until a
+@samp{vFlashDone} packet is received.  The contents of the affected
+regions of flash memory are unpredictable until the @samp{vFlashDone}
+request is completed.
+
 @item X @var{addr},@var{length}:@var{XX@dots{}}
 @anchor{X packet}
 @cindex @samp{X} packet
@@ -23534,6 +23588,11 @@
 @tab @samp{-}
 @tab Yes
 
+@item @samp{qXfer:memory-map:read}
+@tab No
+@tab @samp{-}
+@tab Yes
+
 @end multitable
 
 These are the currently defined stub features, in more detail:
@@ -23652,6 +23711,17 @@
 by suppling an appropriate @samp{qSupported} response (@pxref{qSupported}).
 @end table
 
+@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
+@var{annex} must be empty.
+
+
+This packet is not probed by default; the remote stub must request it,
+by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
+@end table
+
 Reply:
 @table @samp
 @item m @var{data}
@@ -25132,6 +25202,88 @@
 <- @code{T02}
 @end smallexample
 
+@node Memory map format
+@section Memory map format
+@cindex memory map format
+
+To be able to write into flash memory, @value{GDBN} needs to obtain
+memory map from the target.  This section describes the format of the
+memory map.
+
+The memory map is obtained using the @samp{qXfer:memory-map:read}
+(@pxref{qXfer memory map read}) packet and is an XML document that
+lists memory regions.  The top-level structure of the document is shown below:
+
+@smallexample
+<?xml version="1.0"?>
+<!DOCTYPE memory-map
+          PUBLIC "+//IDN gnu.org//DTD GDB Memory Map V1.0//EN"
+                 "http://sourceware.org/gdb/gdb-memory-map.dtd";>
+<memory-map>
+    region...
+</memory-map>
+@end smallexample
+
+Each region can be either:
+
+@itemize
+
+@item 
+A region of RAM starting at @var{addr} and extending for @var{length}
+bytes from there:
+
+@smallexample
+<memory type="ram" start="@var{addr}" length="@var{length}"/>
+@end smallexample
+
+
+@item 
+A region of read-only memory:
+
+@smallexample
+<memory type="rom" start="@var{addr}" length="@var{length}"/>
+@end smallexample
+
+
+@item 
+A region of flash memory, with erasure blocks @var{blocksize}
+bytes in length:
+
+@smallexample
+<memory type="flash" start="@var{addr}" length="@var{length}">
+  <property name="blocksize">@var{blocksize}</property>
+</memory>
+@end smallexample
+
+@end itemize
+
+Regions must not overlap.  @value{GDBN} assumes that areas of memory not covered
+by the memory map are RAM, and uses the ordinary @samp{M} and @samp{X}
+packets to write to addresses in such ranges.
+
+The formal DTD for memory map format is given below:
+
+@smallexample
+<!-- ................................................... -->
+<!-- Memory Map XML DTD ................................ -->
+<!-- File: memory-map.dtd .............................. -->
+<!-- .................................... .............. -->
+<!-- memory-map.dtd -->
+<!-- memory-map: Root element with versioning -->
+<!ELEMENT memory-map (memory | property)>
+<!ATTLIST memory-map    version CDATA   #FIXED  "1.0.0">
+<!ELEMENT memory (property)>
+<!-- memory: Specifies a memory region, 
+             and its type, or device. -->
+<!ATTLIST memory        type    CDATA   #REQUIRED
+                        start   CDATA   #REQUIRED
+                        length  CDATA   #REQUIRED
+                        device  CDATA   #IMPLIED>
+<!-- property: Generic attribute tag -->
+<!ELEMENT property (#PCDATA | property)*>
+<!ATTLIST property      name    CDATA   #REQUIRED>
+@end smallexample
+
 @include agentexpr.texi
 
 @include gpl.texi

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]