Return-path: Received: from eusmtp01.atmel.com ([212.144.249.243]:13890 "EHLO eusmtp01.atmel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751977AbbLXDM7 (ORCPT ); Wed, 23 Dec 2015 22:12:59 -0500 Message-ID: <567B63A5.4070405@atmel.com> (sfid-20151224_041303_361942_9DFB9816) Date: Thu, 24 Dec 2015 12:16:53 +0900 From: glen lee MIME-Version: 1.0 To: Julian Calaby CC: Greg KH , "devel@driverdev.osuosl.org" , linux-wireless , Tony Cho , , Austin Shin , , , Subject: Re: [PATCH] staging: wilc1000: remove wilc_sdio_init References: <1450838003-24379-1-git-send-email-glen.lee@atmel.com> <567B6140.1090907@atmel.com> In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Sender: linux-wireless-owner@vger.kernel.org List-ID: On 2015년 12월 24일 12:07, Julian Calaby wrote: > 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. Hi julian, Thanks for your advise, I will make two patches to fix this. regards, glen lee. > > Thanks, >