On Mon, Jul 17, 2017 at 04:16:59PM -0400, David Kershner wrote:
> From: Sameer Wadgaonkar <sameer.wadgaon...@unisys.com>
> 
> Added checks for functions probe, remove, pause and resume in
> visorbus_register_visor_driver() and removed these checks from
> the individual functions itself.
> 
> Signed-off-by: Sameer Wadgaonkar <sameer.wadgaon...@unisys.com>
> Signed-off-by: David Kershner <david.kersh...@unisys.com>
> Reviewed-by: David Binder <david.bin...@unisys.com>
> ---
>  drivers/staging/unisys/visorbus/visorbus_main.c | 22 ++++++++++--------
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c 
> b/drivers/staging/unisys/visorbus/visorbus_main.c
> index 57f59cb..4849054 100644
> --- a/drivers/staging/unisys/visorbus/visorbus_main.c
> +++ b/drivers/staging/unisys/visorbus/visorbus_main.c
> @@ -490,8 +490,7 @@ static int visordriver_remove_device(struct device *xdev)
>  
>       mutex_lock(&dev->visordriver_callback_lock);
>       dev->being_removed = true;
> -     if (drv->remove)
> -             drv->remove(dev);
> +     drv->remove(dev);
>       mutex_unlock(&dev->visordriver_callback_lock);
>  
>       dev_stop_periodic_work(dev);
> @@ -868,8 +867,6 @@ static int visordriver_probe_device(struct device *xdev)
>  
>       dev = to_visor_device(xdev);
>       drv = to_visor_driver(xdev->driver);
> -     if (!drv->probe)
> -             return -ENODEV;
>  
>       mutex_lock(&dev->visordriver_callback_lock);
>       dev->being_removed = false;
> @@ -940,6 +937,18 @@ int visorbus_register_visor_driver(struct visor_driver 
> *drv)
>       if (!initialized)
>               return -ENODEV;
>  
> +     if (!drv->probe)
> +             return -ENODEV;
> +
> +     if (!drv->remove)
> +             return -ENODEV;
> +
> +     if (!drv->pause)
> +             return -ENODEV;
> +
> +     if (!drv->resume)
> +             return -ENODEV;

It's not a "no device present" error, it's a "you gave me invalid data"
error, so perhaps -EINVAL instead?

> +
>       drv->driver.name = drv->name;
>       drv->driver.bus = &visorbus_type;
>       drv->driver.probe = visordriver_probe_device;
> @@ -1158,9 +1167,6 @@ static int 
> visorchipset_initiate_device_pause_resume(struct visor_device *dev,
>               return -EBUSY;
>  
>       if (is_pause) {
> -             if (!drv->pause)
> -                     return -EINVAL;

See, you say it there :)

I'll take this, but future change please.

thanks,

greg k-h
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to