Re: [PATCH] staging: wilc1000: remove wilc_sdio_init

[glen lee <glen.lee@atmel.com>]

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,
>