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

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

 





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@xxxxxxxxx>
Reviewed-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
---
  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@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