[v3,07/27] net/nfp: standard the comment style

Message ID 20230915091551.1459606-8-chaoyong.he@corigine.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series refact the nfpcore module |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Chaoyong He Sept. 15, 2023, 9:15 a.m. UTC
  Follow the DPDK coding style, use the kdoc comment style.
Also add some comment to help understand logic.

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
---
 drivers/net/nfp/nfpcore/nfp_cpp.h          | 504 ++++-----------------
 drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c |  39 +-
 drivers/net/nfp/nfpcore/nfp_cppcore.c      | 484 ++++++++++++++++----
 drivers/net/nfp/nfpcore/nfp_hwinfo.c       |  21 +-
 drivers/net/nfp/nfpcore/nfp_hwinfo.h       |   2 +
 drivers/net/nfp/nfpcore/nfp_mip.c          |  43 +-
 drivers/net/nfp/nfpcore/nfp_mutex.c        |  69 +--
 drivers/net/nfp/nfpcore/nfp_nffw.c         |  49 +-
 drivers/net/nfp/nfpcore/nfp_nffw.h         |   6 +-
 drivers/net/nfp/nfpcore/nfp_nsp.c          |  53 ++-
 drivers/net/nfp/nfpcore/nfp_nsp.h          | 108 ++---
 drivers/net/nfp/nfpcore/nfp_nsp_eth.c      | 170 ++++---
 drivers/net/nfp/nfpcore/nfp_resource.c     | 103 +++--
 drivers/net/nfp/nfpcore/nfp_resource.h     |  28 +-
 drivers/net/nfp/nfpcore/nfp_rtsym.c        |  59 ++-
 drivers/net/nfp/nfpcore/nfp_rtsym.h        |  12 +-
 drivers/net/nfp/nfpcore/nfp_target.c       |   2 +-
 17 files changed, 888 insertions(+), 864 deletions(-)
  

Comments

Ferruh Yigit Sept. 15, 2023, 1:44 p.m. UTC | #1
On 9/15/2023 10:15 AM, Chaoyong He wrote:
> Follow the DPDK coding style, use the kdoc comment style.
> Also add some comment to help understand logic.
> 
> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>

<...>

>  
> -/*
> - * Set the model id
> - *
> - * @param   cpp     NFP CPP operations structure
> - * @param   model   Model ID
> - */
>  void nfp_cpp_model_set(struct nfp_cpp *cpp, uint32_t model);
>  

As I see some comments are removed in this patch, there is no enforced
comment style for internal functions, for API doxygen comments must
provided.

Of course consistency matters, but if comments are removed because of
the syntax, I think better option to keep them with their own syntax.
  
Chaoyong He Sept. 18, 2023, 1:28 a.m. UTC | #2
> On 9/15/2023 10:15 AM, Chaoyong He wrote:
> > Follow the DPDK coding style, use the kdoc comment style.
> > Also add some comment to help understand logic.
> >
> > Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
> 
> <...>
> 
> >
> > -/*
> > - * Set the model id
> > - *
> > - * @param   cpp     NFP CPP operations structure
> > - * @param   model   Model ID
> > - */
> >  void nfp_cpp_model_set(struct nfp_cpp *cpp, uint32_t model);
> >
> 
> As I see some comments are removed in this patch, there is no enforced
> comment style for internal functions, for API doxygen comments must
> provided.
> 
> Of course consistency matters, but if comments are removed because of the
> syntax, I think better option to keep them with their own syntax.

Sorry for that, we may be missing some when rebase the code, we will check and add them.
Thanks.
  
Chaoyong He Sept. 18, 2023, 2:08 a.m. UTC | #3
> On 9/15/2023 10:15 AM, Chaoyong He wrote:
> > Follow the DPDK coding style, use the kdoc comment style.
> > Also add some comment to help understand logic.
> >
> > Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
> 
> <...>
> 
> >
> > -/*
> > - * Set the model id
> > - *
> > - * @param   cpp     NFP CPP operations structure
> > - * @param   model   Model ID
> > - */
> >  void nfp_cpp_model_set(struct nfp_cpp *cpp, uint32_t model);
> >
> 
> As I see some comments are removed in this patch, there is no enforced
> comment style for internal functions, for API doxygen comments must
> provided.
> 
> Of course consistency matters, but if comments are removed because of the
> syntax, I think better option to keep them with their own syntax.

Oh, I understand what you mean now.
We are not removing it, we just keep the comment of functions in the place where it was defined (the implement file).
I will make it clear in the commit message in the next version, thanks.
  

Patch

diff --git a/drivers/net/nfp/nfpcore/nfp_cpp.h b/drivers/net/nfp/nfpcore/nfp_cpp.h
index 139752f85a..82189e9910 100644
--- a/drivers/net/nfp/nfpcore/nfp_cpp.h
+++ b/drivers/net/nfp/nfpcore/nfp_cpp.h
@@ -10,9 +10,7 @@ 
 
 struct nfp_cpp_mutex;
 
-/*
- * NFP CPP handle
- */
+/* NFP CPP handle */
 struct nfp_cpp {
 	uint32_t model;
 	uint32_t interface;
@@ -37,9 +35,7 @@  struct nfp_cpp {
 	int driver_lock_needed;
 };
 
-/*
- * NFP CPP device area handle
- */
+/* NFP CPP device area handle */
 struct nfp_cpp_area {
 	struct nfp_cpp *cpp;
 	char *name;
@@ -127,35 +123,45 @@  struct nfp_cpp_operations {
 
 #define NFP_CPP_TARGET_ID_MASK 0x1f
 
-/*
+/**
  * Pack target, token, and action into a CPP ID.
  *
  * Create a 32-bit CPP identifier representing the access to be made.
  * These identifiers are used as parameters to other NFP CPP functions.
  * Some CPP devices may allow wildcard identifiers to be specified.
  *
- * @target      NFP CPP target id
- * @action      NFP CPP action id
- * @token       NFP CPP token id
+ * @param target
+ *   NFP CPP target id
+ * @param action
+ *   NFP CPP action id
+ * @param token
+ *   NFP CPP token id
  *
- * @return      NFP CPP ID
+ * @return
+ *   NFP CPP ID
  */
 #define NFP_CPP_ID(target, action, token)                               \
 		((((target) & 0x7f) << 24) | (((token) & 0xff) << 16) | \
 		(((action) & 0xff) << 8))
 
-/*
+/**
  * Pack target, token, action, and island into a CPP ID.
- * @target      NFP CPP target id
- * @action      NFP CPP action id
- * @token       NFP CPP token id
- * @island      NFP CPP island id
  *
  * Create a 32-bit CPP identifier representing the access to be made.
  * These identifiers are used as parameters to other NFP CPP functions.
  * Some CPP devices may allow wildcard identifiers to be specified.
  *
- * @return      NFP CPP ID
+ * @param target
+ *   NFP CPP target id
+ * @param action
+ *   NFP CPP action id
+ * @param token
+ *   NFP CPP token id
+ * @param island
+ *   NFP CPP island id
+ *
+ * @return
+ *   NFP CPP ID
  */
 #define NFP_CPP_ISLAND_ID(target, action, token, island)                \
 		((((target) & 0x7f) << 24) | (((token) & 0xff) << 16) | \
@@ -163,9 +169,12 @@  struct nfp_cpp_operations {
 
 /**
  * Return the NFP CPP target of a NFP CPP ID
- * @id          NFP CPP ID
  *
- * @return      NFP CPP target
+ * @param id
+ *   NFP CPP ID
+ *
+ * @return
+ *   NFP CPP target
  */
 static inline uint8_t
 NFP_CPP_ID_TARGET_of(uint32_t id)
@@ -173,11 +182,14 @@  NFP_CPP_ID_TARGET_of(uint32_t id)
 	return (id >> 24) & NFP_CPP_TARGET_ID_MASK;
 }
 
-/*
+/**
  * Return the NFP CPP token of a NFP CPP ID
- * @id          NFP CPP ID
  *
- * @return      NFP CPP token
+ * @param id
+ *   NFP CPP ID
+ *
+ * @return
+ *   NFP CPP token
  */
 static inline uint8_t
 NFP_CPP_ID_TOKEN_of(uint32_t id)
@@ -185,11 +197,14 @@  NFP_CPP_ID_TOKEN_of(uint32_t id)
 	return (id >> 16) & 0xff;
 }
 
-/*
+/**
  * Return the NFP CPP action of a NFP CPP ID
- * @id          NFP CPP ID
  *
- * @return      NFP CPP action
+ * @param id
+ *   NFP CPP ID
+ *
+ * @return
+ *   NFP CPP action
  */
 static inline uint8_t
 NFP_CPP_ID_ACTION_of(uint32_t id)
@@ -197,11 +212,14 @@  NFP_CPP_ID_ACTION_of(uint32_t id)
 	return (id >> 8) & 0xff;
 }
 
-/*
+/**
  * Return the NFP CPP island of a NFP CPP ID
- * @id          NFP CPP ID
  *
- * @return      NFP CPP island
+ * @param id
+ *   NFP CPP ID
+ *
+ * @return
+ *   NFP CPP island
  */
 static inline uint8_t
 NFP_CPP_ID_ISLAND_of(uint32_t id)
@@ -215,109 +233,57 @@  NFP_CPP_ID_ISLAND_of(uint32_t id)
  */
 const struct nfp_cpp_operations *nfp_cpp_transport_operations(void);
 
-/*
- * Set the model id
- *
- * @param   cpp     NFP CPP operations structure
- * @param   model   Model ID
- */
 void nfp_cpp_model_set(struct nfp_cpp *cpp, uint32_t model);
 
-/*
- * Set the private instance owned data of a nfp_cpp struct
- *
- * @param   cpp     NFP CPP operations structure
- * @param   interface Interface ID
- */
 void nfp_cpp_interface_set(struct nfp_cpp *cpp, uint32_t interface);
 
-/*
- * Set the private instance owned data of a nfp_cpp struct
- *
- * @param   cpp     NFP CPP operations structure
- * @param   serial  NFP serial byte array
- * @param   len     Length of the serial byte array
- */
 int nfp_cpp_serial_set(struct nfp_cpp *cpp, const uint8_t *serial,
 		size_t serial_len);
 
-/*
- * Set the private data of the nfp_cpp instance
- *
- * @param   cpp NFP CPP operations structure
- * @return      Opaque device pointer
- */
 void nfp_cpp_priv_set(struct nfp_cpp *cpp, void *priv);
 
-/*
- * Return the private data of the nfp_cpp instance
- *
- * @param   cpp NFP CPP operations structure
- * @return      Opaque device pointer
- */
 void *nfp_cpp_priv(struct nfp_cpp *cpp);
 
-/*
- * Get the privately allocated portion of a NFP CPP area handle
- *
- * @param   cpp_area    NFP CPP area handle
- * @return          Pointer to the private area, or NULL on failure
- */
 void *nfp_cpp_area_priv(struct nfp_cpp_area *cpp_area);
 
 uint32_t __nfp_cpp_model_autodetect(struct nfp_cpp *cpp, uint32_t *model);
 
-/*
- * NFP CPP core interface for CPP clients.
- */
-
-/*
- * Open a NFP CPP handle to a CPP device
- *
- * @param[in]	id	0-based ID for the CPP interface to use
- *
- * @return NFP CPP handle, or NULL on failure.
- */
+/* NFP CPP core interface for CPP clients */
 struct nfp_cpp *nfp_cpp_from_device_name(struct rte_pci_device *dev,
 		int driver_lock_needed);
 
-/*
- * Free a NFP CPP handle
- *
- * @param[in]	cpp	NFP CPP handle
- */
 void nfp_cpp_free(struct nfp_cpp *cpp);
 
 #define NFP_CPP_MODEL_INVALID   0xffffffff
 
-/*
- * NFP_CPP_MODEL_CHIP_of - retrieve the chip ID from the model ID
+/**
+ * Retrieve the chip ID from the model ID
  *
  * The chip ID is a 16-bit BCD+A-F encoding for the chip type.
  *
- * @param[in]   model   NFP CPP model id
- * @return      NFP CPP chip id
+ * @param model
+ *   NFP CPP model id
+ *
+ * @return
+ *   NFP CPP chip id
  */
 #define NFP_CPP_MODEL_CHIP_of(model)        (((model) >> 16) & 0xffff)
 
-/*
- * NFP_CPP_MODEL_IS_6000 - Check for the NFP6000 family of devices
+/**
+ * Check for the NFP6000 family of devices
  *
  * NOTE: The NFP4000 series is considered as a NFP6000 series variant.
  *
- * @param[in]	model	NFP CPP model id
- * @return		true if model is in the NFP6000 family, false otherwise.
+ * @param model
+ *   NFP CPP model id
+ *
+ * @return
+ *   true if model is in the NFP6000 family, false otherwise.
  */
 #define NFP_CPP_MODEL_IS_6000(model)		     \
 		((NFP_CPP_MODEL_CHIP_of(model) >= 0x3800) && \
 		(NFP_CPP_MODEL_CHIP_of(model) < 0x7000))
 
-/*
- * nfp_cpp_model - Retrieve the Model ID of the NFP
- *
- * @param[in]	cpp	NFP CPP handle
- * @return		NFP CPP Model ID
- */
 uint32_t nfp_cpp_model(struct nfp_cpp *cpp);
 
 /*
@@ -330,7 +296,7 @@  uint32_t nfp_cpp_model(struct nfp_cpp *cpp);
 #define NFP_CPP_INTERFACE_TYPE_RPC		0x3
 #define NFP_CPP_INTERFACE_TYPE_ILA		0x4
 
-/*
+/**
  * Construct a 16-bit NFP Interface ID
  *
  * Interface IDs consists of 4 bits of interface type, 4 bits of unit
@@ -340,422 +306,138 @@  uint32_t nfp_cpp_model(struct nfp_cpp *cpp);
  * which use the MU Atomic CompareAndWrite operation - hence the limit to 16
  * bits to be able to use the NFP Interface ID as a lock owner.
  *
- * @param[in]	type	NFP Interface Type
- * @param[in]	unit	Unit identifier for the interface type
- * @param[in]	channel	Channel identifier for the interface unit
- * @return		Interface ID
+ * @param type
+ *   NFP Interface Type
+ * @param unit
+ *   Unit identifier for the interface type
+ * @param channel
+ *   Channel identifier for the interface unit
+ *
+ * @return
+ *   Interface ID
  */
 #define NFP_CPP_INTERFACE(type, unit, channel)	\
 	((((type) & 0xf) << 12) | \
 	 (((unit) & 0xf) <<  8) | \
 	 (((channel) & 0xff) << 0))
 
-/*
+/**
  * Get the interface type of a NFP Interface ID
- * @param[in]	interface	NFP Interface ID
- * @return			NFP Interface ID's type
+ *
+ * @param interface
+ *   NFP Interface ID
+ *
+ * @return
+ *   NFP Interface ID's type
  */
 #define NFP_CPP_INTERFACE_TYPE_of(interface)	(((interface) >> 12) & 0xf)
 
-/*
+/**
  * Get the interface unit of a NFP Interface ID
- * @param[in]	interface	NFP Interface ID
- * @return			NFP Interface ID's unit
+ *
+ * @param interface
+ *   NFP Interface ID
+ *
+ * @return
+ *   NFP Interface ID's unit
  */
 #define NFP_CPP_INTERFACE_UNIT_of(interface)	(((interface) >>  8) & 0xf)
 
-/*
+/**
  * Get the interface channel of a NFP Interface ID
- * @param[in]	interface	NFP Interface ID
- * @return			NFP Interface ID's channel
+ *
+ * @param interface
+ *   NFP Interface ID
+ *
+ * @return
+ *   NFP Interface ID's channel
  */
 #define NFP_CPP_INTERFACE_CHANNEL_of(interface)	(((interface) >>  0) & 0xff)
 
-/*
- * Retrieve the Interface ID of the NFP
- * @param[in]	cpp	NFP CPP handle
- * @return		NFP CPP Interface ID
- */
+
 uint16_t nfp_cpp_interface(struct nfp_cpp *cpp);
 
-/*
- * Retrieve the NFP Serial Number (unique per NFP)
- * @param[in]	cpp	NFP CPP handle
- * @param[out]	serial	Pointer to reference the serial number array
- *
- * @return	size of the NFP6000 serial number, in bytes
- */
 int nfp_cpp_serial(struct nfp_cpp *cpp, const uint8_t **serial);
 
-/*
- * Allocate a NFP CPP area handle, as an offset into a CPP ID
- * @param[in]	cpp	NFP CPP handle
- * @param[in]	cpp_id	NFP CPP ID
- * @param[in]	address	Offset into the NFP CPP ID address space
- * @param[in]	size	Size of the area to reserve
- *
- * @return NFP CPP handle, or NULL on failure.
- */
 struct nfp_cpp_area *nfp_cpp_area_alloc(struct nfp_cpp *cpp, uint32_t cpp_id,
 		uint64_t address, size_t size);
 
-/*
- * Allocate a NFP CPP area handle, as an offset into a CPP ID, by a named owner
- * @param[in]	cpp	NFP CPP handle
- * @param[in]	cpp_id	NFP CPP ID
- * @param[in]	name	Name of owner of the area
- * @param[in]	address	Offset into the NFP CPP ID address space
- * @param[in]	size	Size of the area to reserve
- *
- * @return NFP CPP handle, or NULL on failure.
- */
 struct nfp_cpp_area *nfp_cpp_area_alloc_with_name(struct nfp_cpp *cpp,
 		uint32_t cpp_id, const char *name, uint64_t address,
 		uint32_t size);
 
-/*
- * Free an allocated NFP CPP area handle
- * @param[in]	area	NFP CPP area handle
- */
 void nfp_cpp_area_free(struct nfp_cpp_area *area);
 
-/*
- * Acquire the resources needed to access the NFP CPP area handle
- *
- * @param[in]	area	NFP CPP area handle
- *
- * @return 0 on success, -1 on failure.
- */
 int nfp_cpp_area_acquire(struct nfp_cpp_area *area);
 
-/*
- * Release the resources needed to access the NFP CPP area handle
- *
- * @param[in]	area	NFP CPP area handle
- */
 void nfp_cpp_area_release(struct nfp_cpp_area *area);
 
-/*
- * Allocate, then acquire the resources needed to access the NFP CPP area handle
- * @param[in]	cpp	NFP CPP handle
- * @param[in]	cpp_id	NFP CPP ID
- * @param[in]	address	Offset into the NFP CPP ID address space
- * @param[in]	size	Size of the area to reserve
- *
- * @return NFP CPP handle, or NULL on failure.
- */
 struct nfp_cpp_area *nfp_cpp_area_alloc_acquire(struct nfp_cpp *cpp,
 		uint32_t cpp_id, uint64_t address, size_t size);
 
-/*
- * Release the resources, then free the NFP CPP area handle
- * @param[in]	area	NFP CPP area handle
- */
 void nfp_cpp_area_release_free(struct nfp_cpp_area *area);
 
 uint8_t *nfp_cpp_map_area(struct nfp_cpp *cpp, uint32_t cpp_id,
 		uint64_t addr, uint32_t size, struct nfp_cpp_area **area);
 
-/*
- * Read from a NFP CPP area handle into a buffer. The area must be acquired with
- * 'nfp_cpp_area_acquire()' before calling this operation.
- *
- * @param[in]	area	NFP CPP area handle
- * @param[in]	offset	Offset into the area
- * @param[in]	buffer	Location of buffer to receive the data
- * @param[in]	length	Length of the data to read
- *
- * @return bytes read on success, negative value on failure.
- *
- */
 int nfp_cpp_area_read(struct nfp_cpp_area *area, uint32_t offset,
 		void *buffer, size_t length);
 
-/*
- * Write to a NFP CPP area handle from a buffer. The area must be acquired with
- * 'nfp_cpp_area_acquire()' before calling this operation.
- *
- * @param[in]	area	NFP CPP area handle
- * @param[in]	offset	Offset into the area
- * @param[in]	buffer	Location of buffer that holds the data
- * @param[in]	length	Length of the data to read
- *
- * @return bytes written on success, negative value on failure.
- */
 int nfp_cpp_area_write(struct nfp_cpp_area *area, uint32_t offset,
 		const void *buffer, size_t length);
 
-/*
- * nfp_cpp_area_iomem() - get IOMEM region for CPP area
- * @area:       CPP area handle
- *
- * Returns an iomem pointer for use with readl()/writel() style operations.
- *
- * NOTE: Area must have been locked down with an 'acquire'.
- *
- * Return: pointer to the area, or NULL
- */
 void *nfp_cpp_area_iomem(struct nfp_cpp_area *area);
 
-/*
- * Get the NFP CPP handle that is the parent of a NFP CPP area handle
- *
- * @param	cpp_area	NFP CPP area handle
- * @return			NFP CPP handle
- */
 struct nfp_cpp *nfp_cpp_area_cpp(struct nfp_cpp_area *cpp_area);
 
-/*
- * Get the name passed during allocation of the NFP CPP area handle
- *
- * @param	cpp_area	NFP CPP area handle
- * @return			Pointer to the area's name
- */
 const char *nfp_cpp_area_name(struct nfp_cpp_area *cpp_area);
 
-/*
- * Read a block of data from a NFP CPP ID
- *
- * @param[in]	cpp	NFP CPP handle
- * @param[in]	cpp_id	NFP CPP ID
- * @param[in]	address	Offset into the NFP CPP ID address space
- * @param[in]	kernel_vaddr	Buffer to copy read data to
- * @param[in]	length	Size of the area to reserve
- *
- * @return bytes read on success, -1 on failure.
- */
 int nfp_cpp_read(struct nfp_cpp *cpp, uint32_t cpp_id,
 		uint64_t address, void *kernel_vaddr, size_t length);
 
-/*
- * Write a block of data to a NFP CPP ID
- *
- * @param[in]	cpp	NFP CPP handle
- * @param[in]	cpp_id	NFP CPP ID
- * @param[in]	address	Offset into the NFP CPP ID address space
- * @param[in]	kernel_vaddr	Buffer to copy write data from
- * @param[in]	length	Size of the area to reserve
- *
- * @return bytes written on success, -1 on failure.
- */
 int nfp_cpp_write(struct nfp_cpp *cpp, uint32_t cpp_id,
 		uint64_t address, const void *kernel_vaddr, size_t length);
 
-/*
- * Read a single 32-bit value from a NFP CPP area handle
- *
- * @param area		NFP CPP area handle
- * @param offset	offset into NFP CPP area handle
- * @param value		output value
- *
- * The area must be acquired with 'nfp_cpp_area_acquire()' before calling this
- * operation.
- *
- * NOTE: offset must be 32-bit aligned.
- *
- * @return 0 on success, or -1 on error.
- */
 int nfp_cpp_area_readl(struct nfp_cpp_area *area, uint32_t offset,
 		uint32_t *value);
 
-/*
- * Write a single 32-bit value to a NFP CPP area handle
- *
- * @param area		NFP CPP area handle
- * @param offset	offset into NFP CPP area handle
- * @param value		value to write
- *
- * The area must be acquired with 'nfp_cpp_area_acquire()' before calling this
- * operation.
- *
- * NOTE: offset must be 32-bit aligned.
- *
- * @return 0 on success, or -1 on error.
- */
 int nfp_cpp_area_writel(struct nfp_cpp_area *area, uint32_t offset,
 		uint32_t value);
 
-/*
- * Read a single 64-bit value from a NFP CPP area handle
- *
- * @param area		NFP CPP area handle
- * @param offset	offset into NFP CPP area handle
- * @param value		output value
- *
- * The area must be acquired with 'nfp_cpp_area_acquire()' before calling this
- * operation.
- *
- * NOTE: offset must be 64-bit aligned.
- *
- * @return 0 on success, or -1 on error.
- */
 int nfp_cpp_area_readq(struct nfp_cpp_area *area, uint32_t offset,
 		uint64_t *value);
 
-/*
- * Write a single 64-bit value to a NFP CPP area handle
- *
- * @param area		NFP CPP area handle
- * @param offset	offset into NFP CPP area handle
- * @param value		value to write
- *
- * The area must be acquired with 'nfp_cpp_area_acquire()' before calling this
- * operation.
- *
- * NOTE: offset must be 64-bit aligned.
- *
- * @return 0 on success, or -1 on error.
- */
 int nfp_cpp_area_writeq(struct nfp_cpp_area *area, uint32_t offset,
 		uint64_t value);
 
-/*
- * Write a single 32-bit value on the XPB bus
- *
- * @param cpp           NFP CPP device handle
- * @param xpb_tgt	XPB target and address
- * @param value         value to write
- *
- * @return 0 on success, or -1 on failure.
- */
 int nfp_xpb_writel(struct nfp_cpp *cpp, uint32_t xpb_tgt, uint32_t value);
 
-/*
- * Read a single 32-bit value from the XPB bus
- *
- * @param cpp           NFP CPP device handle
- * @param xpb_tgt	XPB target and address
- * @param value         output value
- *
- * @return 0 on success, or -1 on failure.
- */
 int nfp_xpb_readl(struct nfp_cpp *cpp, uint32_t xpb_tgt, uint32_t *value);
 
-/*
- * Read a 32-bit word from a NFP CPP ID
- *
- * @param cpp           NFP CPP handle
- * @param cpp_id        NFP CPP ID
- * @param address       offset into the NFP CPP ID address space
- * @param value         output value
- *
- * @return 0 on success, or -1 on failure.
- */
 int nfp_cpp_readl(struct nfp_cpp *cpp, uint32_t cpp_id,
 		uint64_t address, uint32_t *value);
 
-/*
- * Write a 32-bit value to a NFP CPP ID
- *
- * @param cpp           NFP CPP handle
- * @param cpp_id        NFP CPP ID
- * @param address       offset into the NFP CPP ID address space
- * @param value         value to write
- *
- * @return 0 on success, or -1 on failure.
- *
- */
 int nfp_cpp_writel(struct nfp_cpp *cpp, uint32_t cpp_id,
 		uint64_t address, uint32_t value);
 
-/*
- * Read a 64-bit work from a NFP CPP ID
- *
- * @param cpp           NFP CPP handle
- * @param cpp_id        NFP CPP ID
- * @param address       offset into the NFP CPP ID address space
- * @param value         output value
- *
- * @return 0 on success, or -1 on failure.
- */
 int nfp_cpp_readq(struct nfp_cpp *cpp, uint32_t cpp_id,
 		uint64_t address, uint64_t *value);
 
-/*
- * Write a 64-bit value to a NFP CPP ID
- *
- * @param cpp           NFP CPP handle
- * @param cpp_id        NFP CPP ID
- * @param address       offset into the NFP CPP ID address space
- * @param value         value to write
- *
- * @return 0 on success, or -1 on failure.
- */
 int nfp_cpp_writeq(struct nfp_cpp *cpp, uint32_t cpp_id,
 		uint64_t address, uint64_t value);
 
-/*
- * Initialize a mutex location
-
- * The CPP target:address must point to a 64-bit aligned location, and will
- * initialize 64 bits of data at the location.
- *
- * This creates the initial mutex state, as locked by this nfp_cpp_interface().
- *
- * This function should only be called when setting up the initial lock state
- * upon boot-up of the system.
- *
- * @param cpp		NFP CPP handle
- * @param target	NFP CPP target ID
- * @param address	Offset into the address space of the NFP CPP target ID
- * @param key_id	Unique 32-bit value for this mutex
- *
- * @return 0 on success, negative value on failure.
- */
 int nfp_cpp_mutex_init(struct nfp_cpp *cpp, int target,
 		uint64_t address, uint32_t key_id);
 
-/*
- * Create a mutex handle from an address controlled by a MU Atomic engine
- *
- * The CPP target:address must point to a 64-bit aligned location, and reserve
- * 64 bits of data at the location for use by the handle.
- *
- * Only target/address pairs that point to entities that support the MU Atomic
- * Engine's CmpAndSwap32 command are supported.
- *
- * @param cpp		NFP CPP handle
- * @param target	NFP CPP target ID
- * @param address	Offset into the address space of the NFP CPP target ID
- * @param key_id	32-bit unique key (must match the key at this location)
- *
- * @return		A non-NULL struct nfp_cpp_mutex * on success, NULL on
- *                      failure.
- */
 struct nfp_cpp_mutex *nfp_cpp_mutex_alloc(struct nfp_cpp *cpp, int target,
 		uint64_t address, uint32_t key_id);
 
-/*
- * Free a mutex handle - does not alter the lock state
- *
- * @param mutex		NFP CPP Mutex handle
- */
 void nfp_cpp_mutex_free(struct nfp_cpp_mutex *mutex);
 
-/*
- * Lock a mutex handle, using the NFP MU Atomic Engine
- *
- * @param mutex		NFP CPP Mutex handle
- *
- * @return 0 on success, negative value on failure.
- */
 int nfp_cpp_mutex_lock(struct nfp_cpp_mutex *mutex);
 
-/*
- * Unlock a mutex handle, using the NFP MU Atomic Engine
- *
- * @param mutex		NFP CPP Mutex handle
- *
- * @return 0 on success, negative value on failure.
- */
 int nfp_cpp_mutex_unlock(struct nfp_cpp_mutex *mutex);
 
-/*
- * Attempt to lock a mutex handle, using the NFP MU Atomic Engine
- *
- * @param mutex		NFP CPP Mutex handle
- * @return		0 if the lock succeeded, negative value on failure.
- */
 int nfp_cpp_mutex_trylock(struct nfp_cpp_mutex *mutex);
 
 uint32_t nfp_cpp_mu_locality_lsb(struct nfp_cpp *cpp);
diff --git a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
index bdf4a658f5..7e94bfb611 100644
--- a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
+++ b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
@@ -58,7 +58,7 @@ 
  * Minimal size of the PCIe cfg memory we depend on being mapped,
  * queue controller and DMA controller don't have to be covered.
  */
-#define NFP_PCI_MIN_MAP_SIZE				0x080000
+#define NFP_PCI_MIN_MAP_SIZE				0x080000        /* 512K */
 
 #define NFP_PCIE_P2C_FIXED_SIZE(bar)               (1 << (bar)->bitsize)
 #define NFP_PCIE_P2C_BULK_SIZE(bar)                (1 << (bar)->bitsize)
@@ -72,40 +72,25 @@ 
 #define NFP_PCIE_CPP_BAR_PCIETOCPPEXPBAR(bar, slot) \
 	(((bar) * 8 + (slot)) * 4)
 
-/*
- * Define to enable a bit more verbose debug output.
- * Set to 1 to enable a bit more verbose debug output.
- */
 struct nfp_pcie_user;
 struct nfp6000_area_priv;
 
-/*
- * struct nfp_bar - describes BAR configuration and usage
- * @nfp:	backlink to owner
- * @barcfg:	cached contents of BAR config CSR
- * @base:	the BAR's base CPP offset
- * @mask:       mask for the BAR aperture (read only)
- * @bitsize:	bitsize of BAR aperture (read only)
- * @index:	index of the BAR
- * @lock:	lock to specify if bar is in use
- * @refcnt:	number of current users
- * @iomem:	mapped IO memory
- */
+/* Describes BAR configuration and usage */
 #define NFP_BAR_MIN 1
 #define NFP_BAR_MID 5
 #define NFP_BAR_MAX 7
 
 struct nfp_bar {
-	struct nfp_pcie_user *nfp;
-	uint32_t barcfg;
-	uint64_t base;		/* CPP address base */
-	uint64_t mask;		/* Bit mask of the bar */
-	uint32_t bitsize;	/* Bit size of the bar */
-	uint32_t index;
-	int lock;
+	struct nfp_pcie_user *nfp;    /**< Backlink to owner */
+	uint32_t barcfg;     /**< BAR config CSR */
+	uint64_t base;       /**< Base CPP offset */
+	uint64_t mask;       /**< Mask of the BAR aperture (read only) */
+	uint32_t bitsize;    /**< Bit size of the BAR aperture (read only) */
+	uint32_t index;      /**< Index of the BAR */
+	int lock;            /**< If the BAR has been locked */
 
 	char *csr;
-	char *iomem;
+	char *iomem;         /**< mapped IO memory */
 };
 
 #define BUSDEV_SZ	13
@@ -360,9 +345,7 @@  nfp_disable_bars(struct nfp_pcie_user *nfp)
 	}
 }
 
-/*
- * Generic CPP bus access interface.
- */
+/* Generic CPP bus access interface. */
 
 struct nfp6000_area_priv {
 	struct nfp_bar *bar;
diff --git a/drivers/net/nfp/nfpcore/nfp_cppcore.c b/drivers/net/nfp/nfpcore/nfp_cppcore.c
index e2af888a28..0e8372576e 100644
--- a/drivers/net/nfp/nfpcore/nfp_cppcore.c
+++ b/drivers/net/nfp/nfpcore/nfp_cppcore.c
@@ -26,6 +26,15 @@ 
 #define NFP_PL_DEVICE_MODEL_MASK               (NFP_PL_DEVICE_PART_MASK | \
 						NFP_PL_DEVICE_ID_MASK)
 
+/**
+ * Set the private data of the nfp_cpp instance
+ *
+ * @param cpp
+ *   NFP CPP operations structure
+ *
+ * @return
+ *   Opaque device pointer
+ */
 void
 nfp_cpp_priv_set(struct nfp_cpp *cpp,
 		void *priv)
@@ -33,12 +42,29 @@  nfp_cpp_priv_set(struct nfp_cpp *cpp,
 	cpp->priv = priv;
 }
 
+/**
+ * Return the private data of the nfp_cpp instance
+ *
+ * @param cpp
+ *   NFP CPP operations structure
+ *
+ * @return
+ *   Opaque device pointer
+ */
 void *
 nfp_cpp_priv(struct nfp_cpp *cpp)
 {
 	return cpp->priv;
 }
 
+/**
+ * Set the model id
+ *
+ * @param cpp
+ *   NFP CPP operations structure
+ * @param model
+ *   Model ID
+ */
 void
 nfp_cpp_model_set(struct nfp_cpp *cpp,
 		uint32_t model)
@@ -46,6 +72,15 @@  nfp_cpp_model_set(struct nfp_cpp *cpp,
 	cpp->model = model;
 }
 
+/**
+ * Retrieve the Model ID of the NFP
+ *
+ * @param cpp
+ *   NFP CPP handle
+ *
+ * @return
+ *   NFP CPP Model ID
+ */
 uint32_t
 nfp_cpp_model(struct nfp_cpp *cpp)
 {
@@ -63,6 +98,14 @@  nfp_cpp_model(struct nfp_cpp *cpp)
 	return model;
 }
 
+/**
+ * Set the private instance owned data of a nfp_cpp struct
+ *
+ * @param cpp
+ *   NFP CPP operations structure
+ * @param interface
+ *   Interface ID
+ */
 void
 nfp_cpp_interface_set(struct nfp_cpp *cpp,
 		uint32_t interface)
@@ -70,6 +113,17 @@  nfp_cpp_interface_set(struct nfp_cpp *cpp,
 	cpp->interface = interface;
 }
 
+/**
+ * Retrieve the Serial ID of the NFP
+ *
+ * @param cpp
+ *   NFP CPP handle
+ * @param serial
+ *   Pointer to NFP serial number
+ *
+ * @return
+ *   Length of NFP serial number
+ */
 int
 nfp_cpp_serial(struct nfp_cpp *cpp,
 		const uint8_t **serial)
@@ -78,6 +132,16 @@  nfp_cpp_serial(struct nfp_cpp *cpp,
 	return cpp->serial_len;
 }
 
+/**
+ * Set the private instance owned data of a nfp_cpp struct
+ *
+ * @param cpp
+ *   NFP CPP operations structure
+ * @param serial
+ *   NFP serial byte array
+ * @param serial_len
+ *   Length of the serial byte array
+ */
 int
 nfp_cpp_serial_set(struct nfp_cpp *cpp,
 		const uint8_t *serial,
@@ -96,6 +160,15 @@  nfp_cpp_serial_set(struct nfp_cpp *cpp,
 	return 0;
 }
 
+/**
+ * Retrieve the Interface ID of the NFP
+ *
+ * @param cpp
+ *   NFP CPP handle
+ *
+ * @return
+ *   NFP CPP Interface ID
+ */
 uint16_t
 nfp_cpp_interface(struct nfp_cpp *cpp)
 {
@@ -105,18 +178,45 @@  nfp_cpp_interface(struct nfp_cpp *cpp)
 	return cpp->interface;
 }
 
+/**
+ * Get the privately allocated portion of a NFP CPP area handle
+ *
+ * @param cpp_area
+ *   NFP CPP area handle
+ *
+ * @return
+ *   Pointer to the private area, or NULL on failure
+ */
 void *
 nfp_cpp_area_priv(struct nfp_cpp_area *cpp_area)
 {
 	return &cpp_area[1];
 }
 
+/**
+ * Get the NFP CPP handle that is the pci_dev of a NFP CPP area handle
+ *
+ * @param cpp_area
+ *   NFP CPP area handle
+ *
+ * @return
+ *   NFP CPP handle
+ */
 struct nfp_cpp *
 nfp_cpp_area_cpp(struct nfp_cpp_area *cpp_area)
 {
 	return cpp_area->cpp;
 }
 
+/**
+ * Get the name passed during allocation of the NFP CPP area handle
+ *
+ * @param cpp_area
+ *   NFP CPP area handle
+ *
+ * @return
+ *   Pointer to the area's name
+ */
 const char *
 nfp_cpp_area_name(struct nfp_cpp_area *cpp_area)
 {
@@ -153,15 +253,24 @@  nfp_cpp_mu_locality_lsb(struct nfp_cpp *cpp)
 	return cpp->mu_locality_lsb;
 }
 
-/*
- * nfp_cpp_area_alloc - allocate a new CPP area
- * @cpp:    CPP handle
- * @dest:   CPP id
- * @address:    start address on CPP target
- * @size:   size of area in bytes
+/**
+ * Allocate and initialize a CPP area structure.
+ * The area must later be locked down with an 'acquire' before
+ * it can be safely accessed.
  *
- * Allocate and initialize a CPP area structure.  The area must later
- * be locked down with an 'acquire' before it can be safely accessed.
+ * @param cpp
+ *   CPP device handle
+ * @param dest
+ *   CPP id
+ * @param name
+ *   Name of region
+ * @param address
+ *   Address of region
+ * @param size
+ *   Size of region
+ *
+ * @return
+ *   NFP CPP area handle, or NULL
  *
  * NOTE: @address and @size must be 32-bit aligned values.
  */
@@ -211,6 +320,25 @@  nfp_cpp_area_alloc_with_name(struct nfp_cpp *cpp,
 	return area;
 }
 
+/**
+ * Allocate and initialize a CPP area structure.
+ * The area must later be locked down with an 'acquire' before
+ * it can be safely accessed.
+ *
+ * @param cpp
+ *   CPP device handle
+ * @param dest
+ *   CPP id
+ * @param address
+ *   Address of region
+ * @param size
+ *   Size of region
+ *
+ * @return
+ *   NFP CPP area handle, or NULL
+ *
+ * NOTE: @address and @size must be 32-bit aligned values.
+ */
 struct nfp_cpp_area *
 nfp_cpp_area_alloc(struct nfp_cpp *cpp,
 		uint32_t dest,
@@ -220,17 +348,22 @@  nfp_cpp_area_alloc(struct nfp_cpp *cpp,
 	return nfp_cpp_area_alloc_with_name(cpp, dest, NULL, address, size);
 }
 
-/*
- * nfp_cpp_area_alloc_acquire - allocate a new CPP area and lock it down
- *
- * @cpp:    CPP handle
- * @dest:   CPP id
- * @address:    start address on CPP target
- * @size:   size of area
- *
+/**
  * Allocate and initialize a CPP area structure, and lock it down so
  * that it can be accessed directly.
  *
+ * @param cpp
+ *   CPP device handle
+ * @param destination
+ *   CPP id
+ * @param address
+ *   Address of region
+ * @param size
+ *   Size of region
+ *
+ * @return
+ *   NFP CPP area handle, or NULL
+ *
  * NOTE: @address and @size must be 32-bit aligned values.
  *
  * NOTE: The area must also be 'released' when the structure is freed.
@@ -258,11 +391,11 @@  nfp_cpp_area_alloc_acquire(struct nfp_cpp *cpp,
 	return area;
 }
 
-/*
- * nfp_cpp_area_free - free up the CPP area
- * area:    CPP area handle
- *
+/**
  * Frees up memory resources held by the CPP area.
+ *
+ * @param area
+ *   CPP area handle
  */
 void
 nfp_cpp_area_free(struct nfp_cpp_area *area)
@@ -272,11 +405,11 @@  nfp_cpp_area_free(struct nfp_cpp_area *area)
 	free(area);
 }
 
-/*
- * nfp_cpp_area_release_free - release CPP area and free it
- * area:    CPP area handle
+/**
+ * Releases CPP area and frees up memory resources held by it.
  *
- * Releases CPP area and frees up memory resources held by the it.
+ * @param area
+ *   CPP area handle
  */
 void
 nfp_cpp_area_release_free(struct nfp_cpp_area *area)
@@ -285,12 +418,15 @@  nfp_cpp_area_release_free(struct nfp_cpp_area *area)
 	nfp_cpp_area_free(area);
 }
 
-/*
- * nfp_cpp_area_acquire - lock down a CPP area for access
- * @area:   CPP area handle
+/**
+ * Locks down the CPP area for a potential long term activity.
+ * Area must always be locked down before being accessed.
  *
- * Locks down the CPP area for a potential long term activity.  Area
- * must always be locked down before being accessed.
+ * @param area
+ *   CPP area handle
+ *
+ * @return
+ *   0 on success, -1 on failure.
  */
 int
 nfp_cpp_area_acquire(struct nfp_cpp_area *area)
@@ -307,11 +443,11 @@  nfp_cpp_area_acquire(struct nfp_cpp_area *area)
 	return 0;
 }
 
-/*
- * nfp_cpp_area_release - release a locked down CPP area
- * @area:   CPP area handle
- *
+/**
  * Releases a previously locked down CPP area.
+ *
+ * @param area
+ *   CPP area handle
  */
 void
 nfp_cpp_area_release(struct nfp_cpp_area *area)
@@ -320,16 +456,16 @@  nfp_cpp_area_release(struct nfp_cpp_area *area)
 		area->cpp->op->area_release(area);
 }
 
-/*
- * nfp_cpp_area_iomem() - get IOMEM region for CPP area
+/**
+ * Returns an iomem pointer for use with readl()/writel() style operations.
  *
- * @area:       CPP area handle
+ * @param area
+ *   CPP area handle
  *
- * Returns an iomem pointer for use with readl()/writel() style operations.
+ * @return
+ *   Pointer to the area, or NULL
  *
  * NOTE: Area must have been locked down with an 'acquire'.
- *
- * Return: pointer to the area, or NULL
  */
 void *
 nfp_cpp_area_iomem(struct nfp_cpp_area *area)
@@ -342,18 +478,22 @@  nfp_cpp_area_iomem(struct nfp_cpp_area *area)
 	return iomem;
 }
 
-/*
- * nfp_cpp_area_read - read data from CPP area
+/**
+ * Read data from indicated CPP region.
  *
- * @area:       CPP area handle
- * @offset:     offset into CPP area
- * @kernel_vaddr:   kernel address to put data into
- * @length:     number of bytes to read
+ * @param area
+ *   CPP area handle
+ * @param offset
+ *   Offset into CPP area
+ * @param kernel_vaddr
+ *   Address to put data into
+ * @param length
+ *   Number of bytes to read
  *
- * Read data from indicated CPP region.
+ * @return
+ *   Length of io, or -ERRNO
  *
  * NOTE: @offset and @length must be 32-bit aligned values.
- *
  * NOTE: Area must have been locked down with an 'acquire'.
  */
 int
@@ -368,18 +508,22 @@  nfp_cpp_area_read(struct nfp_cpp_area *area,
 	return area->cpp->op->area_read(area, kernel_vaddr, offset, length);
 }
 
-/*
- * nfp_cpp_area_write - write data to CPP area
+/**
+ * Write data to indicated CPP region.
  *
- * @area:       CPP area handle
- * @offset:     offset into CPP area
- * @kernel_vaddr:   kernel address to read data from
- * @length:     number of bytes to write
+ * @param area
+ *   CPP area handle
+ * @param offset
+ *   Offset into CPP area
+ * @param kernel_vaddr
+ *   Address to put data into
+ * @param length
+ *   Number of bytes to read
  *
- * Write data to indicated CPP region.
+ * @return
+ *   Length of io, or -ERRNO
  *
  * NOTE: @offset and @length must be 32-bit aligned values.
- *
  * NOTE: Area must have been locked down with an 'acquire'.
  */
 int
@@ -436,6 +580,19 @@  nfp_xpb_to_cpp(struct nfp_cpp *cpp,
 	return xpb;
 }
 
+/**
+ * Read a uint32_t value from an area
+ *
+ * @param area
+ *   CPP Area handle
+ * @param offset
+ *   Offset into area
+ * @param value
+ *   Pointer to read buffer
+ *
+ * @return
+ *   0 on success, or -ERRNO
+ */
 int
 nfp_cpp_area_readl(struct nfp_cpp_area *area,
 		uint32_t offset,
@@ -450,6 +607,19 @@  nfp_cpp_area_readl(struct nfp_cpp_area *area,
 	return (sz == sizeof(*value)) ? 0 : -1;
 }
 
+/**
+ * Write a uint32_t vale to an area
+ *
+ * @param area
+ *   CPP Area handle
+ * @param offset
+ *   Offset into area
+ * @param value
+ *   Value to write
+ *
+ * @return
+ *   0 on success, or -ERRNO
+ */
 int
 nfp_cpp_area_writel(struct nfp_cpp_area *area,
 		uint32_t offset,
@@ -462,6 +632,19 @@  nfp_cpp_area_writel(struct nfp_cpp_area *area,
 	return (sz == sizeof(value)) ? 0 : -1;
 }
 
+/**
+ * Read a uint64_t value from an area
+ *
+ * @param area
+ *   CPP Area handle
+ * @param offset
+ *   Offset into area
+ * @param value
+ *   Pointer to read buffer
+ *
+ * @return
+ *   0 on success, or -ERRNO
+ */
 int
 nfp_cpp_area_readq(struct nfp_cpp_area *area,
 		uint32_t offset,
@@ -476,6 +659,19 @@  nfp_cpp_area_readq(struct nfp_cpp_area *area,
 	return (sz == sizeof(*value)) ? 0 : -1;
 }
 
+/**
+ * Write a uint64_t vale to an area
+ *
+ * @param area
+ *   CPP Area handle
+ * @param offset
+ *   Offset into area
+ * @param value
+ *   Value to write
+ *
+ * @return
+ *   0 on success, or -ERRNO
+ */
 int
 nfp_cpp_area_writeq(struct nfp_cpp_area *area,
 		uint32_t offset,
@@ -489,6 +685,21 @@  nfp_cpp_area_writeq(struct nfp_cpp_area *area,
 	return (sz == sizeof(value)) ? 0 : -1;
 }
 
+/**
+ * Read a uint32_t value from a CPP location
+ *
+ * @param cpp
+ *   CPP device handle
+ * @param cpp_id
+ *   CPP ID for operation
+ * @param address
+ *   Address for operation
+ * @param value
+ *   Pointer to read buffer
+ *
+ * @return
+ *   0 on success, or -ERRNO
+ */
 int
 nfp_cpp_readl(struct nfp_cpp *cpp,
 		uint32_t cpp_id,
@@ -504,6 +715,21 @@  nfp_cpp_readl(struct nfp_cpp *cpp,
 	return (sz == sizeof(*value)) ? 0 : -1;
 }
 
+/**
+ * Write a uint32_t value to a CPP location
+ *
+ * @param cpp
+ *   CPP device handle
+ * @param cpp_id
+ *   CPP ID for operation
+ * @param address
+ *   Address for operation
+ * @param value
+ *   Value to write
+ *
+ * @return
+ *   0 on success, or -ERRNO
+ */
 int
 nfp_cpp_writel(struct nfp_cpp *cpp,
 		uint32_t cpp_id,
@@ -518,6 +744,21 @@  nfp_cpp_writel(struct nfp_cpp *cpp,
 	return (sz == sizeof(value)) ? 0 : -1;
 }
 
+/**
+ * Read a uint64_t value from a CPP location
+ *
+ * @param cpp
+ *   CPP device handle
+ * @param cpp_id
+ *   CPP ID for operation
+ * @param address
+ *   Address for operation
+ * @param value
+ *   Pointer to read buffer
+ *
+ * @return
+ *   0 on success, or -ERRNO
+ */
 int
 nfp_cpp_readq(struct nfp_cpp *cpp,
 		uint32_t cpp_id,
@@ -533,6 +774,21 @@  nfp_cpp_readq(struct nfp_cpp *cpp,
 	return (sz == sizeof(*value)) ? 0 : -1;
 }
 
+/**
+ * Write a uint64_t value to a CPP location
+ *
+ * @param cpp
+ *   CPP device handle
+ * @param cpp_id
+ *   CPP ID for operation
+ * @param address
+ *   Address for operation
+ * @param value
+ *   Value to write
+ *
+ * @return
+ *   0 on success, or -ERRNO
+ */
 int
 nfp_cpp_writeq(struct nfp_cpp *cpp,
 		uint32_t cpp_id,
@@ -547,6 +803,19 @@  nfp_cpp_writeq(struct nfp_cpp *cpp,
 	return (sz == sizeof(value)) ? 0 : -1;
 }
 
+/**
+ * Write a uint32_t word to a XPB location
+ *
+ * @param cpp
+ *   CPP device handle
+ * @param xpb_addr
+ *   XPB target and address
+ * @param value
+ *   Value to write
+ *
+ * @return
+ *   0 on success, or -ERRNO
+ */
 int
 nfp_xpb_writel(struct nfp_cpp *cpp,
 		uint32_t xpb_addr,
@@ -559,6 +828,19 @@  nfp_xpb_writel(struct nfp_cpp *cpp,
 	return nfp_cpp_writel(cpp, cpp_dest, xpb_addr, value);
 }
 
+/**
+ * Read a uint32_t value from a XPB location
+ *
+ * @param cpp
+ *   CPP device handle
+ * @param xpb_addr
+ *   XPB target and address
+ * @param value
+ *   Pointer to read buffer
+ *
+ * @return
+ *   0 on success, or -ERRNO
+ */
 int
 nfp_xpb_readl(struct nfp_cpp *cpp,
 		uint32_t xpb_addr,
@@ -625,9 +907,11 @@  nfp_cpp_alloc(struct rte_pci_device *dev,
 	return cpp;
 }
 
-/*
- * nfp_cpp_free - free the CPP handle
- * @cpp:    CPP handle
+/**
+ * Free the CPP handle
+ *
+ * @param cpp
+ *   CPP handle
  */
 void
 nfp_cpp_free(struct nfp_cpp *cpp)
@@ -641,6 +925,19 @@  nfp_cpp_free(struct nfp_cpp *cpp)
 	free(cpp);
 }
 
+/**
+ * Create a NFP CPP handle from device
+ *
+ * @param dev
+ *   PCI device
+ * @param driver_lock_needed
+ *   Driver lock flag
+ *
+ * @return
+ *   NFP CPP handle on success, NULL on failure
+ *
+ * NOTE: On failure, cpp_ops->free will be called!
+ */
 struct nfp_cpp *
 nfp_cpp_from_device_name(struct rte_pci_device *dev,
 		int driver_lock_needed)
@@ -648,13 +945,22 @@  nfp_cpp_from_device_name(struct rte_pci_device *dev,
 	return nfp_cpp_alloc(dev, driver_lock_needed);
 }
 
-/*
- * nfp_cpp_read - read from CPP target
- * @cpp:        CPP handle
- * @destination:    CPP id
- * @address:        offset into CPP target
- * @kernel_vaddr:   kernel buffer for result
- * @length:     number of bytes to read
+/**
+ * Read from CPP target
+ *
+ * @param cpp
+ *   CPP handle
+ * @param destination
+ *   CPP id
+ * @param address
+ *   Offset into CPP target
+ * @param kernel_vaddr
+ *   Buffer for result
+ * @param length
+ *   Number of bytes to read
+ *
+ * @return
+ *   Length of io, or -ERRNO
  */
 int
 nfp_cpp_read(struct nfp_cpp *cpp,
@@ -678,13 +984,22 @@  nfp_cpp_read(struct nfp_cpp *cpp,
 	return err;
 }
 
-/*
- * nfp_cpp_write - write to CPP target
- * @cpp:        CPP handle
- * @destination:    CPP id
- * @address:        offset into CPP target
- * @kernel_vaddr:   kernel buffer to read from
- * @length:     number of bytes to write
+/**
+ * Write to CPP target
+ *
+ * @param cpp
+ *   CPP handle
+ * @param destination
+ *   CPP id
+ * @param address
+ *   Offset into CPP target
+ * @param kernel_vaddr
+ *   Buffer to read from
+ * @param length
+ *   Number of bytes to write
+ *
+ * @return
+ *   Length of io, or -ERRNO
  */
 int
 nfp_cpp_write(struct nfp_cpp *cpp,
@@ -731,18 +1046,23 @@  __nfp_cpp_model_autodetect(struct nfp_cpp *cpp,
 	return 0;
 }
 
-/*
- * nfp_cpp_map_area() - Helper function to map an area
- * @cpp:    NFP CPP handler
- * @cpp_id: CPP ID
- * @addr:   CPP address
- * @size:   Size of the area
- * @area:   Area handle (output)
+/**
+ * Map an area of IOMEM access.
+ * To undo the effect of this function call @nfp_cpp_area_release_free(*area).
  *
- * Map an area of IOMEM access.  To undo the effect of this function call
- * @nfp_cpp_area_release_free(*area).
+ * @param cpp
+ *   NFP CPP handler
+ * @param cpp_id
+ *   CPP id
+ * @param addr
+ *   CPP address
+ * @param size
+ *   Size of the area
+ * @param area
+ *   Area handle (output)
  *
- * Return: Pointer to memory mapped area or NULL
+ * @return
+ *   Pointer to memory mapped area or NULL
  */
 uint8_t *
 nfp_cpp_map_area(struct nfp_cpp *cpp,
diff --git a/drivers/net/nfp/nfpcore/nfp_hwinfo.c b/drivers/net/nfp/nfpcore/nfp_hwinfo.c
index b658b5e900..f5579ab60f 100644
--- a/drivers/net/nfp/nfpcore/nfp_hwinfo.c
+++ b/drivers/net/nfp/nfpcore/nfp_hwinfo.c
@@ -3,7 +3,8 @@ 
  * All rights reserved.
  */
 
-/* Parse the hwinfo table that the ARM firmware builds in the ARM scratch SRAM
+/*
+ * Parse the hwinfo table that the ARM firmware builds in the ARM scratch SRAM
  * after chip reset.
  *
  * Examples of the fields:
@@ -146,7 +147,7 @@  nfp_hwinfo_fetch(struct nfp_cpp *cpp,
 	struct nfp_hwinfo *db;
 
 	wait.tv_sec = 0;
-	wait.tv_nsec = 10000000;
+	wait.tv_nsec = 10000000;    /* 10ms */
 
 	for (;;) {
 		db = nfp_hwinfo_try_fetch(cpp, hwdb_size);
@@ -154,7 +155,7 @@  nfp_hwinfo_fetch(struct nfp_cpp *cpp,
 			return db;
 
 		nanosleep(&wait, NULL);
-		if (count++ > 200) {
+		if (count++ > 200) {    /* 10ms * 200 = 2s */
 			PMD_DRV_LOG(ERR, "NFP access error");
 			return NULL;
 		}
@@ -180,12 +181,16 @@  nfp_hwinfo_read(struct nfp_cpp *cpp)
 	return db;
 }
 
-/*
- * nfp_hwinfo_lookup() - Find a value in the HWInfo table by name
- * @hwinfo:	NFP HWinfo table
- * @lookup:	HWInfo name to search for
+/**
+ * Find a value in the HWInfo table by name
+ *
+ * @param hwinfo
+ *   NFP HWInfo table
+ * @param lookup
+ *   HWInfo name to search for
  *
- * Return: Value of the HWInfo name, or NULL
+ * @return
+ *   Value of the HWInfo name, or NULL
  */
 const char *
 nfp_hwinfo_lookup(struct nfp_hwinfo *hwinfo,
diff --git a/drivers/net/nfp/nfpcore/nfp_hwinfo.h b/drivers/net/nfp/nfpcore/nfp_hwinfo.h
index a3da7512db..424db8035d 100644
--- a/drivers/net/nfp/nfpcore/nfp_hwinfo.h
+++ b/drivers/net/nfp/nfpcore/nfp_hwinfo.h
@@ -59,6 +59,8 @@ 
  * Packed UTF8Z strings, ie 'key1\000value1\000key2\000value2\000'
  *
  * Unsorted.
+ *
+ * Note: Only the HwInfo v2 Table be supported now.
  */
 
 #define NFP_HWINFO_VERSION_1 ('H' << 24 | 'I' << 16 | 1 << 8 | 0 << 1 | 0)
diff --git a/drivers/net/nfp/nfpcore/nfp_mip.c b/drivers/net/nfp/nfpcore/nfp_mip.c
index 086e82db70..0892c99e96 100644
--- a/drivers/net/nfp/nfpcore/nfp_mip.c
+++ b/drivers/net/nfp/nfpcore/nfp_mip.c
@@ -87,15 +87,16 @@  nfp_mip_read_resource(struct nfp_cpp *cpp,
 	return err;
 }
 
-/*
- * nfp_mip_open() - Get device MIP structure
- * @cpp:	NFP CPP Handle
- *
- * Copy MIP structure from NFP device and return it.  The returned
+/**
+ * Copy MIP structure from NFP device and return it. The returned
  * structure is handled internally by the library and should be
- * freed by calling nfp_mip_close().
+ * freed by calling @nfp_mip_close().
+ *
+ * @param cpp
+ *   NFP CPP Handle
  *
- * Return: pointer to mip, NULL on failure.
+ * @return
+ *   Pointer to MIP, NULL on failure.
  */
 struct nfp_mip *
 nfp_mip_open(struct nfp_cpp *cpp)
@@ -131,11 +132,15 @@  nfp_mip_name(const struct nfp_mip *mip)
 	return mip->name;
 }
 
-/*
- * nfp_mip_symtab() - Get the address and size of the MIP symbol table
- * @mip:	MIP handle
- * @addr:	Location for NFP DDR address of MIP symbol table
- * @size:	Location for size of MIP symbol table
+/**
+ * Get the address and size of the MIP symbol table.
+ *
+ * @param mip
+ *   MIP handle
+ * @param addr
+ *   Location for NFP DDR address of MIP symbol table
+ * @param size
+ *   Location for size of MIP symbol table
  */
 void
 nfp_mip_symtab(const struct nfp_mip *mip,
@@ -146,11 +151,15 @@  nfp_mip_symtab(const struct nfp_mip *mip,
 	*size = rte_le_to_cpu_32(mip->symtab_size);
 }
 
-/*
- * nfp_mip_strtab() - Get the address and size of the MIP symbol name table
- * @mip:	MIP handle
- * @addr:	Location for NFP DDR address of MIP symbol name table
- * @size:	Location for size of MIP symbol name table
+/**
+ * Get the address and size of the MIP symbol name table.
+ *
+ * @param mip
+ *   MIP handle
+ * @param addr
+ *   Location for NFP DDR address of MIP symbol name table
+ * @param size
+ *   Location for size of MIP symbol name table
  */
 void
 nfp_mip_strtab(const struct nfp_mip *mip,
diff --git a/drivers/net/nfp/nfpcore/nfp_mutex.c b/drivers/net/nfp/nfpcore/nfp_mutex.c
index 82919d8270..404d4fa938 100644
--- a/drivers/net/nfp/nfpcore/nfp_mutex.c
+++ b/drivers/net/nfp/nfpcore/nfp_mutex.c
@@ -53,7 +53,7 @@  _nfp_cpp_mutex_validate(uint32_t model,
 	return 0;
 }
 
-/*
+/**
  * Initialize a mutex location
  *
  * The CPP target:address must point to a 64-bit aligned location, and
@@ -65,13 +65,17 @@  _nfp_cpp_mutex_validate(uint32_t model,
  * This function should only be called when setting up
  * the initial lock state upon boot-up of the system.
  *
- * @param mutex     NFP CPP Mutex handle
- * @param target    NFP CPP target ID (ie NFP_CPP_TARGET_CLS or
- *		    NFP_CPP_TARGET_MU)
- * @param address   Offset into the address space of the NFP CPP target ID
- * @param key       Unique 32-bit value for this mutex
+ * @param cpp
+ *   NFP CPP handle
+ * @param target
+ *   NFP CPP target ID (ie NFP_CPP_TARGET_CLS or NFP_CPP_TARGET_MU)
+ * @param address
+ *   Offset into the address space of the NFP CPP target ID
+ * @param key
+ *   Unique 32-bit value for this mutex
  *
- * @return 0 on success, or negative value on failure.
+ * @return
+ *   0 on success, or negative value on failure
  */
 int
 nfp_cpp_mutex_init(struct nfp_cpp *cpp,
@@ -99,7 +103,7 @@  nfp_cpp_mutex_init(struct nfp_cpp *cpp,
 	return 0;
 }
 
-/*
+/**
  * Create a mutex handle from an address controlled by a MU Atomic engine
  *
  * The CPP target:address must point to a 64-bit aligned location, and
@@ -108,13 +112,17 @@  nfp_cpp_mutex_init(struct nfp_cpp *cpp,
  * Only target/address pairs that point to entities that support the
  * MU Atomic Engine are supported.
  *
- * @param cpp       NFP CPP handle
- * @param target    NFP CPP target ID (ie NFP_CPP_TARGET_CLS or
- *		    NFP_CPP_TARGET_MU)
- * @param address   Offset into the address space of the NFP CPP target ID
- * @param key       32-bit unique key (must match the key at this location)
+ * @param cpp
+ *   NFP CPP handle
+ * @param target
+ *   NFP CPP target ID (ie NFP_CPP_TARGET_CLS or NFP_CPP_TARGET_MU)
+ * @param address
+ *   Offset into the address space of the NFP CPP target ID
+ * @param key
+ *   32-bit unique key (must match the key at this location)
  *
- * @return      A non-NULL struct nfp_cpp_mutex * on success, NULL on failure.
+ * @return
+ *   A non-NULL struct nfp_cpp_mutex * on success, NULL on failure.
  */
 struct nfp_cpp_mutex *
 nfp_cpp_mutex_alloc(struct nfp_cpp *cpp,
@@ -178,10 +186,11 @@  nfp_cpp_mutex_alloc(struct nfp_cpp *cpp,
 	return mutex;
 }
 
-/*
+/**
  * Free a mutex handle - does not alter the lock state
  *
- * @param mutex     NFP CPP Mutex handle
+ * @param mutex
+ *   NFP CPP Mutex handle
  */
 void
 nfp_cpp_mutex_free(struct nfp_cpp_mutex *mutex)
@@ -203,12 +212,14 @@  nfp_cpp_mutex_free(struct nfp_cpp_mutex *mutex)
 	free(mutex);
 }
 
-/*
+/**
  * Lock a mutex handle, using the NFP MU Atomic Engine
  *
- * @param mutex     NFP CPP Mutex handle
+ * @param mutex
+ *   NFP CPP Mutex handle
  *
- * @return 0 on success, or negative value on failure.
+ * @return
+ *   0 on success, or negative value on failure.
  */
 int
 nfp_cpp_mutex_lock(struct nfp_cpp_mutex *mutex)
@@ -229,12 +240,14 @@  nfp_cpp_mutex_lock(struct nfp_cpp_mutex *mutex)
 	return 0;
 }
 
-/*
+/**
  * Unlock a mutex handle, using the NFP MU Atomic Engine
  *
- * @param mutex     NFP CPP Mutex handle
+ * @param mutex
+ *   NFP CPP Mutex handle
  *
- * @return 0 on success, or negative value on failure.
+ * @return
+ *   0 on success, or negative value on failure
  */
 int
 nfp_cpp_mutex_unlock(struct nfp_cpp_mutex *mutex)
@@ -280,16 +293,18 @@  nfp_cpp_mutex_unlock(struct nfp_cpp_mutex *mutex)
 	return err;
 }
 
-/*
+/**
  * Attempt to lock a mutex handle, using the NFP MU Atomic Engine
  *
  * Valid lock states:
- *
  *      0x....0000      - Unlocked
  *      0x....000f      - Locked
  *
- * @param mutex     NFP CPP Mutex handle
- * @return      0 if the lock succeeded, negative value on failure.
+ * @param mutex
+ *   NFP CPP Mutex handle
+ *
+ * @return
+ *   0 if the lock succeeded, negative value on failure.
  */
 int
 nfp_cpp_mutex_trylock(struct nfp_cpp_mutex *mutex)
@@ -352,7 +367,7 @@  nfp_cpp_mutex_trylock(struct nfp_cpp_mutex *mutex)
 		 * If there was another contending for this lock, then
 		 * the lock state would be 0x....000f
 		 *
-		 * Write our owner ID into the lock
+		 * Write our owner ID into the lock.
 		 * While not strictly necessary, this helps with
 		 * debug and bookkeeping.
 		 */
diff --git a/drivers/net/nfp/nfpcore/nfp_nffw.c b/drivers/net/nfp/nfpcore/nfp_nffw.c
index b5a354137d..5f004e3b21 100644
--- a/drivers/net/nfp/nfpcore/nfp_nffw.c
+++ b/drivers/net/nfp/nfpcore/nfp_nffw.c
@@ -52,7 +52,7 @@  nffw_fwinfo_mip_mu_da_get(const struct nffw_fwinfo *fi)
 	return (fi->loaded__mu_da__mip_off_hi >> 8) & 1;
 }
 
-/* mip_offset = (loaded__mu_da__mip_off_hi<7:0> << 8) | mip_offset_lo */
+/* mip_offset = (loaded__mu_da__mip_off_hi<7:0> << 32) | mip_offset_lo */
 static uint64_t
 nffw_fwinfo_mip_offset_get(const struct nffw_fwinfo *fi)
 {
@@ -112,11 +112,14 @@  nffw_res_fwinfos(struct nfp_nffw_info_data *fwinf,
 	}
 }
 
-/*
- * nfp_nffw_info_open() - Acquire the lock on the NFFW table
- * @cpp:	NFP CPP handle
+/**
+ * Acquire the lock on the NFFW table
+ *
+ * @param cpp
+ *   NFP CPP handle
  *
- * Return: nffw info pointer, or NULL on failure
+ * @return
+ *   NFFW info pointer, or NULL on failure
  */
 struct nfp_nffw_info *
 nfp_nffw_info_open(struct nfp_cpp *cpp)
@@ -168,11 +171,11 @@  nfp_nffw_info_open(struct nfp_cpp *cpp)
 	return NULL;
 }
 
-/*
- * nfp_nffw_info_close() - Release the lock on the NFFW table
- * @state:	NFP FW info state
+/**
+ * Release the lock on the NFFW table
  *
- * Return: void
+ * @param state
+ *   NFFW info pointer
  */
 void
 nfp_nffw_info_close(struct nfp_nffw_info *state)
@@ -181,11 +184,14 @@  nfp_nffw_info_close(struct nfp_nffw_info *state)
 	free(state);
 }
 
-/*
- * nfp_nffw_info_fwid_first() - Return the first firmware ID in the NFFW
- * @state:	NFP FW info state
+/**
+ * Return the first firmware ID in the NFFW
  *
- * Return: First NFFW firmware info, NULL on failure
+ * @param state
+ *   NFFW info pointer
+ *
+ * @return:
+ *   First NFFW firmware info, NULL on failure
  */
 static struct nffw_fwinfo *
 nfp_nffw_info_fwid_first(struct nfp_nffw_info *state)
@@ -205,13 +211,18 @@  nfp_nffw_info_fwid_first(struct nfp_nffw_info *state)
 	return NULL;
 }
 
-/*
- * nfp_nffw_info_mip_first() - Retrieve the location of the first FW's MIP
- * @state:	NFP FW info state
- * @cpp_id:	Pointer to the CPP ID of the MIP
- * @off:	Pointer to the CPP Address of the MIP
+/**
+ * Retrieve the location of the first FW's MIP
+ *
+ * @param state
+ *   NFFW info pointer
+ * @param cpp_id
+ *   Pointer to the CPP ID of the MIP
+ * @param off
+ *   Pointer to the CPP Address of the MIP
  *
- * Return: 0, or -ERRNO
+ * @return
+ *   0, or -ERRNO
  */
 int
 nfp_nffw_info_mip_first(struct nfp_nffw_info *state,
diff --git a/drivers/net/nfp/nfpcore/nfp_nffw.h b/drivers/net/nfp/nfpcore/nfp_nffw.h
index 46ac8a8d07..e032b6cce7 100644
--- a/drivers/net/nfp/nfpcore/nfp_nffw.h
+++ b/drivers/net/nfp/nfpcore/nfp_nffw.h
@@ -8,7 +8,8 @@ 
 
 #include "nfp_cpp.h"
 
-/* Init-CSR owner IDs for firmware map to firmware IDs which start at 4.
+/*
+ * Init-CSR owner IDs for firmware map to firmware IDs which start at 4.
  * Lower IDs are reserved for target and loader IDs.
  */
 #define NFFW_FWID_EXT   3 /* For active MEs that we didn't load. */
@@ -16,7 +17,7 @@ 
 
 #define NFFW_FWID_ALL   255
 
-/**
+/*
  * NFFW_INFO_VERSION history:
  * 0: This was never actually used (before versioning), but it refers to
  *    the previous struct which had FWINFO_CNT = MEINFO_CNT = 120 that later
@@ -35,6 +36,7 @@ 
 #define NFFW_MEINFO_CNT_V2 200
 #define NFFW_FWINFO_CNT_V2 200
 
+/* nfp.nffw meinfo */
 struct nffw_meinfo {
 	uint32_t ctxmask__fwid__meid;
 };
diff --git a/drivers/net/nfp/nfpcore/nfp_nsp.c b/drivers/net/nfp/nfpcore/nfp_nsp.c
index 76d418d478..039e4729bd 100644
--- a/drivers/net/nfp/nfpcore/nfp_nsp.c
+++ b/drivers/net/nfp/nfpcore/nfp_nsp.c
@@ -109,9 +109,11 @@  nfp_nsp_check(struct nfp_nsp *state)
 	return 0;
 }
 
-/*
- * nfp_nsp_open() - Prepare for communication and lock the NSP resource.
- * @cpp:	NFP CPP Handle
+/**
+ * Prepare for communication and lock the NSP resource.
+ *
+ * @param cpp
+ *   NFP CPP Handle
  */
 struct nfp_nsp *
 nfp_nsp_open(struct nfp_cpp *cpp)
@@ -145,9 +147,11 @@  nfp_nsp_open(struct nfp_cpp *cpp)
 	return state;
 }
 
-/*
- * nfp_nsp_close() - Clean up and unlock the NSP resource.
- * @state:	NFP SP state
+/**
+ * Clean up and unlock the NSP resource.
+ *
+ * @param state
+ *   NFP SP state
  */
 void
 nfp_nsp_close(struct nfp_nsp *state)
@@ -181,7 +185,7 @@  nfp_nsp_wait_reg(struct nfp_cpp *cpp,
 	struct timespec wait;
 
 	wait.tv_sec = 0;
-	wait.tv_nsec = 25000000;
+	wait.tv_nsec = 25000000;     /* 25ms */
 
 	for (;;) {
 		err = nfp_cpp_readq(cpp, nsp_cpp, addr, reg);
@@ -194,28 +198,27 @@  nfp_nsp_wait_reg(struct nfp_cpp *cpp,
 			return 0;
 
 		nanosleep(&wait, 0);
-		if (count++ > 1000)
+		if (count++ > 1000)     /* 25ms * 1000 = 25s */
 			return -ETIMEDOUT;
 	}
 }
 
-/*
- * nfp_nsp_command() - Execute a command on the NFP Service Processor
- * @state:	NFP SP state
- * @code:	NFP SP Command Code
- * @option:	NFP SP Command Argument
- * @buff_cpp:	NFP SP Buffer CPP Address info
- * @buff_addr:	NFP SP Buffer Host address
- *
- * Return: 0 for success with no result
+/**
+ * Execute a command on the NFP Service Processor
  *
- *	 positive value for NSP completion with a result code
+ * @param state
+ *   NFP SP state
+ * @param arg
+ *   NFP command argument structure
  *
- *	-EAGAIN if the NSP is not yet present
- *	-ENODEV if the NSP is not a supported model
- *	-EBUSY if the NSP is stuck
- *	-EINTR if interrupted while waiting for completion
- *	-ETIMEDOUT if the NSP took longer than 30 seconds to complete
+ * @return
+ *   - 0 for success with no result
+ *   - Positive value for NSP completion with a result code
+ *   - -EAGAIN if the NSP is not yet present
+ *   - -ENODEV if the NSP is not a supported model
+ *   - -EBUSY if the NSP is stuck
+ *   - -EINTR if interrupted while waiting for completion
+ *   - -ETIMEDOUT if the NSP took longer than @timeout_sec seconds to complete
  */
 static int
 nfp_nsp_command(struct nfp_nsp *state,
@@ -383,7 +386,7 @@  nfp_nsp_wait(struct nfp_nsp *state)
 	struct timespec wait;
 
 	wait.tv_sec = 0;
-	wait.tv_nsec = 25000000;
+	wait.tv_nsec = 25000000;    /* 25ms */
 
 	for (;;) {
 		err = nfp_nsp_command(state, SPCODE_NOOP, 0, 0, 0);
@@ -392,7 +395,7 @@  nfp_nsp_wait(struct nfp_nsp *state)
 
 		nanosleep(&wait, 0);
 
-		if (count++ > 1000) {
+		if (count++ > 1000) {    /* 25ms * 1000 = 25s */
 			err = -ETIMEDOUT;
 			break;
 		}
diff --git a/drivers/net/nfp/nfpcore/nfp_nsp.h b/drivers/net/nfp/nfpcore/nfp_nsp.h
index edb56e26ca..0fcb21e99c 100644
--- a/drivers/net/nfp/nfpcore/nfp_nsp.h
+++ b/drivers/net/nfp/nfpcore/nfp_nsp.h
@@ -158,72 +158,45 @@  enum nfp_eth_fec {
 #define NFP_FEC_REED_SOLOMON	RTE_BIT32(NFP_FEC_REED_SOLOMON_BIT)
 #define NFP_FEC_DISABLED	RTE_BIT32(NFP_FEC_DISABLED_BIT)
 
-/**
- * struct nfp_eth_table - ETH table information
- * @count:	number of table entries
- * @max_index:	max of @index fields of all @ports
- * @ports:	table of ports
- *
- * @eth_index:	port index according to legacy ethX numbering
- * @index:	chip-wide first channel index
- * @nbi:	NBI index
- * @base:	first channel index (within NBI)
- * @lanes:	number of channels
- * @speed:	interface speed (in Mbps)
- * @interface:	interface (module) plugged in
- * @media:	media type of the @interface
- * @fec:	forward error correction mode
- * @aneg:	auto negotiation mode
- * @mac_addr:	interface MAC address
- * @label_port:	port id
- * @label_subport:  id of interface within port (for split ports)
- * @enabled:	is enabled?
- * @tx_enabled:	is TX enabled?
- * @rx_enabled:	is RX enabled?
- * @override_changed: is media reconfig pending?
- *
- * @port_type:	one of %PORT_* defines for ethtool
- * @port_lanes:	total number of lanes on the port (sum of lanes of all subports)
- * @is_split:	is interface part of a split port
- * @fec_modes_supported:	bitmap of FEC modes supported
- */
+/* ETH table information */
 struct nfp_eth_table {
-	uint32_t count;
-	uint32_t max_index;
+	uint32_t count;     /**< Number of table entries */
+	uint32_t max_index; /**< Max of @index fields of all @ports */
 	struct nfp_eth_table_port {
+		/** Port index according to legacy ethX numbering */
 		uint32_t eth_index;
-		uint32_t index;
-		uint32_t nbi;
-		uint32_t base;
-		uint32_t lanes;
-		uint32_t speed;
+		uint32_t index;  /**< Chip-wide first channel index */
+		uint32_t nbi;    /**< NBI index */
+		uint32_t base;   /**< First channel index (within NBI) */
+		uint32_t lanes;  /**< Number of channels */
+		uint32_t speed;  /**< Interface speed (in Mbps) */
 
-		uint32_t interface;
-		enum nfp_eth_media media;
+		uint32_t interface;  /**< Interface (module) plugged in */
+		enum nfp_eth_media media; /**< Media type of the @interface */
 
-		enum nfp_eth_fec fec;
-		enum nfp_eth_aneg aneg;
+		enum nfp_eth_fec fec;     /**< Forward Error Correction mode */
+		enum nfp_eth_aneg aneg;   /**< Auto negotiation mode */
 
-		struct rte_ether_addr mac_addr;
+		struct rte_ether_addr mac_addr;  /**< Interface MAC address */
 
-		uint8_t label_port;
+		uint8_t label_port;    /**< Port id */
+		/** Id of interface within port (for split ports) */
 		uint8_t label_subport;
 
-		int enabled;
-		int tx_enabled;
-		int rx_enabled;
-
-		int override_changed;
+		int enabled;     /**< Enable port */
+		int tx_enabled;  /**< Enable TX */
+		int rx_enabled;  /**< Enable RX */
 
-		/* Computed fields */
-		uint8_t port_type;
+		int override_changed;  /**< Media reconfig pending */
 
+		uint8_t port_type;    /**< One of %PORT_* */
+		/** Sum of lanes of all subports of this port */
 		uint32_t port_lanes;
 
-		int is_split;
+		int is_split;   /**< Split port */
 
-		uint32_t fec_modes_supported;
-	} ports[];
+		uint32_t fec_modes_supported;  /**< Bitmap of FEC modes supported */
+	} ports[]; /**< Table of ports */
 };
 
 struct nfp_eth_table *nfp_eth_read_ports(struct nfp_cpp *cpp);
@@ -263,28 +236,17 @@  int __nfp_eth_set_aneg(struct nfp_nsp *nsp, enum nfp_eth_aneg mode);
 int __nfp_eth_set_speed(struct nfp_nsp *nsp, uint32_t speed);
 int __nfp_eth_set_split(struct nfp_nsp *nsp, uint32_t lanes);
 
-/**
- * struct nfp_nsp_identify - NSP static information
- * @version:      opaque version string
- * @flags:        version flags
- * @br_primary:   branch id of primary bootloader
- * @br_secondary: branch id of secondary bootloader
- * @br_nsp:       branch id of NSP
- * @primary:      version of primary bootloader
- * @secondary:    version id of secondary bootloader
- * @nsp:          version id of NSP
- * @sensor_mask:  mask of present sensors available on NIC
- */
+/* NSP static information */
 struct nfp_nsp_identify {
-	char version[40];
-	uint8_t flags;
-	uint8_t br_primary;
-	uint8_t br_secondary;
-	uint8_t br_nsp;
-	uint16_t primary;
-	uint16_t secondary;
-	uint16_t nsp;
-	uint64_t sensor_mask;
+	char version[40];      /**< Opaque version string */
+	uint8_t flags;         /**< Version flags */
+	uint8_t br_primary;    /**< Branch id of primary bootloader */
+	uint8_t br_secondary;  /**< Branch id of secondary bootloader */
+	uint8_t br_nsp;        /**< Branch id of NSP */
+	uint16_t primary;      /**< Version of primary bootloader */
+	uint16_t secondary;    /**< Version id of secondary bootloader */
+	uint16_t nsp;          /**< Version id of NSP */
+	uint64_t sensor_mask;  /**< Mask of present sensors available on NIC */
 };
 
 struct nfp_nsp_identify *__nfp_nsp_identify(struct nfp_nsp *nsp);
diff --git a/drivers/net/nfp/nfpcore/nfp_nsp_eth.c b/drivers/net/nfp/nfpcore/nfp_nsp_eth.c
index 74daa92aed..cb090d2a47 100644
--- a/drivers/net/nfp/nfpcore/nfp_nsp_eth.c
+++ b/drivers/net/nfp/nfpcore/nfp_nsp_eth.c
@@ -264,7 +264,8 @@  __nfp_eth_read_ports(struct nfp_nsp *nsp)
 		goto err;
 	}
 
-	/* The NFP3800 NIC support 8 ports, but only 2 ports are valid,
+	/*
+	 * The NFP3800 NIC support 8 ports, but only 2 ports are valid,
 	 * the rest 6 ports mac are all 0, ensure we don't use these port
 	 */
 	for (i = 0; i < NSP_ETH_MAX_COUNT; i++) {
@@ -274,7 +275,8 @@  __nfp_eth_read_ports(struct nfp_nsp *nsp)
 			cnt++;
 	}
 
-	/* Some versions of flash will give us 0 instead of port count. For
+	/*
+	 * Some versions of flash will give us 0 instead of port count. For
 	 * those that give a port count, verify it against the value calculated
 	 * above.
 	 */
@@ -312,14 +314,16 @@  __nfp_eth_read_ports(struct nfp_nsp *nsp)
 	return NULL;
 }
 
-/*
- * nfp_eth_read_ports() - retrieve port information
- * @cpp:	NFP CPP handle
+/**
+ * Read the port information from the device.
+ *
+ * Returned structure should be freed once no longer needed.
  *
- * Read the port information from the device.  Returned structure should
- * be freed with kfree() once no longer needed.
+ * @param cpp
+ *   NFP CPP handle
  *
- * Return: populated ETH table or NULL on error.
+ * @return
+ *   Populated ETH table or NULL on error.
  */
 struct nfp_eth_table *
 nfp_eth_read_ports(struct nfp_cpp *cpp)
@@ -387,19 +391,19 @@  nfp_eth_config_cleanup_end(struct nfp_nsp *nsp)
 	free(entries);
 }
 
-/*
- * nfp_eth_config_commit_end() - perform recorded configuration changes
- * @nsp:	NFP NSP handle returned from nfp_eth_config_start()
- *
+/**
  * Perform the configuration which was requested with __nfp_eth_set_*()
- * helpers and recorded in @nsp state.  If device was already configured
- * as requested or no __nfp_eth_set_*() operations were made no NSP command
+ * helpers and recorded in @nsp state. If device was already configured
+ * as requested or no __nfp_eth_set_*() operations were made, no NSP command
  * will be performed.
  *
- * Return:
- * 0 - configuration successful;
- * 1 - no changes were needed;
- * -ERRNO - configuration failed.
+ * @param nsp
+ *   NFP NSP handle returned from nfp_eth_config_start()
+ *
+ * @return
+ *   - (0) Configuration successful
+ *   - (1) No changes were needed
+ *   - (-ERRNO) Configuration failed
  */
 int
 nfp_eth_config_commit_end(struct nfp_nsp *nsp)
@@ -417,19 +421,21 @@  nfp_eth_config_commit_end(struct nfp_nsp *nsp)
 	return ret;
 }
 
-/*
- * nfp_eth_set_mod_enable() - set PHY module enable control bit
- * @cpp:	NFP CPP handle
- * @idx:	NFP chip-wide port index
- * @enable:	Desired state
- *
+/**
  * Enable or disable PHY module (this usually means setting the TX lanes
  * disable bits).
  *
- * Return:
- * 0 - configuration successful;
- * 1 - no changes were needed;
- * -ERRNO - configuration failed.
+ * @param cpp
+ *   NFP CPP handle
+ * @param idx
+ *   NFP chip-wide port index
+ * @param enable
+ *   Desired state
+ *
+ * @return
+ *   - (0) Configuration successful
+ *   - (1) No changes were needed
+ *   - (-ERRNO) Configuration failed
  */
 int
 nfp_eth_set_mod_enable(struct nfp_cpp *cpp,
@@ -460,18 +466,20 @@  nfp_eth_set_mod_enable(struct nfp_cpp *cpp,
 	return nfp_eth_config_commit_end(nsp);
 }
 
-/*
- * nfp_eth_set_configured() - set PHY module configured control bit
- * @cpp:	NFP CPP handle
- * @idx:	NFP chip-wide port index
- * @configed:	Desired state
- *
+/**
  * Set the ifup/ifdown state on the PHY.
  *
- * Return:
- * 0 - configuration successful;
- * 1 - no changes were needed;
- * -ERRNO - configuration failed.
+ * @param cpp
+ *   NFP CPP handle
+ * @param idx
+ *   NFP chip-wide port index
+ * @param configured
+ *   Desired state
+ *
+ * @return
+ *   - (0) Configuration successful
+ *   - (1) No changes were needed
+ *   - (-ERRNO) Configuration failed
  */
 int
 nfp_eth_set_configured(struct nfp_cpp *cpp,
@@ -525,7 +533,7 @@  nfp_eth_set_bit_config(struct nfp_nsp *nsp,
 
 	/*
 	 * Note: set features were added in ABI 0.14 but the error
-	 *	 codes were initially not populated correctly.
+	 * codes were initially not populated correctly.
 	 */
 	if (nfp_nsp_get_abi_ver_minor(nsp) < 17) {
 		PMD_DRV_LOG(ERR, "set operations not supported, please update flash");
@@ -555,15 +563,17 @@  nfp_eth_set_bit_config(struct nfp_nsp *nsp,
 				val, ctrl_bit);			\
 	}))
 
-/*
- * __nfp_eth_set_aneg() - set PHY autonegotiation control bit
- * @nsp:	NFP NSP handle returned from nfp_eth_config_start()
- * @mode:	Desired autonegotiation mode
- *
+/**
  * Allow/disallow PHY module to advertise/perform autonegotiation.
  * Will write to hwinfo overrides in the flash (persistent config).
  *
- * Return: 0 or -ERRNO.
+ * @param nsp
+ *   NFP NSP handle returned from nfp_eth_config_start()
+ * @param mode
+ *   Desired autonegotiation mode
+ *
+ * @return
+ *   0 or -ERRNO
  */
 int
 __nfp_eth_set_aneg(struct nfp_nsp *nsp,
@@ -573,15 +583,17 @@  __nfp_eth_set_aneg(struct nfp_nsp *nsp,
 			NSP_ETH_STATE_ANEG, mode, NSP_ETH_CTRL_SET_ANEG);
 }
 
-/*
- * __nfp_eth_set_fec() - set PHY forward error correction control bit
- * @nsp:	NFP NSP handle returned from nfp_eth_config_start()
- * @mode:	Desired fec mode
- *
+/**
  * Set the PHY module forward error correction mode.
  * Will write to hwinfo overrides in the flash (persistent config).
  *
- * Return: 0 or -ERRNO.
+ * @param nsp
+ *   NFP NSP handle returned from nfp_eth_config_start()
+ * @param mode
+ *   Desired fec mode
+ *
+ * @return
+ *   0 or -ERRNO
  */
 static int
 __nfp_eth_set_fec(struct nfp_nsp *nsp,
@@ -591,16 +603,20 @@  __nfp_eth_set_fec(struct nfp_nsp *nsp,
 			NSP_ETH_STATE_FEC, mode, NSP_ETH_CTRL_SET_FEC);
 }
 
-/*
- * nfp_eth_set_fec() - set PHY forward error correction control mode
- * @cpp:	NFP CPP handle
- * @idx:	NFP chip-wide port index
- * @mode:	Desired fec mode
+/**
+ * Set PHY forward error correction control mode
+ *
+ * @param cpp
+ *   NFP CPP handle
+ * @param idx
+ *   NFP chip-wide port index
+ * @param mode
+ *   Desired fec mode
  *
- * Return:
- * 0 - configuration successful;
- * 1 - no changes were needed;
- * -ERRNO - configuration failed.
+ * @return
+ *   - (0) Configuration successful
+ *   - (1) No changes were needed
+ *   - (-ERRNO) Configuration failed
  */
 int
 nfp_eth_set_fec(struct nfp_cpp *cpp,
@@ -623,17 +639,19 @@  nfp_eth_set_fec(struct nfp_cpp *cpp,
 	return nfp_eth_config_commit_end(nsp);
 }
 
-/*
- * __nfp_eth_set_speed() - set interface speed/rate
- * @nsp:	NFP NSP handle returned from nfp_eth_config_start()
- * @speed:	Desired speed (per lane)
- *
- * Set lane speed.  Provided @speed value should be subport speed divided
- * by number of lanes this subport is spanning (i.e. 10000 for 40G, 25000 for
- * 50G, etc.)
+/**
+ * Set lane speed.
+ * Provided @speed value should be subport speed divided by number of
+ * lanes this subport is spanning (i.e. 10000 for 40G, 25000 for 50G, etc.)
  * Will write to hwinfo overrides in the flash (persistent config).
  *
- * Return: 0 or -ERRNO.
+ * @param nsp
+ *   NFP NSP handle returned from nfp_eth_config_start()
+ * @param speed
+ *   Desired speed (per lane)
+ *
+ * @return
+ *   0 or -ERRNO
  */
 int
 __nfp_eth_set_speed(struct nfp_nsp *nsp,
@@ -651,15 +669,17 @@  __nfp_eth_set_speed(struct nfp_nsp *nsp,
 			NSP_ETH_STATE_RATE, rate, NSP_ETH_CTRL_SET_RATE);
 }
 
-/*
- * __nfp_eth_set_split() - set interface lane split
- * @nsp:	NFP NSP handle returned from nfp_eth_config_start()
- * @lanes:	Desired lanes per port
- *
+/**
  * Set number of lanes in the port.
  * Will write to hwinfo overrides in the flash (persistent config).
  *
- * Return: 0 or -ERRNO.
+ * @param nsp
+ *   NFP NSP handle returned from nfp_eth_config_start()
+ * @param lanes
+ *   Desired lanes per port
+ *
+ * @return
+ *   0 or -ERRNO
  */
 int
 __nfp_eth_set_split(struct nfp_nsp *nsp,
diff --git a/drivers/net/nfp/nfpcore/nfp_resource.c b/drivers/net/nfp/nfpcore/nfp_resource.c
index 363f7d6198..bdebf5c3aa 100644
--- a/drivers/net/nfp/nfpcore/nfp_resource.c
+++ b/drivers/net/nfp/nfpcore/nfp_resource.c
@@ -22,32 +22,23 @@ 
 
 #define NFP_RESOURCE_ENTRY_NAME_SZ	8
 
-/*
- * struct nfp_resource_entry - Resource table entry
- * @owner:		NFP CPP Lock, interface owner
- * @key:		NFP CPP Lock, posix_crc32(name, 8)
- * @region:		Memory region descriptor
- * @name:		ASCII, zero padded name
- * @reserved
- * @cpp_action:		CPP Action
- * @cpp_token:		CPP Token
- * @cpp_target:		CPP Target ID
- * @page_offset:	256-byte page offset into target's CPP address
- * @page_size:		size, in 256-byte pages
- */
+/* Resource table entry */
 struct nfp_resource_entry {
 	struct nfp_resource_entry_mutex {
-		uint32_t owner;
-		uint32_t key;
+		uint32_t owner;  /**< NFP CPP Lock, interface owner */
+		uint32_t key;    /**< NFP CPP Lock, posix_crc32(name, 8) */
 	} mutex;
+	/* Memory region descriptor */
 	struct nfp_resource_entry_region {
+		/** ASCII, zero padded name */
 		uint8_t  name[NFP_RESOURCE_ENTRY_NAME_SZ];
 		uint8_t  reserved[5];
-		uint8_t  cpp_action;
-		uint8_t  cpp_token;
-		uint8_t  cpp_target;
+		uint8_t  cpp_action;  /**< CPP Action */
+		uint8_t  cpp_token;   /**< CPP Token */
+		uint8_t  cpp_target;  /**< CPP Target ID */
+		/** 256-byte page offset into target's CPP address */
 		uint32_t page_offset;
-		uint32_t page_size;
+		uint32_t page_size;   /**< Size, in 256-byte pages */
 	} region;
 };
 
@@ -147,14 +138,18 @@  nfp_resource_try_acquire(struct nfp_cpp *cpp,
 	return err;
 }
 
-/*
- * nfp_resource_acquire() - Acquire a resource handle
- * @cpp:	NFP CPP handle
- * @name:	Name of the resource
+/**
+ * Acquire a resource handle
+ *
+ * Note: This function locks the acquired resource.
  *
- * NOTE: This function locks the acquired resource
+ * @param cpp
+ *   NFP CPP handle
+ * @param name
+ *   Name of the resource
  *
- * Return: NFP Resource handle, or NULL
+ * @return
+ *   NFP Resource handle, or NULL
  */
 struct nfp_resource *
 nfp_resource_acquire(struct nfp_cpp *cpp,
@@ -183,7 +178,7 @@  nfp_resource_acquire(struct nfp_cpp *cpp,
 	}
 
 	wait.tv_sec = 0;
-	wait.tv_nsec = 1000000;
+	wait.tv_nsec = 1000000;    /* 1ms */
 
 	for (;;) {
 		err = nfp_resource_try_acquire(cpp, res, dev_mutex);
@@ -194,7 +189,7 @@  nfp_resource_acquire(struct nfp_cpp *cpp,
 			goto err_free;
 		}
 
-		if (count++ > 1000) {
+		if (count++ > 1000) {    /* 1ms * 1000 = 1s */
 			PMD_DRV_LOG(ERR, "Error: resource %s timed out", name);
 			err = -EBUSY;
 			goto err_free;
@@ -213,11 +208,13 @@  nfp_resource_acquire(struct nfp_cpp *cpp,
 	return NULL;
 }
 
-/*
- * nfp_resource_release() - Release a NFP Resource handle
- * @res:	NFP Resource handle
+/**
+ * Release a NFP Resource handle
  *
- * NOTE: This function implicitly unlocks the resource handle
+ * NOTE: This function implicitly unlocks the resource handle.
+ *
+ * @param res
+ *   NFP Resource handle
  */
 void
 nfp_resource_release(struct nfp_resource *res)
@@ -227,11 +224,14 @@  nfp_resource_release(struct nfp_resource *res)
 	free(res);
 }
 
-/*
- * nfp_resource_cpp_id() - Return the cpp_id of a resource handle
- * @res:        NFP Resource handle
+/**
+ * Return the cpp_id of a resource handle
+ *
+ * @param res
+ *   NFP Resource handle
  *
- * Return: NFP CPP ID
+ * @return
+ *   NFP CPP ID
  */
 uint32_t
 nfp_resource_cpp_id(const struct nfp_resource *res)
@@ -239,11 +239,14 @@  nfp_resource_cpp_id(const struct nfp_resource *res)
 	return res->cpp_id;
 }
 
-/*
- * nfp_resource_name() - Return the name of a resource handle
- * @res:        NFP Resource handle
+/**
+ * Return the name of a resource handle
  *
- * Return: const char pointer to the name of the resource
+ * @param res
+ *   NFP Resource handle
+ *
+ * @return
+ *   Const char pointer to the name of the resource
  */
 const char *
 nfp_resource_name(const struct nfp_resource *res)
@@ -251,11 +254,14 @@  nfp_resource_name(const struct nfp_resource *res)
 	return res->name;
 }
 
-/*
- * nfp_resource_address() - Return the address of a resource handle
- * @res:        NFP Resource handle
+/**
+ * Return the address of a resource handle
+ *
+ * @param res
+ *   NFP Resource handle
  *
- * Return: Address of the resource
+ * @return
+ *   Address of the resource
  */
 uint64_t
 nfp_resource_address(const struct nfp_resource *res)
@@ -263,11 +269,14 @@  nfp_resource_address(const struct nfp_resource *res)
 	return res->addr;
 }
 
-/*
- * nfp_resource_size() - Return the size in bytes of a resource handle
- * @res:        NFP Resource handle
+/**
+ * Return the size in bytes of a resource handle
+ *
+ * @param res
+ *   NFP Resource handle
  *
- * Return: Size of the resource in bytes
+ * @return
+ *   Size of the resource in bytes
  */
 uint64_t
 nfp_resource_size(const struct nfp_resource *res)
diff --git a/drivers/net/nfp/nfpcore/nfp_resource.h b/drivers/net/nfp/nfpcore/nfp_resource.h
index 009b7359a4..4236950caf 100644
--- a/drivers/net/nfp/nfpcore/nfp_resource.h
+++ b/drivers/net/nfp/nfpcore/nfp_resource.h
@@ -8,43 +8,27 @@ 
 
 #include "nfp_cpp.h"
 
+/* Netronone Flow Firmware Table */
 #define NFP_RESOURCE_NFP_NFFW           "nfp.nffw"
+
+/* NFP Hardware Info Database */
 #define NFP_RESOURCE_NFP_HWINFO         "nfp.info"
+
+/* Service Processor */
 #define NFP_RESOURCE_NSP		"nfp.sp"
 
-/**
- * Opaque handle to a NFP Resource
- */
+/* Opaque handle to a NFP Resource */
 struct nfp_resource;
 
 struct nfp_resource *nfp_resource_acquire(struct nfp_cpp *cpp,
 		const char *name);
 
-/**
- * Release a NFP Resource, and free the handle
- * @param[in]   res     NFP Resource handle
- */
 void nfp_resource_release(struct nfp_resource *res);
 
-/**
- * Return the CPP ID of a NFP Resource
- * @param[in]   res     NFP Resource handle
- * @return      CPP ID of the NFP Resource
- */
 uint32_t nfp_resource_cpp_id(const struct nfp_resource *res);
 
-/**
- * Return the name of a NFP Resource
- * @param[in]   res     NFP Resource handle
- * @return      Name of the NFP Resource
- */
 const char *nfp_resource_name(const struct nfp_resource *res);
 
-/**
- * Return the target address of a NFP Resource
- * @param[in]   res     NFP Resource handle
- * @return      Address of the NFP Resource
- */
 uint64_t nfp_resource_address(const struct nfp_resource *res);
 
 uint64_t nfp_resource_size(const struct nfp_resource *res);
diff --git a/drivers/net/nfp/nfpcore/nfp_rtsym.c b/drivers/net/nfp/nfpcore/nfp_rtsym.c
index d15a920752..0e6c0f9fe1 100644
--- a/drivers/net/nfp/nfpcore/nfp_rtsym.c
+++ b/drivers/net/nfp/nfpcore/nfp_rtsym.c
@@ -162,11 +162,14 @@  __nfp_rtsym_table_read(struct nfp_cpp *cpp,
 	return NULL;
 }
 
-/*
- * nfp_rtsym_count() - Get the number of RTSYM descriptors
- * @rtbl:	NFP RTsym table
+/**
+ * Get the number of RTSYM descriptors
+ *
+ * @param rtbl
+ *   NFP RTSYM table
  *
- * Return: Number of RTSYM descriptors
+ * @return
+ *   Number of RTSYM descriptors
  */
 int
 nfp_rtsym_count(struct nfp_rtsym_table *rtbl)
@@ -177,12 +180,16 @@  nfp_rtsym_count(struct nfp_rtsym_table *rtbl)
 	return rtbl->num;
 }
 
-/*
- * nfp_rtsym_get() - Get the Nth RTSYM descriptor
- * @rtbl:	NFP RTsym table
- * @idx:	Index (0-based) of the RTSYM descriptor
+/**
+ * Get the Nth RTSYM descriptor
+ *
+ * @param rtbl
+ *   NFP RTSYM table
+ * @param idx
+ *   Index (0-based) of the RTSYM descriptor
  *
- * Return: const pointer to a struct nfp_rtsym descriptor, or NULL
+ * @return
+ *   Const pointer to a struct nfp_rtsym descriptor, or NULL
  */
 const struct nfp_rtsym *
 nfp_rtsym_get(struct nfp_rtsym_table *rtbl,
@@ -197,12 +204,16 @@  nfp_rtsym_get(struct nfp_rtsym_table *rtbl,
 	return &rtbl->symtab[idx];
 }
 
-/*
- * nfp_rtsym_lookup() - Return the RTSYM descriptor for a symbol name
- * @rtbl:	NFP RTsym table
- * @name:	Symbol name
+/**
+ * Return the RTSYM descriptor for a symbol name
+ *
+ * @param rtbl
+ *   NFP RTSYM table
+ * @param name
+ *   Symbol name
  *
- * Return: const pointer to a struct nfp_rtsym descriptor, or NULL
+ * @return
+ *   Const pointer to a struct nfp_rtsym descriptor, or NULL
  */
 const struct nfp_rtsym *
 nfp_rtsym_lookup(struct nfp_rtsym_table *rtbl,
@@ -227,7 +238,8 @@  nfp_rtsym_size(const struct nfp_rtsym *sym)
 	case NFP_RTSYM_TYPE_NONE:
 		PMD_DRV_LOG(ERR, "The type of rtsym '%s' is NONE", sym->name);
 		return 0;
-	case NFP_RTSYM_TYPE_OBJECT:    /* Fall through */
+	case NFP_RTSYM_TYPE_OBJECT:
+		/* FALLTHROUGH */
 	case NFP_RTSYM_TYPE_FUNCTION:
 		return sym->size;
 	case NFP_RTSYM_TYPE_ABS:
@@ -327,17 +339,22 @@  nfp_rtsym_readq(struct nfp_cpp *cpp,
 	return nfp_cpp_readq(cpp, cpp_id, addr, value);
 }
 
-/*
- * nfp_rtsym_read_le() - Read a simple unsigned scalar value from symbol
- * @rtbl:	NFP RTsym table
- * @name:	Symbol name
- * @error:	Pointer to error code (optional)
+/**
+ * Read a simple unsigned scalar value from symbol
  *
  * Lookup a symbol, map, read it and return it's value. Value of the symbol
  * will be interpreted as a simple little-endian unsigned value. Symbol can
  * be 4 or 8 bytes in size.
  *
- * Return: value read, on error sets the error and returns ~0ULL.
+ * @param rtbl
+ *   NFP RTSYM table
+ * @param name
+ *   Symbol name
+ * @param error
+ *   Pointer to error code (optional)
+ *
+ * @return
+ *   Value read, on error sets the error and returns ~0ULL.
  */
 uint64_t
 nfp_rtsym_read_le(struct nfp_rtsym_table *rtbl,
diff --git a/drivers/net/nfp/nfpcore/nfp_rtsym.h b/drivers/net/nfp/nfpcore/nfp_rtsym.h
index e7295258b3..ff1facbd17 100644
--- a/drivers/net/nfp/nfpcore/nfp_rtsym.h
+++ b/drivers/net/nfp/nfpcore/nfp_rtsym.h
@@ -31,12 +31,12 @@ 
  * of "sram" symbols for backward compatibility, which are viewed as global.
  */
 struct nfp_rtsym {
-	const char *name;
-	uint64_t addr;
-	uint64_t size;
-	int type;
-	int target;
-	int domain;
+	const char *name;  /**< Symbol name */
+	uint64_t addr;     /**< Address in the domain/target's address space */
+	uint64_t size;     /**< Size (in bytes) of the symbol */
+	int type;          /**< NFP_RTSYM_TYPE_* of the symbol */
+	int target;        /**< CPP target identifier, or NFP_RTSYM_TARGET_* */
+	int domain;        /**< CPP target domain */
 };
 
 struct nfp_rtsym_table;
diff --git a/drivers/net/nfp/nfpcore/nfp_target.c b/drivers/net/nfp/nfpcore/nfp_target.c
index 611848e233..540b242a43 100644
--- a/drivers/net/nfp/nfpcore/nfp_target.c
+++ b/drivers/net/nfp/nfpcore/nfp_target.c
@@ -767,7 +767,7 @@  nfp_encode_basic(uint64_t *addr,
 		/*
 		 * Make sure we compare against isldN values by clearing the
 		 * LSB. This is what the silicon does.
-		 **/
+		 */
 		isld[0] &= ~1;
 		isld[1] &= ~1;