From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dmitry Torokhov Subject: Re: [PATCHv5] Input: keypad: Add smsc ece1099 keypad driver Date: Mon, 29 Oct 2012 13:45:20 -0700 Message-ID: <20121029204520.GC13256@core.coreip.homeip.net> References: <1351507129-10229-1-git-send-email-sourav.poddar@ti.com> <20121029162045.GK27566@arwen.pp.htv.fi> <20121029170633.GA11558@core.coreip.homeip.net> <20121029190516.GA29230@arwen.pp.htv.fi> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20121029190516.GA29230-S8G//mZuvNWo5Im9Ml3/Zg@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org Sender: "devicetree-discuss" To: Felipe Balbi Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, santosh.shilimkar-l0cyMroinI0@public.gmane.org, linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Sourav Poddar , linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-input@vger.kernel.org On Mon, Oct 29, 2012 at 09:05:53PM +0200, Felipe Balbi wrote: > Hi, > > On Mon, Oct 29, 2012 at 10:06:33AM -0700, Dmitry Torokhov wrote: > > [ big snip ] > > > > > +static int __devexit smsc_remove(struct platform_device *pdev) > > > > +{ > > > > > > shouldn't you unregister the input device here ?? > > > > And that is why I do not like devm_* interface myself... But no, since > > input device was allocated with devm_input_allocate_device() it does not > > need to be explicitly freed or unregistered. > > IMHO, that's a fragility on current devm implementation for input > devices, then. > > devm_input_allocate_device() is *only* allocating the input device (at > least judging by the name). Looks like you should introduce > devm_input_register_device() ? What happens if I > devm_input_allocate_device() but don't go as far as > input_register_device() (some error happens in-between) ? > It will be freed automatically. > I'm sure you have some proper handling for it, but it's quite misleading > the way this was implemented. Well, I could add devm_input_register_device(), devm_input_unregister_device(), devm_input_free_device() and then add checks to "normal" input_register_device(), input_unregister_device() and input_free_device() to throw warnings and errors if they are used with managed resources, and similarly add checks to devm_* API so that they are not called with unmanaged devices and make a big mess out of it. _OR_ I could just add one new call devm_input_allocate_device() to create managed input devices and make the rest of old API work with both managed and unmanaged input devices. I think the latter is much better. Thanks. -- Dmitry From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760593Ab2J2Upc (ORCPT ); Mon, 29 Oct 2012 16:45:32 -0400 Received: from mail-pa0-f46.google.com ([209.85.220.46]:59702 "EHLO mail-pa0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757343Ab2J2Up0 (ORCPT ); Mon, 29 Oct 2012 16:45:26 -0400 Date: Mon, 29 Oct 2012 13:45:20 -0700 From: Dmitry Torokhov To: Felipe Balbi Cc: Sourav Poddar , linux-input@vger.kernel.org, linux-omap@vger.kernel.org, b-cousson@ti.com, santosh.shilimkar@ti.com, devicetree-discuss@lists.ozlabs.org, linux-kernel@vger.kernel.org Subject: Re: [PATCHv5] Input: keypad: Add smsc ece1099 keypad driver Message-ID: <20121029204520.GC13256@core.coreip.homeip.net> References: <1351507129-10229-1-git-send-email-sourav.poddar@ti.com> <20121029162045.GK27566@arwen.pp.htv.fi> <20121029170633.GA11558@core.coreip.homeip.net> <20121029190516.GA29230@arwen.pp.htv.fi> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20121029190516.GA29230@arwen.pp.htv.fi> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Oct 29, 2012 at 09:05:53PM +0200, Felipe Balbi wrote: > Hi, > > On Mon, Oct 29, 2012 at 10:06:33AM -0700, Dmitry Torokhov wrote: > > [ big snip ] > > > > > +static int __devexit smsc_remove(struct platform_device *pdev) > > > > +{ > > > > > > shouldn't you unregister the input device here ?? > > > > And that is why I do not like devm_* interface myself... But no, since > > input device was allocated with devm_input_allocate_device() it does not > > need to be explicitly freed or unregistered. > > IMHO, that's a fragility on current devm implementation for input > devices, then. > > devm_input_allocate_device() is *only* allocating the input device (at > least judging by the name). Looks like you should introduce > devm_input_register_device() ? What happens if I > devm_input_allocate_device() but don't go as far as > input_register_device() (some error happens in-between) ? > It will be freed automatically. > I'm sure you have some proper handling for it, but it's quite misleading > the way this was implemented. Well, I could add devm_input_register_device(), devm_input_unregister_device(), devm_input_free_device() and then add checks to "normal" input_register_device(), input_unregister_device() and input_free_device() to throw warnings and errors if they are used with managed resources, and similarly add checks to devm_* API so that they are not called with unmanaged devices and make a big mess out of it. _OR_ I could just add one new call devm_input_allocate_device() to create managed input devices and make the rest of old API work with both managed and unmanaged input devices. I think the latter is much better. Thanks. -- Dmitry