Re: [PATCH] Input: drivers/joystick: use parallel port device model

[Sudip Mukherjee]

On Fri, Jul 31, 2015 at 01:43:06PM -0700, Dmitry Torokhov wrote:
> > 
> > Converting to the "new" api is the end goal here, no need to keep the
> > old one around anymore.
> 
> OK, then I guess we can do the conversion right (dropping db9_base
> module-global) and see if anyone screams at us.
Hi Dmitry,
It might become a scream-able condition if we put the pointer to db9 in
the private of pardevice. I was doing par_dev->private = db9;
and while in detach I was doing
struct db9 *db9 = port->physport->devices->private;

Now consider these three situtaions:
1) We have two parallel ports. db9 is attached and registered with
parport0. Now parport1 is removed. So the removal of parport1 will be
announced to all the drivers which are registered with parallel port
bus. And so our detach callback is called with parport1 as the argument.
Now the detach function should check if it is using that port but db9
has the portnumber data and we have stored db9 in the private so we
try to do: struct db9 *db9 = port->physport->devices->private; and BANG
(we have not used parport1).

2) Again we have two parallel ports. we are connected to parport1 and
parport0 is empty. We try to remove db9 module.
parport_unregister_driver() will call the detach callback with all the
ports available with the parallel port bus. So db9_detach() is called
with parport0 and we try to do:
struct db9 *db9 = port->physport->devices->private; and OOPS, no device
is using parport0 and so physport->devices is still NULL.

3) We have one parallel port and lp is already loaded. We try to load
db9 and the driver is loaded but it is not able to register the device.
So we try to rmmod the module and the detach is called with the
parport0. But we still have not registred with parport0 and since we
donot have db9_base with us we have no way of knowing that we are
registered or not.

If you still want to remove db9_base we can do by checking the device
name in the detach function and by testing port->physport->devices for
NULL, but the code will get unnecessary complicated. And these are not
just hypothetical situtation I got them while testing.
I am ready to make the changes, just want your confirmation if it is
worth complicatng the code and taking risk just for removing one global
variable.

regards
sudip
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/