diff mbox

[V2,1/5] staging: wilc1000: #ifdef conditionals cover entire functions

Message ID 20150730115647.GA12823@sudip-pc (mailing list archive)
State Not Applicable
Delegated to: Kalle Valo
Headers show

Commit Message

Sudip Mukherjee July 30, 2015, 11:56 a.m. UTC 
On Thu, Jul 30, 2015 at 06:10:10PM +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@atmel.com>
> ---
<snip>
>  
> +#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 */
instead of changing #ifndef to #ifdef i think the following would have
been easier:


>  
>  uint32_t sdio_xfer_cnt(void)
<snip>
> +#ifdef WILC_SDIO_IRQ_GPIO
>  static int sdio_clear_int_ext(uint32_t val)
>  {
>  	int ret;
>  
> -	if (g_sdio.has_thrpt_enh3) {
> +	if(g_sdio.has_thrpt_enh3) {
why changing this? The original style is according to the kernel coding
style.

regards
sudip
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Greg KH July 31, 2015, 3:48 a.m. UTC | #1 
On Fri, Jul 31, 2015 at 12:27:04PM +0900, tony.cho wrote:
> 
> 
> On 2015? 07? 30? 20:56, Sudip Mukherjee wrote:
> >On Thu, Jul 30, 2015 at 06:10:10PM +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@atmel.com>
> >>---
> ><snip>
> >>+#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 */
> >instead of changing #ifndef to #ifdef i think the following would have
> >been easier:
> 
> Yes, I agree with you.

Great!

> I will rewrite them after important patches are accepted.

This is the first patch in the series, I'm not going to take it if you
are going to redo it later, please fix it correctly.

I'll drop this series from my queue.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/staging/wilc1000/wilc_sdio.c b/drivers/staging/wilc1000/wilc_sdio.c
index 5a18148..5cd4d45 100644
--- a/drivers/staging/wilc1000/wilc_sdio.c
+++ b/drivers/staging/wilc1000/wilc_sdio.c
@@ -155,9 +155,9 @@  _fail_:
 	return 0;
 }
 
+#ifndef WILC_SDIO_IRQ_GPIO
 static int sdio_clear_int(void)
 {
-#ifndef WILC_SDIO_IRQ_GPIO
 	/* uint32_t sts; */
 	sdio_cmd52_t cmd;
 
@@ -170,7 +170,10 @@  static int sdio_clear_int(void)
 	int_clrd++;
 
 	return cmd.data;
+}
 #else
+static int sdio_clear_int(void)
+{
 	uint32_t reg;
 
 	if (!sdio_read_reg(WILC_HOST_RX_CTRL_0, &reg)) {
@@ -181,9 +184,8 @@  static int sdio_clear_int(void)
 	sdio_write_reg(WILC_HOST_RX_CTRL_0, reg);
 	int_clrd++;
 	return 1;
-#endif
-
 }
+#endif
 
 uint32_t sdio_xfer_cnt(void)
 {