Patchwork [U-Boot] WIP: cbfs: Add docbook template

login
register
mail settings
Submitter Simon Glass
Date Oct. 31, 2012, 10:27 p.m.
Message ID <1351722443-8281-1-git-send-email-sjg@chromium.org>
Download mbox | patch
Permalink /patch/196021/
State RFC
Delegated to: Simon Glass
Headers show

Comments

Simon Glass - Oct. 31, 2012, 10:27 p.m.
This adds a docbook template for fs, and makes CBFS use it.

Problems (advise please as I have enough 500pp books in my bookshelf):

1. It requires the function names to be repeated. I would like to do this:

/**
 * Get the name of a file in CBFS.
 *
 * @file:		The handle to the file.
 *
 * @return The name of the file, NULL on error.
 */
const char *file_cbfs_name(const struct cbfs_cachenode *file);

but I must do this:

/**
 * file_cbfs_name() - Get the name of a file in CBFS.
 *
 * @file:		The handle to the file.
 *
 * @return The name of the file, NULL on error.
 */
const char *file_cbfs_name(const struct cbfs_cachenode *file);

2. It will not accept the doxygen @param style for parameters:

/**
 * file_cbfs_name() - Get the name of a file in CBFS.
 *
 * @param file		The handle to the file.
 *
 * @return The name of the file, NULL on error.
 */
const char *file_cbfs_name(const struct cbfs_cachenode *file);

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 doc/DocBook/Makefile |    2 +-
 include/cbfs.h       |   45 +++++++++++++++++++++++----------------------
 2 files changed, 24 insertions(+), 23 deletions(-)
Marek Vasut - Oct. 31, 2012, 11:39 p.m.
Dear Simon Glass,

> This adds a docbook template for fs, and makes CBFS use it.
> 
> Problems (advise please as I have enough 500pp books in my bookshelf):
> 
> 1. It requires the function names to be repeated. I would like to do this:

Repeating function names is good, you can be sure what the comment is associated 
with.

[...]

> 2. It will not accept the doxygen @param style for parameters:

It's not doxygen, sorry. Read [1].

btw. I'd like some kind of @return, but maybe describing the return value in 
comment is enough.

[...]

[1] http://www.denx.de/wiki/U-Boot/CodingStyle

Best regards,
Marek Vasut
Simon Glass - Nov. 2, 2012, 2:23 a.m.
Hi Marek,

On Wed, Oct 31, 2012 at 4:39 PM, Marek Vasut <marex@denx.de> wrote:
> Dear Simon Glass,
>
>> This adds a docbook template for fs, and makes CBFS use it.
>>
>> Problems (advise please as I have enough 500pp books in my bookshelf):
>>
>> 1. It requires the function names to be repeated. I would like to do this:
>
> Repeating function names is good, you can be sure what the comment is associated
> with.
>

Hmmm, well I already know that, since the comment appears immediately
above the function...

> [...]
>
>> 2. It will not accept the doxygen @param style for parameters:
>
> It's not doxygen, sorry. Read [1].

Yes, I was rather hoping there was an easy way to fix these two
things. Never mind.

>
> btw. I'd like some kind of @return, but maybe describing the return value in
> comment is enough.

Maybe, but it's nice to be explicit. The return value is at least as
important as the parameters, and they are called out.

Anyway I will rev the patch with the above in mind.

Regads,
Simon

>
> [...]
>
> [1] http://www.denx.de/wiki/U-Boot/CodingStyle
>
> Best regards,
> Marek Vasut
Marek Vasut - Nov. 2, 2012, 3:39 a.m.
Dear Simon Glass,

> Hi Marek,
> 
> On Wed, Oct 31, 2012 at 4:39 PM, Marek Vasut <marex@denx.de> wrote:
> > Dear Simon Glass,
> > 
> >> This adds a docbook template for fs, and makes CBFS use it.
> >> 
> >> Problems (advise please as I have enough 500pp books in my bookshelf):
> > 
> >> 1. It requires the function names to be repeated. I would like to do this:
> > Repeating function names is good, you can be sure what the comment is
> > associated with.
> 
> Hmmm, well I already know that, since the comment appears immediately
> above the function...

At the time of writing, yes. But some adjustment of code might mess that up.

> > [...]
> > 
> >> 2. It will not accept the doxygen @param style for parameters:
> > It's not doxygen, sorry. Read [1].
> 
> Yes, I was rather hoping there was an easy way to fix these two
> things. Never mind.

I'd say there is, but let's fix it in Linux kernel too. Actually, I have a pile 
of patches for Linux kernel's documentation, but it's hard to get any 
documentation patches in :-C

