From mboxrd@z Thu Jan 1 00:00:00 1970 From: Igor Grinberg Subject: Re: [PATCHv2 1/2] ads7846: OMAP3: Removal of warnings backtrace in bootup Date: Thu, 03 Feb 2011 12:17:55 +0200 Message-ID: <4D4A80D3.2060407@compulab.co.il> References: <1296726514-32146-1-git-send-email-sourav.poddar@ti.com> <4D4A7F13.8080502@compulab.co.il> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4D4A7F13.8080502@compulab.co.il> Sender: linux-input-owner@vger.kernel.org To: Sourav Poddar Cc: dmitry.torokhov@gmail.com, balbi@ti.com, Kishon Vijay Abraham I , charu@ti.com, linux-input@vger.kernel.org, linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org, gadiyar@ti.com List-Id: linux-omap@vger.kernel.org Hi again, On 02/03/11 12:10, Igor Grinberg wrote: > Hi, > > On 02/03/11 11:48, Sourav Poddar wrote: > >> gpio_pendown in ads7846_probe is not getting initalized (defaulted to 0) >> resulting in gpio_free to be called without a gpio_request. This >> results in the following backtrace in bootup. >> >> ------------[ cut here ]------------ >> WARNING: at drivers/gpio/gpiolib.c:1258 gpio_free+0x100/0x12c() >> Modules linked in: >> [] (unwind_backtrace+0x0/0xe4) from [] >> (warn_slowpath_common+0x4c/0x64) >> [] (warn_slowpath_common+0x4c/0x64) from [] >> (warn_slowpath_null+0x18/0x1c) >> [] (warn_slowpath_null+0x18/0x1c) from [] >> (gpio_free+0x100/0x12c) >> [] (gpio_free+0x100/0x12c) from [] >> (ads7846_probe+0xa38/0xc5c) >> [] (ads7846_probe+0xa38/0xc5c) from [] >> (spi_drv_probe+0x18/0x1c) >> [] (spi_drv_probe+0x18/0x1c) from [] >> (driver_probe_device+0xc8/0x184) >> [] (driver_probe_device+0xc8/0x184) from [] >> (__driver_attach+0x68/0x8c) >> [] (__driver_attach+0x68/0x8c) from [] >> (bus_for_each_dev+0x48/0x74) >> [] (bus_for_each_dev+0x48/0x74) from [] >> (bus_add_driver+0xa0/0x220) >> [] (bus_add_driver+0xa0/0x220) from [] >> (driver_register+0xa8/0x134) >> [] (driver_register+0xa8/0x134) from [] >> (do_one_initcall+0xcc/0x1a4) >> [] (do_one_initcall+0xcc/0x1a4) from [] >> (kernel_init+0x14c/0x214) >> [] (kernel_init+0x14c/0x214) from [] >> (kernel_thread_exit+0x0/0x8) >> ---[ end trace 4053287f8a5ec18f ]--- >> >> Initializing gpio_pendown in ads7846_probe to -1 before >> ads7846_setup_pendown function removes the above backtrace >> warning. >> >> Signed-off-by: Sourav Poddar >> Signed-off-by: Kishon Vijay Abraham I >> --- >> drivers/input/touchscreen/ads7846.c | 1 + >> 1 files changed, 1 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c >> index 14ea54b..036f245 100644 >> --- a/drivers/input/touchscreen/ads7846.c >> +++ b/drivers/input/touchscreen/ads7846.c >> @@ -1221,6 +1221,7 @@ static int __devinit ads7846_probe(struct spi_device *spi) >> ts->input = input_dev; >> ts->vref_mv = pdata->vref_mv; >> ts->swap_xy = pdata->swap_xy; >> + ts->gpio_pendown = -1; > Wouldn't it be better putting this into ads7846_setup_pendown() function? > This will keep the whole gpio_pendown initialization code close together. > Something like: > > if (pdata->get_pendown_state) { > ts->get_pendown_state = pdata->get_pendown_state; > ts->gpio_pendown = -1; > return 0; > } Sorry for the above links (this is embarrassing, but I've got played by my email client), I meant of course: if (pdata->get_pendown_state) { ts->get_pendown_state = pdata->get_pendown_state; ts->gpio_pendown = -1; return 0; } > > Also, why don't we use -EINVAL for the invalid gpio number instead of -1 constant? > >> >> mutex_init(&ts->lock); >> init_waitqueue_head(&ts->wait); -- Regards, Igor.