staging: comedi: adl_pci7296: factor out the PCI device code

PATCH In review H Hartley Sweeten 3 participants 7 messages May 23, 2012 – May 24, 2012 Staging

Share & tools

H Hartley Sweeten

author

2012-05-23 21:29 UTC #0 lore raw

PATCH 1/1 staging: comedi: adl_pci7296: factor out the PCI device code

Factor out the code that finds a matching PCI device from attach function. This allows reducing the indent level of the remaining code in the attach function.

Signed-off-by H Hartley Sweeten <[email protected]>

Cc Ian Abbott <[email protected]>

Cc Mori Hess <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

---

drivers/staging/comedi/drivers/adl_pci7296.c +56 ~~-53~~

index b4dae3b..4df57b1 100644
@@ -55,19 +55,40 @@ struct adl_pci7296_private {
 
 #define devpriv ((struct adl_pci7296_private *)dev->private)
 
+static struct pci_dev *adl_pci7296_find_pci(struct comedi_device *dev,
+					    struct comedi_devconfig *it)
+{
+	struct pci_dev *pcidev = NULL;
+	int bus = it->options[0];
+	int slot = it->options[1];
+
+	for_each_pci_dev(pcidev) {
+		if (pcidev->vendor != PCI_VENDOR_ID_ADLINK ||
+		    pcidev->device != PCI_DEVICE_ID_PCI7296)
+			continue;
+		if (bus || slot) {
+			/* requested particular bus/slot */
+			if (pcidev->bus->number != bus ||
+			    PCI_SLOT(pcidev->devfn) != slot)
+				continue;
+		}
+		return pcidev;
+	}
+	printk(KERN_ERR
+		"comedi%d: no supported board found! (req. bus/slot : %d/%d)\n",
+	       dev->minor, bus, slot);
+	return NULL;
+}
+
 static int adl_pci7296_attach(struct comedi_device *dev,
 			      struct comedi_devconfig *it)
 {
-	struct pci_dev *pcidev = NULL;
 	struct comedi_subdevice *s;
-	int bus, slot;
 	int ret;
 
 	printk(KERN_INFO "comedi%d: attach adl_pci7432\n", dev->minor);
 
 	dev->board_name = "pci7432";
-	bus = it->options[0];
-	slot = it->options[1];
 
 	if (alloc_private(dev, sizeof(struct adl_pci7296_private)) < 0)
 		return -ENOMEM;

@@ -75,63 +96,45 @@ static int adl_pci7296_attach(struct comedi_device *dev,
 	if (alloc_subdevices(dev, 4) < 0)
 		return -ENOMEM;
 
-	for_each_pci_dev(pcidev) {
-		if (pcidev->vendor == PCI_VENDOR_ID_ADLINK &&
-		    pcidev->device == PCI_DEVICE_ID_PCI7296) {
-			if (bus || slot) {
-				/* requested particular bus/slot */
-				if (pcidev->bus->number != bus
-				    || PCI_SLOT(pcidev->devfn) != slot) {
-					continue;
-				}
-			}
-			devpriv->pci_dev = pcidev;
-			if (comedi_pci_enable(pcidev, "adl_pci7296") < 0) {
-				printk(KERN_ERR "comedi%d: Failed to enable PCI device and request regions\n",
-				     dev->minor);
-				return -EIO;
-			}
+	devpriv->pci_dev = adl_pci7296_find_pci(dev, it);
+	if (!devpriv->pci_dev)
+		return -EIO;
 
-			dev->iobase = pci_resource_start(pcidev, 2);
-			printk(KERN_INFO "comedi: base addr %4lx\n",
-				dev->iobase);
+	if (comedi_pci_enable(devpriv->pci_dev, "adl_pci7296") < 0) {
+		printk(KERN_ERR
+			"comedi%d: Failed to enable PCI device and request regions\n",
+			dev->minor);
+		return -EIO;
+	}
 
-			/*  four 8255 digital io subdevices */
-			s = dev->subdevices + 0;
-			subdev_8255_init(dev, s, NULL,
-					 (unsigned long)(dev->iobase));
+	dev->iobase = pci_resource_start(devpriv->pci_dev, 2);
+	printk(KERN_INFO "comedi: base addr %4lx\n", dev->iobase);
 
-			s = dev->subdevices + 1;
-			ret = subdev_8255_init(dev, s, NULL,
-					       (unsigned long)(dev->iobase +
-							       PORT2A));
-			if (ret < 0)
-				return ret;
+	/*  four 8255 digital io subdevices */
+	s = dev->subdevices + 0;
+	subdev_8255_init(dev, s, NULL, (unsigned long)(dev->iobase));
 
-			s = dev->subdevices + 2;
-			ret = subdev_8255_init(dev, s, NULL,
-					       (unsigned long)(dev->iobase +
-							       PORT3A));
-			if (ret < 0)
-				return ret;
+	s = dev->subdevices + 1;
+	ret = subdev_8255_init(dev, s, NULL,
+				(unsigned long)(dev->iobase + PORT2A));
+	if (ret < 0)
+		return ret;
 
-			s = dev->subdevices + 3;
-			ret = subdev_8255_init(dev, s, NULL,
-					       (unsigned long)(dev->iobase +
-							       PORT4A));
-			if (ret < 0)
-				return ret;
+	s = dev->subdevices + 2;
+	ret = subdev_8255_init(dev, s, NULL,
+				(unsigned long)(dev->iobase + PORT3A));
+	if (ret < 0)
+		return ret;
 
-			printk(KERN_DEBUG "comedi%d: adl_pci7432 attached\n",
-				dev->minor);
+	s = dev->subdevices + 3;
+	ret = subdev_8255_init(dev, s, NULL,
+				(unsigned long)(dev->iobase + PORT4A));
+	if (ret < 0)
+		return ret;
 
-			return 1;
-		}
-	}
+	printk(KERN_DEBUG "comedi%d: adl_pci7432 attached\n", dev->minor);
 
-	printk(KERN_ERR "comedi%d: no supported board found! (req. bus/slot : %d/%d)\n",
-	       dev->minor, bus, slot);
-	return -EIO;
+	return 0;
 }
 
 static void adl_pci7296_detach(struct comedi_device *dev)

Dan Carpenter

2012-05-23 21:40 UTC #1 lore raw

Hartley, you're doing a great job with all these patches, but it's going to be weeks before the merge window closes. We're going to end up getting confused with all the patches floating around.

Do you think you could maybe put up a git tree with the current stuff so people know when they're stepping on each other's toes?

regards,
dan carpenter

H Hartley Sweeten

author

2012-05-23 21:43 UTC #2 lore raw

Quoted 6 lines · Dan Carpenter

Hartley, you're doing a great job with all these patches, but it's going to be weeks before the merge window closes. We're going to end up getting confused with all the patches floating around.
Do you think you could maybe put up a git tree with the current stuff so people know when they're stepping on each other's toes?

I would but really don't know how/were to put one.

Also, I'm still a bit of a newbie with git. I would need some help initially to make sure I didn't screw it up. ;-)

Hartley

Dan Carpenter

2012-05-24 06:28 UTC #3 lore raw

Quoted 10 lines · H Hartley Sweeten

Quoted 6 lines · Dan Carpenter
Hartley, you're doing a great job with all these patches, but it's going to be weeks before the merge window closes. We're going to end up getting confused with all the patches floating around.
Do you think you could maybe put up a git tree with the current stuff so people know when they're stepping on each other's toes?
I would but really don't know how/were to put one.

I can't speak for Greg but probably he'd still end up taking patches from the list, not from the git tree. So that makes it less of a big deal where you put it, since we'd only use it to just track which patches are out there.

So if someone submits a patch to comedi, we'd tell them, "Uh. This conflicts with some previously submitted patches. Please pull from Hartley's tree and modify it so it applies."

There are several free git hosting projects out there. I use
http://repo.or.cz/ for Smatch.

Quoted 2 lines

Also, I'm still a bit of a newbie with git. I would need some help initially to make sure I didn't screw it up. ;-)

