Re: [RFC][patch] sh: maple: Dreamcast mouse driver

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

Hi Adrian,

On Sunday 28 December 2008 15:49:57 Adrian McMenamin wrote:
> +
> +	mdev = to_maple_dev(dev);
> +	mdrv = to_maple_driver(dev->driver);
> +	if (!mdrv||!mdev) {
> +		error = EINVAL;
> +		goto fail;
> +	}

This still does not make sense, mdev and mdrv will never be NULL.

> +fail:
> +	return -error;

You just negated the error returned by input_register_device() and the module 
loading code will think that probe succeeded.

> +
> +static int remove_maple_mouse(struct device *dev)
> +{
> +	struct maple_device *mdev = to_maple_dev(dev);
> +	struct dc_mouse *mse = maple_get_drvdata(mdev);
> +
> +	mdev->callback = NULL;

I am uneasy about just whacking a NULL there. The other driver you sent 
implemented open and close methods and used maple_getcond_callback to 
manipulate callbacks which I assume is safer than just resetting the pointer.

-- 
Dmitry