linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Greg KH <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org, linux-input@vger.kernel.org
Subject: Re: [PATCH] Input: drivers/joystick: use parallel port device model
Date: Mon, 3 Aug 2015 20:32:20 +0530	[thread overview]
Message-ID: <20150803150220.GA6221@sudip-pc> (raw)
In-Reply-To: <20150731204306.GM5613@dtor-ws>

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

  parent reply	other threads:[~2015-08-03 15:02 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-30 14:06 [PATCH] Input: drivers/joystick: use parallel port device model Sudip Mukherjee
2015-07-30 16:53 ` Dmitry Torokhov
2015-07-31 10:15   ` Sudip Mukherjee
2015-07-31 16:54     ` Dmitry Torokhov
2015-07-31 17:34       ` Sudip Mukherjee
2015-07-31 20:36         ` Greg KH
2015-07-31 20:43           ` Dmitry Torokhov
2015-08-01 12:43             ` Sudip Mukherjee
2015-08-03 12:26               ` Pali Rohár
2015-08-03 12:36                 ` Sudip Mukherjee
2015-08-03 12:41                   ` Pali Rohár
2015-08-04 18:37                     ` Pali Rohár
2015-08-03 15:02             ` Sudip Mukherjee [this message]
2015-08-04 18:41               ` Dmitry Torokhov
2015-08-05  6:27                 ` Sudip Mukherjee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150803150220.GA6221@sudip-pc \
    --to=sudipm.mukherjee@gmail.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).