Register a project.
Do a fork of http://repo.or.cz/w/linux-2.6.git
Push Greg's stuff.
Go through the email list and download all the comedi patches which Greg doesn't have (raw emails including headers and everything).

cat email.txt | git am -s

If it doesn't apply complain to the submitter. Don't fix anyone's patches or changelogs. Just complain and make them do the work.

Make sure when you apply a patch that the authorship information is preserved.

git push ssh://[email protected]/srv/git/project.git master

regards,
dan carpenter

H Hartley Sweeten

author

2012-05-23 21:52 UTC #4 lore raw

In reply to Dan Carpenter

Quoted 3 lines · Dan Carpenter

Hartley, you're doing a great job with all these patches, but it's going to be weeks before the merge window closes. We're going to end up getting confused with all the patches floating around.

I guess I should wait until the merge window closes and Greg starts taking patches into the staging tree before continuing...

Regards,
Hartley

Dan Carpenter

2012-05-23 22:03 UTC #5 lore raw

Quoted 8 lines · H Hartley Sweeten

Quoted 3 lines · Dan Carpenter
Hartley, you're doing a great job with all these patches, but it's going to be weeks before the merge window closes. We're going to end up getting confused with all the patches floating around.
I guess I should wait until the merge window closes and Greg starts taking patches into the staging tree before continuing...

Wait! No no. The first answer was the right one, where you volunteered to do a bunch of work.

regards,
dan carpenter

[email protected]

maintainer

2012-05-24 02:11 UTC #6 lore raw

Quoted 6 lines · Dan Carpenter

Quoted 5 lines · H Hartley Sweeten
Quoted 3 lines · Dan Carpenter
Hartley, you're doing a great job with all these patches, but it's going to be weeks before the merge window closes. We're going to end up getting confused with all the patches floating around.

Quoted 6 lines

Quoted 3 lines
I guess I should wait until the merge window closes and Greg starts taking patches into the staging tree before continuing...
Wait! No no. The first answer was the right one, where you volunteered to do a bunch of work.

Yes, that's right, don't stop, I can queue these all up in my "to-apply" mailbox just fine and will start applying them when 3.5-rc1 is out, to my staging-next tree. No need for you to take a break at all :)

thanks,

greg k-h