Explorar el Código

reviewd old tasks comments for doxygen and update

zilio nicolas hace 8 años
padre
commit
0d460b28c7
Se han modificado 6 ficheros con 162 adiciones y 120 borrados
  1. 14 18
      pcilib/lock.c
  2. 51 32
      pcilib/lock.h
  3. 18 10
      pcilib/locking.c
  4. 51 3
      pcilib/locking.h
  5. 7 35
      protocols/software.c
  6. 21 22
      protocols/software.h

+ 14 - 18
pcilib/lock.c

@@ -18,22 +18,21 @@
 #include "lock.h"
 #include "pci.h"
 
-
+/**
+ * structure to define a lock
+ */
 struct pcilib_lock_s {
-    pthread_mutex_t mutex;
-    pcilib_lock_flags_t flags;
+  pthread_mutex_t mutex; /**< the pthread robust mutex itself*/
+  pcilib_lock_flags_t flags; /**<flag to define the property of the mutex*/
 #ifdef HAVE_STDATOMIC_H
-    volatile atomic_uint refs;
+  volatile atomic_uint refs; /**< approximate number of processes that may require access to this lock*/ 
 #else /* HAVE_STDATOMIC_H */
-    volatile uint32_t refs;
+    volatile uint32_t refs;/**< approximate number of processes that may require access to this lock*/
 #endif /* HAVE_STDATOMIC_H */
-    char name[];
+  char name[]; /**< identifier name to search for a lock*/
 };
 
 
-/**
- * this function initialize a new semaphore in the kernel if it's not already initialized given the key that permits to differentiate semaphores, and then return the integer that points to the semaphore that have been initialized or to a previously already initialized semaphore
- */
 int pcilib_init_lock(pcilib_lock_t *lock, pcilib_lock_flags_t flags, const char *lock_id) {
     int err;
     pthread_mutexattr_t attr;
@@ -53,11 +52,13 @@ int pcilib_init_lock(pcilib_lock_t *lock, pcilib_lock_flags_t flags, const char
 	return PCILIB_ERROR_FAILED;
     }
 
+	/* we declare the mutex as possibly shared amongst different processes*/
     if ((err = pthread_mutexattr_setpshared(&attr, PTHREAD_PROCESS_SHARED))!=0) {
 	pcilib_error("Can't configure a shared mutex attribute, errno %i", errno);
 	return PCILIB_ERROR_FAILED;
     }
 
+	/* we set the mutex as robust, so it would be automatically unlocked if the application crash*/
     if ((flags&PCILIB_LOCK_FLAG_PERSISTENT)==0) {
 	if ((err = pthread_mutexattr_setrobust(&attr, PTHREAD_MUTEX_ROBUST))!=0) {
 	    pcilib_error("Can't configure a robust mutex attribute, errno: %i", errno);
@@ -78,9 +79,6 @@ int pcilib_init_lock(pcilib_lock_t *lock, pcilib_lock_flags_t flags, const char
 }
 
 
-/*
- * we uninitialize a mutex and set its name to 0 pointed by lock_ctx with this function. setting name to is the real destroying operation, but we need to unitialize the lock to initialize it again after
- */
 void pcilib_free_lock(pcilib_lock_t *lock) {
     int err;
 
@@ -134,9 +132,6 @@ const char *pcilib_lock_get_name(pcilib_lock_t *lock) {
     return NULL;
 }
 
-/*
- * this function will take the lock for the semaphore pointed by semId
- */
 int pcilib_lock_custom(pcilib_lock_t *lock, pcilib_lock_flags_t flags, pcilib_timeout_t timeout) {
     int err;
 
@@ -149,12 +144,15 @@ int pcilib_lock_custom(pcilib_lock_t *lock, pcilib_lock_flags_t flags, pcilib_ti
 
     switch (timeout) {
      case PCILIB_TIMEOUT_INFINITE:
+	/* the process will be hold till it can gain acquire the lock*/
         err = pthread_mutex_lock(&lock->mutex);
 	break;
      case PCILIB_TIMEOUT_IMMEDIATE:
+	/* the function returns immediatly if it can't acquire the lock*/
         err = pthread_mutex_trylock(&lock->mutex);
 	break;
      default:
+	/* the process will be hold till it can acquire the lock and timeout is not reached*/
         clock_gettime(CLOCK_REALTIME, &tm);
         tm.tv_nsec += 1000 * (timeout%1000000);
         if (tm.tv_nsec < 1000000000)
@@ -171,6 +169,7 @@ int pcilib_lock_custom(pcilib_lock_t *lock, pcilib_lock_flags_t flags, pcilib_ti
 
     switch (err) {
      case EOWNERDEAD:
+	/*in the case an application with a lock acquired crashes, this lock becomes inconsistent. we have so to make it consistent again to use it again.*/
         err = pthread_mutex_consistent(&lock->mutex);
         if (err) {
 	    pcilib_error("Failed to mark mutex as consistent, errno %i", err);
@@ -195,9 +194,6 @@ int pcilib_try_lock(pcilib_lock_t* lock) {
     return pcilib_lock_custom(lock, PCILIB_LOCK_FLAGS_DEFAULT, PCILIB_TIMEOUT_IMMEDIATE);
 }
 
-/**
- * this function will unlock the semaphore pointed by lock_ctx.
- */
 void pcilib_unlock(pcilib_lock_t *lock) {
     int err;
 

+ 51 - 32
pcilib/lock.h

@@ -1,29 +1,31 @@
 /**
  * @file lock.h
- * @skip author zilio nicolas, nicolas.zilio@hotmail.fr
  * @brief this file is the header file for the functions that implement a semaphore API for the pcitool program, using pthread robust mutexes.
- * @details the use of pthread robust mutexes  was chosen due to the fact we privilege security over fastness, and that pthread mutexes permits to recover semaphores even with crash ,and that it does not require access to resources that can be easily accessible from extern usage as flock file locking mechanism. A possible other locking mechanism could be the sysv semaphores, but we have a problem of how determine a perfect hash for the init function, and more, benchmarks proves that sysv semaphore aren't that stable and that with more than 10 locks/unlocks, pthread is better in performance, so that should suits more to the final pcitool program. 
- * We considered that mutex implmentation is enough compared to a reader/writer implementation. If it should change, please go to sysv semaphore.
- * Basic explanation on how semaphores here work: a semaphore here is a positive integer, thus that can't go below zero, which is initiated with a value. when a process want access to the critical resource, it asks to decrement the value of the semaphore, and when it has finished, it reincrements it.basically, when the semaphore is equal to zero, any process must have to wait for it to be reincremented before decrementing it again. Here are defined two types of access to the semaphore corresponding to the reader/writer problem : an exclusive lock, which means that no other process than the one who have the resource can access it; a shared lock, which means that other processes who want to access to the resource with a shared lock can have the access, but a concurrent process who want to access the semaphore with an exclusive lock won't be able to.
- * explanation on locks here : here locks are registered in kernel memory, where they are defined by a pthread_mutex_t and a name, which corresponds to a register or processus. The iterations like searching a lock are done on names.
+ * @details 		the use of pthread robust mutexes  was chosen due to the fact we privilege security over fastness, and that pthread mutexes permits to recover semaphores even with crash ,and that it does not require access to resources that can be easily accessible from extern usage as flock file locking mechanism. A possible other locking mechanism could be the sysv semaphores, but we have a problem of how determine a perfect hash for the init function, and more, benchmarks proves that sysv semaphore aren't that stable. For pure locking/unlocking, pthread is better in performance than sysV, but it suffers from big initialization times. In this sense, a kernel memory space is used for saving the locks, and persistence permits to avoid initializations over uses.
+ *
+ * 		We considered that mutex implmentation is enough compared to a reader/writer implementation. If it should change, please go to sysv semaphore.
+ *
+ * 		Basic explanation on how semaphores here work: a semaphore here is a positive integer, thus that can't go below zero, which is initiated with a value. when a process want access to the critical resource, it asks to decrement the value of the semaphore, and when it has finished, it reincrements it.basically, when the semaphore is equal to zero, any process must have to wait for it to be reincremented before decrementing it again. Here are defined two types of access to the semaphore corresponding to the reader/writer problem : an exclusive lock, which means that no other process than the one who have the resource can access it; a shared lock, which means that other processes who want to access to the resource with a shared lock can have the access, but a concurrent process who want to access the semaphore with an exclusive lock won't be able to.
+ * explanation on locks here : here locks are registered in kernel memory, where they are defined by a pthread_mutex_t and an identifier name, which corresponds most of the time to a mix of the register associated name and processus (but it's up to the user). The iterations like searching a lock are done on this id name.
  */
 
 #ifndef _PCILIB_LOCK_H
 #define _PCILIB_LOCK_H
 
-#define PCILIB_LOCK_SIZE 128		/**< size of one lock, determine so the size of the protocol_name in the way locks are registered. 40 bytes are necessary for the mutex structure, so we have a protocol name of length LOCK_SIZE-40*/
+#define PCILIB_LOCK_SIZE 128		/**< size of one lock. indeed, as we can't allocate easily on the fly memory in the kernel, fixed size have been chosen. determines so the size of the identifier name in the way locks are registered. 40 bytes are necessary for the mutex structure, so we have an id name of length LOCK_SIZE-40*/
 
 #include <pcilib.h>
 
 /**
- * type that defines possible flags when locking a lock by calling pcilib_lock
+ * type that defines possible flags for a lock, defining how a lock should be handled by the locking functions
  */
 typedef enum {
     PCILIB_LOCK_FLAGS_DEFAULT = 0,	/**< Default flags */
-    PCILIB_LOCK_FLAG_UNLOCKED = 1,	/**< Perform operation unlocked (protected by global flock during initialization of locking subsystem) */
+    PCILIB_LOCK_FLAG_UNLOCKED = 1,	/**< Perform operations unlocked, thus without taking care of the lock (protected by global flock during initialization of locking subsystem) */
     PCILIB_LOCK_FLAG_PERSISTENT = 2	/**< Do not create robust mutexes, but preserve the lock across application launches */
 } pcilib_lock_flags_t;
 
+/** structure defining a lock*/
 typedef struct pcilib_lock_s pcilib_lock_t;
 
 
@@ -32,70 +34,87 @@ extern "C" {
 #endif
 
 /**
- * this function initialize a new semaphore in the kernel given a name that corresponds to a specific processus if the semaphore is not already initialized given the name that permits to differentiate semaphores, and then return the integer that points to the semaphore that have been initialized or to a previously already initialized semaphore.
- * @param[in] lock - pointer to lock to initialize
- * @param[in] flags - flags
+ *this function initializes a lock, by setting correctly its property given the flags associated.
+ * @param[in,out] lock - pointer to lock to initialize
+ * @param[in] flags - flags: if it's set to two, then not a robust mutex is created
  * @param[in] lock_id - lock identificator
  * @return error code or 0 on success
  */
 int pcilib_init_lock(pcilib_lock_t *lock, pcilib_lock_flags_t flags, const char *lock_id);
 
 /**
- * this function uninitialize a lock in kernel memory and set the corresponding name to 0
- * @param[in] lock_ctx the pointer that points to the lock.
+ * this function will unref the defined lock. Any subsequent tries to lock it without reinitializaing it will fail.
+ * @param[in,out] lock_ctx - the pointer that points to the lock.
  */
 void pcilib_free_lock(pcilib_lock_t *lock_ctx);
 
-
+/**
+ * this function gives the identifier name associated to a lock in the kernel space
+ * @param[in] loc - pointer to the lock we want the name
+ * @return string corresponding to the name
+ */
 const char *pcilib_lock_get_name(pcilib_lock_t *lock);
 
 
 /**
- * Increment reference count. Not thread/process safe unless system supports stdatomic (gcc 4.9+).
- * In this case, the access should be synchronized by the caller
- * @param[in] lock - pointer to initialized lock
+ * Increment reference count(number of processes that may access the given lock).
+ * Not thread/process safe unless system supports stdatomic (gcc 4.9+). In this case, the access should be synchronized by the caller.
+ * @param[in,out] lock - pointer to initialized lock
  */
 void pcilib_lock_ref(pcilib_lock_t *lock);
 
 /**
- * Decrement reference count. Not thread/process safe unless system supports stdatomic (gcc 4.9+).
- * In this case, the access should be synchronized by the caller
- * @param[in] lock - pointer to initialized lock
+ * Decrement reference count(number of processes that may access the given lock).
+ * Not thread/process safe unless system supports stdatomic (gcc 4.9+). In this case, the access should be synchronized by the caller
+ * @param[in,out] lock - pointer to initialized lock
  */
 void pcilib_lock_unref(pcilib_lock_t *lock);
 
 /**
- * Return _approximate_ number of lock references. The crashed applications will may not unref.
- * @param[in] lock - pointer to initialized lock
+ * Return _approximate_ number of lock references as the crashed applications will may not unref.
+ * @param[in,out] lock - pointer to initialized lock
+ * @return the number of lock refs
  */
 size_t pcilib_lock_get_refs(pcilib_lock_t *lock);
 
+/**
+ * gets the flags associated to the given lock
+ * @param[in] lock - the lock we want to know the flags
+ * @return value of the flag associated
+ */
 pcilib_lock_flags_t pcilib_lock_get_flags(pcilib_lock_t *lock);
 
 /**
- * this function will take a lock for the mutex pointed by lock
- * @param[in] lock the pointer to the mutex
- * @param[in] flags define the type of lock wanted 
- * @param[in] timeout defines timeout 
+ * this function will call different locking functions to acquire the given lock. Given the flags, it is thus possible to: 
+ * 	1) the process requesting the lock will be held till it can acquire it
+ *	2)the lock is tried to be acquired, if the lock can be acquired then it is, if not, then the function returns immediatly and the lock is not taken at all
+ *	3) same than previous, but it's possible to define a waiting time to acquire the lock before returning
+ *
+ * @param[in] lock - the pointer to the mutex
+ * @param[in] flags - define the type of lock wanted 
+ * @param[in] timeout - the waiting time if asked, before the function returns without having obtained the lock, in micro seconds
+ * @return error code or 0 for correctness
  */
 int pcilib_lock_custom(pcilib_lock_t* lock, pcilib_lock_flags_t flags, pcilib_timeout_t timeout);
 
 /**
- * this function will take a lock for the mutex pointed by lock
- * @param[in] lock the pointer to the mutex
+ * function to acquire a lock, and wait till the lock can be acquire
+ * @param[in] lock - the pointer to the mutex
+ * @return error code or 0 for correctness
  */
 int pcilib_lock(pcilib_lock_t* lock);
 
 /**
- * this function will try to take a lock for the mutex pointed by lock
- * @param[in] lock the pointer to the mutex
+ * this function will try to take a lock for the mutex pointed by lockfunction to acquire a lock, but that returns immediatly if the lock can't be acquired on first try
+ * @param[in] lock - the pointer to the mutex
+ * @return error code or 0 for correctness
  */
 int pcilib_try_lock(pcilib_lock_t* lock);
 
 
 /**
-  * this function will unlock the lock pointed by lock 
- * @param[in] lock the integer that points to the semaphore
+  * this function unlocks the lock pointed by lock 
+ * @param[in] lock - the integer that points to the semaphore
  */
 void pcilib_unlock(pcilib_lock_t* lock);
 

+ 18 - 10
pcilib/locking.c

@@ -21,10 +21,12 @@ int pcilib_init_locking(pcilib_t* ctx) {
     pcilib_kmem_reuse_state_t reused;
 
     assert(PCILIB_LOCK_PAGES * PCILIB_KMEM_PAGE_SIZE >= PCILIB_MAX_LOCKS * PCILIB_LOCK_SIZE);
-
+	
+	/*protection against multiple creations of kernel space*/
     err = pcilib_lock_global(ctx);
     if (err) return err;
 
+	/* by default, this kernel space is persistent and will be reused, in order to avoid the big initialization times for robust mutexes each time we run pcitool*/
     ctx->locks.kmem = pcilib_alloc_kernel_memory(ctx, PCILIB_KMEM_TYPE_PAGE, PCILIB_LOCK_PAGES, PCILIB_KMEM_PAGE_SIZE, 0, PCILIB_KMEM_USE(PCILIB_KMEM_USE_LOCKS,0), PCILIB_KMEM_FLAG_REUSE|PCILIB_KMEM_FLAG_PERSISTENT);
     if (!ctx->locks.kmem) {
 	pcilib_unlock_global(ctx);
@@ -46,6 +48,7 @@ int pcilib_init_locking(pcilib_t* ctx) {
         }
     }
 
+	/* the lock that has been used for the creation of kernel space is declared unlocked, has we shouldnot use it anymore*/
     ctx->locks.locking = pcilib_get_lock(ctx, PCILIB_LOCK_FLAG_UNLOCKED, "locking");
 
     pcilib_unlock_global(ctx);
@@ -59,7 +62,7 @@ int pcilib_init_locking(pcilib_t* ctx) {
 }
 
 /*
- * this functions destroy all locks and then free the kernel memory allocated for them
+ * this function free the kernel memory allocated for them and destroys locks by setting memory to 0
  */
 void pcilib_free_locking(pcilib_t *ctx) {
     if (ctx->locks.locking)
@@ -75,7 +78,7 @@ void pcilib_free_locking(pcilib_t *ctx) {
 int pcilib_lock_global(pcilib_t *ctx) {
     int err;
     
-    /* we flock() to make sure to not have two initialization in the same time (possible long time to init) */
+    /* we flock() on the board's device file to make sure to not have two initialization in the same time (possible long time to init) */
     if ((err = flock(ctx->handle, LOCK_EX))==-1) {
 	pcilib_error("Can't get flock on device file");
 	return PCILIB_ERROR_FAILED;
@@ -105,7 +108,7 @@ pcilib_lock_t *pcilib_get_lock(pcilib_t *ctx, pcilib_lock_flags_t flags, const c
     pcilib_lock_t *lock;
     char buffer[PCILIB_LOCK_SIZE];
 
-
+	/* we construct the complete lock_id given the parameters of the function*/
     va_list pa;
     va_start(pa, lock_id);
     ret = vsnprintf(buffer, PCILIB_LOCK_SIZE, lock_id, pa);
@@ -115,7 +118,9 @@ pcilib_lock_t *pcilib_get_lock(pcilib_t *ctx, pcilib_lock_flags_t flags, const c
 	pcilib_error("Failed to construct the lock id, probably arguments does not match the format string (%s)...", lock_id);
 	return NULL;
     }
-
+	
+	
+	/* we iterate through locks to see if there is one already with the same name*/	
 	// Would be nice to have hash here
     for (i = 0; i < PCILIB_MAX_LOCKS; i++) {
 	lock = pcilib_get_lock_by_id(ctx, i);
@@ -141,6 +146,7 @@ pcilib_lock_t *pcilib_get_lock(pcilib_t *ctx, pcilib_lock_flags_t flags, const c
 		}
 	    }
 #endif /* ! HAVE_STDATOMIC_H */
+	/* if yes, we increment its ref variable*/
 	    pcilib_lock_ref(lock);
 #ifndef HAVE_STDATOMIC_H
 	    if ((flags&PCILIB_LOCK_FLAG_UNLOCKED)==0)
@@ -192,6 +198,7 @@ pcilib_lock_t *pcilib_get_lock(pcilib_t *ctx, pcilib_lock_flags_t flags, const c
 	return NULL;
     }
 
+	/* if the lock did not exist before, then we create it*/
     err = pcilib_init_lock(lock, flags, buffer);
     
     if (err) {
@@ -229,11 +236,11 @@ void pcilib_return_lock(pcilib_t *ctx, pcilib_lock_flags_t flags, pcilib_lock_t
 }
 
 
-/**
-  * Destroy all existing locks. This is unsafe call as this and other running applications
-  * will still have all initialized lock pointers. It is user responsibility to issue this
-  * command when no other application is running.
-  */
+/*
+ * Destroy all existing locks. This is unsafe call as this and other running applications
+ * will still have all initialized lock pointers. It is user responsibility to issue this
+ * command when no other application is running.
+ */
 int pcilib_destroy_all_locks(pcilib_t *ctx, int force) {
     int err;
     pcilib_lock_id_t i;
@@ -269,6 +276,7 @@ int pcilib_destroy_all_locks(pcilib_t *ctx, int force) {
 	return 0;
     }
 
+	/* if we run in non-forced case, then if it may be still processes that can have access to the locks, they are not destroyed*/
     if (!force) {
 	for (i = 0; i < PCILIB_MAX_LOCKS; i++) {
 	    pcilib_lock_t *lock = pcilib_get_lock_by_id(ctx, i);

+ 51 - 3
pcilib/locking.h

@@ -1,5 +1,5 @@
 /**
- * @file lock_global.h
+ * @file locking.h
  * @brief this file is the header file for functions that touch all locks allocated for software registers.
  * @details for more details about implementation choice, please read the file lock.h
  */
@@ -14,12 +14,16 @@
 #include <pcilib/kmem.h>
 #include <pcilib/lock.h>
 
-typedef uint32_t pcilib_lock_id_t;
+typedef uint32_t pcilib_lock_id_t; /**< type to represent the index of a lock in the table of locks in the kernel space*/
 
 typedef struct pcilib_locking_s pcilib_locking_t;
+
+/**
+ * structure defining the kernel space used for locks
+ */
 struct pcilib_locking_s {
     pcilib_kmem_handle_t *kmem;							/**< kmem used to store mutexes */
-    pcilib_lock_t *locking;							/**< lock used while intializing other locks */
+    pcilib_lock_t *locking;							/**< lock used while intializing kernel space */
 //    pcilib_lock_t *mmap;							/**< lock used to protect mmap operation */
 };
 
@@ -27,17 +31,61 @@ struct pcilib_locking_s {
 extern "C" {
 #endif
 
+/**
+ *this function gets the kernel space for the locks : if this space have been already initialized, then the previous space is returned. If not, the space is created. this function has to be protected, in order to avoid the simultaneous creation of 2 kernel spaces. For that, we use pcilib_lock_global.
+ *@param[in,out] ctx - the pcilib_t structure running, getting filled with a ref to locks' kernel space
+ */
 int pcilib_init_locking(pcilib_t *ctx);
+
+/**
+ *this function cleans the memory from all locks : the kernel space is freed, and locks references in pcilib_t are destroyed by setting memory to 0
+ *@param[in,out] ctx - the pcilib_t structure running
+ */
 void pcilib_free_locking(pcilib_t *ctx);
 
+/**
+ * this function use flock locking mechanism on the ALPS platform device file, to make sure to not create two kernel spaces for locks
+ *@param[in,out] ctx - the pcilib_t structure running
+ */
 int pcilib_lock_global(pcilib_t *ctx);
+
+/**
+ *function to remove the lock created by flock on the ALPS platform device file
+ *@param[in,out] ctx - the pcilib_t structure running
+ */
 void pcilib_unlock_global(pcilib_t *ctx);
 
+/**
+ * this function returns the lock at the index in the kernel space equal to id
+ *@param[in] ctx - the pcilib_t structure running
+ *@param[in] id - the index of the lock
+ *@return the lock structure corresponding
+ */
 pcilib_lock_t *pcilib_get_lock_by_id(pcilib_t *ctx, pcilib_lock_id_t id);
 
+/**
+ *this function verify if the lock requested exists in the kernel space. If yes, then nothing is done, else we create the lock in the kernel space. This function also gives the number of processes that may request the lock afterwards, including the one that just created it. 
+ *@param[in] ctx - the pcilib_t structure running
+ *@param[in] flags - the flag defining the property of the lock
+ *@param[in@ lock_id - the identifier name for the lock
+ *@return the corresponding lock, or a new one if it did not exist before
+ */
 pcilib_lock_t *pcilib_get_lock(pcilib_t *ctx, pcilib_lock_flags_t flags, const char *lock_id, ...);
+
+/**
+ *this function is to decrement the variable in a lock containing the number of processes that may access to this lock(refs)
+ *@param[in] ctx - the pcilib_t structure running
+ *@param[in] flags - the flag defining the property of the lock
+ *@param[in] lock - pointer to the lock we want to modify
+ */
 void pcilib_return_lock(pcilib_t *ctx, pcilib_lock_flags_t flags, pcilib_lock_t *lock);
 
+/**
+ * this function destroy all the locks that have been created(unref the locks + set memory to 0), and so is used when we want to clean properly the kernel space. If force is set to 1, then we don't care about other processes that may request locks. If not, if there is locks that may be requested by other processes, then the operation is stopped. Of course, destroying locks that may be requested by other processes results in an undefined behaviour. Thus, it is user responsibility to issue this command with force set to 1
+ *@param[in,out] ctx - the pcilib_t structure running
+ *@param[in] force - should the operation be forced or not
+ * @return error code : 0 if everything was ok
+ */
 int pcilib_destroy_all_locks(pcilib_t *ctx, int force);
 
 

+ 7 - 35
protocols/software.c

@@ -12,33 +12,21 @@
 
 typedef struct pcilib_software_register_bank_context_s pcilib_software_register_bank_context_t;
 
+/**
+ * structure defining the context of software registers
+ */
 struct pcilib_software_register_bank_context_s {
     pcilib_register_bank_context_t bank_ctx;	 /**< the bank context associated with the software registers */
-
     pcilib_kmem_handle_t *kmem;			/**< the kernel memory for software registers */
     void *addr;					/**< the virtual adress of the allocated kernel memory*/
 };
 
-/**
- * pcilib_software_registers_close
- * this function clear the kernel memory space that could have been allocated for software registers
- * @param[in] bank_ctx the bank context running that we get from the initialisation function
- */	
 void pcilib_software_registers_close(pcilib_t *ctx, pcilib_register_bank_context_t *bank_ctx) {
 	if (((pcilib_software_register_bank_context_t*)bank_ctx)->kmem)
 	    pcilib_free_kernel_memory(ctx, ((pcilib_software_register_bank_context_t*)bank_ctx)->kmem, PCILIB_KMEM_FLAG_REUSE);
 	free(bank_ctx);
 }
 
-/**
- * pcilib_software_registers_open
- * this function initializes the kernel space memory and stores in it the default values of the registers of the given bank index, if it was not initialized by a concurrent process, and return a bank context containing the adress of this kernel space. It the kernel space memory was already initialized by a concurrent process, then this function just return the bank context with the adress of this kernel space already used
- * @param[in] ctx the pcilib_t structure running
- * @param[in] bank the bank index that will permits to get the bank we want registers from
- * @param[in] model not used
- * @param[in] args not used
- * @return a bank context with the adress of the kernel space memory related to it
- */
 pcilib_register_bank_context_t* pcilib_software_registers_open(pcilib_t *ctx, pcilib_register_bank_t bank, const char* model, const void *args) {
 	int err;
 	pcilib_software_register_bank_context_t *bank_ctx;
@@ -59,6 +47,7 @@ pcilib_register_bank_context_t* pcilib_software_registers_open(pcilib_t *ctx, pc
 	    return NULL;
 	}
 
+	/*we get a lock to protect the creation of the kernel space for software registers against multiple creations*/
 	lock = pcilib_get_lock(ctx, PCILIB_LOCK_FLAGS_DEFAULT, "softreg/%s", bank_desc->name);
 	if (!lock) {
 	    pcilib_software_registers_close(ctx, (pcilib_register_bank_context_t*)bank_ctx);
@@ -74,7 +63,7 @@ pcilib_register_bank_context_t* pcilib_software_registers_open(pcilib_t *ctx, pc
 	    return NULL;
 	}
 
-
+	/*creation of the kernel space*/
 	handle = pcilib_alloc_kernel_memory(ctx, PCILIB_KMEM_TYPE_PAGE, 1, PCILIB_KMEM_PAGE_SIZE, 0, PCILIB_KMEM_USE(PCILIB_KMEM_USE_SOFTWARE_REGISTERS, bank_desc->addr), PCILIB_KMEM_FLAG_REUSE|PCILIB_KMEM_FLAG_PERSISTENT);
 	if (!handle) {
 	    pcilib_unlock(lock);
@@ -98,7 +87,8 @@ pcilib_register_bank_context_t* pcilib_software_registers_open(pcilib_t *ctx, pc
 		pcilib_error("Inconsistent software registers are found (only part of required buffers is available)");
 		return NULL;
 	    }
-		
+
+	/* here we fill the software registers with their default value*/		
 	    for (i = 0; ctx->model_info.registers[i].name != NULL; i++) {
 		if ((ctx->model_info.registers[i].bank == ctx->banks[bank].addr)&&(ctx->model_info.registers[i].type == PCILIB_REGISTER_STANDARD)) {
 		    *(pcilib_register_value_t*)(bank_ctx->addr + ctx->model_info.registers[i].addr) = ctx->model_info.registers[i].defvalue;
@@ -112,15 +102,6 @@ pcilib_register_bank_context_t* pcilib_software_registers_open(pcilib_t *ctx, pc
 	return (pcilib_register_bank_context_t*)bank_ctx;
 }
 
-/**
- * pcilib_software_registers_read
- * this function read the value of a said register in the kernel space
- * @param[in] ctx the pcilib_t structure runnning
- * @param[in] bank_ctx the bank context that was returned by the initialisation function
- * @param[in] addr the adress of the register we want to read
- * @param[out] value the value of the register
- * @return 0 in case of success
- */
 int pcilib_software_registers_read(pcilib_t *ctx, pcilib_register_bank_context_t *bank_ctx, pcilib_register_addr_t addr, pcilib_register_value_t *value){
 	if ((addr + sizeof(pcilib_register_value_t)) > bank_ctx->bank->size) {
 	    pcilib_error("Trying to access space outside of the define register bank (bank: %s, addr: 0x%lx)", bank_ctx->bank->name, addr);
@@ -132,15 +113,6 @@ int pcilib_software_registers_read(pcilib_t *ctx, pcilib_register_bank_context_t
 	return 0;
 }
 
-/**
- * pcilib_software_registers_write
- * this function write the said value to a said register in the kernel space
- * @param[in] ctx the pcilib_t structure runnning
- * @param[in] bank_ctx the bank context that was returned by the initialisation function
- * @param[in] addr the adress of the register we want to write in
- * @param[out] value the value we want to write in the register
- * @return 0 in case of success
- */
 int pcilib_software_registers_write(pcilib_t *ctx, pcilib_register_bank_context_t *bank_ctx, pcilib_register_addr_t addr, pcilib_register_value_t value) {
 	if ((addr + sizeof(pcilib_register_value_t)) > bank_ctx->bank->size) {
 	    pcilib_error("Trying to access space outside of the define register bank (bank: %s, addr: 0x%lx)", bank_ctx->bank->name, addr);

+ 21 - 22
protocols/software.h

@@ -1,7 +1,6 @@
 /**
  * @file software.h
- * @skip author nicolas zilio, nicolas.zilio@hotmail.fr
- * @brief header file for implementation of the protocol with software registers allocated in the main memory.
+ * @brief header file for implementation of the protocol with software registers allocated in the kernel space, for parameters sharing between concurrent pcitool instances
  */
 
 #ifndef _PCILIB_SOFTWARE_H
@@ -13,44 +12,44 @@
 
 /**
  * this function initialize the kernel space memory for the use of software register. it initializes the kernel space memory and stores in it the default values of the registers of the given bank index, if it was not initialized by a concurrent process, and return a bank context containing the adress of this kernel space. If the kernel space memory was already initialized by a concurrent process, then this function just return the bank context with the adress of this kernel space already used
- * @param[in] ctx the pcilib_t structure running
- * @param[in] bank the bank index that will permits to get the bank we want registers from
- * @param[in] model not used
- * @param[in] args not used
- * @return a bank context with the adress of the kernel space memory related to it
+ * @param[in] - ctx the pcilib_t structure running
+ * @param[in] - bank the bank index that will permits to get the bank we want registers from
+ * @param[in] - model not used
+ * @param[in] - args not used
+ * @return a bank context with the adress of the kernel space memory related to it, as we could have for BAR
  */
 pcilib_register_bank_context_t* pcilib_software_registers_open(pcilib_t *ctx, pcilib_register_bank_t bank, const char* model, const void *args);
 
 /**
- * this function clear the kernel memory space that could have been allocated for software registers.
- * @param[in] ctx the pcilib_t structure runnning
- * @param[in] bank_ctx the bank context running that we get from the initialisation function
+ * this function clear the kernel memory space that could have been allocated for software registers and the bank context that correspond to it too
+ * @param[in] ctx - the pcilib_t structure runnning
+ * @param[in] bank_ctx - the bank context running that we get from the initialisation function
  */
 void pcilib_software_registers_close(pcilib_t *ctx, pcilib_register_bank_context_t *bank_ctx);
 
 /**
  * this function read the value of a said register in the kernel space.
- * @param[in] ctx the pcilib_t structure runnning
- * @param[in] bank_ctx the bank context that was returned by the initialisation function
- * @param[in] addr the adress of the register we want to read
- * @param[out] value the value of the register
- * @return 0 in case of success
+ * @param[in] - ctx the pcilib_t structure runnning
+ * @param[in] - bank_ctx the bank context that was returned by the initialisation function
+ * @param[in] - addr the adress of the register we want to read
+ * @param[out] - value the value of the register
+ * @return error code : 0 in case of success
  */
 int  pcilib_software_registers_read(pcilib_t *ctx, pcilib_register_bank_context_t* bank_ctx, pcilib_register_addr_t addr, pcilib_register_value_t *value);
 
 /**
- * this function write the said value to a said register in the kernel space
- * @param[in] ctx the pcilib_t structure runnning
- * @param[in] bank_ctx the bank context that was returned by the initialisation function
- * @param[in] addr the adress of the register we want to write in
- * @param[out] value the value we want to write in the register
- * @return 0 in case of success
+ * this function write the given value to a given register in the kernel space
+ * @param[in] ctx - the pcilib_t structure runnning
+ * @param[in] bank_ctx - the bank context that was returned by the initialisation function
+ * @param[in] addr - the adress of the register we want to write in
+ * @param[in] value - the value we want to write in the register
+ * @return error code : 0 in case of success
  */
 int pcilib_software_registers_write(pcilib_t *ctx,pcilib_register_bank_context_t* bank_ctx, pcilib_register_addr_t addr, pcilib_register_value_t value);
 
 #ifdef _PCILIB_EXPORT_C
 /**
- * new protocol addition to the protocol api.
+ * software protocol addition to the protocol api.
  */
 const pcilib_register_protocol_api_description_t pcilib_register_software_protocol_api =
   { PCILIB_VERSION, pcilib_software_registers_open, pcilib_software_registers_close,pcilib_software_registers_read, pcilib_software_registers_write }; /**< we add there the protocol to the list of possible protocols*/