[PATCH 1/3] Staging: ipack/bridges/tpci200: delete sysfs files

[PATCH 1/3] Staging: ipack/bridges/tpci200: delete sysfs files

[Samuel Iglesias Gonsalvez]

To perform the installation of a mezzanine it was needed to write on these
files, now it is not needed at all as the device model is properly implemented.

Signed-off-by: Samuel Iglesias Gonsalvez <siglesias@igalia.com>
---
 drivers/staging/ipack/bridges/tpci200.c |  225 +++----------------------------
 1 file changed, 15 insertions(+), 210 deletions(-)

diff --git a/drivers/staging/ipack/bridges/tpci200.c b/drivers/staging/ipack/bridges/tpci200.c
index ad28707..c6cc67e 100644
--- a/drivers/staging/ipack/bridges/tpci200.c
+++ b/drivers/staging/ipack/bridges/tpci200.c
@@ -365,207 +365,9 @@ static irqreturn_t tpci200_interrupt(int irq, void *dev_id)
 	return ret;
 }
 
-#ifdef CONFIG_SYSFS
-
-static struct ipack_device *tpci200_slot_register(unsigned int tpci200_number,
-						  unsigned int slot_position)
-{
-	int found = 0;
-	struct ipack_device  *dev;
-	struct tpci200_board *tpci200;
-
-	list_for_each_entry(tpci200, &tpci200_list, list) {
-		if (tpci200->number == tpci200_number) {
-			found = 1;
-			break;
-		}
-	}
-
-	if (!found) {
-		pr_err("carrier board not found for the device\n");
-		return NULL;
-	}
-
-	if (slot_position >= TPCI200_NB_SLOT) {
-		pr_info("Slot [%d:%d] doesn't exist!\n", tpci200_number,
-			slot_position);
-		return NULL;
-	}
-
-	if (mutex_lock_interruptible(&tpci200->mutex))
-		return NULL;
-
-	if (tpci200->slots[slot_position].dev != NULL) {
-		pr_err("Slot [%d:%d] already installed !\n", tpci200_number,
-		       slot_position);
-		goto err_unlock;
-	}
-
-	/*
-	 * Give the same IRQ number as the slot number.
-	 * The TPCI200 has assigned his own two IRQ by PCI bus driver
-	 */
-	dev = ipack_device_register(tpci200->info->ipack_bus,
-				    slot_position, slot_position);
-	if (dev == NULL) {
-		pr_info("Slot [%d:%d] Unable to register an ipack device\n",
-			tpci200_number, slot_position);
-		goto err_unlock;
-	}
-
-	tpci200->slots[slot_position].dev = dev;
-	mutex_unlock(&tpci200->mutex);
-	return dev;
-
-err_unlock:
-	mutex_unlock(&tpci200->mutex);
-	return NULL;
-}
-
-static ssize_t tpci200_store_board(struct device *pdev, const char *buf,
-				   size_t count, int slot)
-{
-	struct tpci200_board *card = dev_get_drvdata(pdev);
-	struct ipack_device *dev = card->slots[slot].dev;
-
-	if (dev != NULL)
-		return -EBUSY;
-
-	dev = tpci200_slot_register(card->number, slot);
-	if (dev == NULL)
-		return -ENODEV;
-
-	return count;
-}
-
-static ssize_t tpci200_show_board(struct device *pdev, char *buf, int slot)
-{
-	struct tpci200_board *card = dev_get_drvdata(pdev);
-	struct ipack_device *dev = card->slots[slot].dev;
-
-	if (dev != NULL)
-		return snprintf(buf, PAGE_SIZE, "%s\n", dev_name(&dev->dev));
-	else
-		return snprintf(buf, PAGE_SIZE, "none\n");
-}
-
-static ssize_t tpci200_show_description(struct device *pdev,
-					struct device_attribute *attr,
-					char *buf)
-{
-	return snprintf(buf, PAGE_SIZE,
-			"TEWS tpci200 carrier PCI for Industry-pack mezzanines.\n");
-}
-
-static ssize_t tpci200_show_board_slot0(struct device *pdev,
-					struct device_attribute *attr,
-					char *buf)
-{
-	return tpci200_show_board(pdev, buf, 0);
-}
-
-static ssize_t tpci200_store_board_slot0(struct device *pdev,
-					 struct device_attribute *attr,
-					 const char *buf, size_t count)
-{
-	return tpci200_store_board(pdev, buf, count, 0);
-}
-
-static ssize_t tpci200_show_board_slot1(struct device *pdev,
-					struct device_attribute *attr,
-					char *buf)
-{
-	return tpci200_show_board(pdev, buf, 1);
-}
-
-static ssize_t tpci200_store_board_slot1(struct device *pdev,
-					 struct device_attribute *attr,
-					 const char *buf, size_t count)
-{
-	return tpci200_store_board(pdev, buf, count, 1);
-}
-
-static ssize_t tpci200_show_board_slot2(struct device *pdev,
-					struct device_attribute *attr,
-					char *buf)
-{
-	return tpci200_show_board(pdev, buf, 2);
-}
-
-static ssize_t tpci200_store_board_slot2(struct device *pdev,
-					 struct device_attribute *attr,
-					 const char *buf, size_t count)
-{
-	return tpci200_store_board(pdev, buf, count, 2);
-}
-
-
-static ssize_t tpci200_show_board_slot3(struct device *pdev,
-					struct device_attribute *attr,
-					char *buf)
-{
-	return tpci200_show_board(pdev, buf, 3);
-}
-
-static ssize_t tpci200_store_board_slot3(struct device *pdev,
-					 struct device_attribute *attr,
-					 const char *buf, size_t count)
-{
-	return tpci200_store_board(pdev, buf, count, 3);
-}
-
-/* Declaration of the device attributes for the TPCI200 */
-static DEVICE_ATTR(description, S_IRUGO,
-		   tpci200_show_description, NULL);
-static DEVICE_ATTR(board_slot0, S_IRUGO | S_IWUSR,
-		   tpci200_show_board_slot0, tpci200_store_board_slot0);
-static DEVICE_ATTR(board_slot1, S_IRUGO | S_IWUSR,
-		   tpci200_show_board_slot1, tpci200_store_board_slot1);
-static DEVICE_ATTR(board_slot2, S_IRUGO | S_IWUSR,
-		   tpci200_show_board_slot2, tpci200_store_board_slot2);
-static DEVICE_ATTR(board_slot3, S_IRUGO | S_IWUSR,
-		   tpci200_show_board_slot3, tpci200_store_board_slot3);
-
-static struct attribute *tpci200_attrs[] = {
-	&dev_attr_description.attr,
-	&dev_attr_board_slot0.attr,
-	&dev_attr_board_slot1.attr,
-	&dev_attr_board_slot2.attr,
-	&dev_attr_board_slot3.attr,
-	NULL,
-};
-
-static struct attribute_group tpci200_attr_group = {
-	.attrs = tpci200_attrs,
-};
-
-static int tpci200_create_sysfs_files(struct tpci200_board *card)
-{
-	return sysfs_create_group(&card->info->pdev->dev.kobj,
-				  &tpci200_attr_group);
-}
-
-static void tpci200_remove_sysfs_files(struct tpci200_board *card)
-{
-	sysfs_remove_group(&card->info->pdev->dev.kobj, &tpci200_attr_group);
-}
-
-#else
-
-static int tpci200_create_sysfs_files(struct tpci200_board *card)
-{
-	return 0;
-}
-
-static void tpci200_remove_sysfs_files(struct tpci200_board *card)
-{
-}
-
-#endif /* CONFIG_SYSFS */
-
 static int tpci200_register(struct tpci200_board *tpci200)
 {
-	int  i;
+	int i;
 	int res;
 	unsigned long ioidint_base;
 	unsigned long mem_base;
@@ -574,12 +376,6 @@ static int tpci200_register(struct tpci200_board *tpci200)
 	if (pci_enable_device(tpci200->info->pdev) < 0)
 		return -ENODEV;
 
-	if (tpci200_create_sysfs_files(tpci200) < 0) {
-		pr_err("failed creating sysfs files\n");
-		res = -EFAULT;
-		goto out_disable_pci;
-	}
-
 	/* Request IP interface register (Bar 2) */
 	res = pci_request_region(tpci200->info->pdev, TPCI200_IP_INTERFACE_BAR,
 				 "Carrier IP interface registers");
@@ -587,7 +383,7 @@ static int tpci200_register(struct tpci200_board *tpci200)
 		pr_err("(bn 0x%X, sn 0x%X) failed to allocate PCI resource for BAR 2 !",
 		       tpci200->info->pdev->bus->number,
 		       tpci200->info->pdev->devfn);
-		goto out_remove_sysfs;
+		goto out_disable_pci;
 	}
 
 	/* Request IO ID INT space (Bar 3) */
@@ -659,6 +455,13 @@ static int tpci200_register(struct tpci200_board *tpci200)
 
 		writew(slot_ctrl, (tpci200->info->interface_regs +
 				   control_reg[i]));
+		/*
+		 * Give the same IRQ number as the slot number.
+		 * The TPCI200 has assigned his own two IRQ by PCI bus driver
+		 */
+		tpci200->slots[i].dev =
+			ipack_device_register(tpci200->info->ipack_bus, i, i);
+
 	}
 
 	res = request_irq(tpci200->info->pdev->irq,
@@ -674,15 +477,18 @@ static int tpci200_register(struct tpci200_board *tpci200)
 
 	return 0;
 
+out_err:
+	for (i = 0; i < TPCI200_NB_SLOT; i++) {
+		ipack_device_unregister(tpci200->slots[i].dev);
+		tpci200->slots[i].dev = NULL;
+	}
+
 out_release_ioid_int_space:
 	pci_release_region(tpci200->info->pdev, TPCI200_IO_ID_INT_SPACES_BAR);
 out_release_ip_space:
 	pci_release_region(tpci200->info->pdev, TPCI200_IP_INTERFACE_BAR);
-out_remove_sysfs:
-	tpci200_remove_sysfs_files(tpci200);
 out_disable_pci:
 	pci_disable_device(tpci200->info->pdev);
-out_err:
 	return res;
 }
 
@@ -1087,7 +893,6 @@ static int tpci200_pciprobe(struct pci_dev *pdev,
 static void __tpci200_pci_remove(struct tpci200_board *tpci200)
 {
 	tpci200_uninstall(tpci200);
-	tpci200_remove_sysfs_files(tpci200);
 	list_del(&tpci200->list);
 	ipack_bus_unregister(tpci200->info->ipack_bus);
 	kfree(tpci200->info);
-- 
1.7.10

[PATCH 2/3] Staging: ipack: fix failure registering an ipack device

[Samuel Iglesias Gonsalvez]

Trying to install an ipack device it always failed in the match() function. This
patch fixes all the bugs present there.

Signed-off-by: Samuel Iglesias Gonsalvez <siglesias@igalia.com>
---
 drivers/staging/ipack/bridges/tpci200.c |   60 +++++++++++--------------------
 drivers/staging/ipack/ipack.c           |    3 +-
 2 files changed, 22 insertions(+), 41 deletions(-)

diff --git a/drivers/staging/ipack/bridges/tpci200.c b/drivers/staging/ipack/bridges/tpci200.c
index c6cc67e..676f338 100644
--- a/drivers/staging/ipack/bridges/tpci200.c
+++ b/drivers/staging/ipack/bridges/tpci200.c
@@ -59,13 +59,6 @@ static struct tpci200_board *check_slot(struct ipack_device *dev)
 		return NULL;
 	}
 
-	BUG_ON(tpci200->slots == NULL);
-	if (tpci200->slots[dev->slot].dev == NULL) {
-		pr_info("Slot [%d:%d] is not registered !\n", dev->bus_nr,
-			dev->slot);
-		return NULL;
-	}
-
 	return tpci200;
 }
 
@@ -455,13 +448,6 @@ static int tpci200_register(struct tpci200_board *tpci200)
 
 		writew(slot_ctrl, (tpci200->info->interface_regs +
 				   control_reg[i]));
-		/*
-		 * Give the same IRQ number as the slot number.
-		 * The TPCI200 has assigned his own two IRQ by PCI bus driver
-		 */
-		tpci200->slots[i].dev =
-			ipack_device_register(tpci200->info->ipack_bus, i, i);
-
 	}
 
 	res = request_irq(tpci200->info->pdev->irq,
@@ -471,18 +457,11 @@ static int tpci200_register(struct tpci200_board *tpci200)
 		pr_err("(bn 0x%X, sn 0x%X) unable to register IRQ !",
 		       tpci200->info->pdev->bus->number,
 		       tpci200->info->pdev->devfn);
-		tpci200_unregister(tpci200);
-		goto out_err;
+		goto out_release_ioid_int_space;
 	}
 
 	return 0;
 
-out_err:
-	for (i = 0; i < TPCI200_NB_SLOT; i++) {
-		ipack_device_unregister(tpci200->slots[i].dev);
-		tpci200->slots[i].dev = NULL;
-	}
-
 out_release_ioid_int_space:
 	pci_release_region(tpci200->info->pdev, TPCI200_IO_ID_INT_SPACES_BAR);
 out_release_ip_space:
@@ -565,20 +544,15 @@ out:
 
 static int tpci200_slot_unmap_space(struct ipack_device *dev, int space)
 {
-	int res;
 	struct ipack_addr_space *virt_addr_space;
 	struct tpci200_board *tpci200;
 
 	tpci200 = check_slot(dev);
-	if (tpci200 == NULL) {
-		res = -EINVAL;
-		goto out;
-	}
+	if (tpci200 == NULL)
+		return -EINVAL;
 
-	if (mutex_lock_interruptible(&tpci200->mutex)) {
-		res = -ERESTARTSYS;
-		goto out;
-	}
+	if (mutex_lock_interruptible(&tpci200->mutex))
+		return -ERESTARTSYS;
 
 	switch (space) {
 	case IPACK_IO_SPACE:
@@ -601,16 +575,15 @@ static int tpci200_slot_unmap_space(struct ipack_device *dev, int space)
 		if (dev->mem_space.address == NULL) {
 			pr_info("Slot [%d:%d] MEM space not mapped !\n",
 				dev->bus_nr, dev->slot);
-		goto out_unlock;
+			goto out_unlock;
 		}
 		virt_addr_space = &dev->mem_space;
 		break;
 	default:
 		pr_err("Slot [%d:%d] space number %d doesn't exist !\n",
 		       dev->bus_nr, dev->slot, space);
-		res = -EINVAL;
-		goto out_unlock;
-		break;
+		mutex_unlock(&tpci200->mutex);
+		return -EINVAL;
 	}
 
 	iounmap(virt_addr_space->address);
@@ -619,8 +592,7 @@ static int tpci200_slot_unmap_space(struct ipack_device *dev, int space)
 	virt_addr_space->size = 0;
 out_unlock:
 	mutex_unlock(&tpci200->mutex);
-out:
-	return res;
+	return 0;
 }
 
 static int tpci200_slot_unregister(struct ipack_device *dev)
@@ -649,7 +621,7 @@ static int tpci200_slot_unregister(struct ipack_device *dev)
 static int tpci200_slot_map_space(struct ipack_device *dev,
 				  unsigned int memory_size, int space)
 {
-	int res;
+	int res = 0;
 	unsigned int size_to_map;
 	void __iomem *phys_address;
 	struct ipack_addr_space *virt_addr_space;
@@ -785,6 +757,8 @@ out:
 static void tpci200_slot_remove(struct tpci200_slot *slot)
 {
 	if ((slot->dev == NULL) ||
+	    (slot->dev->driver == NULL) ||
+	    (slot->dev->driver->ops == NULL) ||
 	    (slot->dev->driver->ops->remove == NULL))
 		return;
 
@@ -844,7 +818,7 @@ out_err:
 static int tpci200_pciprobe(struct pci_dev *pdev,
 			    const struct pci_device_id *id)
 {
-	int ret;
+	int ret, i;
 	struct tpci200_board *tpci200;
 
 	tpci200 = kzalloc(sizeof(struct tpci200_board), GFP_KERNEL);
@@ -887,6 +861,14 @@ static int tpci200_pciprobe(struct pci_dev *pdev,
 	dev_set_drvdata(&pdev->dev, tpci200);
 	/* add the registered device in an internal linked list */
 	list_add_tail(&tpci200->list, &tpci200_list);
+
+	/*
+	 * Give the same IRQ number as the slot number.
+	 * The TPCI200 has assigned his own two IRQ by PCI bus driver
+	 */
+	for (i = 0; i < TPCI200_NB_SLOT; i++)
+		tpci200->slots[i].dev =
+			ipack_device_register(tpci200->info->ipack_bus, i, i);
 	return ret;
 }
 
diff --git a/drivers/staging/ipack/ipack.c b/drivers/staging/ipack/ipack.c
index 2b4fa51..378c3e5 100644
--- a/drivers/staging/ipack/ipack.c
+++ b/drivers/staging/ipack/ipack.c
@@ -48,7 +48,7 @@ static int ipack_bus_match(struct device *device, struct device_driver *driver)
 	if (ret)
 		dev->driver = drv;
 
-	return 0;
+	return ret;
 }
 
 static int ipack_bus_probe(struct device *device)
@@ -172,7 +172,6 @@ struct ipack_device *ipack_device_register(struct ipack_bus_device *bus,
 	ret = device_register(&dev->dev);
 	if (ret < 0) {
 		pr_err("error registering the device.\n");
-		dev->driver->ops->remove(dev);
 		kfree(dev);
 		return NULL;
 	}
-- 
1.7.10

Re: [PATCH 2/3] Staging: ipack: fix failure registering an ipack device

[Dan Carpenter]

On Wed, May 23, 2012 at 11:13:14AM +0200, Samuel Iglesias Gonsalvez wrote:
> Trying to install an ipack device it always failed in the match() function. This
> patch fixes all the bugs present there.
> 

This looks like part of it is undoing stuff introduced in
[patch 1/3]?  I was going to ask what the stuff was in 1/3 and then
it got removed which makes me wonder why even more.  Could you write
something about why in your changelog?  Or if it's not needed then
remove it from the first patch.

Really it would be easier to review if you broke it up into one
patch per bug, with a description of what the bugs were instead of
"patch fixes all the bugs present there".

Some of these changes don't seem necessary.  It feels like you were
debugging and then when you got match() to work you just committed
all the changes instead of only the lines which are needed.

regards,
dan carpenter

Re: [PATCH 2/3] Staging: ipack: fix failure registering an ipack device

[Samuel Iglesias Gonsálvez]

[-- Attachment #1: Type: text/plain, Size: 1062 bytes --]

On Wed, 2012-05-23 at 13:41 +0300, Dan Carpenter wrote:
> On Wed, May 23, 2012 at 11:13:14AM +0200, Samuel Iglesias Gonsalvez wrote:
> > Trying to install an ipack device it always failed in the match() function. This
> > patch fixes all the bugs present there.
> > 
> 
> This looks like part of it is undoing stuff introduced in
> [patch 1/3]?  I was going to ask what the stuff was in 1/3 and then
> it got removed which makes me wonder why even more.  Could you write
> something about why in your changelog?  Or if it's not needed then
> remove it from the first patch.
> 
> Really it would be easier to review if you broke it up into one
> patch per bug, with a description of what the bugs were instead of
> "patch fixes all the bugs present there".
> 
> Some of these changes don't seem necessary.  It feels like you were
> debugging and then when you got match() to work you just committed
> all the changes instead of only the lines which are needed.
> 

Yes, you are right. I will work again on these patches.

Thanks,

Sam


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[PATCH 3/3] Staging: ipack/bridges/tpci200: remove name field from slot_irq

[Samuel Iglesias Gonsalvez]

This field is not needed at all, as the IRQ is registered for the carrier not
for the mezzanine.

Signed-off-by: Samuel Iglesias Gonsalvez <siglesias@igalia.com>
---
 drivers/staging/ipack/bridges/tpci200.c |    1 -
 drivers/staging/ipack/bridges/tpci200.h |    2 --
 2 files changed, 3 deletions(-)

diff --git a/drivers/staging/ipack/bridges/tpci200.c b/drivers/staging/ipack/bridges/tpci200.c
index 676f338..6751625 100644
--- a/drivers/staging/ipack/bridges/tpci200.c
+++ b/drivers/staging/ipack/bridges/tpci200.c
@@ -743,7 +743,6 @@ static int tpci200_request_irq(struct ipack_device *dev, int vector,
 	slot_irq->vector = vector;
 	slot_irq->handler = handler;
 	slot_irq->arg = arg;
-	slot_irq->name = dev_name(&dev->dev);
 
 	tpci200->slots[dev->slot].irq = slot_irq;
 	res = __tpci200_request_irq(tpci200, dev);
diff --git a/drivers/staging/ipack/bridges/tpci200.h b/drivers/staging/ipack/bridges/tpci200.h
index 0b547ee..55f6776 100644
--- a/drivers/staging/ipack/bridges/tpci200.h
+++ b/drivers/staging/ipack/bridges/tpci200.h
@@ -106,14 +106,12 @@
  * @vector	Vector number
  * @handler	Handler called when IRQ arrives
  * @arg		Handler argument
- * @name	IRQ name
  *
  */
 struct slot_irq {
 	int		vector;
 	int		(*handler)(void *);
 	void		*arg;
-	const char	*name;
 };
 
 /**
-- 
1.7.10