On Mon, Jul 17, 2017 at 04:16:59PM -0400, David Kershner wrote:
> From: Sameer Wadgaonkar <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: David Kershner <[email protected]>
> Reviewed-by: David Binder <[email protected]>
> ---
> 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
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
