[PATCH] staging: comedi: adl_pci7296: factor out the PCI device code

[PATCH] staging: comedi: adl_pci7296: factor out the PCI device code

[H Hartley Sweeten]

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 <hsweeten@visionengravers.com>
Cc: Ian Abbott <abbotti@mev.co.uk>
Cc: Mori Hess <fmhess@users.sourceforge.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---

diff --git a/drivers/staging/comedi/drivers/adl_pci7296.c b/drivers/staging/comedi/drivers/adl_pci7296.c
index b4dae3b..4df57b1 100644
--- a/drivers/staging/comedi/drivers/adl_pci7296.c
+++ b/drivers/staging/comedi/drivers/adl_pci7296.c
@@ -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)

Re: [PATCH] staging: comedi: adl_pci7296: factor out the PCI device code

[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?

regards,
dan carpenter

RE: [PATCH] staging: comedi: adl_pci7296: factor out the PCI device code

[H Hartley Sweeten]

On Wednesday, May 23, 2012 2:44 PM, Dan Carpenter wrote:
> 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

Re: [PATCH] staging: comedi: adl_pci7296: factor out the PCI device code

[Dan Carpenter]

On Wed, May 23, 2012 at 04:43:34PM -0500, H Hartley Sweeten wrote:
> On Wednesday, May 23, 2012 2:44 PM, Dan Carpenter wrote:
> > 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.

> 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://yourname@repo.or.cz/srv/git/project.git master

regards,
dan carpenter

RE: [PATCH] staging: comedi: adl_pci7296: factor out the PCI device code

[H Hartley Sweeten]

On Wednesday, May 23, 2012 2:44 PM, Dan Carpenter wrote:
> 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

Re: [PATCH] staging: comedi: adl_pci7296: factor out the PCI device code

[Dan Carpenter]

On Wed, May 23, 2012 at 04:52:47PM -0500, H Hartley Sweeten wrote:
> On Wednesday, May 23, 2012 2:44 PM, Dan Carpenter wrote:
> > 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

Re: [PATCH] staging: comedi: adl_pci7296: factor out the PCI device code

[gregkh]

On Thu, May 24, 2012 at 01:07:02AM +0300, Dan Carpenter wrote:
> On Wed, May 23, 2012 at 04:52:47PM -0500, H Hartley Sweeten wrote:
> > On Wednesday, May 23, 2012 2:44 PM, Dan Carpenter wrote:
> > > 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.

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