> > btw. I'd like some kind of @return, but maybe describing the return value
> > in comment is enough.
> 
> Maybe, but it's nice to be explicit. The return value is at least as
> important as the parameters, and they are called out.

I agree (!).

> Anyway I will rev the patch with the above in mind.
[...]

btw. It'd be nice to be able to patch the upstream (linux's) kerneldoc :-(

Best regards,
Marek Vasut

Patch

diff --git a/doc/DocBook/Makefile b/doc/DocBook/Makefile
index da88b32..521e8bc 100644
--- a/doc/DocBook/Makefile
+++ b/doc/DocBook/Makefile
@@ -8,7 +8,7 @@ 
 
 include $(TOPDIR)/config.mk
 
-DOCBOOKS := linker_lists.xml stdio.xml
+DOCBOOKS := fs.xml linker_lists.xml stdio.xml
 
 ###
 # The build process is as follows (targets):
diff --git a/include/cbfs.h b/include/cbfs.h
index 6ea3f35..dc17109 100644
--- a/include/cbfs.h
+++ b/include/cbfs.h
@@ -77,46 +77,46 @@  struct cbfs_cachenode {
 
 extern enum cbfs_result file_cbfs_result;
 
-/*
+/**
  * Return a string describing the most recent error condition.
  *
  * @return A pointer to the constant string.
  */
 const char *file_cbfs_error(void);
 
-/*
+/**
  * Initialize the CBFS driver and load metadata into RAM.
  *
- * @param end_of_rom	Points to the end of the ROM the CBFS should be read
+ * @end_of_rom: Points to the end of the ROM the CBFS should be read
  *                      from.
  */
 void file_cbfs_init(uintptr_t end_of_rom);
 
-/*
+/**
  * Get the header structure for the current CBFS.
  *
  * @return A pointer to the constant structure, or NULL if there is none.
  */
 const struct cbfs_header *file_cbfs_get_header(void);
 
-/*
+/**
  * Get a handle for the first file in CBFS.
  *
  * @return A handle for the first file in CBFS, NULL on error.
  */
 const struct cbfs_cachenode *file_cbfs_get_first(void);
 
-/*
+/**
  * Get a handle to the file after this one in CBFS.
  *
- * @param file		A pointer to the handle to advance.
+ * @file:		A pointer to the handle to advance.
  */
 void file_cbfs_get_next(const struct cbfs_cachenode **file);
 
-/*
+/**
  * Find a file with a particular name in CBFS.
  *
- * @param name		The name to search for.
+ * @name:		The name to search for.
  *
  * @return A handle to the file, or NULL on error.
  */
@@ -127,53 +127,54 @@  const struct cbfs_cachenode *file_cbfs_find(const char *name);
 /* All of the functions below can be used without first initializing CBFS. */
 /***************************************************************************/
 
-/*
+/**
  * Find a file with a particular name in CBFS without using the heap.
  *
- * @param end_of_rom	Points to the end of the ROM the CBFS should be read
+ * @end_of_rom:		Points to the end of the ROM the CBFS should be read
  *                      from.
- * @param name		The name to search for.
+ * @name:		The name to search for.
  *
  * @return A handle to the file, or NULL on error.
  */
 const struct cbfs_cachenode *file_cbfs_find_uncached(uintptr_t end_of_rom,
 						     const char *name);
 
-/*
+/**
  * Get the name of a file in CBFS.
  *
- * @param file		The handle to the file.
+ * @file:		The handle to the file.
  *
  * @return The name of the file, NULL on error.
  */
 const char *file_cbfs_name(const struct cbfs_cachenode *file);
 
-/*
+/**
  * Get the size of a file in CBFS.
  *
- * @param file		The handle to the file.
+ * @file:		The handle to the file.
  *
  * @return The size of the file, zero on error.
  */
 u32 file_cbfs_size(const struct cbfs_cachenode *file);
 
-/*
+/**
  * Get the type of a file in CBFS.
  *
- * @param file		The handle to the file.
+ * @file:		The handle to the file.
  *
  * @return The type of the file, zero on error.
  */
 u32 file_cbfs_type(const struct cbfs_cachenode *file);
 
-/*
+/**
  * Read a file from CBFS into RAM
  *
- * @param file		A handle to the file to read.
- * @param buffer	Where to read it into memory.
+ * @file:		A handle to the file to read.
+ * @buffer:		Where to read it into memory.
+ * @maxsize:		Maximum number of bytes to read
  *
  * @return If positive or zero, the number of characters read. If negative, an
- *         error occurred.
+ *	   error occurred.
  */
 long file_cbfs_read(const struct cbfs_cachenode *file, void *buffer,
 		    unsigned long maxsize);