* [PATCH] staging: wilc1000: remove wilc_sdio_init @ 2015-12-23 2:33 Glen Lee 2015-12-24 2:39 ` Julian Calaby 0 siblings, 1 reply; 6+ messages in thread From: Glen Lee @ 2015-12-23 2:33 UTC (permalink / raw) To: gregkh Cc: devel, linux-wireless, tony.cho, glen.lee, leo.kim, austin.shin, adel.noureldin, adham.abozaeid, Nicolas.FERRE wilc_sdio_init return always 1. It is needless, so just remove it and it's related codes also. Signed-off-by: Glen Lee <glen.lee@atmel.com> --- 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; - } - /** * function 0 csa enable **/ -- 1.9.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: wilc1000: remove wilc_sdio_init 2015-12-23 2:33 [PATCH] staging: wilc1000: remove wilc_sdio_init Glen Lee @ 2015-12-24 2:39 ` Julian Calaby 2015-12-24 3:06 ` glen lee 0 siblings, 1 reply; 6+ messages in thread From: Julian Calaby @ 2015-12-24 2:39 UTC (permalink / raw) To: Glen Lee Cc: Greg KH, devel@driverdev.osuosl.org, linux-wireless, Tony Cho, leo.kim, Austin Shin, adel.noureldin, adham.abozaeid, Nicolas.FERRE Hi Glen, On Wed, Dec 23, 2015 at 1:33 PM, Glen Lee <glen.lee@atmel.com> 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 <glen.lee@atmel.com> > --- > 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. Thanks, -- Julian Calaby Email: julian.calaby@gmail.com Profile: http://www.google.com/profiles/julian.calaby/ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: wilc1000: remove wilc_sdio_init 2015-12-24 2:39 ` Julian Calaby @ 2015-12-24 3:06 ` glen lee 2015-12-24 3:07 ` Julian Calaby 0 siblings, 1 reply; 6+ messages in thread From: glen lee @ 2015-12-24 3:06 UTC (permalink / raw) To: Julian Calaby Cc: Greg KH, devel@driverdev.osuosl.org, linux-wireless, Tony Cho, leo.kim, Austin Shin, adel.noureldin, adham.abozaeid, Nicolas.FERRE On 2015년 12월 24일 11:39, Julian Calaby wrote: > Hi Glen, > > On Wed, Dec 23, 2015 at 1:33 PM, Glen Lee <glen.lee@atmel.com> 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 <glen.lee@atmel.com> >> --- >> 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? regards, glen lee. > > Thanks, > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: wilc1000: remove wilc_sdio_init 2015-12-24 3:06 ` glen lee @ 2015-12-24 3:07 ` Julian Calaby 2015-12-24 3:16 ` glen lee 2015-12-24 6:59 ` Dan Carpenter 0 siblings, 2 replies; 6+ messages in thread From: Julian Calaby @ 2015-12-24 3:07 UTC (permalink / raw) To: glen lee Cc: Greg KH, devel@driverdev.osuosl.org, linux-wireless, Tony Cho, leo.kim, Austin Shin, adel.noureldin, adham.abozaeid, Nicolas.FERRE Hi Glen, On Thu, Dec 24, 2015 at 2:06 PM, glen lee <glen.lee@atmel.com> wrote: > > > On 2015년 12월 24일 11:39, Julian Calaby wrote: >> >> Hi Glen, >> >> On Wed, Dec 23, 2015 at 1:33 PM, Glen Lee <glen.lee@atmel.com> 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 <glen.lee@atmel.com> >>> --- >>> 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/ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: wilc1000: remove wilc_sdio_init 2015-12-24 3:07 ` Julian Calaby @ 2015-12-24 3:16 ` glen lee 2015-12-24 6:59 ` Dan Carpenter 1 sibling, 0 replies; 6+ messages in thread From: glen lee @ 2015-12-24 3:16 UTC (permalink / raw) To: Julian Calaby Cc: Greg KH, devel@driverdev.osuosl.org, linux-wireless, Tony Cho, leo.kim, Austin Shin, adel.noureldin, adham.abozaeid, Nicolas.FERRE On 2015년 12월 24일 12:07, Julian Calaby wrote: > Hi Glen, > > On Thu, Dec 24, 2015 at 2:06 PM, glen lee <glen.lee@atmel.com> wrote: >> >> On 2015년 12월 24일 11:39, Julian Calaby wrote: >>> Hi Glen, >>> >>> On Wed, Dec 23, 2015 at 1:33 PM, Glen Lee <glen.lee@atmel.com> 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 <glen.lee@atmel.com> >>>> --- >>>> 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, > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: wilc1000: remove wilc_sdio_init 2015-12-24 3:07 ` Julian Calaby 2015-12-24 3:16 ` glen lee @ 2015-12-24 6:59 ` Dan Carpenter 1 sibling, 0 replies; 6+ messages in thread From: Dan Carpenter @ 2015-12-24 6:59 UTC (permalink / raw) To: Julian Calaby Cc: glen lee, devel@driverdev.osuosl.org, Austin Shin, Greg KH, linux-wireless, Nicolas.FERRE, adel.noureldin, Tony Cho, leo.kim, adham.abozaeid I kind of feel like we have started to err on the side of breaking things up into too many patches. Linus has said the same thing... This patch is not hard to review, except that the title was bad. Originally it was sent as: [PATCH] staging: wilc1000: fix bug in sdio/spi which is the correct title, but the Fixes tag was in the wrong format so Greg rejected it. Now the Fixes tag is missing entirely which is useless and it is disguised as a cleanup. Cleanup patches should not change run time and fixes should be marked clearly. regards, dan carpenter ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-12-24 6:59 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-12-23 2:33 [PATCH] staging: wilc1000: remove wilc_sdio_init Glen Lee 2015-12-24 2:39 ` Julian Calaby 2015-12-24 3:06 ` glen lee 2015-12-24 3:07 ` Julian Calaby 2015-12-24 3:16 ` glen lee 2015-12-24 6:59 ` Dan Carpenter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).