[PATCH 09/21] staging: wilc1000: #ifdef conditionals cover entire functions

[tony.cho]

On 2015년 07월 30일 05:54, Greg KH wrote:
> On Tue, Jul 28, 2015 at 05:47:28PM +0900, Tony Cho wrote:
>> This patch lets preprocessor conditionals (#ifdef) related to
>> WILC_SDIO_IRQ_GPIO to compile out the entire functions. Compiling out
>> the entire functions is preferred rather than portions of functions or
>> expressions becausue doing so makes code harder to read.
>>
>> Signed-off-by: Tony Cho <tony.cho at atmel.com>
>> Reviewed-by: Dan Carpenter <dan.carpenter at oracle.com>
>> ---
>>   drivers/staging/wilc1000/wilc_sdio.c | 482 +++++++++++++++++++++--------------
>>   1 file changed, 293 insertions(+), 189 deletions(-)
>>
>> diff --git a/drivers/staging/wilc1000/wilc_sdio.c b/drivers/staging/wilc1000/wilc_sdio.c
>> index 8f674ad..83b8da9 100644
>> --- a/drivers/staging/wilc1000/wilc_sdio.c
>> +++ b/drivers/staging/wilc1000/wilc_sdio.c
>> @@ -155,22 +155,9 @@ _fail_:
>>   	return 0;
>>   }
>>   
>> +#ifdef WILC_SDIO_IRQ_GPIO
>>   static int sdio_clear_int(void)
>>   {
>> -#ifndef WILC_SDIO_IRQ_GPIO
>> -	/* uint32_t sts; */
>> -	sdio_cmd52_t cmd;
>> -
>> -	cmd.read_write = 0;
>> -	cmd.function = 1;
>> -	cmd.raw = 0;
>> -	cmd.address = 0x4;
>> -	cmd.data = 0;
>> -	g_sdio.sdio_cmd52(&cmd);
>> -	int_clrd++;
>> -
>> -	return cmd.data;
>> -#else
>>   	uint32_t reg;
>>   
>>   	if (!sdio_read_reg(WILC_HOST_RX_CTRL_0, &reg)) {
>> @@ -181,9 +168,23 @@ static int sdio_clear_int(void)
>>   	sdio_write_reg(WILC_HOST_RX_CTRL_0, reg);
>>   	int_clrd++;
>>   	return 1;
>> -#endif
>> +}
>> +#else
>> +static int sdio_clear_int(void)
>> +{
>> +	sdio_cmd52_t cmd;
>>   
>> +	cmd.read_write = 0;
>> +	cmd.function = 1;
>> +	cmd.raw = 0;
>> +	cmd.address = 0x4;
>> +	cmd.data = 0;
>> +	g_sdio.sdio_cmd52(&cmd);
>> +	int_clrd++;
>> +
>> +	return cmd.data;
>>   }
>> +#endif /* WILC_SDIO_IRQ_GPIO */
> Why does this have to be a config option anyway?  Shouldn't this be
> dynamically determined?  How should someone know which to choose?

I agree with your concern. I will check it again.

>>   uint32_t sdio_xfer_cnt(void)
>>   {
>> @@ -521,14 +522,16 @@ static int sdio_deinit(void *pv)
>>   	return 1;
>>   }
>>   
>> +#ifdef WILC_SDIO_IRQ_GPIO
>>   static int sdio_sync(void)
>>   {
>>   	uint32_t reg;
>> +	int ret;
>>   
>>   	/**
>>   	 *      Disable power sequencer
>>   	 **/
>> -	if (!sdio_read_reg(WILC_MISC, &reg)) {
>> +	if(!sdio_read_reg(WILC_MISC, &reg)) {
> You just reverted this coding style change, why?

No, I just had mistake while rewriting the codes. I will take care more from now on.

>
> I can't take this patch, sorry.  Please sync up and make sure you do not
> revert changes that other people have already made to the code, and
> resend the remaining patches in this series.
>
> thanks,
>
> greg k-h

-- 
Cho, Tony
Manager, Staff Engineer | Connectivity System Software Team | Atmel Korea (Wireless solutions BU)
#409, Kins Tower, Jeongja-Dong, Bundang-Gu, Seongnam-Si, Gyeonggi-Do, 463-782, Korea
Phone:82 31 784 8400(Ext. 317); Mobile: 82 10 7232 1523
email: tony.cho at atmel.com