Merge branch 'fix-review-vic'

kernel/spec-fmc.c

@@ -124,9 +124,12 @@ static void spec_shared_irq_ack(struct fmc_device *fmc);
 static irqreturn_t spec_vic_irq_handler(int id, void *data)
 {
 	struct fmc_device *fmc = (struct fmc_device *)data;
+	struct spec_dev *spec = (struct spec_dev *)fmc->carrier_data;
 	irqreturn_t rv;
 
-	rv = spec_vic_irq_dispatch((struct spec_dev *)fmc->carrier_data);
+	spin_lock(&spec->irq_lock);
+	rv = spec_vic_irq_dispatch(spec);
+	spin_unlock(&spec->irq_lock);
 
 	spec_shared_irq_ack(fmc);
 	return IRQ_HANDLED;
@@ -150,29 +153,33 @@ static int spec_irq_request(struct fmc_device *fmc, irq_handler_t handler,
 			    char *name, int flags)
 {
 	struct spec_dev *spec = fmc->carrier_data;
-	int rv;
+	int rv, first_time;
 
 	/* VIC mode interrupt */
 	if (!(flags & IRQF_SHARED)) {
-		int first_time = !spec->vic;
+		spin_lock(&spec->irq_lock);
+		first_time = !spec->vic;
 
 		/* configure the VIC */
 		rv = spec_vic_irq_request(spec, fmc, fmc->irq, handler);
-
-		if (rv)
+		if (rv) {
+			spin_unlock(&spec->irq_lock);
 			return rv;
+		}
 
 		/* on first IRQ, configure VIC "master" handler and GPIO too */
 		if (first_time) {
 			rv = spec_shared_irq_request(fmc, spec_vic_irq_handler,
 						     "spec-vic", IRQF_SHARED);
-			if (rv)
+			if (rv) {
+				spin_unlock(&spec->irq_lock);
 				return rv;
+			}
 
 			fmc->op->gpio_config(fmc, spec_vic_gpio_cfg,
 					     ARRAY_SIZE(spec_vic_gpio_cfg));
 		}
-
+		spin_unlock(&spec->irq_lock);
 	} else {
 		rv = spec_shared_irq_request(fmc, handler, name, flags);
 		pr_debug("Requesting irq '%s' in shared mode (rv %d)\n", name,
@@ -207,23 +214,30 @@ static void spec_irq_ack(struct fmc_device *fmc)
 	/* Nothing for VIC here, all irqs are acked by master VIC handler */
 }
 
-static int spec_shared_irq_free(struct fmc_device *fmc)
+static void spec_shared_irq_free(struct fmc_device *fmc)
 {
 	struct spec_dev *spec = fmc->carrier_data;
 
 	gennum_writel(spec, 0xffff, GNGPIO_INT_MASK_SET);	/* disable */
 	free_irq(spec->pdev->irq, fmc);
-	return 0;
 }
 
 static int spec_irq_free(struct fmc_device *fmc)
 {
 	struct spec_dev *spec = fmc->carrier_data;
 
+	spin_lock(&spec->irq_lock);
 	if (spec->vic)
-		return spec_vic_irq_free(spec, spec->pdev->irq);
-	else
-		return spec_shared_irq_free(fmc);
+		spec_vic_irq_free(spec, fmc->irq);
+
+	/*
+	 * If we were not using the VIC, or we released all the VIC handler, then
+	 * release the PCI IRQ handler
+	 */
+	if (!spec->vic)
+		spec_shared_irq_free(fmc);
+	spin_unlock(&spec->irq_lock);
+	return 0;
 }
 
 /* This is the mapping from virtual GPIO pin numbers to raw gpio numbers */
@@ -392,6 +406,7 @@ static int spec_irq_init(struct fmc_device *fmc)
 	uint32_t value;
 	int i;
 
+	spin_lock_init(&spec->irq_lock);
 	if (spec_use_msi) {
 		/*
 		 * Enable multiple-msi to work around a chip design bug.
@@ -447,12 +462,7 @@ static void spec_irq_exit(struct fmc_device *fmc)
 		gennum_writel(spec, 0, GNINT_CFG(i));
 	fmc->op->irq_ack(fmc); /* just to be safe */
 
-	/* VIC mode: release VIC resources and disable VIC master IRQ line */
-	if (spec->vic) {
-		spec_vic_cleanup(spec);
-		gennum_writel(spec, 0xffff, GNGPIO_INT_MASK_SET); /* disable */
-		free_irq(spec->pdev->irq, fmc);
-	}
+	WARN(spec->vic, "A Mezzanine driver didn't release all its IRQ handlers\n");
 }
 
 static int check_golden(struct fmc_device *fmc)

kernel/spec-vic.c

@@ -24,6 +24,8 @@
 
 /* A Vectored Interrupt Controller object */
 struct vic_irq_controller {
+	/* It protects the handlers' vector */
+	spinlock_t vec_lock;
 	/* already-initialized flag */
 	int initialized;
 	/* Base address (FPGA-relative) */
@@ -79,6 +81,7 @@ static int spec_vic_init(struct spec_dev *spec, struct fmc_device *fmc)
 	if (!vic)
 		return -ENOMEM;
 
+	spin_lock_init(&vic->vec_lock);
 	vic->kernel_va = spec->remap[0] + vic_base;
 	vic->base = (uint32_t) vic_base;
 
@@ -99,24 +102,26 @@ static int spec_vic_init(struct spec_dev *spec, struct fmc_device *fmc)
 	return 0;
 }
 
-void spec_vic_cleanup(struct spec_dev *spec)
+static void spec_vic_exit(struct vic_irq_controller *vic)
 {
-	if (!spec->vic)
+	if (!vic)
 		return;
 
 	/* Disable all irq lines and the VIC in general */
-	vic_writel(spec->vic, 0xffffffff, VIC_REG_IDR);
-	vic_writel(spec->vic, 0, VIC_REG_CTL);
-	kfree(spec->vic);
-	spec->vic = NULL;
+	vic_writel(vic, 0xffffffff, VIC_REG_IDR);
+	vic_writel(vic, 0, VIC_REG_CTL);
+	kfree(vic);
 }
 
+/* NOTE: this function must be called while holding irq_lock */
 irqreturn_t spec_vic_irq_dispatch(struct spec_dev *spec)
 {
 	struct vic_irq_controller *vic = spec->vic;
 	int index, rv;
 	struct vector *vec;
 
+	if (unlikely(!vic))
+		goto fail;
 	/*
 	 * Our parent IRQ handler: read the index value
 	 * from the Vector Address Register, and find matching handler
@@ -139,6 +144,7 @@ fail:
 	return 0;
 }
 
+/* NOTE: this function must be called while holding irq_lock */
 int spec_vic_irq_request(struct spec_dev *spec, struct fmc_device *fmc,
 			 unsigned long id, irq_handler_t handler)
 {
@@ -157,41 +163,59 @@ int spec_vic_irq_request(struct spec_dev *spec, struct fmc_device *fmc,
 	for (i = 0; i < VIC_MAX_VECTORS; i++) {
 		/* find vector in stored table, assign handle, enable */
 		if (vic->vectors[i].saved_id == id) {
-			spin_lock(&spec->irq_lock);
+			spin_lock(&spec->vic->vec_lock);
 
 			vic_writel(vic, i, VIC_IVT_RAM_BASE + 4 * i);
 			vic->vectors[i].requestor = fmc;
 			vic->vectors[i].handler = handler;
 			vic_writel(vic, (1 << i), VIC_REG_IER);
 
-			spin_unlock(&spec->irq_lock);
+			spin_unlock(&spec->vic->vec_lock);
 			return 0;
 		}
 	}
 
-
 	return -EINVAL;
+}
+
 
+/*
+ * vic_handler_count
+ * It counts how many handlers are registered within the VIC controller
+ */
+static inline int vic_handler_count(struct vic_irq_controller *vic)
+{
+	int i, count;
+
+	for (i = 0, count = 0; i < VIC_MAX_VECTORS; ++i)
+		if (vic->vectors[i].handler)
+			count++;
+
+	return count;
 }
 
-int spec_vic_irq_free(struct spec_dev *spec, unsigned long id)
+/* NOTE: this function must be called while holding irq_lock */
+void spec_vic_irq_free(struct spec_dev *spec, unsigned long id)
 {
 	int i;
 
 	for (i = 0; i < VIC_MAX_VECTORS; i++) {
-		uint32_t vec = spec->vic->vectors[i].saved_id;
-		if (vec == id) {
-			spin_lock(&spec->irq_lock);
+		if (spec->vic->vectors[i].saved_id == id) {
+			spin_lock(&spec->vic->vec_lock);
 
 			vic_writel(spec->vic, 1 << i, VIC_REG_IDR);
-			vic_writel(spec->vic, vec, VIC_IVT_RAM_BASE + 4 * i);
+			vic_writel(spec->vic, id, VIC_IVT_RAM_BASE + 4 * i);
 			spec->vic->vectors[i].handler = NULL;
 
-			spin_unlock(&spec->irq_lock);
+			spin_unlock(&spec->vic->vec_lock);
 		}
 	}
 
-	return 0;
+	/* Clean up the VIC if there are no more handlers */
+	if (!vic_handler_count(spec->vic)) {
+		spec_vic_exit(spec->vic);
+		spec->vic = NULL;
+	}
 }
 
 void spec_vic_irq_ack(struct spec_dev *spec, unsigned long id)

kernel/spec.h

@@ -144,11 +144,11 @@ extern int spec_eeprom_write(struct fmc_device *fmc, uint32_t offset,
 extern int spec_gpio_init(struct fmc_device *fmc);
 extern void spec_gpio_exit(struct fmc_device *fmc);
 
+/* Functions in spec-vic.c */
+/* NOTE: these functions must be called while holding irq_lock */
 int spec_vic_irq_request(struct spec_dev *spec, struct fmc_device *fmc,
 			 unsigned long id, irq_handler_t handler);
-
-int spec_vic_irq_free(struct spec_dev *spec, unsigned long id);
+void spec_vic_irq_free(struct spec_dev *spec, unsigned long id);
 irqreturn_t spec_vic_irq_dispatch(struct spec_dev *spec);
-void spec_vic_cleanup(struct spec_dev *spec);
 
 #endif /* __SPEC_H__ */