From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755906AbbGFTdN (ORCPT ); Mon, 6 Jul 2015 15:33:13 -0400 Received: from mail-pd0-f170.google.com ([209.85.192.170]:35498 "EHLO mail-pd0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755844AbbGFTdK (ORCPT ); Mon, 6 Jul 2015 15:33:10 -0400 From: Joshua Clayton To: Dmitry Torokhov Cc: linux-input@vger.kernel.org, Mark Brown , linux-kernel@vger.kernel.org Subject: Re: [PATCH] Input: arizona-haptic - convert to use managed input devices Date: Mon, 06 Jul 2015 12:33:06 -0700 Message-ID: <10887748.SfjriXAezU@jclayton-pc> User-Agent: KMail/4.14.6 (Linux/3.19.0-21-generic; KDE/4.14.6; x86_64; ; ) In-Reply-To: <20150706173841.GA31110@dtor-ws> References: <20150706173841.GA31110@dtor-ws> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, It seems to me that swapping error for ret buries the functional changes in the patch... would it be better to split them into a second cleanup patch? On Monday, July 06, 2015 10:38:41 AM Dmitry Torokhov wrote: > Using managed input device (via devm_input_allocate_device) simplifies > error handling and driver removal paths and also silences CID# 712569. > > Signed-off-by: Dmitry Torokhov > --- > drivers/input/misc/arizona-haptics.c | 52 > ++++++++++++------------------------ 1 file changed, 17 insertions(+), 35 > deletions(-) > > diff --git a/drivers/input/misc/arizona-haptics.c > b/drivers/input/misc/arizona-haptics.c index 4dbbed7..54ebef2 100644 > --- a/drivers/input/misc/arizona-haptics.c > +++ b/drivers/input/misc/arizona-haptics.c > @@ -152,7 +152,7 @@ static int arizona_haptics_probe(struct platform_device > *pdev) { > struct arizona *arizona = dev_get_drvdata(pdev->dev.parent); > struct arizona_haptics *haptics; > - int ret; > + int error; > > haptics = devm_kzalloc(&pdev->dev, sizeof(*haptics), GFP_KERNEL); > if (!haptics) > @@ -160,18 +160,18 @@ static int arizona_haptics_probe(struct > platform_device *pdev) > > haptics->arizona = arizona; > > - ret = regmap_update_bits(arizona->regmap, ARIZONA_HAPTICS_CONTROL_1, > - ARIZONA_HAP_ACT, arizona->pdata.hap_act); > - if (ret != 0) { > + error = regmap_update_bits(arizona->regmap, ARIZONA_HAPTICS_CONTROL_1, > + ARIZONA_HAP_ACT, arizona->pdata.hap_act); > + if (error) { > dev_err(arizona->dev, "Failed to set haptics actuator: %d\n", > - ret); > - return ret; > + error); > + return error; > } > > INIT_WORK(&haptics->work, arizona_haptics_work); > > - haptics->input_dev = input_allocate_device(); > - if (haptics->input_dev == NULL) { > + haptics->input_dev = devm_input_allocate_device(&pdev->dev); > + if (!haptics->input_dev) { > dev_err(arizona->dev, "Failed to allocate input device\n"); > return -ENOMEM; > } > @@ -183,46 +183,28 @@ static int arizona_haptics_probe(struct > platform_device *pdev) haptics->input_dev->close = arizona_haptics_close; > __set_bit(FF_RUMBLE, haptics->input_dev->ffbit); > > - ret = input_ff_create_memless(haptics->input_dev, NULL, > - arizona_haptics_play); > - if (ret < 0) { > + error = input_ff_create_memless(haptics->input_dev, NULL, > + arizona_haptics_play); > + if (error) { This looks OK. No change in behavior. Underlying functions return 0 or a negative number. > dev_err(arizona->dev, "input_ff_create_memless() failed: %d\n", > - ret); > - goto err_ialloc; > + error); > + return error; > } > > - ret = (haptics->input_dev); > - if (ret < 0) { > + error = input_register_device(haptics->input_dev); > + if (error) { Same here. ... -- ~Joshua Clayton