Re: [PATCH 9/9] staging: comedi: das08: Add attach_pci() hook

[Dan Carpenter <dan.carpenter@xxxxxxxxxx>]

On Wed, May 23, 2012 at 05:50:28PM +0100, Ian Abbott wrote:
> +
> +#if IS_ENABLED(CONFIG_COMEDI_DAS08_PCI)
> +static const struct das08_board_struct
> +*das08_find_pci_board(struct pci_dev *pdev)

Normally the * would go on the first line.

static const struct das08_board_struct *
das08_find_pci_board(struct pci_dev *pdev)

That way before cscope was around you can grep for functions by
doing:  egrep -B1 '^[a-z]+\(' *.c

There are 30 some places in the kernel which put the * on the second
line but they are mostly in ugly subsystems.

> +{
> +	unsigned int i;
> +	for (i = 0; i < ARRAY_SIZE(das08_boards); i++)
> +		if (das08_boards[i].bustype == pci &&
> +	            pdev->device == das08_boards[i].id)
> +			return &das08_boards[i];
> +	return NULL;
> +}
> +#endif
> +
> +#if IS_ENABLED(CONFIG_COMEDI_DAS08_PCI)
> +/* only called in the PCI probe path, via comedi_pci_auto_config() */
> +static int __devinit das08_attach_pci(struct comedi_device *dev,
> +				   struct pci_dev *pdev)
> +{
> +	int ret;

Blank line here.

> +	ret = alloc_private(dev, sizeof(struct das08_private_struct));
> +	if (ret < 0)
> +		return ret;
> +	dev_info(dev->class_dev, "attach pci %s\n", pci_name(pdev));
> +	dev->board_ptr = das08_find_pci_board(pdev);
> +	if (dev->board_ptr == NULL) {
> +		dev_err(dev->class_dev, "BUG! cannot determine board type!\n");
> +		return -EINVAL;
> +	}
> +	return das08_pci_attach_common(dev, pdev);
> +}
> +#endif
> +
> +#if IS_ENABLED(CONFIG_COMEDI_DAS08_PCI)
>  static int das08_find_pci(struct comedi_device *dev, int bus, int slot,
>  			  struct pci_dev **pci_dev_p)
>  {
> @@ -995,19 +1063,12 @@ static int das08_find_pci(struct comedi_device *dev, int bus, int slot,
>  			continue;
>  		if (thisboard->id == PCI_ANY_ID) {
>  			/* wildcard board matches any supported PCI board */
> -			unsigned int i;
> -			for (i = 0; i < ARRAY_SIZE(das08_boards); i++) {
> -				if (das08_boards[i].bustype != pci)
> -					continue;
> -				if (pdev->device == das08_boards[i].id) {
> -					/* replace wildcard board_ptr */
> -					dev->board_ptr = &das08_boards[i];
> -					thisboard = comedi_board(dev);
> -					break;
> -				}
> -			}
> -			if (i == ARRAY_SIZE(das08_boards))
> +			const struct das08_board_struct *foundboard =
> +				das08_find_pci_board(pdev);

You could break this up onto a couple lines:
			const struct das08_board_struct *foundboard;

			foundboard = das08_find_pci_board(pdev);
			if (!foundboard)
				continue;

These are nit picks.  Sorry for that.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel