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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@xxxxxxxxx>
---
<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. I will rewrite them after a big patch is accepted.
Thanks for your advice.
Tony.


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)
  {

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

--
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@xxxxxxxxx

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux