On Tue, Feb 15, 2011 at 07:15:37AM -0800, K. Y. Srinivasan wrote:
>
> Signed-off-by: K. Y. Srinivasan <kys@xxxxxxxxxxxxx>
> Signed-off-by: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>
> Signed-off-by: Hank Janssen <hjanssen@xxxxxxxxxxxxx>
>
> ---
> drivers/staging/hv/vmbus_drv.c | 49 +++++++++++++++++++++++++++------------
> 1 files changed, 34 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
> index 459c707..f279e6a 100644
> --- a/drivers/staging/hv/vmbus_drv.c
> +++ b/drivers/staging/hv/vmbus_drv.c
> @@ -36,9 +36,7 @@
> #include "vmbus_private.h"
>
>
> -/* FIXME! We need to do this dynamically for PIC and APIC system */
> -#define VMBUS_IRQ 0x5
> -#define VMBUS_IRQ_VECTOR IRQ5_VECTOR
> +static int vmbus_irq;
>
> /* Main vmbus driver data structure */
> struct vmbus_driver_context {
> @@ -57,6 +55,25 @@ struct vmbus_driver_context {
> struct vm_device device_ctx;
> };
>
> +/*
> + * Find an un-used IRQ that the VMBUS can use. If none is available; return -1.
> + */
> +
> +static int vmbus_get_irq(void)
No extra blank line between function comment and function.
> +{
> + unsigned int avail_irq_mask;
> + int irq = -1;
> + /*
Put an extra blank line after your variables please.
> + * Pick the first unused interrupt. HyperV can
> + * interrupt us on any interrupt line we specify.
> + */
> + avail_irq_mask = probe_irq_on();
> + if (avail_irq_mask != 0)
> + irq = ffs(avail_irq_mask);
> + probe_irq_off(avail_irq_mask);
> + return irq;
> +}
> +
> static int vmbus_match(struct device *device, struct device_driver *driver);
> static int vmbus_probe(struct device *device);
> static int vmbus_remove(struct device *device);
> @@ -80,7 +97,6 @@ EXPORT_SYMBOL(vmbus_loglevel);
> /* (ALL_MODULES << 16 | DEBUG_LVL_ENTEREXIT); */
> /* (((VMBUS | VMBUS_DRV)<<16) | DEBUG_LVL_ENTEREXIT); */
>
> -static int vmbus_irq = VMBUS_IRQ;
>
> /* Set up per device attributes in /sys/bus/vmbus/devices/<bus device> */
> static struct device_attribute vmbus_device_attrs[] = {
> @@ -466,7 +482,7 @@ static int vmbus_bus_init(void)
> struct hv_driver *driver = &vmbus_drv.drv_obj;
> struct vm_device *dev_ctx = &vmbus_drv.device_ctx;
> int ret;
> - unsigned int vector;
> + unsigned int vector, prev_irq = ~0;
>
> DPRINT_INFO(VMBUS, "+++++++ HV Driver version = %s +++++++",
> HV_DRV_VERSION);
> @@ -518,19 +534,23 @@ static int vmbus_bus_init(void)
> }
>
> /* Get the interrupt resource */
> - ret = request_irq(vmbus_irq, vmbus_isr, IRQF_SAMPLE_RANDOM,
> - driver->name, NULL);
> -
> - if (ret != 0) {
> - DPRINT_ERR(VMBUS_DRV, "ERROR - Unable to request IRQ %d",
> - vmbus_irq);
> -
> +get_irq_again:
> + vmbus_irq = vmbus_get_irq();
> + if ((vmbus_irq < 0) || (prev_irq == vmbus_irq)) {
> + printk(KERN_WARNING "VMBUS_DRV: Failed to allocate IRQ\n");
> bus_unregister(&vmbus_drv_ctx->bus);
> -
> ret = -1;
Please provide a "real" error code.
> goto cleanup;
> }
> - vector = VMBUS_IRQ_VECTOR;
> + prev_irq = vmbus_irq;
> +
> + ret = request_irq(vmbus_irq, vmbus_isr, IRQF_SAMPLE_RANDOM,
> + driver->name, NULL);
> +
> + if (ret != 0)
> + goto get_irq_again;
You just set up the potential for an endless loop. You almost _never_
want to goto backwards in a function. Please don't do this.
> +
> + vector = IRQ0_VECTOR + vmbus_irq;
What's this for?
> DPRINT_INFO(VMBUS_DRV, "irq 0x%x vector 0x%x", vmbus_irq, vector);
And why are you still printing this out for the whole world to see?
thanks,
greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
