[PATCH 0/8] Staging: hv: Driver cleanup

[PATCH 0/8] Staging: hv: Driver cleanup

[K. Y. Srinivasan]

Further cleanup of the hv drivers:

	1) Cleanup all remaining checkpatch warnings/errors.

	2) Get rid of items from the TODO file that have been either
	   completed or cannot be done.

	3) Update the TODO file to reflect the current status.
	  

Regards,

K. Y 

[PATCH 1/8] Staging: hv: vmbus: Fix a checkpatch warning in ring_buffer.c

[K. Y. Srinivasan]

Fix a checkpatch warning in ring_buffer.c (line over 80 characters).

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/staging/hv/ring_buffer.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/hv/ring_buffer.c b/drivers/staging/hv/ring_buffer.c
index 9212699..e41d206 100644
--- a/drivers/staging/hv/ring_buffer.c
+++ b/drivers/staging/hv/ring_buffer.c
@@ -34,7 +34,8 @@
 
 
 /* Amount of space to write to */
-#define BYTES_AVAIL_TO_WRITE(r, w, z) ((w) >= (r)) ? ((z) - ((w) - (r))) : ((r) - (w))
+#define BYTES_AVAIL_TO_WRITE(r, w, z) \
+(((w) >= (r)) ? ((z) - ((w) - (r))) : ((r) - (w)))
 
 
 /*
-- 
1.7.4.1

[PATCH 2/8] Staging: hv: vmbus: Fix checkpatch warnings in connection.c

[K. Y. Srinivasan]

Fix bunch of  checkpatch warnings in connection.c


Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/staging/hv/connection.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/hv/connection.c b/drivers/staging/hv/connection.c
index 6aab802..66b7c4b 100644
--- a/drivers/staging/hv/connection.c
+++ b/drivers/staging/hv/connection.c
@@ -224,11 +224,11 @@ static void process_chn_event(u32 relid)
 	channel = relid2channel(relid);
 
 	spin_lock_irqsave(&channel->inbound_lock, flags);
-	if (channel && (channel->onchannel_callback != NULL)) {
+	if (channel && (channel->onchannel_callback != NULL))
 		channel->onchannel_callback(channel->channel_callback_context);
-	} else {
+	else
 		pr_err("channel not found for relid - %u\n", relid);
-	}
+
 	spin_unlock_irqrestore(&channel->inbound_lock, flags);
 }
 
@@ -250,16 +250,17 @@ void vmbus_on_event(unsigned long data)
 		if (!recv_int_page[dword])
 			continue;
 		for (bit = 0; bit < 32; bit++) {
-			if (sync_test_and_clear_bit(bit, (unsigned long *)&recv_int_page[dword])) {
+			if (sync_test_and_clear_bit(bit,
+				(unsigned long *)&recv_int_page[dword])) {
 				relid = (dword << 5) + bit;
 
-				if (relid == 0) {
+				if (relid == 0)
 					/*
 					 * Special case - vmbus
 					 * channel protocol msg
 					 */
 					continue;
-				}
+
 				process_chn_event(relid);
 			}
 		}
-- 
1.7.4.1

[PATCH 3/8] Staging: hv: tools: Fix a checkpatch warning in hv_kvp_daemon.c

[K. Y. Srinivasan]

Fix a checkpatch warning in hv_kvp_daemon.c.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/staging/hv/tools/hv_kvp_daemon.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/hv/tools/hv_kvp_daemon.c b/drivers/staging/hv/tools/hv_kvp_daemon.c
index 33f0f1c..c230bae 100644
--- a/drivers/staging/hv/tools/hv_kvp_daemon.c
+++ b/drivers/staging/hv/tools/hv_kvp_daemon.c
@@ -117,7 +117,7 @@ void kvp_get_os_info(void)
 
 	uname(&uts_buf);
 	os_build = uts_buf.release;
-	processor_arch= uts_buf.machine;
+	processor_arch = uts_buf.machine;
 
 	file = fopen("/etc/SuSE-release", "r");
 	if (file != NULL)
-- 
1.7.4.1

[PATCH 4/8] Staging: hv: vmbus: Fix checkpatch warnings

[K. Y. Srinivasan]

Fix  checkpatch warnings in hv.c

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/staging/hv/hv.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/hv/hv.c b/drivers/staging/hv/hv.c
index e733173..14e6315 100644
--- a/drivers/staging/hv/hv.c
+++ b/drivers/staging/hv/hv.c
@@ -111,7 +111,7 @@ static u64 do_hypercall(u64 control, void *input, void *output)
 	u64 hv_status = 0;
 	u64 input_address = (input) ? virt_to_phys(input) : 0;
 	u64 output_address = (output) ? virt_to_phys(output) : 0;
-	volatile void *hypercall_page = hv_context.hypercall_page;
+	void *hypercall_page = hv_context.hypercall_page;
 
 	__asm__ __volatile__("mov %0, %%r8" : : "r" (output_address) : "r8");
 	__asm__ __volatile__("call *%3" : "=a" (hv_status) :
@@ -132,7 +132,7 @@ static u64 do_hypercall(u64 control, void *input, void *output)
 	u64 output_address = (output) ? virt_to_phys(output) : 0;
 	u32 output_address_hi = output_address >> 32;
 	u32 output_address_lo = output_address & 0xFFFFFFFF;
-	volatile void *hypercall_page = hv_context.hypercall_page;
+	void *hypercall_page = hv_context.hypercall_page;
 
 	__asm__ __volatile__ ("call *%8" : "=d"(hv_status_hi),
 			      "=a"(hv_status_lo) : "d" (control_hi),
-- 
1.7.4.1

Re: [PATCH 4/8] Staging: hv: vmbus: Fix checkpatch warnings

[Greg KH]

On Tue, Jul 19, 2011 at 11:44:21AM -0700, K. Y. Srinivasan wrote:
> Fix  checkpatch warnings in hv.c
> 
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
>  drivers/staging/hv/hv.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/hv/hv.c b/drivers/staging/hv/hv.c
> index e733173..14e6315 100644
> --- a/drivers/staging/hv/hv.c
> +++ b/drivers/staging/hv/hv.c
> @@ -111,7 +111,7 @@ static u64 do_hypercall(u64 control, void *input, void *output)
>  	u64 hv_status = 0;
>  	u64 input_address = (input) ? virt_to_phys(input) : 0;
>  	u64 output_address = (output) ? virt_to_phys(output) : 0;
> -	volatile void *hypercall_page = hv_context.hypercall_page;
> +	void *hypercall_page = hv_context.hypercall_page;

Are you sure?  This was just someone being foolish?  No other reason
someone tried to use volatile here?

greg k-h

RE: [PATCH 4/8] Staging: hv: vmbus: Fix checkpatch warnings

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Tuesday, August 23, 2011 7:17 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang
> Subject: Re: [PATCH 4/8] Staging: hv: vmbus: Fix checkpatch warnings
> 
> On Tue, Jul 19, 2011 at 11:44:21AM -0700, K. Y. Srinivasan wrote:
> > Fix  checkpatch warnings in hv.c
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > ---
> >  drivers/staging/hv/hv.c |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/hv/hv.c b/drivers/staging/hv/hv.c
> > index e733173..14e6315 100644
> > --- a/drivers/staging/hv/hv.c
> > +++ b/drivers/staging/hv/hv.c
> > @@ -111,7 +111,7 @@ static u64 do_hypercall(u64 control, void *input, void
> *output)
> >  	u64 hv_status = 0;
> >  	u64 input_address = (input) ? virt_to_phys(input) : 0;
> >  	u64 output_address = (output) ? virt_to_phys(output) : 0;
> > -	volatile void *hypercall_page = hv_context.hypercall_page;
> > +	void *hypercall_page = hv_context.hypercall_page;
> 
> Are you sure?  This was just someone being foolish?  No other reason
> someone tried to use volatile here?

I cannot see any reason why this needs to be volatile.

Regards,

K. Y
> 
> greg k-h

[PATCH 5/8] Staging: hv: mousevsc: Fix checkpatch errors and warnings

[K. Y. Srinivasan]

Fix checkpatch errors and warnings.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/staging/hv/hv_mouse.c |   58 +++++++++++++++++++++-------------------
 1 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/drivers/staging/hv/hv_mouse.c b/drivers/staging/hv/hv_mouse.c
index 28bd2c5..e95bd3a 100644
--- a/drivers/staging/hv/hv_mouse.c
+++ b/drivers/staging/hv/hv_mouse.c
@@ -53,7 +53,7 @@ struct hv_input_dev_info {
 					 (SYNTHHID_INPUT_VERSION_MAJOR << 16))
 
 
-#pragma pack(push,1)
+#pragma pack(push, 1)
 /*
  * Message types in the synthetic input protocol
  */
@@ -119,8 +119,8 @@ struct synthhid_input_report {
 
 #pragma pack(pop)
 
-#define INPUTVSC_SEND_RING_BUFFER_SIZE		10*PAGE_SIZE
-#define INPUTVSC_RECV_RING_BUFFER_SIZE		10*PAGE_SIZE
+#define INPUTVSC_SEND_RING_BUFFER_SIZE		(10 * PAGE_SIZE)
+#define INPUTVSC_RECV_RING_BUFFER_SIZE		(10 * PAGE_SIZE)
 
 #define NBITS(x) (((x)/BITS_PER_LONG)+1)
 
@@ -177,7 +177,8 @@ struct mousevsc_dev {
 
 static const char *driver_name = "mousevsc";
 
-static void deviceinfo_callback(struct hv_device *dev, struct hv_input_dev_info *info);
+static void deviceinfo_callback(struct hv_device *dev,
+				 struct hv_input_dev_info *info);
 static void inputreport_callback(struct hv_device *dev, void *packet, u32 len);
 static void reportdesc_callback(struct hv_device *dev, void *packet, u32 len);
 
@@ -336,7 +337,8 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device,
 	input_device->hid_desc = kzalloc(desc->bLength, GFP_KERNEL);
 
 	if (!input_device->hid_desc) {
-		pr_err("unable to allocate hid descriptor - size %d", desc->bLength);
+		pr_err("unable to allocate hid descriptor - size %d",
+			 desc->bLength);
 		goto cleanup;
 	}
 
@@ -502,22 +504,21 @@ static void mousevsc_on_channel_callback(void *context)
 				desc = (struct vmpacket_descriptor *)buffer;
 
 				switch (desc->type) {
-					case VM_PKT_COMP:
-						mousevsc_on_send_completion(
-							device, desc);
-						break;
-
-					case VM_PKT_DATA_INBAND:
-						mousevsc_on_receive(
-							device, desc);
-						break;
-
-					default:
-						pr_err("unhandled packet type %d, tid %llx len %d\n",
-							   desc->type,
-							   req_id,
-							   bytes_recvd);
-						break;
+				case VM_PKT_COMP:
+					mousevsc_on_send_completion(
+						device, desc);
+					break;
+
+				case VM_PKT_DATA_INBAND:
+					mousevsc_on_receive(
+						device, desc);
+					break;
+
+				default:
+					pr_err("unhandled packet type %d, tid %llx len %d\n",
+						desc->type, req_id,
+						bytes_recvd);
+					break;
 				}
 
 				/* reset */
@@ -596,12 +597,12 @@ static int mousevsc_connect_to_vsp(struct hv_device *device)
 	pr_info("synthhid protocol request...");
 
 	ret = vmbus_sendpacket(device->channel, request,
-					sizeof(struct pipe_prt_msg) -
-					sizeof(unsigned char) +
-					sizeof(struct synthhid_protocol_request),
-					(unsigned long)request,
-					VM_PKT_DATA_INBAND,
-					VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
+				sizeof(struct pipe_prt_msg) -
+				sizeof(unsigned char) +
+				sizeof(struct synthhid_protocol_request),
+				(unsigned long)request,
+				VM_PKT_DATA_INBAND,
+				VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
 	if (ret != 0) {
 		pr_err("unable to send synthhid protocol request.");
 		goto cleanup;
@@ -766,7 +767,8 @@ struct input_device_context {
 };
 
 
-static void deviceinfo_callback(struct hv_device *dev, struct hv_input_dev_info *info)
+static void deviceinfo_callback(struct hv_device *dev,
+				struct hv_input_dev_info *info)
 {
 	struct input_device_context *input_device_ctx =
 		dev_get_drvdata(&dev->device);
-- 
1.7.4.1

Re: [PATCH 5/8] Staging: hv: mousevsc: Fix checkpatch errors and warnings

[Joe Perches]

On Tue, 2011-07-19 at 11:44 -0700, K. Y. Srinivasan wrote:
> Fix checkpatch errors and warnings.
[]
> diff --git a/drivers/staging/hv/hv_mouse.c b/drivers/staging/hv/hv_mouse.c
[]
> @@ -53,7 +53,7 @@ struct hv_input_dev_info {
> -#pragma pack(push,1)
> +#pragma pack(push, 1)
>  /*
>   * Message types in the synthetic input protocol
>   */

Perhaps it's better and more consistent with
other kernel style uses to remove #pragma pack[...]
and mark the individual structs with __packed;

$ grep -rP --include=*.[ch] "pragma.*pack" * | wc -l
221
$ grep -rP --include=*.[ch] "(__attribute__.*packed|\b__packed\b)" * | wc -l
5553

RE: [PATCH 5/8] Staging: hv: mousevsc: Fix checkpatch errors and warnings

[KY Srinivasan]

> -----Original Message-----
> From: Joe Perches [mailto:joe@perches.com]
> Sent: Tuesday, July 19, 2011 3:29 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang
> Subject: Re: [PATCH 5/8] Staging: hv: mousevsc: Fix checkpatch errors and
> warnings
> 
> On Tue, 2011-07-19 at 11:44 -0700, K. Y. Srinivasan wrote:
> > Fix checkpatch errors and warnings.
> []
> > diff --git a/drivers/staging/hv/hv_mouse.c b/drivers/staging/hv/hv_mouse.c
> []
> > @@ -53,7 +53,7 @@ struct hv_input_dev_info {
> > -#pragma pack(push,1)
> > +#pragma pack(push, 1)
> >  /*
> >   * Message types in the synthetic input protocol
> >   */
> 
> Perhaps it's better and more consistent with
> other kernel style uses to remove #pragma pack[...]
> and mark the individual structs with __packed;

Good point. Currently, this driver is not functional. When the driver
is finally functional, there is a whole lot of cleanup that is needed and I will
do what you are suggesting then. For this go around, I just wanted to address
checkpatch issues with minimal change.

Regards,

K. Y 

[PATCH 6/8] Staging: hv: Get rid of checkpatch cleanup item

[K. Y. Srinivasan]

All checkpatch warnings/errors have been addressed and fixed. Patches have been
been submitted to address the remaining issues.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/staging/hv/TODO |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/hv/TODO b/drivers/staging/hv/TODO
index 582fd4a..b6335ff 100644
--- a/drivers/staging/hv/TODO
+++ b/drivers/staging/hv/TODO
@@ -1,5 +1,4 @@
 TODO:
-	- fix remaining checkpatch warnings and errors
 	- audit the vmbus to verify it is working properly with the
 	  driver model
 	- see if the vmbus can be merged with the other virtual busses
-- 
1.7.4.1

[PATCH 7/8] Staging: hv: Get rid of the vmbus merge item

[K. Y. Srinivasan]

The host/guest transport is very host centric. Given this, it is unlikely
that we will be able to merge vmbus with other host/guest buses in the kernel.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/staging/hv/TODO |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/hv/TODO b/drivers/staging/hv/TODO
index b6335ff..8b4c97a 100644
--- a/drivers/staging/hv/TODO
+++ b/drivers/staging/hv/TODO
@@ -1,8 +1,6 @@
 TODO:
 	- audit the vmbus to verify it is working properly with the
 	  driver model
-	- see if the vmbus can be merged with the other virtual busses
-	  in the kernel
 	- audit the network driver
 	  - checking for carrier inside open is wrong, network device API
             confusion??
-- 
1.7.4.1

[PATCH 8/8] Staging: hv: Update the TODO file

[K. Y. Srinivasan]

The TODO file has not been updated in a long time. Update this file
to reflect the current status.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/staging/hv/TODO |   21 ++++++++++++++++++++-
 1 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/hv/TODO b/drivers/staging/hv/TODO
index 8b4c97a..23c74ed 100644
--- a/drivers/staging/hv/TODO
+++ b/drivers/staging/hv/TODO
@@ -1,11 +1,30 @@
 TODO:
 	- audit the vmbus to verify it is working properly with the
 	  driver model
+
+		STATUS (July 19, 2011):
+		All outstanding issues (known to us)  with regards
+		to conforming to Linux Driver Model have been addressed.
+
 	- audit the network driver
 	  - checking for carrier inside open is wrong, network device API
             confusion??
+
+		STATUS (July 19, 2011):
+		The network driver has been reviewed by the community.
+		We have addressed the various issues that were raised in this
+		review.
+
 	- audit the block driver
 	- audit the scsi driver
 
+		STATUS (July 19, 2011):
+		One of the longstanding comment on this body of code
+		has been to merge the block and scsi driver. This has been done
+		and patches have been submitted. We have also addressed all the
+		review comments on this body of code.
+
+
 Please send patches for this code to Greg Kroah-Hartman <gregkh@suse.de>,
-Hank Janssen <hjanssen@microsoft.com>, and Haiyang Zhang <haiyangz@microsoft.com>.
+Hank Janssen <hjanssen@microsoft.com>, and Haiyang Zhang <haiyangz@microsoft.com>,
+K. Y. Srinivasan <kys@microsoft.com>.
-- 
1.7.4.1

Re: [PATCH 1/8] Staging: hv: vmbus: Fix a checkpatch warning in ring_buffer.c

[Greg KH]

On Tue, Jul 19, 2011 at 11:44:18AM -0700, K. Y. Srinivasan wrote:
> Fix a checkpatch warning in ring_buffer.c (line over 80 characters).
> 
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
>  drivers/staging/hv/ring_buffer.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/staging/hv/ring_buffer.c b/drivers/staging/hv/ring_buffer.c
> index 9212699..e41d206 100644
> --- a/drivers/staging/hv/ring_buffer.c
> +++ b/drivers/staging/hv/ring_buffer.c
> @@ -34,7 +34,8 @@
>  
>  
>  /* Amount of space to write to */
> -#define BYTES_AVAIL_TO_WRITE(r, w, z) ((w) >= (r)) ? ((z) - ((w) - (r))) : ((r) - (w))
> +#define BYTES_AVAIL_TO_WRITE(r, w, z) \
> +(((w) >= (r)) ? ((z) - ((w) - (r))) : ((r) - (w)))

Please at least indent the second line a little bit, otherwise it's a
pain to read, right?

greg k-h