Re: [patch 1/2] Staging: vme: remove an unused array

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/07/12 09:15, Dan Carpenter wrote:
> We don't use the "bus" array or the "bus_num" variable which store the
> number of elements in the array.  The only user was removed in
> 5d6abf379d ('staging: vme: make match() driver specific to improve
> non-VME64x support').
> 

Hmm, without a parameter to specify the bus (i.e. a specific vme bridge) to
which vme_user is to bind, vme_user will bind to all bridges it finds, hogging
resources on them all.

Ok, so the current implementation of vme_user can only bind to one bus, so
it's worse than that, it will only every bind to the first bridge when more
than one are available.

It appears that the real problem here is that vme_user_match is wrong.

Rather than:

	static int vme_user_match(struct vme_dev *vdev)
	{
		if (vdev->num >= VME_USER_BUS_MAX)
			return 0;
		return 1;
	}

It should be something like:

	static int vme_user_match(struct vme_dev *vdev)
	{
		int i;

		for (i = 0; i < VME_USER_BUS_MAX; i++)
			if (vdev->num == bus[i])
				return 1;
		return 0;
	}

Martyn

> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> 
> diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c
> index e24a6f9..be198c0 100644
> --- a/drivers/staging/vme/devices/vme_user.c
> +++ b/drivers/staging/vme/devices/vme_user.c
> @@ -43,9 +43,6 @@
>  static DEFINE_MUTEX(vme_user_mutex);
>  static const char driver_name[] = "vme_user";
>  
> -static int bus[VME_USER_BUS_MAX];
> -static unsigned int bus_num;
> -
>  /* Currently Documentation/devices.txt defines the following for VME:
>   *
>   * 221 char	VME bus
> @@ -628,41 +625,15 @@ static struct vme_driver vme_user_driver = {
>  
>  static int __init vme_user_init(void)
>  {
> -	int retval = 0;
> -
>  	printk(KERN_INFO "VME User Space Access Driver\n");
>  
> -	if (bus_num == 0) {
> -		printk(KERN_ERR "%s: No cards, skipping registration\n",
> -			driver_name);
> -		retval = -ENODEV;
> -		goto err_nocard;
> -	}
> -
> -	/* Let's start by supporting one bus, we can support more than one
> -	 * in future revisions if that ever becomes necessary.
> -	 */
> -	if (bus_num > VME_USER_BUS_MAX) {
> -		printk(KERN_ERR "%s: Driver only able to handle %d buses\n",
> -			driver_name, VME_USER_BUS_MAX);
> -		bus_num = VME_USER_BUS_MAX;
> -	}
> -
>  	/*
>  	 * Here we just register the maximum number of devices we can and
>  	 * leave vme_user_match() to allow only 1 to go through to probe().
>  	 * This way, if we later want to allow multiple user access devices,
>  	 * we just change the code in vme_user_match().
>  	 */
> -	retval = vme_register_driver(&vme_user_driver, VME_MAX_SLOTS);
> -	if (retval != 0)
> -		goto err_reg;
> -
> -	return retval;
> -
> -err_reg:
> -err_nocard:
> -	return retval;
> +	return vme_register_driver(&vme_user_driver, VME_MAX_SLOTS);
>  }
>  
>  static int vme_user_match(struct vme_dev *vdev)
> @@ -888,10 +859,6 @@ static void __exit vme_user_exit(void)
>  	vme_unregister_driver(&vme_user_driver);
>  }
>  
> -
> -MODULE_PARM_DESC(bus, "Enumeration of VMEbus to which the driver is connected");
> -module_param_array(bus, int, &bus_num, 0);
> -
>  MODULE_DESCRIPTION("VME User Space Access Driver");
>  MODULE_AUTHOR("Martyn Welch <martyn.welch@xxxxxx");
>  MODULE_LICENSE("GPL");
> 


-- 
Martyn Welch (Lead Software Engineer)  | Registered in England and Wales
GE Intelligent Platforms               | (3828642) at 100 Barbirolli Square
T +44(0)1327322748                     | Manchester, M2 3AB
E martyn.welch@xxxxxx                  | VAT:GB 927559189


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


[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux