All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xen: remove DEFINE_XENBUS_DRIVER() macro
@ 2014-09-08 16:46 David Vrabel
  2014-09-09  9:33 ` Jan Beulich
  0 siblings, 1 reply; 4+ messages in thread
From: David Vrabel @ 2014-09-08 16:46 UTC (permalink / raw)
  To: xen-devel; +Cc: Boris Ostrovsky, David Vrabel

The DEFINE_XENBUS_DRIVER() macro looks a bit weird and causes sparse
errors.

Replace the uses with standard structure definitions instead.  This is
similar to pci and usb device registration.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 drivers/block/xen-blkback/xenbus.c         |   12 ++++--------
 drivers/block/xen-blkfront.c               |    6 ++++--
 drivers/char/tpm/xen-tpmfront.c            |   14 ++++++++------
 drivers/input/misc/xen-kbdfront.c          |    6 ++++--
 drivers/net/xen-netback/xenbus.c           |   11 ++++-------
 drivers/net/xen-netfront.c                 |   17 +++++++++--------
 drivers/pci/xen-pcifront.c                 |    6 ++++--
 drivers/tty/hvc/hvc_xen.c                  |    9 ++++-----
 drivers/video/fbdev/xen-fbfront.c          |    6 ++++--
 drivers/xen/xen-pciback/xenbus.c           |    6 ++++--
 drivers/xen/xenbus/xenbus_probe.c          |    6 +++++-
 drivers/xen/xenbus/xenbus_probe.h          |    4 +++-
 drivers/xen/xenbus/xenbus_probe_backend.c  |    8 +++++---
 drivers/xen/xenbus/xenbus_probe_frontend.c |    8 +++++---
 include/xen/xenbus.h                       |   21 ++++++++++++---------
 15 files changed, 79 insertions(+), 61 deletions(-)

diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index 3a8b810..541679d 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -907,22 +907,18 @@ static int connect_ring(struct backend_info *be)
 	return 0;
 }
 
-
-/* ** Driver Registration ** */
-
-
 static const struct xenbus_device_id xen_blkbk_ids[] = {
 	{ "vbd" },
 	{ "" }
 };
 
-
-static DEFINE_XENBUS_DRIVER(xen_blkbk, ,
+static struct xenbus_driver xen_blkbk_driver = {
+	.name = "vbd",
+	.ids  = xen_blkbk_ids,
 	.probe = xen_blkbk_probe,
 	.remove = xen_blkbk_remove,
 	.otherend_changed = frontend_changed
-);
-
+};
 
 int xen_blkif_xenbus_init(void)
 {
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 5deb235..555a04c 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -2055,13 +2055,15 @@ static const struct xenbus_device_id blkfront_ids[] = {
 	{ "" }
 };
 
-static DEFINE_XENBUS_DRIVER(blkfront, ,
+static struct xenbus_driver blkfront_driver = {
+	.name = "vbd",
+	.ids  = blkfront_ids,
 	.probe = blkfront_probe,
 	.remove = blkfront_remove,
 	.resume = blkfront_resume,
 	.otherend_changed = blkback_changed,
 	.is_ready = blkfront_is_ready,
-);
+};
 
 static int __init xlblk_init(void)
 {
diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
index 2064b45..d71fd50 100644
--- a/drivers/char/tpm/xen-tpmfront.c
+++ b/drivers/char/tpm/xen-tpmfront.c
@@ -367,12 +367,14 @@ static const struct xenbus_device_id tpmfront_ids[] = {
 };
 MODULE_ALIAS("xen:vtpm");
 
-static DEFINE_XENBUS_DRIVER(tpmfront, ,
-		.probe = tpmfront_probe,
-		.remove = tpmfront_remove,
-		.resume = tpmfront_resume,
-		.otherend_changed = backend_changed,
-	);
+static struct xenbus_driver tpmfront_driver = {
+	.name = "vtpm",
+	.ids = tpmfront_ids,
+	.probe = tpmfront_probe,
+	.remove = tpmfront_remove,
+	.resume = tpmfront_resume,
+	.otherend_changed = backend_changed,
+};
 
 static int __init xen_tpmfront_init(void)
 {
diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c
index fbfdc10..fb95bb7 100644
--- a/drivers/input/misc/xen-kbdfront.c
+++ b/drivers/input/misc/xen-kbdfront.c
@@ -365,12 +365,14 @@ static const struct xenbus_device_id xenkbd_ids[] = {
 	{ "" }
 };
 
-static DEFINE_XENBUS_DRIVER(xenkbd, ,
+static struct xenbus_driver xenkbd_driver = {
+	.name = "vkbd",
+	.ids = xenkbd_ids,
 	.probe = xenkbd_probe,
 	.remove = xenkbd_remove,
 	.resume = xenkbd_resume,
 	.otherend_changed = xenkbd_backend_changed,
-);
+};
 
 static int __init xenkbd_init(void)
 {
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index 9c47b89..2494efd 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -937,22 +937,19 @@ static int read_xenbus_vif_flags(struct backend_info *be)
 	return 0;
 }
 
-
-/* ** Driver Registration ** */
-
-
 static const struct xenbus_device_id netback_ids[] = {
 	{ "vif" },
 	{ "" }
 };
 
-
-static DEFINE_XENBUS_DRIVER(netback, ,
+static struct xenbus_driver netback_driver = {
+	.name = "vif",
+	.ids = netback_ids,
 	.probe = netback_probe,
 	.remove = netback_remove,
 	.uevent = netback_uevent,
 	.otherend_changed = frontend_changed,
-);
+};
 
 int xenvif_xenbus_init(void)
 {
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index ca82f54..358dc2c 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -2300,12 +2300,6 @@ static void xennet_sysfs_delif(struct net_device *netdev)
 
 #endif /* CONFIG_SYSFS */
 
-static const struct xenbus_device_id netfront_ids[] = {
-	{ "vif" },
-	{ "" }
-};
-
-
 static int xennet_remove(struct xenbus_device *dev)
 {
 	struct netfront_info *info = dev_get_drvdata(&dev->dev);
@@ -2338,12 +2332,19 @@ static int xennet_remove(struct xenbus_device *dev)
 	return 0;
 }
 
-static DEFINE_XENBUS_DRIVER(netfront, ,
+static const struct xenbus_device_id netfront_ids[] = {
+	{ "vif" },
+	{ "" }
+};
+
+static struct xenbus_driver netfront_driver = {
+	.name = "vif",
+	.ids = netfront_ids,
 	.probe = netfront_probe,
 	.remove = xennet_remove,
 	.resume = netfront_resume,
 	.otherend_changed = netback_changed,
-);
+};
 
 static int __init netif_init(void)
 {
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index 53df39a..116ca37 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -1136,11 +1136,13 @@ static const struct xenbus_device_id xenpci_ids[] = {
 	{""},
 };
 
-static DEFINE_XENBUS_DRIVER(xenpci, "pcifront",
+static struct xenbus_driver xenpci_driver = {
+	.name			= "pcifront",
+	.ids			= xenpci_ids,
 	.probe			= pcifront_xenbus_probe,
 	.remove			= pcifront_xenbus_remove,
 	.otherend_changed	= pcifront_backend_changed,
-);
+};
 
 static int __init pcifront_init(void)
 {
diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index 2dc2831..c3d8af9 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -347,8 +347,6 @@ static int xen_console_remove(struct xencons_info *info)
 }
 
 #ifdef CONFIG_HVC_XEN_FRONTEND
-static struct xenbus_driver xencons_driver;
-
 static int xencons_remove(struct xenbus_device *dev)
 {
 	return xen_console_remove(dev_get_drvdata(&dev->dev));
@@ -502,13 +500,14 @@ static const struct xenbus_device_id xencons_ids[] = {
 	{ "" }
 };
 
-
-static DEFINE_XENBUS_DRIVER(xencons, "xenconsole",
+static struct xenbus_driver xencons_driver = {
+	.name = "xenconsole",
+	.ids = xencons_ids,
 	.probe = xencons_probe,
 	.remove = xencons_remove,
 	.resume = xencons_resume,
 	.otherend_changed = xencons_backend_changed,
-);
+};
 #endif /* CONFIG_HVC_XEN_FRONTEND */
 
 static int __init xen_hvc_init(void)
diff --git a/drivers/video/fbdev/xen-fbfront.c b/drivers/video/fbdev/xen-fbfront.c
index 901014b..e01c129 100644
--- a/drivers/video/fbdev/xen-fbfront.c
+++ b/drivers/video/fbdev/xen-fbfront.c
@@ -684,12 +684,14 @@ static const struct xenbus_device_id xenfb_ids[] = {
 	{ "" }
 };
 
-static DEFINE_XENBUS_DRIVER(xenfb, ,
+static struct xenbus_driver xenfb_driver = {
+	.name = "vfb",
+	.ids = xenfb_ids,
 	.probe = xenfb_probe,
 	.remove = xenfb_remove,
 	.resume = xenfb_resume,
 	.otherend_changed = xenfb_backend_changed,
-);
+};
 
 static int __init xenfb_init(void)
 {
diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c
index c214daa..ad8d30c 100644
--- a/drivers/xen/xen-pciback/xenbus.c
+++ b/drivers/xen/xen-pciback/xenbus.c
@@ -719,11 +719,13 @@ static const struct xenbus_device_id xen_pcibk_ids[] = {
 	{""},
 };
 
-static DEFINE_XENBUS_DRIVER(xen_pcibk, DRV_NAME,
+static struct xenbus_driver xen_pcibk_driver = {
+	.name                   = DRV_NAME,
+	.ids                    = xen_pcibk_ids,
 	.probe			= xen_pcibk_xenbus_probe,
 	.remove			= xen_pcibk_xenbus_remove,
 	.otherend_changed	= xen_pcibk_frontend_changed,
-);
+};
 
 const struct xen_pcibk_backend *__read_mostly xen_pcibk_backend;
 
diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
index 3c0a74b..fcc90cd 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -297,9 +297,13 @@ void xenbus_dev_shutdown(struct device *_dev)
 EXPORT_SYMBOL_GPL(xenbus_dev_shutdown);
 
 int xenbus_register_driver_common(struct xenbus_driver *drv,
-				  struct xen_bus_type *bus)
+				  struct xen_bus_type *bus,
+				  struct module *owner, const char *mod_name)
 {
+	drv->driver.name = drv->name;
 	drv->driver.bus = &bus->bus;
+	drv->driver.owner = owner;
+	drv->driver.mod_name = mod_name;
 
 	return driver_register(&drv->driver);
 }
diff --git a/drivers/xen/xenbus/xenbus_probe.h b/drivers/xen/xenbus/xenbus_probe.h
index 1085ec2..c9ec7ca 100644
--- a/drivers/xen/xenbus/xenbus_probe.h
+++ b/drivers/xen/xenbus/xenbus_probe.h
@@ -60,7 +60,9 @@ extern int xenbus_match(struct device *_dev, struct device_driver *_drv);
 extern int xenbus_dev_probe(struct device *_dev);
 extern int xenbus_dev_remove(struct device *_dev);
 extern int xenbus_register_driver_common(struct xenbus_driver *drv,
-					 struct xen_bus_type *bus);
+					 struct xen_bus_type *bus,
+					 struct module *owner,
+					 const char *mod_name);
 extern int xenbus_probe_node(struct xen_bus_type *bus,
 			     const char *type,
 			     const char *nodename);
diff --git a/drivers/xen/xenbus/xenbus_probe_backend.c b/drivers/xen/xenbus/xenbus_probe_backend.c
index 5125dce..04f7f85 100644
--- a/drivers/xen/xenbus/xenbus_probe_backend.c
+++ b/drivers/xen/xenbus/xenbus_probe_backend.c
@@ -234,13 +234,15 @@ int xenbus_dev_is_online(struct xenbus_device *dev)
 }
 EXPORT_SYMBOL_GPL(xenbus_dev_is_online);
 
-int xenbus_register_backend(struct xenbus_driver *drv)
+int __xenbus_register_backend(struct xenbus_driver *drv, struct module *owner,
+			      const char *mod_name)
 {
 	drv->read_otherend_details = read_frontend_details;
 
-	return xenbus_register_driver_common(drv, &xenbus_backend);
+	return xenbus_register_driver_common(drv, &xenbus_backend,
+					     owner, mod_name);
 }
-EXPORT_SYMBOL_GPL(xenbus_register_backend);
+EXPORT_SYMBOL_GPL(__xenbus_register_backend);
 
 static int backend_probe_and_watch(struct notifier_block *notifier,
 				   unsigned long event,
diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c b/drivers/xen/xenbus/xenbus_probe_frontend.c
index cb385c1..bcb53bd 100644
--- a/drivers/xen/xenbus/xenbus_probe_frontend.c
+++ b/drivers/xen/xenbus/xenbus_probe_frontend.c
@@ -317,13 +317,15 @@ static void wait_for_devices(struct xenbus_driver *xendrv)
 			 print_device_status);
 }
 
-int xenbus_register_frontend(struct xenbus_driver *drv)
+int __xenbus_register_frontend(struct xenbus_driver *drv, struct module *owner,
+			       const char *mod_name)
 {
 	int ret;
 
 	drv->read_otherend_details = read_backend_details;
 
-	ret = xenbus_register_driver_common(drv, &xenbus_frontend);
+	ret = xenbus_register_driver_common(drv, &xenbus_frontend,
+					    owner, mod_name);
 	if (ret)
 		return ret;
 
@@ -332,7 +334,7 @@ int xenbus_register_frontend(struct xenbus_driver *drv)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(xenbus_register_frontend);
+EXPORT_SYMBOL_GPL(__xenbus_register_frontend);
 
 static DECLARE_WAIT_QUEUE_HEAD(backend_state_wq);
 static int backend_state;
diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
index 0324c6d..5553e86 100644
--- a/include/xen/xenbus.h
+++ b/include/xen/xenbus.h
@@ -86,6 +86,7 @@ struct xenbus_device_id
 
 /* A xenbus driver. */
 struct xenbus_driver {
+	const char *name;
 	const struct xenbus_device_id *ids;
 	int (*probe)(struct xenbus_device *dev,
 		     const struct xenbus_device_id *id);
@@ -100,20 +101,22 @@ struct xenbus_driver {
 	int (*is_ready)(struct xenbus_device *dev);
 };
 
-#define DEFINE_XENBUS_DRIVER(var, drvname, methods...)		\
-struct xenbus_driver var ## _driver = {				\
-	.driver.name = drvname + 0 ?: var ## _ids->devicetype,	\
-	.driver.owner = THIS_MODULE,				\
-	.ids = var ## _ids, ## methods				\
-}
-
 static inline struct xenbus_driver *to_xenbus_driver(struct device_driver *drv)
 {
 	return container_of(drv, struct xenbus_driver, driver);
 }
 
-int __must_check xenbus_register_frontend(struct xenbus_driver *);
-int __must_check xenbus_register_backend(struct xenbus_driver *);
+int __must_check __xenbus_register_frontend(struct xenbus_driver *drv,
+					    struct module *owner,
+					    const char *mod_name);
+int __must_check __xenbus_register_backend(struct xenbus_driver *drv,
+					   struct module *owner,
+					   const char *mod_name);
+
+#define xenbus_register_frontend(drv) \
+	__xenbus_register_frontend(drv, THIS_MODULE, KBUILD_MODNAME);
+#define xenbus_register_backend(drv) \
+	__xenbus_register_backend(drv, THIS_MODULE, KBUILD_MODNAME);
 
 void xenbus_unregister_driver(struct xenbus_driver *drv);
 
-- 
1.7.10.4

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

* Re: [PATCH] xen: remove DEFINE_XENBUS_DRIVER() macro
  2014-09-08 16:46 [PATCH] xen: remove DEFINE_XENBUS_DRIVER() macro David Vrabel
@ 2014-09-09  9:33 ` Jan Beulich
  2014-09-09  9:53   ` David Vrabel
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2014-09-09  9:33 UTC (permalink / raw)
  To: David Vrabel; +Cc: xen-devel, Boris Ostrovsky

>>> On 08.09.14 at 18:46, <david.vrabel@citrix.com> wrote:
> The DEFINE_XENBUS_DRIVER() macro looks a bit weird and causes sparse
> errors.
> 
> Replace the uses with standard structure definitions instead.  This is
> similar to pci and usb device registration.
> 
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>

I really regret you doing this, re-adding redundancy the elimination
of which the macro got added for (the diffstat would look even less
favorable if your patch didn't eliminate a couple of bogus comments
and blank lines). I'm certainly not in the position to nack this patch,
but if I was I would.

Jan

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

* Re: [PATCH] xen: remove DEFINE_XENBUS_DRIVER() macro
  2014-09-09  9:33 ` Jan Beulich
@ 2014-09-09  9:53   ` David Vrabel
  2014-09-09 10:15     ` Jan Beulich
  0 siblings, 1 reply; 4+ messages in thread
From: David Vrabel @ 2014-09-09  9:53 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel, Boris Ostrovsky

On 09/09/14 10:33, Jan Beulich wrote:
>>>> On 08.09.14 at 18:46, <david.vrabel@citrix.com> wrote:
>> The DEFINE_XENBUS_DRIVER() macro looks a bit weird and causes sparse
>> errors.
>>
>> Replace the uses with standard structure definitions instead.  This is
>> similar to pci and usb device registration.
>>
>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> 
> I really regret you doing this, re-adding redundancy the elimination
> of which the macro got added for (the diffstat would look even less
> favorable if your patch didn't eliminate a couple of bogus comments
> and blank lines). I'm certainly not in the position to nack this patch,
> but if I was I would.

There didn't seem to be a good reason to require that the driver name
match the first device ID.  What should the driver be named if it was
for two or more different types of Xenbus device? And not all drivers
did this anyway.

However, we could do:

  drv->driver.name = drv->name ? drv->name : drv->ids[0];

What that resolve your concern?

David

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

* Re: [PATCH] xen: remove DEFINE_XENBUS_DRIVER() macro
  2014-09-09  9:53   ` David Vrabel
@ 2014-09-09 10:15     ` Jan Beulich
  0 siblings, 0 replies; 4+ messages in thread
