[4/6] api:odp_spinlock.h: Update doxygen comments, renaming of function params

Message ID 1417616254-31760-5-git-send-email-ola.liljedahl@linaro.org
State New
Headers show

Commit Message

Ola Liljedahl Dec. 3, 2014, 2:17 p.m.
Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org>
---
Update doxygen comments, renaming of function params.

 platform/linux-generic/include/api/odp_spinlock.h | 42 ++++++++++++-----------
 1 file changed, 22 insertions(+), 20 deletions(-)

Comments

Mike Holmes Dec. 3, 2014, 9:35 p.m. | #1
On 3 December 2014 at 09:17, Ola Liljedahl <ola.liljedahl@linaro.org> wrote:

> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org>
> ---
> Update doxygen comments, renaming of function params.
>
>  platform/linux-generic/include/api/odp_spinlock.h | 42
> ++++++++++++-----------
>  1 file changed, 22 insertions(+), 20 deletions(-)
>
> diff --git a/platform/linux-generic/include/api/odp_spinlock.h
> b/platform/linux-generic/include/api/odp_spinlock.h
> index 7c299c8..993413d 100644
> --- a/platform/linux-generic/include/api/odp_spinlock.h
> +++ b/platform/linux-generic/include/api/odp_spinlock.h
> @@ -22,60 +22,62 @@ extern "C" {
>  #include <odp_std_types.h>
>
>  /** @addtogroup odp_synchronizers
> - *  Operations on spinlock.
> + *  Operations on spin locks.
>   *  @{
>   */
>
>  /**
> - * ODP spinlock
> + * ODP spinlock type
>   */
>  typedef struct odp_spinlock_t {
> -       char lock;  /**< @private Lock */
> +       char lock;  /**< @private lock flag, should match
> odp_atomic_flag_t */
>  } odp_spinlock_t;
>
>
>  /**
> - * Init spinlock
> + * Initialize spin lock.
>   *
> - * @param spinlock  Spinlock
> + * @param[out] p_splck Pointer to a spin lock
>

Is this an "in" also, don't you need to provide the pointer to the spinlock
to be initialized ?


>   */
> -void odp_spinlock_init(odp_spinlock_t *spinlock);
> +void odp_spinlock_init(odp_spinlock_t *p_splck);
>
>
>  /**
> - * Lock spinlock
> + * Acquire spin lock.
>   *
> - * @param spinlock  Spinlock
> + * @param[in,out] p_splck Pointer to a spin lock
>   */
> -void odp_spinlock_lock(odp_spinlock_t *spinlock);
> +void odp_spinlock_lock(odp_spinlock_t *p_splck);
>
>
>  /**
> - * Try to lock spinlock
> + * Try to acquire spin lock.
>   *
> - * @param spinlock  Spinlock
> + * @param[in,out] p_splck Pointer to a spin lock
>   *
> - * @return 1 if the lock was taken, otherwise 0.
> + * @retval 1 lock aquired
>

acquired

+ * @retval 0 lock not acquired
>   */
> -int odp_spinlock_trylock(odp_spinlock_t *spinlock);
> +int odp_spinlock_trylock(odp_spinlock_t *p_splck);
>
>
>  /**
> - * Unlock spinlock
> + * Release spin lock.
>   *
> - * @param spinlock  Spinlock
> + * @param[in,out] p_splck Pointer to a spin lock
>   */
> -void odp_spinlock_unlock(odp_spinlock_t *spinlock);
> +void odp_spinlock_unlock(odp_spinlock_t *p_splck);
>
>
>  /**
> - * Test if spinlock is locked
> + * Check if spin lock is busy (locked).
>   *
> - * @param spinlock  Spinlock
> + * @param[in] p_splck Pointer to a spin lock
>   *
> - * @return 1 if the lock is locked, otherwise 0.
> + * @retval 1 lock busy (locked)
> + * @retval 0 lock not busy.
>   */
> -int odp_spinlock_is_locked(odp_spinlock_t *spinlock);
> +int odp_spinlock_is_locked(odp_spinlock_t *p_splck);
>
>
>
> --
> 1.9.1
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
Ola Liljedahl Dec. 3, 2014, 10:53 p.m. | #2
On 3 December 2014 at 22:35, Mike Holmes <mike.holmes@linaro.org> wrote:
>
>
> On 3 December 2014 at 09:17, Ola Liljedahl <ola.liljedahl@linaro.org> wrote:
>>
>> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org>
>> ---
>> Update doxygen comments, renaming of function params.
>>
>>  platform/linux-generic/include/api/odp_spinlock.h | 42
>> ++++++++++++-----------
>>  1 file changed, 22 insertions(+), 20 deletions(-)
>>
>> diff --git a/platform/linux-generic/include/api/odp_spinlock.h
>> b/platform/linux-generic/include/api/odp_spinlock.h
>> index 7c299c8..993413d 100644
>> --- a/platform/linux-generic/include/api/odp_spinlock.h
>> +++ b/platform/linux-generic/include/api/odp_spinlock.h
>> @@ -22,60 +22,62 @@ extern "C" {
>>  #include <odp_std_types.h>
>>
>>  /** @addtogroup odp_synchronizers
>> - *  Operations on spinlock.
>> + *  Operations on spin locks.
>>   *  @{
>>   */
>>
>>  /**
>> - * ODP spinlock
>> + * ODP spinlock type
>>   */
>>  typedef struct odp_spinlock_t {
>> -       char lock;  /**< @private Lock */
>> +       char lock;  /**< @private lock flag, should match
>> odp_atomic_flag_t */
>>  } odp_spinlock_t;
>>
>>
>>  /**
>> - * Init spinlock
>> + * Initialize spin lock.
>>   *
>> - * @param spinlock  Spinlock
>> + * @param[out] p_splck Pointer to a spin lock
>
>
> Is this an "in" also, don't you need to provide the pointer to the spinlock
> to be initialized ?
The pointer is used as a reference to a variable and it is the
input/output-ness of this variable that is documented.
We only write the spin lock, don't read it so I assume it is just an
[out] parameter.
The pointer itself is an input but this is on a lower level (which
happens to be visible in C but no in C++ where pass-by-reference is
supported by the language).
Did I misunderstand something on what in and out means?

>
>>
>>   */
>> -void odp_spinlock_init(odp_spinlock_t *spinlock);
>> +void odp_spinlock_init(odp_spinlock_t *p_splck);
>>
>>
>>  /**
>> - * Lock spinlock
>> + * Acquire spin lock.
>>   *
>> - * @param spinlock  Spinlock
>> + * @param[in,out] p_splck Pointer to a spin lock
>>   */
>> -void odp_spinlock_lock(odp_spinlock_t *spinlock);
>> +void odp_spinlock_lock(odp_spinlock_t *p_splck);
>>
>>
>>  /**
>> - * Try to lock spinlock
>> + * Try to acquire spin lock.
>>   *
>> - * @param spinlock  Spinlock
>> + * @param[in,out] p_splck Pointer to a spin lock
>>   *
>> - * @return 1 if the lock was taken, otherwise 0.
>> + * @retval 1 lock aquired
>
>
> acquired
OK. There's always a reason for a v2 of a patch...

>
>> + * @retval 0 lock not acquired
>>   */
>> -int odp_spinlock_trylock(odp_spinlock_t *spinlock);
>> +int odp_spinlock_trylock(odp_spinlock_t *p_splck);
>>
>>
>>  /**
>> - * Unlock spinlock
>> + * Release spin lock.
>>   *
>> - * @param spinlock  Spinlock
>> + * @param[in,out] p_splck Pointer to a spin lock
>>   */
>> -void odp_spinlock_unlock(odp_spinlock_t *spinlock);
>> +void odp_spinlock_unlock(odp_spinlock_t *p_splck);
>>
>>
>>  /**
>> - * Test if spinlock is locked
>> + * Check if spin lock is busy (locked).
>>   *
>> - * @param spinlock  Spinlock
>> + * @param[in] p_splck Pointer to a spin lock
>>   *
>> - * @return 1 if the lock is locked, otherwise 0.
>> + * @retval 1 lock busy (locked)
>> + * @retval 0 lock not busy.
>>   */
>> -int odp_spinlock_is_locked(odp_spinlock_t *spinlock);
>> +int odp_spinlock_is_locked(odp_spinlock_t *p_splck);
>>
>>
>>
>> --
>> 1.9.1
>>
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
>
>
> --
> Mike Holmes
> Linaro  Sr Technical Manager
> LNG - ODP
Mike Holmes Dec. 4, 2014, 1:28 a.m. | #3
On 3 December 2014 at 17:53, Ola Liljedahl <ola.liljedahl@linaro.org> wrote:

> On 3 December 2014 at 22:35, Mike Holmes <mike.holmes@linaro.org> wrote:
> >
> >
> > On 3 December 2014 at 09:17, Ola Liljedahl <ola.liljedahl@linaro.org>
> wrote:
> >>
> >> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org>
> >> ---
> >> Update doxygen comments, renaming of function params.
> >>
> >>  platform/linux-generic/include/api/odp_spinlock.h | 42
> >> ++++++++++++-----------
> >>  1 file changed, 22 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/platform/linux-generic/include/api/odp_spinlock.h
> >> b/platform/linux-generic/include/api/odp_spinlock.h
> >> index 7c299c8..993413d 100644
> >> --- a/platform/linux-generic/include/api/odp_spinlock.h
> >> +++ b/platform/linux-generic/include/api/odp_spinlock.h
> >> @@ -22,60 +22,62 @@ extern "C" {
> >>  #include <odp_std_types.h>
> >>
> >>  /** @addtogroup odp_synchronizers
> >> - *  Operations on spinlock.
> >> + *  Operations on spin locks.
> >>   *  @{
> >>   */
> >>
> >>  /**
> >> - * ODP spinlock
> >> + * ODP spinlock type
> >>   */
> >>  typedef struct odp_spinlock_t {
> >> -       char lock;  /**< @private Lock */
> >> +       char lock;  /**< @private lock flag, should match
> >> odp_atomic_flag_t */
> >>  } odp_spinlock_t;
> >>
> >>
> >>  /**
> >> - * Init spinlock
> >> + * Initialize spin lock.
> >>   *
> >> - * @param spinlock  Spinlock
> >> + * @param[out] p_splck Pointer to a spin lock
> >
> >
> > Is this an "in" also, don't you need to provide the pointer to the
> spinlock
> > to be initialized ?
> The pointer is used as a reference to a variable and it is the
> input/output-ness of this variable that is documented.
> We only write the spin lock, don't read it so I assume it is just an
> [out] parameter.
> The pointer itself is an input but this is on a lower level (which
> happens to be visible in C but no in C++ where pass-by-reference is
> supported by the language).
> Did I misunderstand something on what in and out means?
>

I was not sure, it occured to me as I read it so I asked you  :)
So in effect we passed noting valuable in so it is an out, thanks


>
> >
> >>
> >>   */
> >> -void odp_spinlock_init(odp_spinlock_t *spinlock);
> >> +void odp_spinlock_init(odp_spinlock_t *p_splck);
> >>
> >>
> >>  /**
> >> - * Lock spinlock
> >> + * Acquire spin lock.
> >>   *
> >> - * @param spinlock  Spinlock
> >> + * @param[in,out] p_splck Pointer to a spin lock
> >>   */
> >> -void odp_spinlock_lock(odp_spinlock_t *spinlock);
> >> +void odp_spinlock_lock(odp_spinlock_t *p_splck);
> >>
> >>
> >>  /**
> >> - * Try to lock spinlock
> >> + * Try to acquire spin lock.
> >>   *
> >> - * @param spinlock  Spinlock
> >> + * @param[in,out] p_splck Pointer to a spin lock
> >>   *
> >> - * @return 1 if the lock was taken, otherwise 0.
> >> + * @retval 1 lock aquired
> >
> >
> > acquired
> OK. There's always a reason for a v2 of a patch...
>
> >
> >> + * @retval 0 lock not acquired
> >>   */
> >> -int odp_spinlock_trylock(odp_spinlock_t *spinlock);
> >> +int odp_spinlock_trylock(odp_spinlock_t *p_splck);
> >>
> >>
> >>  /**
> >> - * Unlock spinlock
> >> + * Release spin lock.
> >>   *
> >> - * @param spinlock  Spinlock
> >> + * @param[in,out] p_splck Pointer to a spin lock
> >>   */
> >> -void odp_spinlock_unlock(odp_spinlock_t *spinlock);
> >> +void odp_spinlock_unlock(odp_spinlock_t *p_splck);
> >>
> >>
> >>  /**
> >> - * Test if spinlock is locked
> >> + * Check if spin lock is busy (locked).
> >>   *
> >> - * @param spinlock  Spinlock
> >> + * @param[in] p_splck Pointer to a spin lock
> >>   *
> >> - * @return 1 if the lock is locked, otherwise 0.
> >> + * @retval 1 lock busy (locked)
> >> + * @retval 0 lock not busy.
> >>   */
> >> -int odp_spinlock_is_locked(odp_spinlock_t *spinlock);
> >> +int odp_spinlock_is_locked(odp_spinlock_t *p_splck);
> >>
> >>
> >>
> >> --
> >> 1.9.1
> >>
> >>
> >> _______________________________________________
> >> lng-odp mailing list
> >> lng-odp@lists.linaro.org
> >> http://lists.linaro.org/mailman/listinfo/lng-odp
> >
> >
> >
> >
> > --
> > Mike Holmes
> > Linaro  Sr Technical Manager
> > LNG - ODP
>

Patch

diff --git a/platform/linux-generic/include/api/odp_spinlock.h b/platform/linux-generic/include/api/odp_spinlock.h
index 7c299c8..993413d 100644
--- a/platform/linux-generic/include/api/odp_spinlock.h
+++ b/platform/linux-generic/include/api/odp_spinlock.h
@@ -22,60 +22,62 @@  extern "C" {
 #include <odp_std_types.h>
 
 /** @addtogroup odp_synchronizers
- *  Operations on spinlock.
+ *  Operations on spin locks.
  *  @{
  */
 
 /**
- * ODP spinlock
+ * ODP spinlock type
  */
 typedef struct odp_spinlock_t {
-	char lock;  /**< @private Lock */
+	char lock;  /**< @private lock flag, should match odp_atomic_flag_t */
 } odp_spinlock_t;
 
 
 /**
- * Init spinlock
+ * Initialize spin lock.
  *
- * @param spinlock  Spinlock
+ * @param[out] p_splck Pointer to a spin lock
  */
-void odp_spinlock_init(odp_spinlock_t *spinlock);
+void odp_spinlock_init(odp_spinlock_t *p_splck);
 
 
 /**
- * Lock spinlock
+ * Acquire spin lock.
  *
- * @param spinlock  Spinlock
+ * @param[in,out] p_splck Pointer to a spin lock
  */
-void odp_spinlock_lock(odp_spinlock_t *spinlock);
+void odp_spinlock_lock(odp_spinlock_t *p_splck);
 
 
 /**
- * Try to lock spinlock
+ * Try to acquire spin lock.
  *
- * @param spinlock  Spinlock
+ * @param[in,out] p_splck Pointer to a spin lock
  *
- * @return 1 if the lock was taken, otherwise 0.
+ * @retval 1 lock aquired
+ * @retval 0 lock not acquired
  */
-int odp_spinlock_trylock(odp_spinlock_t *spinlock);
+int odp_spinlock_trylock(odp_spinlock_t *p_splck);
 
 
 /**
- * Unlock spinlock
+ * Release spin lock.
  *
- * @param spinlock  Spinlock
+ * @param[in,out] p_splck Pointer to a spin lock
  */
-void odp_spinlock_unlock(odp_spinlock_t *spinlock);
+void odp_spinlock_unlock(odp_spinlock_t *p_splck);
 
 
 /**
- * Test if spinlock is locked
+ * Check if spin lock is busy (locked).
  *
- * @param spinlock  Spinlock
+ * @param[in] p_splck Pointer to a spin lock
  *
- * @return 1 if the lock is locked, otherwise 0.
+ * @retval 1 lock busy (locked)
+ * @retval 0 lock not busy.
  */
-int odp_spinlock_is_locked(odp_spinlock_t *spinlock);
+int odp_spinlock_is_locked(odp_spinlock_t *p_splck);