All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 4/6] Staging: hv:  Unify the hyperv driver abstractions
@ 2011-02-26  2:07 ` K. Y. Srinivasan
  0 siblings, 0 replies; 10+ messages in thread
From: K. Y. Srinivasan @ 2011-02-26  2:07 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization
  Cc: K. Y. Srinivasan, Haiyang Zhang, Hank Janssen

This patch combines the two driver abstractions into
a single driver abstraction.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>

---
 drivers/staging/hv/blkvsc.c       |    2 +-
 drivers/staging/hv/blkvsc_drv.c   |    2 +-
 drivers/staging/hv/netvsc.c       |    6 +++---
 drivers/staging/hv/netvsc_api.h   |    4 ++--
 drivers/staging/hv/netvsc_drv.c   |    2 +-
 drivers/staging/hv/rndis_filter.c |    4 ++--
 drivers/staging/hv/storvsc.c      |    4 ++--
 drivers/staging/hv/storvsc_api.h  |    6 +++---
 drivers/staging/hv/storvsc_drv.c  |    2 +-
 drivers/staging/hv/vmbus.h        |   10 +++++++++-
 drivers/staging/hv/vmbus_api.h    |   14 +-------------
 drivers/staging/hv/vmbus_drv.c    |   18 +++++++++---------
 12 files changed, 35 insertions(+), 39 deletions(-)

diff --git a/drivers/staging/hv/blkvsc.c b/drivers/staging/hv/blkvsc.c
index 47ccec2..d29af4b 100644
--- a/drivers/staging/hv/blkvsc.c
+++ b/drivers/staging/hv/blkvsc.c
@@ -63,7 +63,7 @@ blk_vsc_on_device_add(struct hyperv_device *device, void *additional_info)
 	return ret;
 }
 
-int blk_vsc_initialize(struct hv_driver *driver)
+int blk_vsc_initialize(struct driver_context *driver)
 {
 	struct storvsc_driver_object *stor_driver;
 	int ret = 0;
diff --git a/drivers/staging/hv/blkvsc_drv.c b/drivers/staging/hv/blkvsc_drv.c
index 305a665..a280f83 100644
--- a/drivers/staging/hv/blkvsc_drv.c
+++ b/drivers/staging/hv/blkvsc_drv.c
@@ -171,7 +171,7 @@ static const struct block_device_operations block_ops = {
 /*
  * blkvsc_drv_init -  BlkVsc driver initialization.
  */
-static int blkvsc_drv_init(int (*drv_init)(struct hv_driver *drv))
+static int blkvsc_drv_init(int (*drv_init)(struct driver_context *drv))
 {
 	struct storvsc_driver_object *storvsc_drv_obj = &g_blkvsc_drv.drv_obj;
 	struct driver_context *drv_ctx = &g_blkvsc_drv.drv_ctx;
diff --git a/drivers/staging/hv/netvsc.c b/drivers/staging/hv/netvsc.c
index 7cf6999..6d8d0a3 100644
--- a/drivers/staging/hv/netvsc.c
+++ b/drivers/staging/hv/netvsc.c
@@ -49,7 +49,7 @@ netvsc_device_add(struct hyperv_device *device, void *additional_info);
 
 static int netvsc_device_remove(struct hyperv_device *device);
 
-static void netvsc_cleanup(struct hv_driver *driver);
+static void netvsc_cleanup(struct driver_context *driver);
 
 static void netvsc_channel_cb(void *context);
 
@@ -178,7 +178,7 @@ static struct netvsc_device *release_inbound_net_device(
 /*
  * netvsc_initialize - Main entry point
  */
-int netvsc_initialize(struct hv_driver *drv)
+int netvsc_initialize(struct driver_context *drv)
 {
 	struct netvsc_driver *driver = (struct netvsc_driver *)drv;
 
@@ -837,7 +837,7 @@ static int netvsc_device_remove(struct hyperv_device *device)
 /*
  * netvsc_cleanup - Perform any cleanup when the driver is removed
  */
-static void netvsc_cleanup(struct hv_driver *drv)
+static void netvsc_cleanup(struct driver_context *drv)
 {
 }
 
diff --git a/drivers/staging/hv/netvsc_api.h b/drivers/staging/hv/netvsc_api.h
index e43ff7b..842f542 100644
--- a/drivers/staging/hv/netvsc_api.h
+++ b/drivers/staging/hv/netvsc_api.h
@@ -84,7 +84,7 @@ struct hv_netvsc_packet {
 struct netvsc_driver {
 	/* Must be the first field */
 	/* Which is a bug FIXME! */
-	struct hv_driver base;
+	struct driver_context base;
 
 	u32 ring_buf_size;
 	u32 req_ext_size;
@@ -109,7 +109,7 @@ struct netvsc_device_info {
 };
 
 /* Interface */
-int netvsc_initialize(struct hv_driver *drv);
+int netvsc_initialize(struct driver_context *drv);
 int rndis_filter_open(struct hyperv_device *dev);
 int rndis_filter_close(struct hyperv_device *dev);
 
diff --git a/drivers/staging/hv/netvsc_drv.c b/drivers/staging/hv/netvsc_drv.c
index fc4eb9b..fbb32f7 100644
--- a/drivers/staging/hv/netvsc_drv.c
+++ b/drivers/staging/hv/netvsc_drv.c
@@ -490,7 +490,7 @@ static void netvsc_drv_exit(void)
 	return;
 }
 
-static int netvsc_drv_init(int (*drv_init)(struct hv_driver *drv))
+static int netvsc_drv_init(int (*drv_init)(struct driver_context *drv))
 {
 	struct netvsc_driver *net_drv_obj = &g_netvsc_drv.drv_obj;
 	struct driver_context *drv_ctx = &g_netvsc_drv.drv_ctx;
diff --git a/drivers/staging/hv/rndis_filter.c b/drivers/staging/hv/rndis_filter.c
index b718fa9..a2b5f4d 100644
--- a/drivers/staging/hv/rndis_filter.c
+++ b/drivers/staging/hv/rndis_filter.c
@@ -90,7 +90,7 @@ static int rndis_filte_device_add(struct hyperv_device *dev,
 
 static int rndis_filter_device_remove(struct hyperv_device *dev);
 
-static void rndis_filter_cleanup(struct hv_driver *drv);
+static void rndis_filter_cleanup(struct driver_context *drv);
 
 static int rndis_filter_send(struct hyperv_device *dev,
 			     struct hv_netvsc_packet *pkt);
@@ -834,7 +834,7 @@ static int rndis_filter_device_remove(struct hyperv_device *dev)
 	return 0;
 }
 
-static void rndis_filter_cleanup(struct hv_driver *drv)
+static void rndis_filter_cleanup(struct driver_context *drv)
 {
 }
 
diff --git a/drivers/staging/hv/storvsc.c b/drivers/staging/hv/storvsc.c
index 6000b0d..c86bc68 100644
--- a/drivers/staging/hv/storvsc.c
+++ b/drivers/staging/hv/storvsc.c
@@ -781,14 +781,14 @@ static int stor_vsc_on_io_request(struct hyperv_device *device,
 /*
  * stor_vsc_on_cleanup - Perform any cleanup when the driver is removed
  */
-static void stor_vsc_on_cleanup(struct hv_driver *driver)
+static void stor_vsc_on_cleanup(struct driver_context *driver)
 {
 }
 
 /*
  * stor_vsc_initialize - Main entry point
  */
-int stor_vsc_initialize(struct hv_driver *driver)
+int stor_vsc_initialize(struct driver_context *driver)
 {
 	struct storvsc_driver_object *stor_driver;
 
diff --git a/drivers/staging/hv/storvsc_api.h b/drivers/staging/hv/storvsc_api.h
index c2915b4..b488d75 100644
--- a/drivers/staging/hv/storvsc_api.h
+++ b/drivers/staging/hv/storvsc_api.h
@@ -80,7 +80,7 @@ struct hv_storvsc_request {
 struct storvsc_driver_object {
 	/* Must be the first field */
 	/* Which is a bug FIXME! */
-	struct hv_driver base;
+	struct driver_context base;
 
 	/* Set by caller (in bytes) */
 	u32 ring_buffer_size;
@@ -103,8 +103,8 @@ struct storvsc_device_info {
 };
 
 /* Interface */
-int stor_vsc_initialize(struct hv_driver *driver);
+int stor_vsc_initialize(struct driver_context *driver);
 int stor_vsc_on_host_reset(struct hyperv_device *device);
-int blk_vsc_initialize(struct hv_driver *driver);
+int blk_vsc_initialize(struct driver_context *driver);
 
 #endif /* _STORVSC_API_H_ */
diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index c094578..282bd66 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -135,7 +135,7 @@ static struct scsi_host_template scsi_driver = {
 /*
  * storvsc_drv_init - StorVsc driver initialization.
  */
-static int storvsc_drv_init(int (*drv_init)(struct hv_driver *drv))
+static int storvsc_drv_init(int (*drv_init)(struct driver_context *drv))
 {
 	int ret;
 	struct storvsc_driver_object *storvsc_drv_obj = &g_storvsc_drv.drv_obj;
diff --git a/drivers/staging/hv/vmbus.h b/drivers/staging/hv/vmbus.h
index 1ac7e56..cad8b4c 100644
--- a/drivers/staging/hv/vmbus.h
+++ b/drivers/staging/hv/vmbus.h
@@ -29,8 +29,16 @@
 #include "vmbus_api.h"
 
 struct driver_context {
+	const char *name;
+
 	struct hv_guid class_id;
 
+	int (*dev_add)(struct hyperv_device *device, void *data);
+
+	int (*dev_rm)(struct hyperv_device *device);
+
+	void (*cleanup)(struct driver_context *driver);
+
 	struct device_driver driver;
 
 	/*
@@ -48,7 +56,7 @@ struct hyperv_device {
 	struct work_struct probe_failed_work_item;
 	struct hv_guid class_id; /* device type id */
 	struct hv_guid device_id; /* device instance id */
-	struct hv_driver *drv;
+	struct driver_context *drv;
 	int probe_error;
 	struct vmbus_channel *channel; /* associated channel to host*/
 	void    *ext;
diff --git a/drivers/staging/hv/vmbus_api.h b/drivers/staging/hv/vmbus_api.h
index 260086f..0d8232e 100644
--- a/drivers/staging/hv/vmbus_api.h
+++ b/drivers/staging/hv/vmbus_api.h
@@ -55,7 +55,7 @@ struct hv_multipage_buffer {
 
 #pragma pack(pop)
 
-struct hv_driver;
+struct driver_context;
 struct hyperv_device;
 
 struct hv_dev_port_info {
@@ -84,16 +84,4 @@ struct hyperv_device_info {
 	struct hv_dev_port_info outbound;
 };
 
-/* Base driver object */
-struct hv_driver {
-	const char *name;
-
-	/* the device type supported by this driver */
-	struct hv_guid class_id;
-
-	int (*dev_add)(struct hyperv_device *device, void *data);
-	int (*dev_rm)(struct hyperv_device *device);
-	void (*cleanup)(struct hv_driver *driver);
-};
-
 #endif /* _VMBUS_API_H_ */
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index a385bd2..3922a07 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -47,7 +47,7 @@ struct vmbus_driver_context {
 	/* The driver field is not used in here. Instead, the bus field is */
 	/* used to represent the driver */
 	struct driver_context drv_ctx;
-	struct hv_driver drv_obj;
+	struct driver_context drv_obj;
 
 	struct bus_type bus;
 	struct tasklet_struct msg_dpc;
@@ -196,7 +196,7 @@ static int vmbus_dev_rm(struct hyperv_device *dev)
 /*
  * vmbus_cleanup - Perform any cleanup when the driver is removed
  */
-static void vmbus_cleanup(struct hv_driver *drv)
+static void vmbus_cleanup(struct driver_context *drv)
 {
 	/* struct vmbus_driver *driver = (struct vmbus_driver *)drv; */
 
@@ -221,7 +221,7 @@ static void vmbus_onmessage_work(struct work_struct *work)
 /*
  * vmbus_on_msg_dpc - DPC routine to handle messages from the hypervisior
  */
-static void vmbus_on_msg_dpc(struct hv_driver *drv)
+static void vmbus_on_msg_dpc(struct driver_context *drv)
 {
 	int cpu = smp_processor_id();
 	void *page_addr = hv_context.synic_message_page[cpu];
@@ -267,7 +267,7 @@ static void vmbus_on_msg_dpc(struct hv_driver *drv)
 /*
  * vmbus_on_isr - ISR routine
  */
-static int vmbus_on_isr(struct hv_driver *drv)
+static int vmbus_on_isr(struct driver_context *drv)
 {
 	int ret = 0;
 	int cpu = smp_processor_id();
@@ -463,7 +463,7 @@ static ssize_t vmbus_show_device_attr(struct device *dev,
 static int vmbus_bus_init(void)
 {
 	struct vmbus_driver_context *vmbus_drv_ctx = &vmbus_drv;
-	struct hv_driver *driver = &vmbus_drv.drv_obj;
+	struct driver_context *driver = &vmbus_drv.drv_obj;
 	struct hyperv_device *dev = &vmbus_drv.device_obj;
 	int ret;
 	unsigned int vector;
@@ -587,7 +587,7 @@ cleanup:
  */
 static void vmbus_bus_exit(void)
 {
-	struct hv_driver *driver = &vmbus_drv.drv_obj;
+	struct driver_context *driver = &vmbus_drv.drv_obj;
 	struct vmbus_driver_context *vmbus_drv_ctx = &vmbus_drv;
 
 	struct hyperv_device *dev = &vmbus_drv.device_obj;
@@ -857,7 +857,7 @@ static int vmbus_match(struct device *device, struct device_driver *driver)
 		   sizeof(struct hv_guid)) == 0) {
 		/*
 		 * !! NOTE: The driver_ctx is not a vmbus_drv_ctx. We typecast
-		 * it here to access the struct hv_driver field
+		 * it here to access the struct driver_context field
 		 */
 		struct vmbus_driver_context *vmbus_drv_ctx =
 			(struct vmbus_driver_context *)driver_ctx;
@@ -1020,7 +1020,7 @@ static void vmbus_device_release(struct device *device)
  */
 static void vmbus_msg_dpc(unsigned long data)
 {
-	struct hv_driver *driver = (struct hv_driver *)data;
+	struct driver_context *driver = (struct driver_context *)data;
 
 	/* Call to bus driver to handle interrupt */
 	vmbus_on_msg_dpc(driver);
@@ -1037,7 +1037,7 @@ static void vmbus_event_dpc(unsigned long data)
 
 static irqreturn_t vmbus_isr(int irq, void *dev_id)
 {
-	struct hv_driver *driver = &vmbus_drv.drv_obj;
+	struct driver_context *driver = &vmbus_drv.drv_obj;
 	int ret;
 
 	/* Call to bus driver to handle interrupt */
-- 
1.5.5.6


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/6] Staging: hv:  Unify the hyperv driver abstractions
@ 2011-02-26  2:07 ` K. Y. Srinivasan
  0 siblings, 0 replies; 10+ messages in thread
From: K. Y. Srinivasan @ 2011-02-26  2:07 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, virtualization

This patch combines the two driver abstractions into
a single driver abstraction.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>

---
 drivers/staging/hv/blkvsc.c       |    2 +-
 drivers/staging/hv/blkvsc_drv.c   |    2 +-
 drivers/staging/hv/netvsc.c       |    6 +++---
 drivers/staging/hv/netvsc_api.h   |    4 ++--
 drivers/staging/hv/netvsc_drv.c   |    2 +-
 drivers/staging/hv/rndis_filter.c |    4 ++--
 drivers/staging/hv/storvsc.c      |    4 ++--
 drivers/staging/hv/storvsc_api.h  |    6 +++---
 drivers/staging/hv/storvsc_drv.c  |    2 +-
 drivers/staging/hv/vmbus.h        |   10 +++++++++-
 drivers/staging/hv/vmbus_api.h    |   14 +-------------
 drivers/staging/hv/vmbus_drv.c    |   18 +++++++++---------
 12 files changed, 35 insertions(+), 39 deletions(-)

diff --git a/drivers/staging/hv/blkvsc.c b/drivers/staging/hv/blkvsc.c
index 47ccec2..d29af4b 100644
--- a/drivers/staging/hv/blkvsc.c
+++ b/drivers/staging/hv/blkvsc.c
@@ -63,7 +63,7 @@ blk_vsc_on_device_add(struct hyperv_device *device, void *additional_info)
 	return ret;
 }
 
-int blk_vsc_initialize(struct hv_driver *driver)
+int blk_vsc_initialize(struct driver_context *driver)
 {
 	struct storvsc_driver_object *stor_driver;
 	int ret = 0;
diff --git a/drivers/staging/hv/blkvsc_drv.c b/drivers/staging/hv/blkvsc_drv.c
index 305a665..a280f83 100644
--- a/drivers/staging/hv/blkvsc_drv.c
+++ b/drivers/staging/hv/blkvsc_drv.c
@@ -171,7 +171,7 @@ static const struct block_device_operations block_ops = {
 /*
  * blkvsc_drv_init -  BlkVsc driver initialization.
  */
-static int blkvsc_drv_init(int (*drv_init)(struct hv_driver *drv))
+static int blkvsc_drv_init(int (*drv_init)(struct driver_context *drv))
 {
 	struct storvsc_driver_object *storvsc_drv_obj = &g_blkvsc_drv.drv_obj;
 	struct driver_context *drv_ctx = &g_blkvsc_drv.drv_ctx;
diff --git a/drivers/staging/hv/netvsc.c b/drivers/staging/hv/netvsc.c
index 7cf6999..6d8d0a3 100644
--- a/drivers/staging/hv/netvsc.c
+++ b/drivers/staging/hv/netvsc.c
@@ -49,7 +49,7 @@ netvsc_device_add(struct hyperv_device *device, void *additional_info);
 
 static int netvsc_device_remove(struct hyperv_device *device);
 
-static void netvsc_cleanup(struct hv_driver *driver);
+static void netvsc_cleanup(struct driver_context *driver);
 
 static void netvsc_channel_cb(void *context);
 
@@ -178,7 +178,7 @@ static struct netvsc_device *release_inbound_net_device(
 /*
  * netvsc_initialize - Main entry point
  */
-int netvsc_initialize(struct hv_driver *drv)
+int netvsc_initialize(struct driver_context *drv)
 {
 	struct netvsc_driver *driver = (struct netvsc_driver *)drv;
 
@@ -837,7 +837,7 @@ static int netvsc_device_remove(struct hyperv_device *device)
 /*
  * netvsc_cleanup - Perform any cleanup when the driver is removed
  */
-static void netvsc_cleanup(struct hv_driver *drv)
+static void netvsc_cleanup(struct driver_context *drv)
 {
 }
 
diff --git a/drivers/staging/hv/netvsc_api.h b/drivers/staging/hv/netvsc_api.h
index e43ff7b..842f542 100644
--- a/drivers/staging/hv/netvsc_api.h
+++ b/drivers/staging/hv/netvsc_api.h
@@ -84,7 +84,7 @@ struct hv_netvsc_packet {
 struct netvsc_driver {
 	/* Must be the first field */
 	/* Which is a bug FIXME! */
-	struct hv_driver base;
+	struct driver_context base;
 
 	u32 ring_buf_size;
 	u32 req_ext_size;
@@ -109,7 +109,7 @@ struct netvsc_device_info {
 };
 
 /* Interface */
-int netvsc_initialize(struct hv_driver *drv);
+int netvsc_initialize(struct driver_context *drv);
 int rndis_filter_open(struct hyperv_device *dev);
 int rndis_filter_close(struct hyperv_device *dev);
 
diff --git a/drivers/staging/hv/netvsc_drv.c b/drivers/staging/hv/netvsc_drv.c
index fc4eb9b..fbb32f7 100644
--- a/drivers/staging/hv/netvsc_drv.c
+++ b/drivers/staging/hv/netvsc_drv.c
@@ -490,7 +490,7 @@ static void netvsc_drv_exit(void)
 	return;
 }
 
-static int netvsc_drv_init(int (*drv_init)(struct hv_driver *drv))
+static int netvsc_drv_init(int (*drv_init)(struct driver_context *drv))
 {
 	struct netvsc_driver *net_drv_obj = &g_netvsc_drv.drv_obj;
 	struct driver_context *drv_ctx = &g_netvsc_drv.drv_ctx;
diff --git a/drivers/staging/hv/rndis_filter.c b/drivers/staging/hv/rndis_filter.c
index b718fa9..a2b5f4d 100644
--- a/drivers/staging/hv/rndis_filter.c
+++ b/drivers/staging/hv/rndis_filter.c
@@ -90,7 +90,7 @@ static int rndis_filte_device_add(struct hyperv_device *dev,
 
 static int rndis_filter_device_remove(struct hyperv_device *dev);
 
-static void rndis_filter_cleanup(struct hv_driver *drv);
+static void rndis_filter_cleanup(struct driver_context *drv);
 
 static int rndis_filter_send(struct hyperv_device *dev,
 			     struct hv_netvsc_packet *pkt);
@@ -834,7 +834,7 @@ static int rndis_filter_device_remove(struct hyperv_device *dev)
 	return 0;
 }
 
-static void rndis_filter_cleanup(struct hv_driver *drv)
+static void rndis_filter_cleanup(struct driver_context *drv)
 {
 }
 
diff --git a/drivers/staging/hv/storvsc.c b/drivers/staging/hv/storvsc.c
index 6000b0d..c86bc68 100644
--- a/drivers/staging/hv/storvsc.c
+++ b/drivers/staging/hv/storvsc.c
@@ -781,14 +781,14 @@ static int stor_vsc_on_io_request(struct hyperv_device *device,
 /*
  * stor_vsc_on_cleanup - Perform any cleanup when the driver is removed
  */
-static void stor_vsc_on_cleanup(struct hv_driver *driver)
+static void stor_vsc_on_cleanup(struct driver_context *driver)
 {
 }
 
 /*
  * stor_vsc_initialize - Main entry point
  */
-int stor_vsc_initialize(struct hv_driver *driver)
+int stor_vsc_initialize(struct driver_context *driver)
 {
 	struct storvsc_driver_object *stor_driver;
 
diff --git a/drivers/staging/hv/storvsc_api.h b/drivers/staging/hv/storvsc_api.h
index c2915b4..b488d75 100644
--- a/drivers/staging/hv/storvsc_api.h
+++ b/drivers/staging/hv/storvsc_api.h
@@ -80,7 +80,7 @@ struct hv_storvsc_request {
 struct storvsc_driver_object {
 	/* Must be the first field */
 	/* Which is a bug FIXME! */
-	struct hv_driver base;
+	struct driver_context base;
 
 	/* Set by caller (in bytes) */
 	u32 ring_buffer_size;
@@ -103,8 +103,8 @@ struct storvsc_device_info {
 };
 
 /* Interface */
-int stor_vsc_initialize(struct hv_driver *driver);
+int stor_vsc_initialize(struct driver_context *driver);
 int stor_vsc_on_host_reset(struct hyperv_device *device);
-int blk_vsc_initialize(struct hv_driver *driver);
+int blk_vsc_initialize(struct driver_context *driver);
 
 #endif /* _STORVSC_API_H_ */
diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index c094578..282bd66 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -135,7 +135,7 @@ static struct scsi_host_template scsi_driver = {
 /*
  * storvsc_drv_init - StorVsc driver initialization.
  */
-static int storvsc_drv_init(int (*drv_init)(struct hv_driver *drv))
+static int storvsc_drv_init(int (*drv_init)(struct driver_context *drv))
 {
 	int ret;
 	struct storvsc_driver_object *storvsc_drv_obj = &g_storvsc_drv.drv_obj;
diff --git a/drivers/staging/hv/vmbus.h b/drivers/staging/hv/vmbus.h
index 1ac7e56..cad8b4c 100644
--- a/drivers/staging/hv/vmbus.h
+++ b/drivers/staging/hv/vmbus.h
@@ -29,8 +29,16 @@
 #include "vmbus_api.h"
 
 struct driver_context {
+	const char *name;
+
 	struct hv_guid class_id;
 
+	int (*dev_add)(struct hyperv_device *device, void *data);
+
+	int (*dev_rm)(struct hyperv_device *device);
+
+	void (*cleanup)(struct driver_context *driver);
+
 	struct device_driver driver;
 
 	/*
@@ -48,7 +56,7 @@ struct hyperv_device {
 	struct work_struct probe_failed_work_item;
 	struct hv_guid class_id; /* device type id */
 	struct hv_guid device_id; /* device instance id */
-	struct hv_driver *drv;
+	struct driver_context *drv;
 	int probe_error;
 	struct vmbus_channel *channel; /* associated channel to host*/
 	void    *ext;
diff --git a/drivers/staging/hv/vmbus_api.h b/drivers/staging/hv/vmbus_api.h
index 260086f..0d8232e 100644
--- a/drivers/staging/hv/vmbus_api.h
+++ b/drivers/staging/hv/vmbus_api.h
@@ -55,7 +55,7 @@ struct hv_multipage_buffer {
 
 #pragma pack(pop)
 
-struct hv_driver;
+struct driver_context;
 struct hyperv_device;
 
 struct hv_dev_port_info {
@@ -84,16 +84,4 @@ struct hyperv_device_info {
 	struct hv_dev_port_info outbound;
 };
 
-/* Base driver object */
-struct hv_driver {
-	const char *name;
-
-	/* the device type supported by this driver */
-	struct hv_guid class_id;
-
-	int (*dev_add)(struct hyperv_device *device, void *data);
-	int (*dev_rm)(struct hyperv_device *device);
-	void (*cleanup)(struct hv_driver *driver);
-};
-
 #endif /* _VMBUS_API_H_ */
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index a385bd2..3922a07 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -47,7 +47,7 @@ struct vmbus_driver_context {
 	/* The driver field is not used in here. Instead, the bus field is */
 	/* used to represent the driver */
 	struct driver_context drv_ctx;
-	struct hv_driver drv_obj;
+	struct driver_context drv_obj;
 
 	struct bus_type bus;
 	struct tasklet_struct msg_dpc;
@@ -196,7 +196,7 @@ static int vmbus_dev_rm(struct hyperv_device *dev)
 /*
  * vmbus_cleanup - Perform any cleanup when the driver is removed
  */
-static void vmbus_cleanup(struct hv_driver *drv)
+static void vmbus_cleanup(struct driver_context *drv)
 {
 	/* struct vmbus_driver *driver = (struct vmbus_driver *)drv; */
 
@@ -221,7 +221,7 @@ static void vmbus_onmessage_work(struct work_struct *work)
 /*
  * vmbus_on_msg_dpc - DPC routine to handle messages from the hypervisior
  */
-static void vmbus_on_msg_dpc(struct hv_driver *drv)
+static void vmbus_on_msg_dpc(struct driver_context *drv)
 {
 	int cpu = smp_processor_id();
 	void *page_addr = hv_context.synic_message_page[cpu];
@@ -267,7 +267,7 @@ static void vmbus_on_msg_dpc(struct hv_driver *drv)
 /*
  * vmbus_on_isr - ISR routine
  */
-static int vmbus_on_isr(struct hv_driver *drv)
+static int vmbus_on_isr(struct driver_context *drv)
 {
 	int ret = 0;
 	int cpu = smp_processor_id();
@@ -463,7 +463,7 @@ static ssize_t vmbus_show_device_attr(struct device *dev,
 static int vmbus_bus_init(void)
 {
 	struct vmbus_driver_context *vmbus_drv_ctx = &vmbus_drv;
-	struct hv_driver *driver = &vmbus_drv.drv_obj;
+	struct driver_context *driver = &vmbus_drv.drv_obj;
 	struct hyperv_device *dev = &vmbus_drv.device_obj;
 	int ret;
 	unsigned int vector;
@@ -587,7 +587,7 @@ cleanup:
  */
 static void vmbus_bus_exit(void)
 {
-	struct hv_driver *driver = &vmbus_drv.drv_obj;
+	struct driver_context *driver = &vmbus_drv.drv_obj;
 	struct vmbus_driver_context *vmbus_drv_ctx = &vmbus_drv;
 
 	struct hyperv_device *dev = &vmbus_drv.device_obj;
@@ -857,7 +857,7 @@ static int vmbus_match(struct device *device, struct device_driver *driver)
 		   sizeof(struct hv_guid)) == 0) {
 		/*
 		 * !! NOTE: The driver_ctx is not a vmbus_drv_ctx. We typecast
-		 * it here to access the struct hv_driver field
+		 * it here to access the struct driver_context field
 		 */
 		struct vmbus_driver_context *vmbus_drv_ctx =
 			(struct vmbus_driver_context *)driver_ctx;
@@ -1020,7 +1020,7 @@ static void vmbus_device_release(struct device *device)
  */
 static void vmbus_msg_dpc(unsigned long data)
 {
-	struct hv_driver *driver = (struct hv_driver *)data;
+	struct driver_context *driver = (struct driver_context *)data;
 
 	/* Call to bus driver to handle interrupt */
 	vmbus_on_msg_dpc(driver);
@@ -1037,7 +1037,7 @@ static void vmbus_event_dpc(unsigned long data)
 
 static irqreturn_t vmbus_isr(int irq, void *dev_id)
 {
-	struct hv_driver *driver = &vmbus_drv.drv_obj;
+	struct driver_context *driver = &vmbus_drv.drv_obj;
 	int ret;
 
 	/* Call to bus driver to handle interrupt */
-- 
1.5.5.6

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/6] Staging: hv:  Unify the hyperv driver abstractions
  2011-02-26  2:07 ` K. Y. Srinivasan
  (?)
@ 2011-03-01  2:52 ` Greg KH
  2011-03-02  1:43   ` KY Srinivasan
  -1 siblings, 1 reply; 10+ messages in thread
From: Greg KH @ 2011-03-01  2:52 UTC (permalink / raw)
  To: K. Y. Srinivasan
  Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang, Hank Janssen

On Fri, Feb 25, 2011 at 06:07:03PM -0800, K. Y. Srinivasan wrote:
> This patch combines the two driver abstractions into
> a single driver abstraction.

Ah, how sweet.  Unfortunatly you don't say "how" you did this.

Nor do you describe _what_ those two driver abstractions were.  Are we
talking i2c and usb abstractions?  gpio and spi?  Driver core and
platform?  We want to know exactly what is going on here.

Think of writing something that when you look back, in 3 years, while
staring at a Linux hyperv driver originally written for the 2.6.9
kernel, that somehow never got forward ported and you are tasked with
doing this, that you can just do a simple 'git log drivers/staging/hv/'
and instantly know just from the changelog comments exactly what you
need to do to your driver to clean it up and properly get it to work on
the new 8.2.2 kernel release.

This changelog entry, would require you to go and dig through the guts
of the patch itself, trying to figure out what abstractions you are
talking about, and exactly how they were combined, all the while
wondering _why_ they were combined.

Please, think of your future self, you will thank him in the years to
come by doing this properly.  Not to mention making other's lives easier
if you happen to have escaped this dire task by then.

Oh, you have an extra space up there in the subject, please fix it next
time.

> -int blk_vsc_initialize(struct hv_driver *driver)
> +int blk_vsc_initialize(struct driver_context *driver)

"struct driver_context"?  Oh please no.

I realize that you are hopefully going to later rename this to something
else, but remember, a few patches back you thought that the "ctx" name
wasn't nice.  And here you go resuscitating it from the graveyard of
pointy bits.

And what happens if your future patch is rejected?  You are stuck with a
"driver_context" structure in a subsystem?  That's a pretty big abuse of
the global namespace, don't you think?  It sounds like something that
should go into include/linux/device.h

Please be careful about names, they mean things, even when you think
they don't.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH 4/6] Staging: hv:  Unify the hyperv driver abstractions
  2011-03-01  2:52 ` Greg KH
@ 2011-03-02  1:43   ` KY Srinivasan
  2011-03-02  5:41     ` Greg KH
  0 siblings, 1 reply; 10+ messages in thread
From: KY Srinivasan @ 2011-03-02  1:43 UTC (permalink / raw)
  To: Greg KH
  Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang, Hank Janssen



> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Monday, February 28, 2011 9:53 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang; Hank
> Janssen
> Subject: Re: [PATCH 4/6] Staging: hv: Unify the hyperv driver abstractions
> 
> On Fri, Feb 25, 2011 at 06:07:03PM -0800, K. Y. Srinivasan wrote:
> > This patch combines the two driver abstractions into
> > a single driver abstraction.
> 
> Ah, how sweet.  Unfortunatly you don't say "how" you did this.
> 
> Nor do you describe _what_ those two driver abstractions were.  Are we
> talking i2c and usb abstractions?  gpio and spi?  Driver core and
> platform?  We want to know exactly what is going on here.

My mistake; I will have a more descriptive comment.

> 
> Think of writing something that when you look back, in 3 years, while
> staring at a Linux hyperv driver originally written for the 2.6.9
> kernel, that somehow never got forward ported and you are tasked with
> doing this, that you can just do a simple 'git log drivers/staging/hv/'
> and instantly know just from the changelog comments exactly what you
> need to do to your driver to clean it up and properly get it to work on
> the new 8.2.2 kernel release.
> 
> This changelog entry, would require you to go and dig through the guts
> of the patch itself, trying to figure out what abstractions you are
> talking about, and exactly how they were combined, all the while
> wondering _why_ they were combined.
> 
> Please, think of your future self, you will thank him in the years to
> come by doing this properly.  Not to mention making other's lives easier
> if you happen to have escaped this dire task by then.
> 
> Oh, you have an extra space up there in the subject, please fix it next
> time.
> 
> > -int blk_vsc_initialize(struct hv_driver *driver)
> > +int blk_vsc_initialize(struct driver_context *driver)
> 
> "struct driver_context"?  Oh please no.

Greg; this is the patch that consolidates the state in  struct hv_driver into 
struct driver_context. In the spirit of doing one thing in a patch;
other relevant changes are made in:
Patch[5/6]: Changes the name driver_context to hyperv_driver
Patch[6/6]: Cleanup all variable names that refer to struct hyperv_driver.
 
> 
> I realize that you are hopefully going to later rename this to something
> else, but remember, a few patches back you thought that the "ctx" name
> wasn't nice.  And here you go resuscitating it from the graveyard of
> pointy bits.

As I noted in a different email, may be the granularity I chose in breaking up
these patches is causing all this confusion.
 
> 
> And what happens if your future patch is rejected?  You are stuck with a
> "driver_context" structure in a subsystem?  That's a pretty big abuse of
> the global namespace, don't you think?  It sounds like something that
> should go into include/linux/device.h
> 
> Please be careful about names, they mean things, even when you think
> they don't.

Greg, would it be better if I just had one patch for dealing with all device related issues and a
Separate patch for dealing all driver related issues?

Regards,

K. Y

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/6] Staging: hv:  Unify the hyperv driver abstractions
  2011-03-02  1:43   ` KY Srinivasan
@ 2011-03-02  5:41     ` Greg KH
  2011-03-03  2:50       ` KY Srinivasan
  0 siblings, 1 reply; 10+ messages in thread
From: Greg KH @ 2011-03-02  5:41 UTC (permalink / raw)
  To: KY Srinivasan
  Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang, Hank Janssen

On Wed, Mar 02, 2011 at 01:43:13AM +0000, KY Srinivasan wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:greg@kroah.com]
> > Sent: Monday, February 28, 2011 9:53 PM
> > To: KY Srinivasan
> > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang; Hank
> > Janssen
> > Subject: Re: [PATCH 4/6] Staging: hv: Unify the hyperv driver abstractions
> > 
> > On Fri, Feb 25, 2011 at 06:07:03PM -0800, K. Y. Srinivasan wrote:
> > > This patch combines the two driver abstractions into
> > > a single driver abstraction.
> > 
> > Ah, how sweet.  Unfortunatly you don't say "how" you did this.
> > 
> > Nor do you describe _what_ those two driver abstractions were.  Are we
> > talking i2c and usb abstractions?  gpio and spi?  Driver core and
> > platform?  We want to know exactly what is going on here.
> 
> My mistake; I will have a more descriptive comment.
> 
> > 
> > Think of writing something that when you look back, in 3 years, while
> > staring at a Linux hyperv driver originally written for the 2.6.9
> > kernel, that somehow never got forward ported and you are tasked with
> > doing this, that you can just do a simple 'git log drivers/staging/hv/'
> > and instantly know just from the changelog comments exactly what you
> > need to do to your driver to clean it up and properly get it to work on
> > the new 8.2.2 kernel release.
> > 
> > This changelog entry, would require you to go and dig through the guts
> > of the patch itself, trying to figure out what abstractions you are
> > talking about, and exactly how they were combined, all the while
> > wondering _why_ they were combined.
> > 
> > Please, think of your future self, you will thank him in the years to
> > come by doing this properly.  Not to mention making other's lives easier
> > if you happen to have escaped this dire task by then.
> > 
> > Oh, you have an extra space up there in the subject, please fix it next
> > time.
> > 
> > > -int blk_vsc_initialize(struct hv_driver *driver)
> > > +int blk_vsc_initialize(struct driver_context *driver)
> > 
> > "struct driver_context"?  Oh please no.
> 
> Greg; this is the patch that consolidates the state in  struct hv_driver into 
> struct driver_context. In the spirit of doing one thing in a patch;
> other relevant changes are made in:
> Patch[5/6]: Changes the name driver_context to hyperv_driver
> Patch[6/6]: Cleanup all variable names that refer to struct hyperv_driver.

Yes, but on its own, this patch is wrong, that is not a valid name, even
if it is a "temporary" name.

> > I realize that you are hopefully going to later rename this to something
> > else, but remember, a few patches back you thought that the "ctx" name
> > wasn't nice.  And here you go resuscitating it from the graveyard of
> > pointy bits.
> 
> As I noted in a different email, may be the granularity I chose in breaking up
> these patches is causing all this confusion.

Yes, as I think you need to go much finer as you were doing more than
one thing in these patches, and not describing them properly at all.

Please try to redo them in a simpler manner, probably breaking it into
more steps, so we can properly review them.

> > And what happens if your future patch is rejected?  You are stuck with a
> > "driver_context" structure in a subsystem?  That's a pretty big abuse of
> > the global namespace, don't you think?  It sounds like something that
> > should go into include/linux/device.h
> > 
> > Please be careful about names, they mean things, even when you think
> > they don't.
> 
> Greg, would it be better if I just had one patch for dealing with all
> device related issues and a Separate patch for dealing all driver
> related issues?

Again, only do one thing per patch.  So yes, of course they should be
separate.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH 4/6] Staging: hv:  Unify the hyperv driver abstractions
  2011-03-02  5:41     ` Greg KH
@ 2011-03-03  2:50       ` KY Srinivasan
  2011-03-03  6:10         ` Greg KH
  0 siblings, 1 reply; 10+ messages in thread
From: KY Srinivasan @ 2011-03-03  2:50 UTC (permalink / raw)
  To: Greg KH
  Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang, Hank Janssen



> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Wednesday, March 02, 2011 12:41 AM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang; Hank
> Janssen
> Subject: Re: [PATCH 4/6] Staging: hv: Unify the hyperv driver abstractions
> 
> On Wed, Mar 02, 2011 at 01:43:13AM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:greg@kroah.com]
> > > Sent: Monday, February 28, 2011 9:53 PM
> > > To: KY Srinivasan
> > > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > > devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang;
> Hank
> > > Janssen
> > > Subject: Re: [PATCH 4/6] Staging: hv: Unify the hyperv driver abstractions
> > >
> > > On Fri, Feb 25, 2011 at 06:07:03PM -0800, K. Y. Srinivasan wrote:
> > > > This patch combines the two driver abstractions into
> > > > a single driver abstraction.
> > >
> > > Ah, how sweet.  Unfortunatly you don't say "how" you did this.
> > >
> > > Nor do you describe _what_ those two driver abstractions were.  Are we
> > > talking i2c and usb abstractions?  gpio and spi?  Driver core and
> > > platform?  We want to know exactly what is going on here.
> >
> > My mistake; I will have a more descriptive comment.
> >
> > >
> > > Think of writing something that when you look back, in 3 years, while
> > > staring at a Linux hyperv driver originally written for the 2.6.9
> > > kernel, that somehow never got forward ported and you are tasked with
> > > doing this, that you can just do a simple 'git log drivers/staging/hv/'
> > > and instantly know just from the changelog comments exactly what you
> > > need to do to your driver to clean it up and properly get it to work on
> > > the new 8.2.2 kernel release.
> > >
> > > This changelog entry, would require you to go and dig through the guts
> > > of the patch itself, trying to figure out what abstractions you are
> > > talking about, and exactly how they were combined, all the while
> > > wondering _why_ they were combined.
> > >
> > > Please, think of your future self, you will thank him in the years to
> > > come by doing this properly.  Not to mention making other's lives easier
> > > if you happen to have escaped this dire task by then.
> > >
> > > Oh, you have an extra space up there in the subject, please fix it next
> > > time.
> > >
> > > > -int blk_vsc_initialize(struct hv_driver *driver)
> > > > +int blk_vsc_initialize(struct driver_context *driver)
> > >
> > > "struct driver_context"?  Oh please no.
> >
> > Greg; this is the patch that consolidates the state in  struct hv_driver into
> > struct driver_context. In the spirit of doing one thing in a patch;
> > other relevant changes are made in:
> > Patch[5/6]: Changes the name driver_context to hyperv_driver
> > Patch[6/6]: Cleanup all variable names that refer to struct hyperv_driver.
> 
> Yes, but on its own, this patch is wrong, that is not a valid name, even
> if it is a "temporary" name.

Greg, the temporary name happens to be the name currently in use in the 
code - this is not the name I introduced. Think of this as the surviving 
data structure after  the hv_driver state is consolidated into 
(the existing) driver_context data structure.  I did this in the spirit of 
doing one thing at a time. If I am going to be
picking a more appropriate name for the consolidated data structure; I 
might as well pick the final name that we want this unified driver abstraction 
to be called. 


> 
> > > I realize that you are hopefully going to later rename this to something
> > > else, but remember, a few patches back you thought that the "ctx" name
> > > wasn't nice.  And here you go resuscitating it from the graveyard of
> > > pointy bits.
> >
> > As I noted in a different email, may be the granularity I chose in breaking up
> > these patches is causing all this confusion.
> 
> Yes, as I think you need to go much finer as you were doing more than
> one thing in these patches, and not describing them properly at all.
> 
> Please try to redo them in a simpler manner, probably breaking it into
> more steps, so we can properly review them.

Based on your comments on intermediate names, would you recommend that
as part  of consolidating the driver abstractions, I also rename this combined 
state. 

Regards,

K. Y

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/6] Staging: hv:  Unify the hyperv driver abstractions
  2011-03-03  2:50       ` KY Srinivasan
@ 2011-03-03  6:10         ` Greg KH
  2011-03-03 21:16           ` KY Srinivasan
  0 siblings, 1 reply; 10+ messages in thread
From: Greg KH @ 2011-03-03  6:10 UTC (permalink / raw)
  To: KY Srinivasan
  Cc: Greg KH, linux-kernel, devel, virtualization, Haiyang Zhang,
	Hank Janssen

On Thu, Mar 03, 2011 at 02:50:00AM +0000, KY Srinivasan wrote:
> > > > "struct driver_context"?  Oh please no.
> > >
> > > Greg; this is the patch that consolidates the state in  struct hv_driver into
> > > struct driver_context. In the spirit of doing one thing in a patch;
> > > other relevant changes are made in:
> > > Patch[5/6]: Changes the name driver_context to hyperv_driver
> > > Patch[6/6]: Cleanup all variable names that refer to struct hyperv_driver.
> > 
> > Yes, but on its own, this patch is wrong, that is not a valid name, even
> > if it is a "temporary" name.
> 
> Greg, the temporary name happens to be the name currently in use in the 
> code - this is not the name I introduced.

There is not a "struct driver_context" in the code that I see today, or
am I missing something?  That's my objection here, please don't use that
name, it's not valid for a subsystem to use, even for a tiny bit.

> Think of this as the surviving data structure after  the hv_driver
> state is consolidated into (the existing) driver_context data
> structure.  I did this in the spirit of doing one thing at a time. If
> I am going to be picking a more appropriate name for the consolidated
> data structure; I might as well pick the final name that we want this
> unified driver abstraction to be called. 

Your final name is fine, it's the intermediate one I'm objecting to.

How about 'struct hv_driver_context' instead?

> > > > I realize that you are hopefully going to later rename this to something
> > > > else, but remember, a few patches back you thought that the "ctx" name
> > > > wasn't nice.  And here you go resuscitating it from the graveyard of
> > > > pointy bits.
> > >
> > > As I noted in a different email, may be the granularity I chose in breaking up
> > > these patches is causing all this confusion.
> > 
> > Yes, as I think you need to go much finer as you were doing more than
> > one thing in these patches, and not describing them properly at all.
> > 
> > Please try to redo them in a simpler manner, probably breaking it into
> > more steps, so we can properly review them.
> 
> Based on your comments on intermediate names, would you recommend that
> as part  of consolidating the driver abstractions, I also rename this combined 
> state. 

Probably, if I understand what you are referring to.  Please post code
so that I really know what you are doing :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH 4/6] Staging: hv:  Unify the hyperv driver abstractions
  2011-03-03  6:10         ` Greg KH
@ 2011-03-03 21:16           ` KY Srinivasan
  2011-03-03 21:22             ` Greg KH
  0 siblings, 1 reply; 10+ messages in thread
From: KY Srinivasan @ 2011-03-03 21:16 UTC (permalink / raw)
  To: Greg KH
  Cc: Greg KH, linux-kernel, devel, virtualization, Haiyang Zhang,
	Hank Janssen



> -----Original Message-----
> From: Greg KH [mailto:gregkh@suse.de]
> Sent: Thursday, March 03, 2011 1:10 AM
> To: KY Srinivasan
> Cc: Greg KH; linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> virtualization@lists.osdl.org; Haiyang Zhang; Hank Janssen
> Subject: Re: [PATCH 4/6] Staging: hv: Unify the hyperv driver abstractions
> 
> On Thu, Mar 03, 2011 at 02:50:00AM +0000, KY Srinivasan wrote:
> > > > > "struct driver_context"?  Oh please no.
> > > >
> > > > Greg; this is the patch that consolidates the state in  struct hv_driver into
> > > > struct driver_context. In the spirit of doing one thing in a patch;
> > > > other relevant changes are made in:
> > > > Patch[5/6]: Changes the name driver_context to hyperv_driver
> > > > Patch[6/6]: Cleanup all variable names that refer to struct hyperv_driver.
> > >
> > > Yes, but on its own, this patch is wrong, that is not a valid name, even
> > > if it is a "temporary" name.
> >
> > Greg, the temporary name happens to be the name currently in use in the
> > code - this is not the name I introduced.
> 
> There is not a "struct driver_context" in the code that I see today, or
> am I missing something?  That's my objection here, please don't use that
> name, it's not valid for a subsystem to use, even for a tiny bit.

Look at the file vmbus.h  you will see struct driver_context. This has been there for as long 
as I have seen this code. 

> 
> > Think of this as the surviving data structure after  the hv_driver
> > state is consolidated into (the existing) driver_context data
> > structure.  I did this in the spirit of doing one thing at a time. If
> > I am going to be picking a more appropriate name for the consolidated
> > data structure; I might as well pick the final name that we want this
> > unified driver abstraction to be called.
> 
> Your final name is fine, it's the intermediate one I'm objecting to.

Ok.

> 
> How about 'struct hv_driver_context' instead?
> 
> > > > > I realize that you are hopefully going to later rename this to something
> > > > > else, but remember, a few patches back you thought that the "ctx" name
> > > > > wasn't nice.  And here you go resuscitating it from the graveyard of
> > > > > pointy bits.
> > > >
> > > > As I noted in a different email, may be the granularity I chose in breaking up
> > > > these patches is causing all this confusion.
> > >
> > > Yes, as I think you need to go much finer as you were doing more than
> > > one thing in these patches, and not describing them properly at all.
> > >
> > > Please try to redo them in a simpler manner, probably breaking it into
> > > more steps, so we can properly review them.
> >
> > Based on your comments on intermediate names, would you recommend that
> > as part  of consolidating the driver abstractions, I also rename this combined
> > state.
> 
> Probably, if I understand what you are referring to.  Please post code
> so that I really know what you are doing :)
> 
> thanks,
> 
> greg k-h

Regards,

K. Y

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/6] Staging: hv:  Unify the hyperv driver abstractions
  2011-03-03 21:16           ` KY Srinivasan
@ 2011-03-03 21:22             ` Greg KH
  2011-03-04 15:18               ` KY Srinivasan
  0 siblings, 1 reply; 10+ messages in thread
From: Greg KH @ 2011-03-03 21:22 UTC (permalink / raw)
  To: KY Srinivasan
  Cc: Greg KH, linux-kernel, devel, virtualization, Haiyang Zhang,
	Hank Janssen

On Thu, Mar 03, 2011 at 09:16:29PM +0000, KY Srinivasan wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@suse.de]
> > Sent: Thursday, March 03, 2011 1:10 AM
> > To: KY Srinivasan
> > Cc: Greg KH; linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > virtualization@lists.osdl.org; Haiyang Zhang; Hank Janssen
> > Subject: Re: [PATCH 4/6] Staging: hv: Unify the hyperv driver abstractions
> > 
> > On Thu, Mar 03, 2011 at 02:50:00AM +0000, KY Srinivasan wrote:
> > > > > > "struct driver_context"?  Oh please no.
> > > > >
> > > > > Greg; this is the patch that consolidates the state in  struct hv_driver into
> > > > > struct driver_context. In the spirit of doing one thing in a patch;
> > > > > other relevant changes are made in:
> > > > > Patch[5/6]: Changes the name driver_context to hyperv_driver
> > > > > Patch[6/6]: Cleanup all variable names that refer to struct hyperv_driver.
> > > >
> > > > Yes, but on its own, this patch is wrong, that is not a valid name, even
> > > > if it is a "temporary" name.
> > >
> > > Greg, the temporary name happens to be the name currently in use in the
> > > code - this is not the name I introduced.
> > 
> > There is not a "struct driver_context" in the code that I see today, or
> > am I missing something?  That's my objection here, please don't use that
> > name, it's not valid for a subsystem to use, even for a tiny bit.
> 
> Look at the file vmbus.h  you will see struct driver_context. This has
> been there for as long as I have seen this code. 

Ok, I am rightly corrected, I totally missed that, you are right.

Feel free to resend after addressing the other issues.

I'll fix up the hv_mouse driver, you don't have to worry about that one
if you don't want to, just ignore it please.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH 4/6] Staging: hv:  Unify the hyperv driver abstractions
  2011-03-03 21:22             ` Greg KH
@ 2011-03-04 15:18               ` KY Srinivasan
  0 siblings, 0 replies; 10+ messages in thread
From: KY Srinivasan @ 2011-03-04 15:18 UTC (permalink / raw)
  To: Greg KH
  Cc: Greg KH, linux-kernel, devel, virtualization, Haiyang Zhang,
	Hank Janssen



> -----Original Message-----
> From: Greg KH [mailto:gregkh@suse.de]
> Sent: Thursday, March 03, 2011 4:22 PM
> To: KY Srinivasan
> Cc: Greg KH; linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> virtualization@lists.osdl.org; Haiyang Zhang; Hank Janssen
> Subject: Re: [PATCH 4/6] Staging: hv: Unify the hyperv driver abstractions
> 
> On Thu, Mar 03, 2011 at 09:16:29PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@suse.de]
> > > Sent: Thursday, March 03, 2011 1:10 AM
> > > To: KY Srinivasan
> > > Cc: Greg KH; linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > > virtualization@lists.osdl.org; Haiyang Zhang; Hank Janssen
> > > Subject: Re: [PATCH 4/6] Staging: hv: Unify the hyperv driver abstractions
> > >
> > > On Thu, Mar 03, 2011 at 02:50:00AM +0000, KY Srinivasan wrote:
> > > > > > > "struct driver_context"?  Oh please no.
> > > > > >
> > > > > > Greg; this is the patch that consolidates the state in  struct hv_driver into
> > > > > > struct driver_context. In the spirit of doing one thing in a patch;
> > > > > > other relevant changes are made in:
> > > > > > Patch[5/6]: Changes the name driver_context to hyperv_driver
> > > > > > Patch[6/6]: Cleanup all variable names that refer to struct
> hyperv_driver.
> > > > >
> > > > > Yes, but on its own, this patch is wrong, that is not a valid name, even
> > > > > if it is a "temporary" name.
> > > >
> > > > Greg, the temporary name happens to be the name currently in use in the
> > > > code - this is not the name I introduced.
> > >
> > > There is not a "struct driver_context" in the code that I see today, or
> > > am I missing something?  That's my objection here, please don't use that
> > > name, it's not valid for a subsystem to use, even for a tiny bit.
> >
> > Look at the file vmbus.h  you will see struct driver_context. This has
> > been there for as long as I have seen this code.
> 
> Ok, I am rightly corrected, I totally missed that, you are right.
> 
> Feel free to resend after addressing the other issues.
> 
> I'll fix up the hv_mouse driver, you don't have to worry about that one
> if you don't want to, just ignore it please.

Greg, I am working on a patch-set that hopefully will address all
the concerns that were raised. As part of this effort, I will also
deal with the mouse driver. I should have these patches out next week.
Thanks for your patience here.

Regards,

K. Y 

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2011-03-04 15:18 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-26  2:07 [PATCH 4/6] Staging: hv: Unify the hyperv driver abstractions K. Y. Srinivasan
2011-02-26  2:07 ` K. Y. Srinivasan
2011-03-01  2:52 ` Greg KH
2011-03-02  1:43   ` KY Srinivasan
2011-03-02  5:41     ` Greg KH
2011-03-03  2:50       ` KY Srinivasan
2011-03-03  6:10         ` Greg KH
2011-03-03 21:16           ` KY Srinivasan
2011-03-03 21:22             ` Greg KH
2011-03-04 15:18               ` KY Srinivasan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.