[PATCH] HVCS fixes

Here are a set of HVCS (drivers/char/hvcs.c) fixes that were suggested by Jeff
Garzik on July 29th in his review of this driver as well as some other fixes
for problems I found while reviewing the driver.  These are all relatively
minor, but necessary.

- Cleaned up curly braces on single line conditional blocks.

- Replaced debug memset(...,0x3F,...) with memset(...,0x00,...).

- Removed explicit '= 0' after static int declarations since these default
  to zero.

- Removed list_for_each_safe() instances and replaced with
  list_for_each_entry() which cut down on amt of code.  The 'safe' version is
  un-needed now that the driver is using spinlocks.

- Changed spin_lock_irqsave() to spin_lock() when locking hvcs_structs_lock
  and hvcs_pi_lock since these are not touched in an int handler.

- changed spin_lock_irqsave() to spin_lock() in interrupt handler.

- Initialized hvcs_structs_lock and hvcs_pi_lock to SPIN_LOCK_UNLOCKED at
  declaration tiem rather than in hvcs_module_init().

- Added spin_lock around list_del() in destroy_hvcs_struct() to protect the
  list traversal from deletion.  The original omission was an oversight.

- Removed '= NULL' from pointer declarations since they are initialized NULL
  by default.

- Removed wmb() instance from hvcs_try_write().  They probably aren't needed
  with locking in place.

- Added check and cleanup for hvcs_pi_buff = kmalloc() in
  hvcs_module_init().

- Exposed hvcs_struct.index via a sysfs attribute so that the coupling
  between /dev/hvcs* and a vty-server can be systematically determined.

- Moved kobject_put() in hvcs_open() outside of the
  spin_unlock_irqrestore().

- In hvcs_probe() changed kmalloc(sizeof(*hvcsd),...) to
  kmalloc(sizeof(struct hvcs_struct)) because hvcsd references a NULL pointer
  at the time of kmalloc.

- Incremented the HVCS_DRIVER_VERSION to 1.3.1

arch/ppc64/kernel/hvcserver.c:

- Changed function documentation of EXPORTed functions to comply with proper
  kernel-doc documentation style.

- Changed 'unsigned int' types to 'uint32_t' to comply with how unit
  addresses and partition IDs are handled in other arch/ppc64 vterm code.

- Cleaned up curly braces on single line conditional blocks.

include/asm-ppc64/hvcserver.h:

- Added kernel-doc style documentation for hvcs_partner_info struct.

- changed 'unsigned int' types to 'uint32_t' to comply with how unit
  addresses and partition IDs are handled in other arch/ppc64 vterm code.

Signed-off-by: Ryan S. Arnold <rsa@us.ibm.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

1 files changed, 58 insertions, 28 deletions

diff --git a/arch/ppc64/kernel/hvcserver.c b/arch/ppc64/kernel/hvcserver.c
index fbe445ec09cd1e..bde8f42da8543a 100644
--- a/arch/ppc64/kernel/hvcserver.c
+++ b/arch/ppc64/kernel/hvcserver.c
@@ -61,14 +61,21 @@ int hvcs_convert(long to_convert)
 	}
 }
 
+/**
+ * hvcs_free_partner_info - free pi allocated by hvcs_get_partner_info
+ * @head: list_head pointer for an allocated list of partner info structs to
+ *	free.
+ *
+ * This function is used to free the partner info list that was returned by
+ * calling hvcs_get_partner_info().
+ */
 int hvcs_free_partner_info(struct list_head *head)
 {
 	struct hvcs_partner_info *pi;
 	struct list_head *element;
 
-	if (!head) {
+	if (!head)
 		return -EINVAL;
-	}
 
 	while (!list_empty(head)) {
 		element = head->next;
@@ -82,7 +89,7 @@ int hvcs_free_partner_info(struct list_head *head)
 EXPORT_SYMBOL(hvcs_free_partner_info);
 
 /* Helper function for hvcs_get_partner_info */
-int hvcs_next_partner(unsigned int unit_address,
+int hvcs_next_partner(uint32_t unit_address,
 		unsigned long last_p_partition_ID,
 		unsigned long last_p_unit_address, unsigned long *pi_buff)
 
@@ -94,25 +101,37 @@ int hvcs_next_partner(unsigned int unit_address,
 	return hvcs_convert(retval);
 }
 
-/*
- * The unit_address parameter is the unit address of the vty-server vdevice
- * in whose partner information the caller is interested.  This function
- * uses a pointer to a list_head instance in which to store the partner info.
+/**
+ * hvcs_get_partner_info - Get all of the partner info for a vty-server adapter
+ * @unit_address: The unit_address of the vty-server adapter for which this
+ *	function is fetching partner info.
+ * @head: An initialized list_head pointer to an empty list to use to return the
+ *	list of partner info fetched from the hypervisor to the caller.
+ * @pi_buff: A page sized buffer pre-allocated prior to calling this function
+ *	that is to be used to be used by firmware as an iterator to keep track
+ *	of the partner info retrieval.
+ *
  * This function returns non-zero on success, or if there is no partner info.
  *
+ * The pi_buff is pre-allocated prior to calling this function because this
+ * function may be called with a spin_lock held and kmalloc of a page is not
+ * recommended as GFP_ATOMIC.
+ *
+ * The first long of this buffer is used to store a partner unit address.  The
+ * second long is used to store a partner partition ID and starting at
+ * pi_buff[2] is the 79 character Converged Location Code (diff size than the
+ * unsigned longs, hence the casting mumbo jumbo you see later).
+ *
  * Invocation of this function should always be followed by an invocation of
  * hvcs_free_partner_info() using a pointer to the SAME list head instance
- * that was used to store the partner_info list.
+ * that was passed as a parameter to this function.
  */
-int hvcs_get_partner_info(unsigned int unit_address, struct list_head *head,
+int hvcs_get_partner_info(uint32_t unit_address, struct list_head *head,
 		unsigned long *pi_buff)
 {
 	/*
-	 * This is a page sized buffer to be passed to hvcall per invocation.
-	 * NOTE: the first long returned is unit_address.  The second long
-	 * returned is the partition ID and starting with pi_buff[2] are
-	 * HVCS_CLC_LENGTH characters, which are diff size than the unsigned
-	 * long, hence the casting mumbojumbo you see later.
+	 * Dealt with as longs because of the hcall interface even though the
+	 * values are uint32_t.
 	 */
 	unsigned long	last_p_partition_ID;
 	unsigned long	last_p_unit_address;
@@ -122,15 +141,12 @@ int hvcs_get_partner_info(unsigned int unit_address, struct list_head *head,
 
 	memset(pi_buff, 0x00, PAGE_SIZE);
 	/* invalid parameters */
-	if (!head)
+	if (!head || !pi_buff)
 		return -EINVAL;
 
 	last_p_partition_ID = last_p_unit_address = ~0UL;
 	INIT_LIST_HEAD(head);
 
-	if (!pi_buff)
-		return -ENOMEM;
-
 	do {
 		retval = hvcs_next_partner(unit_address, last_p_partition_ID,
 				last_p_unit_address, pi_buff);
@@ -183,21 +199,29 @@ int hvcs_get_partner_info(unsigned int unit_address, struct list_head *head,
 }
 EXPORT_SYMBOL(hvcs_get_partner_info);
 
-/*
+/**
+ * hvcs_register_connection - establish a connection between this vty-server and
+ *	a vty.
+ * @unit_address: The unit address of the vty-server adapter that is to be
+ *	establish a connection.
+ * @p_partition_ID: The partition ID of the vty adapter that is to be connected.
+ * @p_unit_address: The unit address of the vty adapter to which the vty-server
+ *	is to be connected.
+ *
  * If this function is called once and -EINVAL is returned it may
  * indicate that the partner info needs to be refreshed for the
  * target unit address at which point the caller must invoke
  * hvcs_get_partner_info() and then call this function again.  If,
  * for a second time, -EINVAL is returned then it indicates that
  * there is probably already a partner connection registered to a
- * different vty-server@ vdevice.  It is also possible that a second
+ * different vty-server adapter.  It is also possible that a second
  * -EINVAL may indicate that one of the parms is not valid, for
- * instance if the link was removed between the vty-server@ vdevice
- * and the vty@ vdevice that you are trying to open.  Don't shoot the
+ * instance if the link was removed between the vty-server adapter
+ * and the vty adapter that you are trying to open.  Don't shoot the
  * messenger.  Firmware implemented it this way.
  */
-int hvcs_register_connection( unsigned int unit_address,
-		unsigned int p_partition_ID, unsigned int p_unit_address)
+int hvcs_register_connection( uint32_t unit_address,
+		uint32_t p_partition_ID, uint32_t p_unit_address)
 {
 	long retval;
 	retval = plpar_hcall_norets(H_REGISTER_VTERM, unit_address,
@@ -206,11 +230,17 @@ int hvcs_register_connection( unsigned int unit_address,
 }
 EXPORT_SYMBOL(hvcs_register_connection);
 
-/*
- * If -EBUSY is returned continue to call this function
- * until 0 is returned.
+/**
+ * hvcs_free_connection - free the connection between a vty-server and vty
+ * @unit_address: The unit address of the vty-server that is to have its
+ *	connection severed.
+ *
+ * This function is used to free the partner connection between a vty-server
+ * adapter and a vty adapter.
+ *
+ * If -EBUSY is returned continue to call this function until 0 is returned.
  */
-int hvcs_free_connection(unsigned int unit_address)
+int hvcs_free_connection(uint32_t unit_address)
 {
 	long retval;
 	retval = plpar_hcall_norets(H_FREE_VTERM, unit_address);