All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marcus Folkesson <marcus.folkesson@gmail.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Alexey Khoroshilov <khoroshilov@ispras.ru>,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] input: pxrc - do not store USB device in private struct
Date: Tue, 24 Jul 2018 20:09:59 +0200	[thread overview]
Message-ID: <20180724180959.GA2864@gmail.com> (raw)
In-Reply-To: <20180724023804.6mnbstzdrcp74rev@penguin>

[-- Attachment #1: Type: text/plain, Size: 1919 bytes --]

Hello Dmitry,

On Tue, Jul 24, 2018 at 02:38:04AM +0000, Dmitry Torokhov wrote:
> Hi Marcus,
> 
> On Mon, Jul 16, 2018 at 04:40:14PM +0200, Marcus Folkesson wrote:
> > The USB device is only needed during setup, so put it back after
> > initialization and do not store it in our private struct.
> > 
> > Also, the USB device is a parent of USB interface so our driver
> > model rules ensure that USB device should not disappear while
> > interface device is still there.
> > So not keep a refcount on the device is safe.
> > 
> > Reported-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
> > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> > ---
> >  drivers/input/joystick/pxrc.c | 22 ++++++++++++----------
> >  1 file changed, 12 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/input/joystick/pxrc.c b/drivers/input/joystick/pxrc.c
> > index 07a0dbd3ced2..46a7acb747bf 100644
> > --- a/drivers/input/joystick/pxrc.c
> > +++ b/drivers/input/joystick/pxrc.c
> ...
> 
> > @@ -204,23 +204,25 @@ static int pxrc_probe(struct usb_interface *intf,
> >  		return -ENOMEM;
> >  
> >  	mutex_init(&pxrc->pm_mutex);
> > -	pxrc->udev = usb_get_dev(interface_to_usbdev(intf));
> > +	udev = usb_get_dev(interface_to_usbdev(intf));
> 
> There is really no need to "get" device for the probe duration, or in
> general, when you are not storing the reference to it.
> 
> I posted series with an updated version of this patch plus couple more
> cleanups/fixes, and would appreciate if you could give it a spin.

Thank you for doing this.

I have reviewed the patchset and tested on real hardware, and it looks good
to me.

For what it's worth:

Reviewed-by: Marcus Folkesson <marcus.folkesson@gmail.com>
Tested-by: Marcus Folkesson <marcus.folkesson@gmail.com> 

On the whole patchset.

> 
> Thanks.
> 
> -- 
> Dmitry

Best regards
Marcus Folkesson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2018-07-24 18:09 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-16 14:40 [PATCH] input: pxrc - do not store USB device in private struct Marcus Folkesson
2018-07-24  2:38 ` Dmitry Torokhov
2018-07-24 18:09   ` Marcus Folkesson [this message]
2018-07-25 23:37     ` Dmitry Torokhov

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=20180724180959.GA2864@gmail.com \
    --to=marcus.folkesson@gmail.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=khoroshilov@ispras.ru \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.