From: Jan Beulich @ 2014-09-09 10:15 UTC (permalink / raw)
  To: David Vrabel; +Cc: xen-devel, Boris Ostrovsky

>>> On 09.09.14 at 11:53, <david.vrabel@citrix.com> wrote:
> On 09/09/14 10:33, Jan Beulich wrote:
>>>>> On 08.09.14 at 18:46, <david.vrabel@citrix.com> wrote:
>>> The DEFINE_XENBUS_DRIVER() macro looks a bit weird and causes sparse
>>> errors.
>>>
>>> Replace the uses with standard structure definitions instead.  This is
>>> similar to pci and usb device registration.
>>>
>>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>> 
>> I really regret you doing this, re-adding redundancy the elimination
>> of which the macro got added for (the diffstat would look even less
>> favorable if your patch didn't eliminate a couple of bogus comments
>> and blank lines). I'm certainly not in the position to nack this patch,
>> but if I was I would.
> 
> There didn't seem to be a good reason to require that the driver name
> match the first device ID.  What should the driver be named if it was
> for two or more different types of Xenbus device?

Is that a reasonable thing to expect?

> And not all drivers did this anyway.
> 
> However, we could do:
> 
>   drv->driver.name = drv->name ? drv->name : drv->ids[0];
> 
> What that resolve your concern?

Only partly - to some degree I also prefer the slightly more terse
code resulting from the macro use. And the original motivation
was anyway to allow reuse of upstream driver code in our forward
ported kernel without needing to patch each individual instance
(iirc largely needed to keep unmodified_drivers/ building, which
would otherwise break due to e.g. new fields in .driver needing
initialization).

Jan

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

end of thread, other threads:[~2014-09-09 10:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-08 16:46 [PATCH] xen: remove DEFINE_XENBUS_DRIVER() macro David Vrabel
2014-09-09  9:33 ` Jan Beulich
2014-09-09  9:53   ` David Vrabel
2014-09-09 10:15     ` Jan Beulich

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.