|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] Re: [PATCH] xen/pciback: miscellaneous adjustments
On Mon, Sep 19, 2011 at 05:42:24PM +0100, Jan Beulich wrote:
> This is a minor bugfix and a set of small cleanups; as it is not clear
> whether this needs splitting into pieces (and if so, at what
> granularity), I'm submitting this as a single combined patch as a
> first try (I'd hope it doesn't need to be one patch per description
> line below):
> - add a missing return statement to an error path in
> kill_domain_by_device()
> - use pci_is_enabled() rather than raw atomic_read()
> - use resource_size() rather than open coding it
I got a patch for that from
commit b1d39d366a412f634fe50c1db6cfc751829157d3
Author: Thomas Meyer <thomas@xxxxxxxx>
Date: Sat Aug 6 11:05:35 2011 +0200
xen/pciback: use resource_size()
Use resource_size function on resource object
instead of explicit computation.
The semantic patch that makes this output is available
in scripts/coccinelle/api/resource_size.cocci.
More information about semantic patching is available at
http://coccinelle.lip6.fr/
Signed-off-by: Thomas Meyer <thomas@xxxxxxxx>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
diff --git a/drivers/xen/xen-pciback/conf_space_header.c
b/drivers/xen/xen-pciback/conf_space_header.c
index da3cbdf..f14b30f 100644
--- a/drivers/xen/xen-pciback/conf_space_header.c
+++ b/drivers/xen/xen-pciback/conf_space_header.c
@@ -187,7 +187,7 @@ static inline void read_dev_bar(struct pci_dev *dev,
bar_info->val = res[pos].start |
(res[pos].flags & PCI_REGION_FLAG_MASK);
- bar_info->len_val = res[pos].end - res[pos].start + 1;
+ bar_info->len_val = resource_size(&res[pos]);
}
static void *bar_init(struct pci_dev *dev, int offset)
so will use that instead, and drop that section from your patch.
> - remove a bogus attempt to zero-terminate an already zero-terminated
> string
> - #define DRV_NAME once uniformly in the shared local header
> - make DRIVER_ATTR() variables static
> - eliminate a pointless use of list_for_each_entry_safe()
> - add MODULE_ALIAS()
> - a little bit of constification
> - adjust a few messages
> - remove stray semicolons from inline function definitions
Otherwsie they look good to me.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> ---
> drivers/xen/xen-pciback/conf_space.c | 1
> drivers/xen/xen-pciback/conf_space_header.c | 5 +---
> drivers/xen/xen-pciback/conf_space_quirks.c | 3 --
> drivers/xen/xen-pciback/passthrough.c | 2 -
> drivers/xen/xen-pciback/pci_stub.c | 31
> +++++++++++-----------------
> drivers/xen/xen-pciback/pciback.h | 30
> +++++++++++++++++----------
> drivers/xen/xen-pciback/pciback_ops.c | 1
> drivers/xen/xen-pciback/vpci.c | 9 +++-----
> drivers/xen/xen-pciback/xenbus.c | 5 +---
> 9 files changed, 42 insertions(+), 45 deletions(-)
>
> --- 3.1-rc6/drivers/xen/xen-pciback/conf_space.c
> +++ 3.1-rc6-xen-pciback-misc/drivers/xen/xen-pciback/conf_space.c
> @@ -15,7 +15,6 @@
> #include "conf_space.h"
> #include "conf_space_quirks.h"
>
> -#define DRV_NAME "xen-pciback"
> static int permissive;
> module_param(permissive, bool, 0644);
>
> --- 3.1-rc6/drivers/xen/xen-pciback/conf_space_header.c
> +++ 3.1-rc6-xen-pciback-misc/drivers/xen/xen-pciback/conf_space_header.c
> @@ -15,7 +15,6 @@ struct pci_bar_info {
> int which;
> };
>
> -#define DRV_NAME "xen-pciback"
> #define is_enable_cmd(value) ((value)&(PCI_COMMAND_MEMORY|PCI_COMMAND_IO))
> #define is_master_cmd(value) ((value)&PCI_COMMAND_MASTER)
>
> @@ -25,7 +24,7 @@ static int command_read(struct pci_dev *
> int ret;
>
> ret = xen_pcibk_read_config_word(dev, offset, value, data);
> - if (!atomic_read(&dev->enable_cnt))
> + if (!pci_is_enabled(dev))
> return ret;
>
> for (i = 0; i < PCI_ROM_RESOURCE; i++) {
> @@ -187,7 +186,7 @@ static inline void read_dev_bar(struct p
>
> bar_info->val = res[pos].start |
> (res[pos].flags & PCI_REGION_FLAG_MASK);
> - bar_info->len_val = res[pos].end - res[pos].start + 1;
> + bar_info->len_val = resource_size(res + pos);
> }
>
> static void *bar_init(struct pci_dev *dev, int offset)
> --- 3.1-rc6/drivers/xen/xen-pciback/conf_space_quirks.c
> +++ 3.1-rc6-xen-pciback-misc/drivers/xen/xen-pciback/conf_space_quirks.c
> @@ -12,7 +12,6 @@
> #include "conf_space_quirks.h"
>
> LIST_HEAD(xen_pcibk_quirks);
> -#define DRV_NAME "xen-pciback"
> static inline const struct pci_device_id *
> match_one_device(const struct pci_device_id *id, const struct pci_dev *dev)
> {
> @@ -36,7 +35,7 @@ static struct xen_pcibk_config_quirk *xe
> goto out;
> tmp_quirk = NULL;
> printk(KERN_DEBUG DRV_NAME
> - ":quirk didn't match any device xen_pciback knows about\n");
> + ": quirk didn't match any device known\n");
> out:
> return tmp_quirk;
> }
> --- 3.1-rc6/drivers/xen/xen-pciback/passthrough.c
> +++ 3.1-rc6-xen-pciback-misc/drivers/xen/xen-pciback/passthrough.c
> @@ -182,7 +182,7 @@ static int __xen_pcibk_get_pcifront_dev(
> return 1;
> }
>
> -struct xen_pcibk_backend xen_pcibk_passthrough_backend = {
> +const struct xen_pcibk_backend xen_pcibk_passthrough_backend = {
> .name = "passthrough",
> .init = __xen_pcibk_init_devices,
> .free = __xen_pcibk_release_devices,
> --- 3.1-rc6/drivers/xen/xen-pciback/pci_stub.c
> +++ 3.1-rc6-xen-pciback-misc/drivers/xen/xen-pciback/pci_stub.c
> @@ -21,8 +21,6 @@
> #include "conf_space.h"
> #include "conf_space_quirks.h"
>
> -#define DRV_NAME "xen-pciback"
> -
> static char *pci_devs_to_hide;
> wait_queue_head_t xen_pcibk_aer_wait_queue;
> /*Add sem for sync AER handling and xen_pcibk remove/reconfigue ops,
> @@ -514,12 +512,14 @@ static void kill_domain_by_device(struct
> int err;
> char nodename[PCI_NODENAME_MAX];
>
> - if (!psdev)
> + if (!psdev) {
> dev_err(&psdev->dev->dev,
> "device is NULL when do AER recovery/kill_domain\n");
> + return;
> + }
> +
> snprintf(nodename, PCI_NODENAME_MAX, "/local/domain/0/backend/pci/%d/0",
> psdev->pdev->xdev->otherend_id);
> - nodename[strlen(nodename)] = '\0';
>
> again:
> err = xenbus_transaction_start(&xbt);
> @@ -605,7 +605,7 @@ static pci_ers_result_t common_process(s
> if (test_bit(_XEN_PCIF_active,
> (unsigned long *)&psdev->pdev->sh_info->flags)) {
> dev_dbg(&psdev->dev->dev,
> - "schedule pci_conf service in xen_pcibk\n");
> + "schedule pci_conf service in " DRV_NAME "\n");
> xen_pcibk_test_and_schedule_op(psdev->pdev);
> }
>
> @@ -995,8 +995,7 @@ out:
> err = count;
> return err;
> }
> -
> -DRIVER_ATTR(new_slot, S_IWUSR, NULL, pcistub_slot_add);
> +static DRIVER_ATTR(new_slot, S_IWUSR, NULL, pcistub_slot_add);
>
> static ssize_t pcistub_slot_remove(struct device_driver *drv, const char
> *buf,
> size_t count)
> @@ -1015,8 +1014,7 @@ out:
> err = count;
> return err;
> }
> -
> -DRIVER_ATTR(remove_slot, S_IWUSR, NULL, pcistub_slot_remove);
> +static DRIVER_ATTR(remove_slot, S_IWUSR, NULL, pcistub_slot_remove);
>
> static ssize_t pcistub_slot_show(struct device_driver *drv, char *buf)
> {
> @@ -1039,8 +1037,7 @@ static ssize_t pcistub_slot_show(struct
>
> return count;
> }
> -
> -DRIVER_ATTR(slots, S_IRUSR, pcistub_slot_show, NULL);
> +static DRIVER_ATTR(slots, S_IRUSR, pcistub_slot_show, NULL);
>
> static ssize_t pcistub_irq_handler_show(struct device_driver *drv, char *buf)
> {
> @@ -1069,8 +1066,7 @@ static ssize_t pcistub_irq_handler_show(
> spin_unlock_irqrestore(&pcistub_devices_lock, flags);
> return count;
> }
> -
> -DRIVER_ATTR(irq_handlers, S_IRUSR, pcistub_irq_handler_show, NULL);
> +static DRIVER_ATTR(irq_handlers, S_IRUSR, pcistub_irq_handler_show, NULL);
>
> static ssize_t pcistub_irq_handler_switch(struct device_driver *drv,
> const char *buf,
> @@ -1106,7 +1102,7 @@ out:
> err = count;
> return err;
> }
> -DRIVER_ATTR(irq_handler_state, S_IWUSR, NULL, pcistub_irq_handler_switch);
> +static DRIVER_ATTR(irq_handler_state, S_IWUSR, NULL,
> pcistub_irq_handler_switch);
>
> static ssize_t pcistub_quirk_add(struct device_driver *drv, const char *buf,
> size_t count)
> @@ -1170,8 +1166,7 @@ out:
>
> return count;
> }
> -
> -DRIVER_ATTR(quirks, S_IRUSR | S_IWUSR, pcistub_quirk_show,
> pcistub_quirk_add);
> +static DRIVER_ATTR(quirks, S_IRUSR | S_IWUSR, pcistub_quirk_show,
> pcistub_quirk_add);
>
> static ssize_t permissive_add(struct device_driver *drv, const char *buf,
> size_t count)
> @@ -1236,8 +1231,7 @@ static ssize_t permissive_show(struct de
> spin_unlock_irqrestore(&pcistub_devices_lock, flags);
> return count;
> }
> -
> -DRIVER_ATTR(permissive, S_IRUSR | S_IWUSR, permissive_show, permissive_add);
> +static DRIVER_ATTR(permissive, S_IRUSR | S_IWUSR, permissive_show,
> permissive_add);
>
> static void pcistub_exit(void)
> {
> @@ -1374,3 +1368,4 @@ module_init(xen_pcibk_init);
> module_exit(xen_pcibk_cleanup);
>
> MODULE_LICENSE("Dual BSD/GPL");
> +MODULE_ALIAS("xen-backend:pci");
> --- 3.1-rc6/drivers/xen/xen-pciback/pciback.h
> +++ 3.1-rc6-xen-pciback-misc/drivers/xen/xen-pciback/pciback.h
> @@ -15,6 +15,8 @@
> #include <linux/atomic.h>
> #include <xen/interface/io/pciif.h>
>
> +#define DRV_NAME "xen-pciback"
> +
> struct pci_dev_entry {
> struct list_head list;
> struct pci_dev *dev;
> @@ -89,7 +91,7 @@ typedef int (*publish_pci_root_cb) (stru
> * passthrough - BDFs are exactly like in the host.
> */
> struct xen_pcibk_backend {
> - char *name;
> + const char *name;
> int (*init)(struct xen_pcibk_device *pdev);
> void (*free)(struct xen_pcibk_device *pdev);
> int (*find)(struct pci_dev *pcidev, struct xen_pcibk_device *pdev,
> @@ -104,9 +106,9 @@ struct xen_pcibk_backend {
> unsigned int devfn);
> };
>
> -extern struct xen_pcibk_backend xen_pcibk_vpci_backend;
> -extern struct xen_pcibk_backend xen_pcibk_passthrough_backend;
> -extern struct xen_pcibk_backend *xen_pcibk_backend;
> +extern const struct xen_pcibk_backend xen_pcibk_vpci_backend;
> +extern const struct xen_pcibk_backend xen_pcibk_passthrough_backend;
> +extern const struct xen_pcibk_backend *xen_pcibk_backend;
>
> static inline int xen_pcibk_add_pci_dev(struct xen_pcibk_device *pdev,
> struct pci_dev *dev,
> @@ -116,13 +118,14 @@ static inline int xen_pcibk_add_pci_dev(
> if (xen_pcibk_backend && xen_pcibk_backend->add)
> return xen_pcibk_backend->add(pdev, dev, devid, publish_cb);
> return -1;
> -};
> +}
> +
> static inline void xen_pcibk_release_pci_dev(struct xen_pcibk_device *pdev,
> struct pci_dev *dev)
> {
> if (xen_pcibk_backend && xen_pcibk_backend->free)
> return xen_pcibk_backend->release(pdev, dev);
> -};
> +}
>
> static inline struct pci_dev *
> xen_pcibk_get_pci_dev(struct xen_pcibk_device *pdev, unsigned int domain,
> @@ -131,7 +134,8 @@ xen_pcibk_get_pci_dev(struct xen_pcibk_d
> if (xen_pcibk_backend && xen_pcibk_backend->get)
> return xen_pcibk_backend->get(pdev, domain, bus, devfn);
> return NULL;
> -};
> +}
> +
> /**
> * Add for domain0 PCIE-AER handling. Get guest domain/bus/devfn in xen_pcibk
> * before sending aer request to pcifront, so that guest could identify
> @@ -148,25 +152,29 @@ static inline int xen_pcibk_get_pcifront
> return xen_pcibk_backend->find(pcidev, pdev, domain, bus,
> devfn);
> return -1;
> -};
> +}
> +
> static inline int xen_pcibk_init_devices(struct xen_pcibk_device *pdev)
> {
> if (xen_pcibk_backend && xen_pcibk_backend->init)
> return xen_pcibk_backend->init(pdev);
> return -1;
> -};
> +}
> +
> static inline int xen_pcibk_publish_pci_roots(struct xen_pcibk_device *pdev,
> publish_pci_root_cb cb)
> {
> if (xen_pcibk_backend && xen_pcibk_backend->publish)
> return xen_pcibk_backend->publish(pdev, cb);
> return -1;
> -};
> +}
> +
> static inline void xen_pcibk_release_devices(struct xen_pcibk_device *pdev)
> {
> if (xen_pcibk_backend && xen_pcibk_backend->free)
> return xen_pcibk_backend->free(pdev);
> -};
> +}
> +
> /* Handles events from front-end */
> irqreturn_t xen_pcibk_handle_event(int irq, void *dev_id);
> void xen_pcibk_do_op(struct work_struct *data);
> --- 3.1-rc6/drivers/xen/xen-pciback/pciback_ops.c
> +++ 3.1-rc6-xen-pciback-misc/drivers/xen/xen-pciback/pciback_ops.c
> @@ -10,7 +10,6 @@
> #include <linux/sched.h>
> #include "pciback.h"
>
> -#define DRV_NAME "xen-pciback"
> int verbose_request;
> module_param(verbose_request, int, 0644);
>
> --- 3.1-rc6/drivers/xen/xen-pciback/vpci.c
> +++ 3.1-rc6-xen-pciback-misc/drivers/xen/xen-pciback/vpci.c
> @@ -12,7 +12,6 @@
> #include "pciback.h"
>
> #define PCI_SLOT_MAX 32
> -#define DRV_NAME "xen-pciback"
>
> struct vpci_dev_data {
> /* Access to dev_list must be protected by lock */
> @@ -150,9 +149,9 @@ static void __xen_pcibk_release_pci_dev(
> spin_lock_irqsave(&vpci_dev->lock, flags);
>
> for (slot = 0; slot < PCI_SLOT_MAX; slot++) {
> - struct pci_dev_entry *e, *tmp;
> - list_for_each_entry_safe(e, tmp, &vpci_dev->dev_list[slot],
> - list) {
> + struct pci_dev_entry *e;
> +
> + list_for_each_entry(e, &vpci_dev->dev_list[slot], list) {
> if (e->dev == dev) {
> list_del(&e->list);
> found_dev = e->dev;
> @@ -247,7 +246,7 @@ static int __xen_pcibk_get_pcifront_dev(
> return found;
> }
>
> -struct xen_pcibk_backend xen_pcibk_vpci_backend = {
> +const struct xen_pcibk_backend xen_pcibk_vpci_backend = {
> .name = "vpci",
> .init = __xen_pcibk_init_devices,
> .free = __xen_pcibk_release_devices,
> --- 3.1-rc6/drivers/xen/xen-pciback/xenbus.c
> +++ 3.1-rc6-xen-pciback-misc/drivers/xen/xen-pciback/xenbus.c
> @@ -13,7 +13,6 @@
> #include <asm/xen/pci.h>
> #include "pciback.h"
>
> -#define DRV_NAME "xen-pciback"
> #define INVALID_EVTCHN_IRQ (-1)
> struct workqueue_struct *xen_pcibk_wq;
>
> @@ -176,7 +175,7 @@ static int xen_pcibk_attach(struct xen_p
> if (magic == NULL || strcmp(magic, XEN_PCI_MAGIC) != 0) {
> xenbus_dev_fatal(pdev->xdev, -EFAULT,
> "version mismatch (%s/%s) with pcifront - "
> - "halting xen_pcibk",
> + "halting " DRV_NAME,
> magic, XEN_PCI_MAGIC);
> goto out;
> }
> @@ -724,7 +723,7 @@ static struct xenbus_driver xenbus_xen_p
> .otherend_changed = xen_pcibk_frontend_changed,
> };
>
> -struct xen_pcibk_backend *xen_pcibk_backend;
> +const struct xen_pcibk_backend *__read_mostly xen_pcibk_backend;
>
> int __init xen_pcibk_xenbus_register(void)
> {
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |