Return-path: Received: from mail-io0-f181.google.com ([209.85.223.181]:33305 "EHLO mail-io0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752975AbbLXDHu convert rfc822-to-8bit (ORCPT ); Wed, 23 Dec 2015 22:07:50 -0500 Received: by mail-io0-f181.google.com with SMTP id 186so235587219iow.0 for ; Wed, 23 Dec 2015 19:07:50 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <567B6140.1090907@atmel.com> References: <1450838003-24379-1-git-send-email-glen.lee@atmel.com> <567B6140.1090907@atmel.com> From: Julian Calaby Date: Thu, 24 Dec 2015 14:07:30 +1100 Message-ID: (sfid-20151224_040753_866002_64062379) Subject: Re: [PATCH] staging: wilc1000: remove wilc_sdio_init To: glen lee Cc: Greg KH , "devel@driverdev.osuosl.org" , linux-wireless , Tony Cho , leo.kim@atmel.com, Austin Shin , adel.noureldin@atmel.com, adham.abozaeid@atmel.com, Nicolas.FERRE@atmel.com Content-Type: text/plain; charset=UTF-8 Sender: linux-wireless-owner@vger.kernel.org List-ID: Hi Glen, On Thu, Dec 24, 2015 at 2:06 PM, glen lee wrote: > > > On 2015년 12월 24일 11:39, Julian Calaby wrote: >> >> Hi Glen, >> >> On Wed, Dec 23, 2015 at 1:33 PM, Glen Lee wrote: >>> >>> wilc_sdio_init return always 1. It is needless, so just remove it and >>> it's >>> related codes also. >>> >>> Signed-off-by: Glen Lee >>> --- >>> drivers/staging/wilc1000/wilc_sdio.c | 12 ------------ >>> 1 file changed, 12 deletions(-) >>> >>> diff --git a/drivers/staging/wilc1000/wilc_sdio.c >>> b/drivers/staging/wilc1000/wilc_sdio.c >>> index e961b50..caad876 100644 >>> --- a/drivers/staging/wilc1000/wilc_sdio.c >>> +++ b/drivers/staging/wilc1000/wilc_sdio.c >>> @@ -185,11 +185,6 @@ static void wilc_sdio_disable_interrupt(struct wilc >>> *dev) >>> dev_info(&func->dev, "wilc_sdio_disable_interrupt OUT\n"); >>> } >>> >>> -static int wilc_sdio_init(void) >>> -{ >>> - return 1; >>> -} >>> - >>> /******************************************** >>> * >>> * Function 0 >>> @@ -611,13 +606,6 @@ static int sdio_init(struct wilc *wilc) >>> >>> g_sdio.irq_gpio = (wilc->dev_irq_num); >>> >>> - if (!wilc_sdio_init()) { >>> - dev_err(&func->dev, "Failed io init bus...\n"); >>> - return 0; >>> - } else { >>> - return 0; >>> - } >>> - >> >> This isn't equivalent code as both arms of the if statement eventually >> call return 0. > > > Hi julian, > > Yes, you are correct. > Actually, The original code was like this before It is patched wrongly. > - if (!wilc_sdio_init()) { > - dev_err(&func->dev, "Failed io init bus...\n"); > - return 0; > - } > I could fix this first and then remove wilc_sdio_init(). > But I thought that this can be fixed by removing wilc_sdio_init which also > fixes always return 0 error. > > Do you think I should fix "always return 0 error" first and then remove > wilc_sdio_init()? > Or update change log about the error which cause this? It should be two patches, my instinct is to do one which fixes it always returning zero, then another that removes the empty function. Fixing bugs then cleaning up seems more logical to me. Thanks, -- Julian Calaby Email: julian.calaby@gmail.com Profile: http://www.google.com/profiles/julian.calaby/