All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/14] cleanups continue
@ 2018-04-12 15:50 Sergio Paracuellos
  2018-04-12 15:50 ` [PATCH 01/14] staging: ks7010: move ROM_FILE definition into source file Sergio Paracuellos
                   ` (13 more replies)
  0 siblings, 14 replies; 19+ messages in thread
From: Sergio Paracuellos @ 2018-04-12 15:50 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, wsa

The following series makes new cleanups for ks7010 SDIO
related code. All of these needs the previous sent patch 
series to be applied before.

Sergio Paracuellos (14):
  staging: ks7010: move ROM_FILE definition into source file
  staging: ks7010: move sdio specific register definitions into source
    file
  staging: ks7010: delete not used definitions in ks7010_sdio source
  staging: ks7010: add REG suffix to sdio register definitions
  staging: ks7010: review comment style in ks7010_sdio source file
  staging: ks7010: review debug and error messages in ks7010_sdio source
  staging: ks7010: avoid one extra level indentation in ks_wlan_hw_rx
    function
  staging: ks7010: move MODULE_DEVICE_TABLE related code
  staging: ks7010: replace create_workqueue with alloc_workqueue
  staging: ks7010: check sdio_set_block_size return value
  staging: ks7010: fix error paths in ks7010_sdio_remove function
  staging: ks7010: use u8 instead of unsigned char for firmware buffers
  staging: ks7010: assign dev_alloc_name() result to variable before
    check it
  staging: ks7010: use linux circular buffer header macros to handle tx
    and rx queues

 drivers/staging/ks7010/ks7010_sdio.c | 295 +++++++++++++++++++++++------------
 drivers/staging/ks7010/ks7010_sdio.h |  71 ---------
 2 files changed, 192 insertions(+), 174 deletions(-)

-- 
2.7.4

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 01/14] staging: ks7010: move ROM_FILE definition into source file
  2018-04-12 15:50 [PATCH 00/14] cleanups continue Sergio Paracuellos
@ 2018-04-12 15:50 ` Sergio Paracuellos
  2018-04-12 15:50 ` [PATCH 02/14] staging: ks7010: move sdio specific register definitions " Sergio Paracuellos
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Sergio Paracuellos @ 2018-04-12 15:50 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, wsa

This commit moves ROM_FILE from header to source file because
there is not being used outside this.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks7010_sdio.c | 1 +
 drivers/staging/ks7010/ks7010_sdio.h | 2 --
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index e475e9f..99f7b1d 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -22,6 +22,7 @@
 #include "ks_hostif.h"
 #include "ks7010_sdio.h"
 
+#define ROM_FILE "ks7010sd.rom"
 #define KS7010_FUNC_NUM 1
 #define KS7010_IO_BLOCK_SIZE 512
 #define KS7010_MAX_CLOCK 25000000
diff --git a/drivers/staging/ks7010/ks7010_sdio.h b/drivers/staging/ks7010/ks7010_sdio.h
index 3f65845..1bcdbe3 100644
--- a/drivers/staging/ks7010/ks7010_sdio.h
+++ b/drivers/staging/ks7010/ks7010_sdio.h
@@ -154,6 +154,4 @@ struct rx_device {
 	spinlock_t rx_dev_lock;	/* protect access to the queue */
 };
 
-#define	ROM_FILE "ks7010sd.rom"
-
 #endif /* _KS7010_SDIO_H */
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 02/14] staging: ks7010: move sdio specific register definitions into source file
  2018-04-12 15:50 [PATCH 00/14] cleanups continue Sergio Paracuellos
  2018-04-12 15:50 ` [PATCH 01/14] staging: ks7010: move ROM_FILE definition into source file Sergio Paracuellos
@ 2018-04-12 15:50 ` Sergio Paracuellos
  2018-04-12 15:50 ` [PATCH 03/14] staging: ks7010: delete not used definitions in ks7010_sdio source Sergio Paracuellos
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Sergio Paracuellos @ 2018-04-12 15:50 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, wsa

This commit moves SDIO related register definitions from header
to source file. There is no need to have those into the header
because they are only being used in specific SDIO code.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks7010_sdio.c | 72 ++++++++++++++++++++++++++++++++++++
 drivers/staging/ks7010/ks7010_sdio.h | 69 ----------------------------------
 2 files changed, 72 insertions(+), 69 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 99f7b1d..db1b749 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -23,6 +23,78 @@
 #include "ks7010_sdio.h"
 
 #define ROM_FILE "ks7010sd.rom"
+
+/*  SDIO KeyStream vendor and device */
+#define SDIO_VENDOR_ID_KS_CODE_A	0x005b
+#define SDIO_VENDOR_ID_KS_CODE_B	0x0023
+
+/* Older sources suggest earlier versions were named 7910 or 79xx */
+#define SDIO_DEVICE_ID_KS_7010		0x7910
+
+/* Read/Write Status Register */
+#define READ_STATUS		0x000000
+#define WRITE_STATUS		0x00000C
+enum reg_status_type {
+	REG_STATUS_BUSY,
+	REG_STATUS_IDLE
+};
+
+/* Read Index Register */
+#define READ_INDEX		0x000004
+
+/* Read Data Size Register */
+#define READ_DATA_SIZE		0x000008
+
+/* Write Index Register */
+#define WRITE_INDEX		0x000010
+
+/* Write Status/Read Data Size Register
+ * for network packet (less than 2048 bytes data)
+ */
+#define WSTATUS_RSIZE		0x000014
+#define WSTATUS_MASK		0x80	/* Write Status Register value */
+#define RSIZE_MASK		0x7F	/* Read Data Size Register value [10:4] */
+
+/* ARM to SD interrupt Enable */
+#define INT_ENABLE		0x000020
+/* ARM to SD interrupt Pending */
+#define INT_PENDING		0x000024
+
+#define INT_GCR_B              BIT(7)
+#define INT_GCR_A              BIT(6)
+#define INT_WRITE_STATUS       BIT(5)
+#define INT_WRITE_INDEX        BIT(4)
+#define INT_WRITE_SIZE         BIT(3)
+#define INT_READ_STATUS        BIT(2)
+#define INT_READ_INDEX         BIT(1)
+#define INT_READ_SIZE          BIT(0)
+
+/* General Communication Register A */
+#define GCR_A			0x000028
+enum gen_com_reg_a {
+	GCR_A_INIT,
+	GCR_A_REMAP,
+	GCR_A_RUN
+};
+
+/* General Communication Register B */
+#define GCR_B			0x00002C
+enum gen_com_reg_b {
+	GCR_B_ACTIVE,
+	GCR_B_DOZE
+};
+
+/* Wakeup Register */
+#define WAKEUP			0x008018
+#define WAKEUP_REQ		0x5a
+
+/* AHB Data Window  0x010000-0x01FFFF */
+#define DATA_WINDOW		0x010000
+#define WINDOW_SIZE		(64 * 1024)
+
+#define KS7010_IRAM_ADDRESS	0x06000000
+
+
 #define KS7010_FUNC_NUM 1
 #define KS7010_IO_BLOCK_SIZE 512
 #define KS7010_MAX_CLOCK 25000000
diff --git a/drivers/staging/ks7010/ks7010_sdio.h b/drivers/staging/ks7010/ks7010_sdio.h
index 1bcdbe3..95ac86b 100644
--- a/drivers/staging/ks7010/ks7010_sdio.h
+++ b/drivers/staging/ks7010/ks7010_sdio.h
@@ -11,75 +11,6 @@
 #ifndef _KS7010_SDIO_H
 #define _KS7010_SDIO_H
 
-/*  SDIO KeyStream vendor and device */
-#define SDIO_VENDOR_ID_KS_CODE_A	0x005b
-#define SDIO_VENDOR_ID_KS_CODE_B	0x0023
-/* Older sources suggest earlier versions were named 7910 or 79xx */
-#define SDIO_DEVICE_ID_KS_7010		0x7910
-
-/* Read/Write Status Register */
-#define READ_STATUS		0x000000
-#define WRITE_STATUS		0x00000C
-enum reg_status_type {
-	REG_STATUS_BUSY,
-	REG_STATUS_IDLE
-};
-
-/* Read Index Register */
-#define READ_INDEX		0x000004
-
-/* Read Data Size Register */
-#define READ_DATA_SIZE		0x000008
-
-/* Write Index Register */
-#define WRITE_INDEX		0x000010
-
-/* Write Status/Read Data Size Register
- * for network packet (less than 2048 bytes data)
- */
-#define WSTATUS_RSIZE		0x000014
-#define WSTATUS_MASK		0x80	/* Write Status Register value */
-#define RSIZE_MASK		0x7F	/* Read Data Size Register value [10:4] */
-
-/* ARM to SD interrupt Enable */
-#define INT_ENABLE		0x000020
-/* ARM to SD interrupt Pending */
-#define INT_PENDING		0x000024
-
-#define INT_GCR_B              BIT(7)
-#define INT_GCR_A              BIT(6)
-#define INT_WRITE_STATUS       BIT(5)
-#define INT_WRITE_INDEX        BIT(4)
-#define INT_WRITE_SIZE         BIT(3)
-#define INT_READ_STATUS        BIT(2)
-#define INT_READ_INDEX         BIT(1)
-#define INT_READ_SIZE          BIT(0)
-
-/* General Communication Register A */
-#define GCR_A			0x000028
-enum gen_com_reg_a {
-	GCR_A_INIT,
-	GCR_A_REMAP,
-	GCR_A_RUN
-};
-
-/* General Communication Register B */
-#define GCR_B			0x00002C
-enum gen_com_reg_b {
-	GCR_B_ACTIVE,
-	GCR_B_DOZE
-};
-
-/* Wakeup Register */
-#define WAKEUP			0x008018
-#define WAKEUP_REQ		0x5a
-
-/* AHB Data Window  0x010000-0x01FFFF */
-#define DATA_WINDOW		0x010000
-#define WINDOW_SIZE		(64 * 1024)
-
-#define KS7010_IRAM_ADDRESS	0x06000000
-
 /**
  * struct ks_sdio_card - SDIO device data.
  *
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 03/14] staging: ks7010: delete not used definitions in ks7010_sdio source
  2018-04-12 15:50 [PATCH 00/14] cleanups continue Sergio Paracuellos
  2018-04-12 15:50 ` [PATCH 01/14] staging: ks7010: move ROM_FILE definition into source file Sergio Paracuellos
  2018-04-12 15:50 ` [PATCH 02/14] staging: ks7010: move sdio specific register definitions " Sergio Paracuellos
@ 2018-04-12 15:50 ` Sergio Paracuellos
  2018-04-12 15:50 ` [PATCH 04/14] staging: ks7010: add REG suffix to sdio register definitions Sergio Paracuellos
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Sergio Paracuellos @ 2018-04-12 15:50 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, wsa

This commit removes two definitions inside ks7010_sdio source file
because they are not being used at all.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks7010_sdio.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index db1b749..063c461 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -94,10 +94,7 @@ enum gen_com_reg_b {
 
 #define KS7010_IRAM_ADDRESS	0x06000000
 
-
-#define KS7010_FUNC_NUM 1
 #define KS7010_IO_BLOCK_SIZE 512
-#define KS7010_MAX_CLOCK 25000000
 
 static const struct sdio_device_id ks7010_sdio_ids[] = {
 	{SDIO_DEVICE(SDIO_VENDOR_ID_KS_CODE_A, SDIO_DEVICE_ID_KS_7010)},
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 04/14] staging: ks7010: add REG suffix to sdio register definitions
  2018-04-12 15:50 [PATCH 00/14] cleanups continue Sergio Paracuellos
                   ` (2 preceding siblings ...)
  2018-04-12 15:50 ` [PATCH 03/14] staging: ks7010: delete not used definitions in ks7010_sdio source Sergio Paracuellos
@ 2018-04-12 15:50 ` Sergio Paracuellos
  2018-04-12 15:50 ` [PATCH 05/14] staging: ks7010: review comment style in ks7010_sdio source file Sergio Paracuellos
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Sergio Paracuellos @ 2018-04-12 15:50 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, wsa

This commit adds REG suffix to register definitions related
with SDIO in order to improve readability.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks7010_sdio.c | 96 ++++++++++++++++++------------------
 1 file changed, 48 insertions(+), 48 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 063c461..befe9e9 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -32,33 +32,33 @@
 #define SDIO_DEVICE_ID_KS_7010		0x7910
 
 /* Read/Write Status Register */
-#define READ_STATUS		0x000000
-#define WRITE_STATUS		0x00000C
+#define READ_STATUS_REG		0x000000
+#define WRITE_STATUS_REG	0x00000C
 enum reg_status_type {
 	REG_STATUS_BUSY,
 	REG_STATUS_IDLE
 };
 
 /* Read Index Register */
-#define READ_INDEX		0x000004
+#define READ_INDEX_REG		0x000004
 
 /* Read Data Size Register */
-#define READ_DATA_SIZE		0x000008
+#define READ_DATA_SIZE_REG	0x000008
 
 /* Write Index Register */
-#define WRITE_INDEX		0x000010
+#define WRITE_INDEX_REG		0x000010
 
 /* Write Status/Read Data Size Register
  * for network packet (less than 2048 bytes data)
  */
-#define WSTATUS_RSIZE		0x000014
+#define WSTATUS_RSIZE_REG	0x000014
 #define WSTATUS_MASK		0x80	/* Write Status Register value */
 #define RSIZE_MASK		0x7F	/* Read Data Size Register value [10:4] */
 
 /* ARM to SD interrupt Enable */
-#define INT_ENABLE		0x000020
+#define INT_ENABLE_REG		0x000020
 /* ARM to SD interrupt Pending */
-#define INT_PENDING		0x000024
+#define INT_PENDING_REG		0x000024
 
 #define INT_GCR_B              BIT(7)
 #define INT_GCR_A              BIT(6)
@@ -70,7 +70,7 @@ enum reg_status_type {
 #define INT_READ_SIZE          BIT(0)
 
 /* General Communication Register A */
-#define GCR_A			0x000028
+#define GCR_A_REG		0x000028
 enum gen_com_reg_a {
 	GCR_A_INIT,
 	GCR_A_REMAP,
@@ -78,14 +78,14 @@ enum gen_com_reg_a {
 };
 
 /* General Communication Register B */
-#define GCR_B			0x00002C
+#define GCR_B_REG		0x00002C
 enum gen_com_reg_b {
 	GCR_B_ACTIVE,
 	GCR_B_DOZE
 };
 
 /* Wakeup Register */
-#define WAKEUP			0x008018
+#define WAKEUP_REG		0x008018
 #define WAKEUP_REQ		0x5a
 
 /* AHB Data Window  0x010000-0x01FFFF */
@@ -187,9 +187,9 @@ static void ks_wlan_hw_sleep_doze_request(struct ks_wlan_private *priv)
 	atomic_set(&priv->sleepstatus.doze_request, 0);
 
 	if (atomic_read(&priv->sleepstatus.status) == 0) {
-		ret = ks7010_sdio_writeb(priv, GCR_B, GCR_B_DOZE);
+		ret = ks7010_sdio_writeb(priv, GCR_B_REG, GCR_B_DOZE);
 		if (ret) {
-			netdev_err(priv->net_dev, " error : GCR_B\n");
+			netdev_err(priv->net_dev, " error : GCR_B_REG\n");
 			goto set_sleep_mode;
 		}
 		atomic_set(&priv->sleepstatus.status, 1);
@@ -208,9 +208,9 @@ static void ks_wlan_hw_sleep_wakeup_request(struct ks_wlan_private *priv)
 	atomic_set(&priv->sleepstatus.wakeup_request, 0);
 
 	if (atomic_read(&priv->sleepstatus.status) == 1) {
-		ret = ks7010_sdio_writeb(priv, WAKEUP, WAKEUP_REQ);
+		ret = ks7010_sdio_writeb(priv, WAKEUP_REG, WAKEUP_REQ);
 		if (ret) {
-			netdev_err(priv->net_dev, " error : WAKEUP\n");
+			netdev_err(priv->net_dev, " error : WAKEUP_REG\n");
 			goto set_sleep_mode;
 		}
 		atomic_set(&priv->sleepstatus.status, 0);
@@ -227,9 +227,9 @@ void ks_wlan_hw_wakeup_request(struct ks_wlan_private *priv)
 	int ret;
 
 	if (atomic_read(&priv->psstatus.status) == PS_SNOOZE) {
-		ret = ks7010_sdio_writeb(priv, WAKEUP, WAKEUP_REQ);
+		ret = ks7010_sdio_writeb(priv, WAKEUP_REG, WAKEUP_REQ);
 		if (ret)
-			netdev_err(priv->net_dev, " error : WAKEUP\n");
+			netdev_err(priv->net_dev, " error : WAKEUP_REG\n");
 
 		priv->last_wakeup = jiffies;
 		++priv->wakeup_count;
@@ -269,17 +269,17 @@ static void _ks_wlan_hw_power_save(struct ks_wlan_private *priv)
 		return;
 	}
 
-	ret = ks7010_sdio_readb(priv, INT_PENDING, &byte);
+	ret = ks7010_sdio_readb(priv, INT_PENDING_REG, &byte);
 	if (ret) {
-		netdev_err(priv->net_dev, " error : INT_PENDING\n");
+		netdev_err(priv->net_dev, " error : INT_PENDING_REG\n");
 		goto queue_delayed_work;
 	}
 	if (byte)
 		goto queue_delayed_work;
 
-	ret = ks7010_sdio_writeb(priv, GCR_B, GCR_B_DOZE);
+	ret = ks7010_sdio_writeb(priv, GCR_B_REG, GCR_B_DOZE);
 	if (ret) {
-		netdev_err(priv->net_dev, " error : GCR_B\n");
+		netdev_err(priv->net_dev, " error : GCR_B_REG\n");
 		goto queue_delayed_work;
 	}
 	atomic_set(&priv->psstatus.status, PS_SNOOZE);
@@ -354,9 +354,9 @@ static int write_to_device(struct ks_wlan_private *priv, unsigned char *buffer,
 		return ret;
 	}
 
-	ret = ks7010_sdio_writeb(priv, WRITE_STATUS, REG_STATUS_BUSY);
+	ret = ks7010_sdio_writeb(priv, WRITE_STATUS_REG, REG_STATUS_BUSY);
 	if (ret) {
-		netdev_err(priv->net_dev, " error : WRITE_STATUS\n");
+		netdev_err(priv->net_dev, " error : WRITE_STATUS_REG\n");
 		return ret;
 	}
 
@@ -462,9 +462,9 @@ static void ks_wlan_hw_rx(struct ks_wlan_private *priv, uint16_t size)
 				     DUMP_PREFIX_OFFSET,
 				     rx_buffer->data, 32);
 #endif
-		ret = ks7010_sdio_writeb(priv, READ_STATUS, REG_STATUS_IDLE);
+		ret = ks7010_sdio_writeb(priv, READ_STATUS_REG, REG_STATUS_IDLE);
 		if (ret)
-			netdev_err(priv->net_dev, " error : READ_STATUS\n");
+			netdev_err(priv->net_dev, " error : READ_STATUS_REG\n");
 
 		/* length check fail */
 		return;
@@ -475,9 +475,9 @@ static void ks_wlan_hw_rx(struct ks_wlan_private *priv, uint16_t size)
 	event = le16_to_cpu(hdr->event);
 	inc_rxqtail(priv);
 
-	ret = ks7010_sdio_writeb(priv, READ_STATUS, REG_STATUS_IDLE);
+	ret = ks7010_sdio_writeb(priv, READ_STATUS_REG, REG_STATUS_IDLE);
 	if (ret)
-		netdev_err(priv->net_dev, " error : READ_STATUS\n");
+		netdev_err(priv->net_dev, " error : READ_STATUS_REG\n");
 
 	if (atomic_read(&priv->psstatus.confirm_wait)) {
 		if (is_hif_conf(event)) {
@@ -536,9 +536,9 @@ static void ks7010_rw_function(struct work_struct *work)
 	}
 
 	/* read (WriteStatus/ReadDataSize FN1:00_0014) */
-	ret = ks7010_sdio_readb(priv, WSTATUS_RSIZE, &byte);
+	ret = ks7010_sdio_readb(priv, WSTATUS_RSIZE_REG, &byte);
 	if (ret) {
-		netdev_err(priv->net_dev, " error : WSTATUS_RSIZE psstatus=%d\n",
+		netdev_err(priv->net_dev, " error : WSTATUS_RSIZE_REG psstatus=%d\n",
 			   atomic_read(&priv->psstatus.status));
 		goto release_host;
 	}
@@ -568,9 +568,9 @@ static void ks_sdio_interrupt(struct sdio_func *func)
 	if (priv->dev_state < DEVICE_STATE_BOOT)
 		goto queue_delayed_work;
 
-	ret = ks7010_sdio_readb(priv, INT_PENDING, &status);
+	ret = ks7010_sdio_readb(priv, INT_PENDING_REG, &status);
 	if (ret) {
-		netdev_err(priv->net_dev, "error : INT_PENDING\n");
+		netdev_err(priv->net_dev, "error : INT_PENDING_REG\n");
 		goto queue_delayed_work;
 	}
 
@@ -581,9 +581,9 @@ static void ks_sdio_interrupt(struct sdio_func *func)
 	/* bit2 -> Read Status Busy  */
 	if (status & INT_GCR_B ||
 	    atomic_read(&priv->psstatus.status) == PS_SNOOZE) {
-		ret = ks7010_sdio_readb(priv, GCR_B, &byte);
+		ret = ks7010_sdio_readb(priv, GCR_B_REG, &byte);
 		if (ret) {
-			netdev_err(priv->net_dev, " error : GCR_B\n");
+			netdev_err(priv->net_dev, " error : GCR_B_REG\n");
 			goto queue_delayed_work;
 		}
 		if (byte == GCR_B_ACTIVE) {
@@ -597,9 +597,9 @@ static void ks_sdio_interrupt(struct sdio_func *func)
 
 	do {
 		/* read (WriteStatus/ReadDataSize FN1:00_0014) */
-		ret = ks7010_sdio_readb(priv, WSTATUS_RSIZE, &byte);
+		ret = ks7010_sdio_readb(priv, WSTATUS_RSIZE_REG, &byte);
 		if (ret) {
-			netdev_err(priv->net_dev, " error : WSTATUS_RSIZE\n");
+			netdev_err(priv->net_dev, " error : WSTATUS_RSIZE_REG\n");
 			goto queue_delayed_work;
 		}
 		rsize = byte & RSIZE_MASK;
@@ -666,11 +666,11 @@ static int ks7010_sdio_update_index(struct ks_wlan_private *priv, u32 index)
 		return -ENOMEM;
 
 	memcpy(data_buf, &index, sizeof(index));
-	ret = ks7010_sdio_write(priv, WRITE_INDEX, data_buf, sizeof(index));
+	ret = ks7010_sdio_write(priv, WRITE_INDEX_REG, data_buf, sizeof(index));
 	if (ret)
 		goto err_free_data_buf;
 
-	ret = ks7010_sdio_write(priv, READ_INDEX, data_buf, sizeof(index));
+	ret = ks7010_sdio_write(priv, READ_INDEX_REG, data_buf, sizeof(index));
 	if (ret)
 		goto err_free_data_buf;
 
@@ -759,7 +759,7 @@ static int ks7010_copy_firmware(struct ks_wlan_private *priv,
 
 	} while (size);
 
-	ret = ks7010_sdio_writeb(priv, GCR_A, GCR_A_REMAP);
+	ret = ks7010_sdio_writeb(priv, GCR_A_REG, GCR_A_REMAP);
 
 free_rom_buf:
 	kfree(rom_buf);
@@ -778,7 +778,7 @@ static int ks7010_upload_firmware(struct ks_sdio_card *card)
 	sdio_claim_host(card->func);
 
 	/* Firmware running ? */
-	ret = ks7010_sdio_readb(priv, GCR_A, &byte);
+	ret = ks7010_sdio_readb(priv, GCR_A_REG, &byte);
 	if (ret)
 		goto release_host;
 	if (byte == GCR_A_RUN) {
@@ -799,7 +799,7 @@ static int ks7010_upload_firmware(struct ks_sdio_card *card)
 	/* Firmware running check */
 	for (n = 0; n < 50; ++n) {
 		mdelay(10);	/* wait_ms(10); */
-		ret = ks7010_sdio_readb(priv, GCR_A, &byte);
+		ret = ks7010_sdio_readb(priv, GCR_A_REG, &byte);
 		if (ret)
 			goto release_firmware;
 
@@ -904,11 +904,11 @@ static int ks7010_sdio_setup_irqs(struct sdio_func *func)
 	int ret;
 
 	/* interrupt disable */
-	sdio_writeb(func, 0, INT_ENABLE, &ret);
+	sdio_writeb(func, 0, INT_ENABLE_REG, &ret);
 	if (ret)
 		goto irq_error;
 
-	sdio_writeb(func, 0xff, INT_PENDING, &ret);
+	sdio_writeb(func, 0xff, INT_PENDING_REG, &ret);
 	if (ret)
 		goto irq_error;
 
@@ -931,18 +931,18 @@ static void ks7010_sdio_init_irqs(struct sdio_func *func,
 	 * (ARMtoSD_InterruptPending FN1:00_0024)
 	 */
 	sdio_claim_host(func);
-	ret = ks7010_sdio_writeb(priv, INT_PENDING, 0xff);
+	ret = ks7010_sdio_writeb(priv, INT_PENDING_REG, 0xff);
 	sdio_release_host(func);
 	if (ret)
-		netdev_err(priv->net_dev, " error : INT_PENDING\n");
+		netdev_err(priv->net_dev, " error : INT_PENDING_REG\n");
 
 	/* enable ks7010sdio interrupt */
 	byte = (INT_GCR_B | INT_READ_STATUS | INT_WRITE_STATUS);
 	sdio_claim_host(func);
-	ret = ks7010_sdio_writeb(priv, INT_ENABLE, byte);
+	ret = ks7010_sdio_writeb(priv, INT_ENABLE_REG, byte);
 	sdio_release_host(func);
 	if (ret)
-		netdev_err(priv->net_dev, " err : INT_ENABLE\n");
+		netdev_err(priv->net_dev, " err : INT_ENABLE_REG\n");
 }
 
 static void ks7010_private_init(struct ks_wlan_private *priv,
@@ -1117,8 +1117,8 @@ static void ks7010_sdio_remove(struct sdio_func *func)
 
 		/* interrupt disable */
 		sdio_claim_host(func);
-		sdio_writeb(func, 0, INT_ENABLE, &ret);
-		sdio_writeb(func, 0xff, INT_PENDING, &ret);
+		sdio_writeb(func, 0, INT_ENABLE_REG, &ret);
+		sdio_writeb(func, 0xff, INT_PENDING_REG, &ret);
 		sdio_release_host(func);
 
 		ret = send_stop_request(func);
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 05/14] staging: ks7010: review comment style in ks7010_sdio source file
  2018-04-12 15:50 [PATCH 00/14] cleanups continue Sergio Paracuellos
                   ` (3 preceding siblings ...)
  2018-04-12 15:50 ` [PATCH 04/14] staging: ks7010: add REG suffix to sdio register definitions Sergio Paracuellos
@ 2018-04-12 15:50 ` Sergio Paracuellos
  2018-04-12 15:50 ` [PATCH 06/14] staging: ks7010: review debug and error messages in ks7010_sdio source Sergio Paracuellos
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Sergio Paracuellos @ 2018-04-12 15:50 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, wsa

This commit reviews comment style used in ks7010_sdio source file
in order to be coherent with the rest of the code. Most comments
in this source are before definitions but only two of them have
been written at the right. So, be coherent moving this two to the
top of definitions. Also fix one multiline comment style to use
the normal preferred kernel style.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks7010_sdio.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index befe9e9..4fde8d8 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -48,12 +48,17 @@ enum reg_status_type {
 /* Write Index Register */
 #define WRITE_INDEX_REG		0x000010
 
-/* Write Status/Read Data Size Register
+/*
+ * Write Status/Read Data Size Register
  * for network packet (less than 2048 bytes data)
  */
 #define WSTATUS_RSIZE_REG	0x000014
-#define WSTATUS_MASK		0x80	/* Write Status Register value */
-#define RSIZE_MASK		0x7F	/* Read Data Size Register value [10:4] */
+
+/* Write Status Register value */
+#define WSTATUS_MASK		0x80
+
+/* Read Data Size Register value [10:4] */
+#define RSIZE_MASK		0x7F
 
 /* ARM to SD interrupt Enable */
 #define INT_ENABLE_REG		0x000020
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 06/14] staging: ks7010: review debug and error messages in ks7010_sdio source
  2018-04-12 15:50 [PATCH 00/14] cleanups continue Sergio Paracuellos
                   ` (4 preceding siblings ...)
  2018-04-12 15:50 ` [PATCH 05/14] staging: ks7010: review comment style in ks7010_sdio source file Sergio Paracuellos
@ 2018-04-12 15:50 ` Sergio Paracuellos
  2018-04-12 15:50 ` [PATCH 07/14] staging: ks7010: avoid one extra level indentation in ks_wlan_hw_rx function Sergio Paracuellos
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Sergio Paracuellos @ 2018-04-12 15:50 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, wsa

This commit reviews debug and error messages in code located
in ks7010_sdio source file avoiding to use 'error' or 'ks7010'
because this file is using netdev_* functions and has non
sense to repeat information in log messages.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks7010_sdio.c | 38 +++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 4fde8d8..19ce50f 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -194,7 +194,7 @@ static void ks_wlan_hw_sleep_doze_request(struct ks_wlan_private *priv)
 	if (atomic_read(&priv->sleepstatus.status) == 0) {
 		ret = ks7010_sdio_writeb(priv, GCR_B_REG, GCR_B_DOZE);
 		if (ret) {
-			netdev_err(priv->net_dev, " error : GCR_B_REG\n");
+			netdev_err(priv->net_dev, "write GCR_B_REG\n");
 			goto set_sleep_mode;
 		}
 		atomic_set(&priv->sleepstatus.status, 1);
@@ -215,7 +215,7 @@ static void ks_wlan_hw_sleep_wakeup_request(struct ks_wlan_private *priv)
 	if (atomic_read(&priv->sleepstatus.status) == 1) {
 		ret = ks7010_sdio_writeb(priv, WAKEUP_REG, WAKEUP_REQ);
 		if (ret) {
-			netdev_err(priv->net_dev, " error : WAKEUP_REG\n");
+			netdev_err(priv->net_dev, "write WAKEUP_REG\n");
 			goto set_sleep_mode;
 		}
 		atomic_set(&priv->sleepstatus.status, 0);
@@ -234,7 +234,7 @@ void ks_wlan_hw_wakeup_request(struct ks_wlan_private *priv)
 	if (atomic_read(&priv->psstatus.status) == PS_SNOOZE) {
 		ret = ks7010_sdio_writeb(priv, WAKEUP_REG, WAKEUP_REQ);
 		if (ret)
-			netdev_err(priv->net_dev, " error : WAKEUP_REG\n");
+			netdev_err(priv->net_dev, "write WAKEUP_REG\n");
 
 		priv->last_wakeup = jiffies;
 		++priv->wakeup_count;
@@ -276,7 +276,7 @@ static void _ks_wlan_hw_power_save(struct ks_wlan_private *priv)
 
 	ret = ks7010_sdio_readb(priv, INT_PENDING_REG, &byte);
 	if (ret) {
-		netdev_err(priv->net_dev, " error : INT_PENDING_REG\n");
+		netdev_err(priv->net_dev, "read INT_PENDING_REG\n");
 		goto queue_delayed_work;
 	}
 	if (byte)
@@ -284,7 +284,7 @@ static void _ks_wlan_hw_power_save(struct ks_wlan_private *priv)
 
 	ret = ks7010_sdio_writeb(priv, GCR_B_REG, GCR_B_DOZE);
 	if (ret) {
-		netdev_err(priv->net_dev, " error : GCR_B_REG\n");
+		netdev_err(priv->net_dev, "write GCR_B_REG\n");
 		goto queue_delayed_work;
 	}
 	atomic_set(&priv->psstatus.status, PS_SNOOZE);
@@ -355,13 +355,13 @@ static int write_to_device(struct ks_wlan_private *priv, unsigned char *buffer,
 
 	ret = ks7010_sdio_write(priv, DATA_WINDOW, buffer, size);
 	if (ret) {
-		netdev_err(priv->net_dev, " write error : retval=%d\n", ret);
+		netdev_err(priv->net_dev, "write DATA_WINDOW\n");
 		return ret;
 	}
 
 	ret = ks7010_sdio_writeb(priv, WRITE_STATUS_REG, REG_STATUS_BUSY);
 	if (ret) {
-		netdev_err(priv->net_dev, " error : WRITE_STATUS_REG\n");
+		netdev_err(priv->net_dev, "write WRITE_STATUS_REG\n");
 		return ret;
 	}
 
@@ -469,7 +469,7 @@ static void ks_wlan_hw_rx(struct ks_wlan_private *priv, uint16_t size)
 #endif
 		ret = ks7010_sdio_writeb(priv, READ_STATUS_REG, REG_STATUS_IDLE);
 		if (ret)
-			netdev_err(priv->net_dev, " error : READ_STATUS_REG\n");
+			netdev_err(priv->net_dev, "write READ_STATUS_REG\n");
 
 		/* length check fail */
 		return;
@@ -482,7 +482,7 @@ static void ks_wlan_hw_rx(struct ks_wlan_private *priv, uint16_t size)
 
 	ret = ks7010_sdio_writeb(priv, READ_STATUS_REG, REG_STATUS_IDLE);
 	if (ret)
-		netdev_err(priv->net_dev, " error : READ_STATUS_REG\n");
+		netdev_err(priv->net_dev, "write READ_STATUS_REG\n");
 
 	if (atomic_read(&priv->psstatus.confirm_wait)) {
 		if (is_hif_conf(event)) {
@@ -543,7 +543,7 @@ static void ks7010_rw_function(struct work_struct *work)
 	/* read (WriteStatus/ReadDataSize FN1:00_0014) */
 	ret = ks7010_sdio_readb(priv, WSTATUS_RSIZE_REG, &byte);
 	if (ret) {
-		netdev_err(priv->net_dev, " error : WSTATUS_RSIZE_REG psstatus=%d\n",
+		netdev_err(priv->net_dev, "read WSTATUS_RSIZE_REG psstatus=%d\n",
 			   atomic_read(&priv->psstatus.status));
 		goto release_host;
 	}
@@ -575,7 +575,7 @@ static void ks_sdio_interrupt(struct sdio_func *func)
 
 	ret = ks7010_sdio_readb(priv, INT_PENDING_REG, &status);
 	if (ret) {
-		netdev_err(priv->net_dev, "error : INT_PENDING_REG\n");
+		netdev_err(priv->net_dev, "read INT_PENDING_REG\n");
 		goto queue_delayed_work;
 	}
 
@@ -588,7 +588,7 @@ static void ks_sdio_interrupt(struct sdio_func *func)
 	    atomic_read(&priv->psstatus.status) == PS_SNOOZE) {
 		ret = ks7010_sdio_readb(priv, GCR_B_REG, &byte);
 		if (ret) {
-			netdev_err(priv->net_dev, " error : GCR_B_REG\n");
+			netdev_err(priv->net_dev, "read GCR_B_REG\n");
 			goto queue_delayed_work;
 		}
 		if (byte == GCR_B_ACTIVE) {
@@ -604,7 +604,7 @@ static void ks_sdio_interrupt(struct sdio_func *func)
 		/* read (WriteStatus/ReadDataSize FN1:00_0014) */
 		ret = ks7010_sdio_readb(priv, WSTATUS_RSIZE_REG, &byte);
 		if (ret) {
-			netdev_err(priv->net_dev, " error : WSTATUS_RSIZE_REG\n");
+			netdev_err(priv->net_dev, "read WSTATUS_RSIZE_REG\n");
 			goto queue_delayed_work;
 		}
 		rsize = byte & RSIZE_MASK;
@@ -939,7 +939,7 @@ static void ks7010_sdio_init_irqs(struct sdio_func *func,
 	ret = ks7010_sdio_writeb(priv, INT_PENDING_REG, 0xff);
 	sdio_release_host(func);
 	if (ret)
-		netdev_err(priv->net_dev, " error : INT_PENDING_REG\n");
+		netdev_err(priv->net_dev, "write INT_PENDING_REG\n");
 
 	/* enable ks7010sdio interrupt */
 	byte = (INT_GCR_B | INT_READ_STATUS | INT_WRITE_STATUS);
@@ -947,7 +947,7 @@ static void ks7010_sdio_init_irqs(struct sdio_func *func,
 	ret = ks7010_sdio_writeb(priv, INT_ENABLE_REG, byte);
 	sdio_release_host(func);
 	if (ret)
-		netdev_err(priv->net_dev, " err : INT_ENABLE_REG\n");
+		netdev_err(priv->net_dev, "write INT_ENABLE_REG\n");
 }
 
 static void ks7010_private_init(struct ks_wlan_private *priv,
@@ -1018,12 +1018,11 @@ static int ks7010_sdio_probe(struct sdio_func *func,
 	/* private memory allocate */
 	netdev = alloc_etherdev(sizeof(*priv));
 	if (!netdev) {
-		dev_err(&card->func->dev, "ks7010 : Unable to alloc new net device\n");
+		dev_err(&card->func->dev, "Unable to alloc new net device\n");
 		goto err_release_irq;
 	}
 	if (dev_alloc_name(netdev, "wlan%d") < 0) {
-		dev_err(&card->func->dev,
-			"ks7010 :  Couldn't get name!\n");
+		dev_err(&card->func->dev, "Couldn't get name!\n");
 		goto err_free_netdev;
 	}
 
@@ -1037,8 +1036,7 @@ static int ks7010_sdio_probe(struct sdio_func *func,
 	ret = ks7010_upload_firmware(card);
 	if (ret) {
 		netdev_err(priv->net_dev,
-			   "ks7010: firmware load failed !! return code = %d\n",
-			   ret);
+			   "firmware load failed !! ret = %d\n", ret);
 		goto err_free_netdev;
 	}
 
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 07/14] staging: ks7010: avoid one extra level indentation in ks_wlan_hw_rx function
  2018-04-12 15:50 [PATCH 00/14] cleanups continue Sergio Paracuellos
                   ` (5 preceding siblings ...)
  2018-04-12 15:50 ` [PATCH 06/14] staging: ks7010: review debug and error messages in ks7010_sdio source Sergio Paracuellos
@ 2018-04-12 15:50 ` Sergio Paracuellos
  2018-04-12 15:50 ` [PATCH 08/14] staging: ks7010: move MODULE_DEVICE_TABLE related code Sergio Paracuellos
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Sergio Paracuellos @ 2018-04-12 15:50 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, wsa

This commit use an and operator in a if condition to avoid one
indentation level which is not needed at all.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks7010_sdio.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 19ce50f..12687ae 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -484,11 +484,9 @@ static void ks_wlan_hw_rx(struct ks_wlan_private *priv, uint16_t size)
 	if (ret)
 		netdev_err(priv->net_dev, "write READ_STATUS_REG\n");
 
-	if (atomic_read(&priv->psstatus.confirm_wait)) {
-		if (is_hif_conf(event)) {
-			netdev_dbg(priv->net_dev, "IS_HIF_CONF true !!\n");
-			atomic_dec(&priv->psstatus.confirm_wait);
-		}
+	if (atomic_read(&priv->psstatus.confirm_wait) && is_hif_conf(event)) {
+		netdev_dbg(priv->net_dev, "IS_HIF_CONF true !!\n");
+		atomic_dec(&priv->psstatus.confirm_wait);
 	}
 
 	tasklet_schedule(&priv->rx_bh_task);
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 08/14] staging: ks7010: move MODULE_DEVICE_TABLE related code
  2018-04-12 15:50 [PATCH 00/14] cleanups continue Sergio Paracuellos
                   ` (6 preceding siblings ...)
  2018-04-12 15:50 ` [PATCH 07/14] staging: ks7010: avoid one extra level indentation in ks_wlan_hw_rx function Sergio Paracuellos
@ 2018-04-12 15:50 ` Sergio Paracuellos
  2018-04-12 15:50 ` [PATCH 09/14] staging: ks7010: replace create_workqueue with alloc_workqueue Sergio Paracuellos
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Sergio Paracuellos @ 2018-04-12 15:50 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, wsa

This commit moves MODULE_DEVICE_TABLE related code to the end of
the file. This is not necessary at all but moving it just before
its use improves readability.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks7010_sdio.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 12687ae..c53f3f4 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -101,13 +101,6 @@ enum gen_com_reg_b {
 
 #define KS7010_IO_BLOCK_SIZE 512
 
-static const struct sdio_device_id ks7010_sdio_ids[] = {
-	{SDIO_DEVICE(SDIO_VENDOR_ID_KS_CODE_A, SDIO_DEVICE_ID_KS_7010)},
-	{SDIO_DEVICE(SDIO_VENDOR_ID_KS_CODE_B, SDIO_DEVICE_ID_KS_7010)},
-	{ /* all zero */ }
-};
-MODULE_DEVICE_TABLE(sdio, ks7010_sdio_ids);
-
 static inline void inc_txqhead(struct ks_wlan_private *priv)
 {
 	priv->tx_dev.qhead = (priv->tx_dev.qhead + 1) % TX_DEVICE_BUFF_SIZE;
@@ -1148,6 +1141,13 @@ static void ks7010_sdio_remove(struct sdio_func *func)
 	kfree(card);
 }
 
+static const struct sdio_device_id ks7010_sdio_ids[] = {
+	{SDIO_DEVICE(SDIO_VENDOR_ID_KS_CODE_A, SDIO_DEVICE_ID_KS_7010)},
+	{SDIO_DEVICE(SDIO_VENDOR_ID_KS_CODE_B, SDIO_DEVICE_ID_KS_7010)},
+	{ /* all zero */ }
+};
+MODULE_DEVICE_TABLE(sdio, ks7010_sdio_ids);
+
 static struct sdio_driver ks7010_sdio_driver = {
 	.name = "ks7010_sdio",
 	.id_table = ks7010_sdio_ids,
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 09/14] staging: ks7010: replace create_workqueue with alloc_workqueue
  2018-04-12 15:50 [PATCH 00/14] cleanups continue Sergio Paracuellos
                   ` (7 preceding siblings ...)
  2018-04-12 15:50 ` [PATCH 08/14] staging: ks7010: move MODULE_DEVICE_TABLE related code Sergio Paracuellos
@ 2018-04-12 15:50 ` Sergio Paracuellos
  2018-04-12 15:50 ` [PATCH 10/14] staging: ks7010: check sdio_set_block_size return value Sergio Paracuellos
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Sergio Paracuellos @ 2018-04-12 15:50 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, wsa

This commit replaces deprecated create_workqueue call with the
alloc_workqueue one which is the one to be used now for this
purpose.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks7010_sdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index c53f3f4..6db429e 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -1035,7 +1035,7 @@ static int ks7010_sdio_probe(struct sdio_func *func,
 
 	priv->dev_state = DEVICE_STATE_BOOT;
 
-	priv->wq = create_workqueue("wq");
+	priv->wq = alloc_workqueue("wq", WQ_MEM_RECLAIM, 1);
 	if (!priv->wq) {
 		netdev_err(priv->net_dev, "create_workqueue failed !!\n");
 		goto err_free_netdev;
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 10/14] staging: ks7010: check sdio_set_block_size return value
  2018-04-12 15:50 [PATCH 00/14] cleanups continue Sergio Paracuellos
                   ` (8 preceding siblings ...)
  2018-04-12 15:50 ` [PATCH 09/14] staging: ks7010: replace create_workqueue with alloc_workqueue Sergio Paracuellos
@ 2018-04-12 15:50 ` Sergio Paracuellos
  2018-04-12 15:50 ` [PATCH 11/14] staging: ks7010: fix error paths in ks7010_sdio_remove function Sergio Paracuellos
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Sergio Paracuellos @ 2018-04-12 15:50 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, wsa

This commit checks sdio_set_block_size function return value.
If it fails abort driver initialization.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks7010_sdio.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 6db429e..cdfbec8 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -988,6 +988,9 @@ static int ks7010_sdio_probe(struct sdio_func *func,
 	sdio_claim_host(func);
 
 	ret = sdio_set_block_size(func, KS7010_IO_BLOCK_SIZE);
+	if (ret)
+		goto err_free_card;
+
 	dev_dbg(&card->func->dev, "multi_block=%d sdio_set_block_size()=%d %d\n",
 		func->card->cccr.multi_block, func->cur_blksize, ret);
 
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 11/14] staging: ks7010: fix error paths in ks7010_sdio_remove function
  2018-04-12 15:50 [PATCH 00/14] cleanups continue Sergio Paracuellos
                   ` (9 preceding siblings ...)
  2018-04-12 15:50 ` [PATCH 10/14] staging: ks7010: check sdio_set_block_size return value Sergio Paracuellos
@ 2018-04-12 15:50 ` Sergio Paracuellos
  2018-04-12 15:50 ` [PATCH 12/14] staging: ks7010: use u8 instead of unsigned char for firmware buffers Sergio Paracuellos
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Sergio Paracuellos @ 2018-04-12 15:50 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, wsa

This commit reviews and fixes error paths in ks7010_sdio_remove
driver function. It change logic to handle error directly
after priv dereference to avoid one level indentation. It also
removes a temporal netdev variable which wasn't being used in all
of the function calls. Also if send_stop_request call fails it
was making a direct 'return' instead of doing a properly cleaning.
Because of this a new 'err_free_card' label has been added.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks7010_sdio.c | 42 ++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index cdfbec8..82bc839 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -1107,39 +1107,39 @@ static void ks7010_sdio_remove(struct sdio_func *func)
 		return;
 
 	priv = card->priv;
-	if (priv) {
-		struct net_device *netdev = priv->net_dev;
+	if (!priv)
+		goto err_free_card;
 
-		ks_wlan_net_stop(netdev);
+	ks_wlan_net_stop(priv->net_dev);
 
-		/* interrupt disable */
-		sdio_claim_host(func);
-		sdio_writeb(func, 0, INT_ENABLE_REG, &ret);
-		sdio_writeb(func, 0xff, INT_PENDING_REG, &ret);
-		sdio_release_host(func);
+	/* interrupt disable */
+	sdio_claim_host(func);
+	sdio_writeb(func, 0, INT_ENABLE_REG, &ret);
+	sdio_writeb(func, 0xff, INT_PENDING_REG, &ret);
+	sdio_release_host(func);
 
-		ret = send_stop_request(func);
-		if (ret)	/* memory allocation failure */
-			return;
+	ret = send_stop_request(func);
+	if (ret)	/* memory allocation failure */
+		goto err_free_card;
 
-		if (priv->wq) {
-			flush_workqueue(priv->wq);
-			destroy_workqueue(priv->wq);
-		}
+	if (priv->wq) {
+		flush_workqueue(priv->wq);
+		destroy_workqueue(priv->wq);
+	}
 
-		hostif_exit(priv);
+	hostif_exit(priv);
 
-		unregister_netdev(netdev);
+	unregister_netdev(priv->net_dev);
 
-		trx_device_exit(priv);
-		free_netdev(priv->net_dev);
-		card->priv = NULL;
-	}
+	trx_device_exit(priv);
+	free_netdev(priv->net_dev);
+	card->priv = NULL;
 
 	sdio_claim_host(func);
 	sdio_release_irq(func);
 	sdio_disable_func(func);
 	sdio_release_host(func);
+err_free_card:
 	sdio_set_drvdata(func, NULL);
 	kfree(card);
 }
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 12/14] staging: ks7010: use u8 instead of unsigned char for firmware buffers
  2018-04-12 15:50 [PATCH 00/14] cleanups continue Sergio Paracuellos
                   ` (10 preceding siblings ...)
  2018-04-12 15:50 ` [PATCH 11/14] staging: ks7010: fix error paths in ks7010_sdio_remove function Sergio Paracuellos
@ 2018-04-12 15:50 ` Sergio Paracuellos
  2018-04-12 15:50 ` [PATCH 13/14] staging: ks7010: assign dev_alloc_name() result to variable before check it Sergio Paracuellos
  2018-04-12 15:50 ` [PATCH 14/14] staging: ks7010: use linux circular buffer header macros to handle tx and rx queues Sergio Paracuellos
  13 siblings, 0 replies; 19+ messages in thread
From: Sergio Paracuellos @ 2018-04-12 15:50 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, wsa

This commit replaces type unsigned char which is the one which
is being used for firmware buffers with u8 type which is preferred.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks7010_sdio.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 82bc839..9fc9808 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -680,10 +680,10 @@ static int ks7010_sdio_update_index(struct ks_wlan_private *priv, u32 index)
 
 #define ROM_BUFF_SIZE (64 * 1024)
 static int ks7010_sdio_data_compare(struct ks_wlan_private *priv, u32 address,
-				    unsigned char *data, unsigned int size)
+				    u8 *data, unsigned int size)
 {
 	int ret;
-	unsigned char *read_buf;
+	u8 *read_buf;
 
 	read_buf = kmalloc(ROM_BUFF_SIZE, GFP_KERNEL);
 	if (!read_buf)
@@ -714,7 +714,7 @@ static int ks7010_copy_firmware(struct ks_wlan_private *priv,
 	unsigned int size;
 	unsigned int offset;
 	unsigned int n = 0;
-	unsigned char *rom_buf;
+	u8 *rom_buf;
 	int ret;
 
 	rom_buf = kmalloc(ROM_BUFF_SIZE, GFP_KERNEL);
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 13/14] staging: ks7010: assign dev_alloc_name() result to variable before check it
  2018-04-12 15:50 [PATCH 00/14] cleanups continue Sergio Paracuellos
                   ` (11 preceding siblings ...)
  2018-04-12 15:50 ` [PATCH 12/14] staging: ks7010: use u8 instead of unsigned char for firmware buffers Sergio Paracuellos
@ 2018-04-12 15:50 ` Sergio Paracuellos
  2018-04-12 15:50 ` [PATCH 14/14] staging: ks7010: use linux circular buffer header macros to handle tx and rx queues Sergio Paracuellos
  13 siblings, 0 replies; 19+ messages in thread
From: Sergio Paracuellos @ 2018-04-12 15:50 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, wsa

This commit assigns dev_alloc_name() call to 'ret' variable to
check it after instead of check directly the call in the if
condition. This improves a bit readability. It also add an empty
line before the new assignment to separate it from the previous
check statement block.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks7010_sdio.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 9fc9808..9c591e0 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -1015,7 +1015,9 @@ static int ks7010_sdio_probe(struct sdio_func *func,
 		dev_err(&card->func->dev, "Unable to alloc new net device\n");
 		goto err_release_irq;
 	}
-	if (dev_alloc_name(netdev, "wlan%d") < 0) {
+
+	ret = dev_alloc_name(netdev, "wlan%d");
+	if (ret < 0) {
 		dev_err(&card->func->dev, "Couldn't get name!\n");
 		goto err_free_netdev;
 	}
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 14/14] staging: ks7010: use linux circular buffer header macros to handle tx and rx queues
  2018-04-12 15:50 [PATCH 00/14] cleanups continue Sergio Paracuellos
                   ` (12 preceding siblings ...)
  2018-04-12 15:50 ` [PATCH 13/14] staging: ks7010: assign dev_alloc_name() result to variable before check it Sergio Paracuellos
@ 2018-04-12 15:50 ` Sergio Paracuellos
  2018-04-13 12:14   ` Dan Carpenter
  13 siblings, 1 reply; 19+ messages in thread
From: Sergio Paracuellos @ 2018-04-12 15:50 UTC (permalink / raw)
  To: gregkh; +Cc: driverdev-devel, wsa

This commit replace current custom implementation of some circular
buffer head and tail logic in favour of the use of macros defined
in linux circ_buf.h header. It also review internal names and adds
a new CIRC_INC macro to make code more readable. Note also that
CIRC_INC does not need to go inside do-while(0) block because its
use is only located in the four functions that make use of it 
so it won't expand into invalid code at all.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/ks7010/ks7010_sdio.c | 59 ++++++++++++++++++++++--------------
 1 file changed, 36 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 9c591e0..9676902 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -10,6 +10,7 @@
  *   published by the Free Software Foundation.
  */
 
+#include <linux/circ_buf.h>
 #include <linux/firmware.h>
 #include <linux/mmc/card.h>
 #include <linux/mmc/sdio_func.h>
@@ -101,38 +102,50 @@ enum gen_com_reg_b {
 
 #define KS7010_IO_BLOCK_SIZE 512
 
+#define CIRC_INC(a, b) if (++a >= b) a = 0
+
 static inline void inc_txqhead(struct ks_wlan_private *priv)
 {
-	priv->tx_dev.qhead = (priv->tx_dev.qhead + 1) % TX_DEVICE_BUFF_SIZE;
+	CIRC_INC(priv->tx_dev.qhead, TX_DEVICE_BUFF_SIZE);
 }
 
 static inline void inc_txqtail(struct ks_wlan_private *priv)
 {
-	priv->tx_dev.qtail = (priv->tx_dev.qtail + 1) % TX_DEVICE_BUFF_SIZE;
+	CIRC_INC(priv->tx_dev.qtail, TX_DEVICE_BUFF_SIZE);
 }
 
-static inline unsigned int cnt_txqbody(struct ks_wlan_private *priv)
+static inline bool txq_has_space(struct ks_wlan_private *priv)
 {
-	unsigned int tx_cnt = priv->tx_dev.qtail - priv->tx_dev.qhead;
-
-	return (tx_cnt + TX_DEVICE_BUFF_SIZE) % TX_DEVICE_BUFF_SIZE;
+	return (CIRC_SPACE(priv->tx_dev.qhead, priv->tx_dev.qtail,
+			   TX_DEVICE_BUFF_SIZE) > 0);
 }
 
 static inline void inc_rxqhead(struct ks_wlan_private *priv)
 {
-	priv->rx_dev.qhead = (priv->rx_dev.qhead + 1) % RX_DEVICE_BUFF_SIZE;
+	CIRC_INC(priv->rx_dev.qhead, RX_DEVICE_BUFF_SIZE);
 }
 
 static inline void inc_rxqtail(struct ks_wlan_private *priv)
 {
-	priv->rx_dev.qtail = (priv->rx_dev.qtail + 1) % RX_DEVICE_BUFF_SIZE;
+	CIRC_INC(priv->rx_dev.qtail, RX_DEVICE_BUFF_SIZE);
 }
 
-static inline unsigned int cnt_rxqbody(struct ks_wlan_private *priv)
+static inline bool rxq_has_space(struct ks_wlan_private *priv)
 {
-	unsigned int rx_cnt = priv->rx_dev.qtail - priv->rx_dev.qhead;
+	return (CIRC_SPACE(priv->rx_dev.qhead, priv->rx_dev.qtail,
+			   RX_DEVICE_BUFF_SIZE) > 0);
+}
 
-	return (rx_cnt + RX_DEVICE_BUFF_SIZE) % RX_DEVICE_BUFF_SIZE;
+static inline unsigned int txq_count(struct ks_wlan_private *priv)
+{
+	return CIRC_CNT_TO_END(priv->tx_dev.qhead, priv->tx_dev.qtail,
+			       TX_DEVICE_BUFF_SIZE);
+}
+
+static inline unsigned int rxq_count(struct ks_wlan_private *priv)
+{
+	return CIRC_CNT_TO_END(priv->rx_dev.qhead, priv->rx_dev.qtail,
+			       RX_DEVICE_BUFF_SIZE);
 }
 
 /* Read single byte from device address into byte (CMD52) */
@@ -258,11 +271,11 @@ static void _ks_wlan_hw_power_save(struct ks_wlan_private *priv)
 		   atomic_read(&priv->psstatus.status),
 		   atomic_read(&priv->psstatus.confirm_wait),
 		   atomic_read(&priv->psstatus.snooze_guard),
-		   cnt_txqbody(priv));
+		   txq_count(priv));
 
 	if (atomic_read(&priv->psstatus.confirm_wait) ||
 	    atomic_read(&priv->psstatus.snooze_guard) ||
-	    cnt_txqbody(priv)) {
+	    txq_has_space(priv)) {
 		queue_delayed_work(priv->wq, &priv->rw_dwork, 0);
 		return;
 	}
@@ -308,7 +321,7 @@ static int enqueue_txdev(struct ks_wlan_private *priv, unsigned char *p,
 		goto err_complete;
 	}
 
-	if ((TX_DEVICE_BUFF_SIZE - 1) <= cnt_txqbody(priv)) {
+	if ((TX_DEVICE_BUFF_SIZE - 1) <= txq_count(priv)) {
 		netdev_err(priv->net_dev, "tx buffer overflow\n");
 		ret = -EOVERFLOW;
 		goto err_complete;
@@ -366,7 +379,7 @@ static void tx_device_task(struct ks_wlan_private *priv)
 	struct tx_device_buffer *sp;
 	int ret;
 
-	if (cnt_txqbody(priv) <= 0 ||
+	if (!txq_has_space(priv) ||
 	    atomic_read(&priv->psstatus.status) == PS_SNOOZE)
 		return;
 
@@ -385,7 +398,7 @@ static void tx_device_task(struct ks_wlan_private *priv)
 		(*sp->complete_handler)(priv, sp->skb);
 	inc_txqhead(priv);
 
-	if (cnt_txqbody(priv) > 0)
+	if (txq_has_space(priv))
 		queue_delayed_work(priv->wq, &priv->rw_dwork, 0);
 }
 
@@ -413,7 +426,7 @@ int ks_wlan_hw_tx(struct ks_wlan_private *priv, void *p, unsigned long size,
 	result = enqueue_txdev(priv, p, size, complete_handler, skb);
 	spin_unlock(&priv->tx_dev.tx_dev_lock);
 
-	if (cnt_txqbody(priv) > 0)
+	if (txq_has_space(priv))
 		queue_delayed_work(priv->wq, &priv->rw_dwork, 0);
 
 	return result;
@@ -424,12 +437,12 @@ static void rx_event_task(unsigned long dev)
 	struct ks_wlan_private *priv = (struct ks_wlan_private *)dev;
 	struct rx_device_buffer *rp;
 
-	if (cnt_rxqbody(priv) > 0 && priv->dev_state >= DEVICE_STATE_BOOT) {
+	if (rxq_has_space(priv) && priv->dev_state >= DEVICE_STATE_BOOT) {
 		rp = &priv->rx_dev.rx_dev_buff[priv->rx_dev.qhead];
 		hostif_receive(priv, rp->data, rp->size);
 		inc_rxqhead(priv);
 
-		if (cnt_rxqbody(priv) > 0)
+		if (rxq_has_space(priv))
 			tasklet_schedule(&priv->rx_bh_task);
 	}
 }
@@ -442,7 +455,7 @@ static void ks_wlan_hw_rx(struct ks_wlan_private *priv, uint16_t size)
 	unsigned short event = 0;
 
 	/* receive data */
-	if (cnt_rxqbody(priv) >= (RX_DEVICE_BUFF_SIZE - 1)) {
+	if (rxq_count(priv) >= (RX_DEVICE_BUFF_SIZE - 1)) {
 		netdev_err(priv->net_dev, "rx buffer overflow\n");
 		return;
 	}
@@ -513,7 +526,7 @@ static void ks7010_rw_function(struct work_struct *work)
 
 	/* power save wakeup */
 	if (atomic_read(&priv->psstatus.status) == PS_SNOOZE) {
-		if (cnt_txqbody(priv) > 0) {
+		if (txq_has_space(priv)) {
 			ks_wlan_hw_wakeup_request(priv);
 			queue_delayed_work(priv->wq, &priv->rw_dwork, 1);
 		}
@@ -604,7 +617,7 @@ static void ks_sdio_interrupt(struct sdio_func *func)
 
 		if (byte & WSTATUS_MASK) {
 			if (atomic_read(&priv->psstatus.status) == PS_SNOOZE) {
-				if (cnt_txqbody(priv)) {
+				if (txq_has_space(priv)) {
 					ks_wlan_hw_wakeup_request(priv);
 					queue_delayed_work(priv->wq,
 							   &priv->rw_dwork, 1);
@@ -641,7 +654,7 @@ static void trx_device_exit(struct ks_wlan_private *priv)
 	struct tx_device_buffer *sp;
 
 	/* tx buffer clear */
-	while (cnt_txqbody(priv) > 0) {
+	while (txq_has_space(priv)) {
 		sp = &priv->tx_dev.tx_dev_buff[priv->tx_dev.qhead];
 		kfree(sp->sendp);
 		if (sp->complete_handler)	/* TX Complete */
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 14/14] staging: ks7010: use linux circular buffer header macros to handle tx and rx queues
  2018-04-12 15:50 ` [PATCH 14/14] staging: ks7010: use linux circular buffer header macros to handle tx and rx queues Sergio Paracuellos
@ 2018-04-13 12:14   ` Dan Carpenter
  2018-04-13 13:19     ` Sergio Paracuellos
  0 siblings, 1 reply; 19+ messages in thread
From: Dan Carpenter @ 2018-04-13 12:14 UTC (permalink / raw)
  To: Sergio Paracuellos; +Cc: gregkh, driverdev-devel, wsa

On Thu, Apr 12, 2018 at 05:50:31PM +0200, Sergio Paracuellos wrote:
> This commit replace current custom implementation of some circular
> buffer head and tail logic in favour of the use of macros defined
> in linux circ_buf.h header. It also review internal names and adds
> a new CIRC_INC macro to make code more readable. Note also that
> CIRC_INC does not need to go inside do-while(0) block because its
> use is only located in the four functions that make use of it 
> so it won't expand into invalid code at all.
> 
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  drivers/staging/ks7010/ks7010_sdio.c | 59 ++++++++++++++++++++++--------------
>  1 file changed, 36 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
> index 9c591e0..9676902 100644
> --- a/drivers/staging/ks7010/ks7010_sdio.c
> +++ b/drivers/staging/ks7010/ks7010_sdio.c
> @@ -10,6 +10,7 @@
>   *   published by the Free Software Foundation.
>   */
>  
> +#include <linux/circ_buf.h>
>  #include <linux/firmware.h>
>  #include <linux/mmc/card.h>
>  #include <linux/mmc/sdio_func.h>
> @@ -101,38 +102,50 @@ enum gen_com_reg_b {
>  
>  #define KS7010_IO_BLOCK_SIZE 512
>  
> +#define CIRC_INC(a, b) if (++a >= b) a = 0

I don't like this macro.  If we're going to call it CIRC_INC() then it
needs to be included in include/linux/circ_buf.h and not here.  I don't
like that the argument is  "b" instead of "size" or something.  It
should be wrapped in a do { } while(0).  There should be parens around
"a" and "b" so I don't have to think about precedence bugs.

regards,
dan carpenter

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 14/14] staging: ks7010: use linux circular buffer header macros to handle tx and rx queues
  2018-04-13 12:14   ` Dan Carpenter
@ 2018-04-13 13:19     ` Sergio Paracuellos
  2018-04-13 13:27       ` Dan Carpenter
  0 siblings, 1 reply; 19+ messages in thread
From: Sergio Paracuellos @ 2018-04-13 13:19 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: Greg KH, driverdev-devel, wsa

On Fri, Apr 13, 2018 at 2:14 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> On Thu, Apr 12, 2018 at 05:50:31PM +0200, Sergio Paracuellos wrote:
>> This commit replace current custom implementation of some circular
>> buffer head and tail logic in favour of the use of macros defined
>> in linux circ_buf.h header. It also review internal names and adds
>> a new CIRC_INC macro to make code more readable. Note also that
>> CIRC_INC does not need to go inside do-while(0) block because its
>> use is only located in the four functions that make use of it
>> so it won't expand into invalid code at all.
>>
>> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
>> ---
>>  drivers/staging/ks7010/ks7010_sdio.c | 59 ++++++++++++++++++++++--------------
>>  1 file changed, 36 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
>> index 9c591e0..9676902 100644
>> --- a/drivers/staging/ks7010/ks7010_sdio.c
>> +++ b/drivers/staging/ks7010/ks7010_sdio.c
>> @@ -10,6 +10,7 @@
>>   *   published by the Free Software Foundation.
>>   */
>>
>> +#include <linux/circ_buf.h>
>>  #include <linux/firmware.h>
>>  #include <linux/mmc/card.h>
>>  #include <linux/mmc/sdio_func.h>
>> @@ -101,38 +102,50 @@ enum gen_com_reg_b {
>>
>>  #define KS7010_IO_BLOCK_SIZE 512
>>
>> +#define CIRC_INC(a, b) if (++a >= b) a = 0
>
> I don't like this macro.  If we're going to call it CIRC_INC() then it
> needs to be included in include/linux/circ_buf.h and not here.  I don't
> like that the argument is  "b" instead of "size" or something.  It
> should be wrapped in a do { } while(0).  There should be parens around
> "a" and "b" so I don't have to think about precedence bugs.

Ok, I'll send v2 using other macros included in
include/linux/circ_buf.h but avoiding
the use of this new one.

Thanks, Dan.

>
> regards,
> dan carpenter

Best regards,
    Sergio Paracuellos
>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 14/14] staging: ks7010: use linux circular buffer header macros to handle tx and rx queues
  2018-04-13 13:19     ` Sergio Paracuellos
@ 2018-04-13 13:27       ` Dan Carpenter
  2018-04-13 13:32         ` Sergio Paracuellos
  0 siblings, 1 reply; 19+ messages in thread
From: Dan Carpenter @ 2018-04-13 13:27 UTC (permalink / raw)
  To: Sergio Paracuellos; +Cc: Greg KH, driverdev-devel, wsa


This is the last patch in the series so I think we could just drop it
without resending the others.

regards,
dan carpenter

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 14/14] staging: ks7010: use linux circular buffer header macros to handle tx and rx queues
  2018-04-13 13:27       ` Dan Carpenter
@ 2018-04-13 13:32         ` Sergio Paracuellos
  0 siblings, 0 replies; 19+ messages in thread
From: Sergio Paracuellos @ 2018-04-13 13:32 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: Greg KH, driverdev-devel, wsa

On Fri, Apr 13, 2018 at 3:27 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> This is the last patch in the series so I think we could just drop it
> without resending the others.

True. Ok, I'll resend the last one modified in my next series.

>
> regards,
> dan carpenter
>

Best regards,
    Sergio Paracuellos
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2018-04-13 13:32 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-12 15:50 [PATCH 00/14] cleanups continue Sergio Paracuellos
2018-04-12 15:50 ` [PATCH 01/14] staging: ks7010: move ROM_FILE definition into source file Sergio Paracuellos
2018-04-12 15:50 ` [PATCH 02/14] staging: ks7010: move sdio specific register definitions " Sergio Paracuellos
2018-04-12 15:50 ` [PATCH 03/14] staging: ks7010: delete not used definitions in ks7010_sdio source Sergio Paracuellos
2018-04-12 15:50 ` [PATCH 04/14] staging: ks7010: add REG suffix to sdio register definitions Sergio Paracuellos
2018-04-12 15:50 ` [PATCH 05/14] staging: ks7010: review comment style in ks7010_sdio source file Sergio Paracuellos
2018-04-12 15:50 ` [PATCH 06/14] staging: ks7010: review debug and error messages in ks7010_sdio source Sergio Paracuellos
2018-04-12 15:50 ` [PATCH 07/14] staging: ks7010: avoid one extra level indentation in ks_wlan_hw_rx function Sergio Paracuellos
2018-04-12 15:50 ` [PATCH 08/14] staging: ks7010: move MODULE_DEVICE_TABLE related code Sergio Paracuellos
2018-04-12 15:50 ` [PATCH 09/14] staging: ks7010: replace create_workqueue with alloc_workqueue Sergio Paracuellos
2018-04-12 15:50 ` [PATCH 10/14] staging: ks7010: check sdio_set_block_size return value Sergio Paracuellos
2018-04-12 15:50 ` [PATCH 11/14] staging: ks7010: fix error paths in ks7010_sdio_remove function Sergio Paracuellos
2018-04-12 15:50 ` [PATCH 12/14] staging: ks7010: use u8 instead of unsigned char for firmware buffers Sergio Paracuellos
2018-04-12 15:50 ` [PATCH 13/14] staging: ks7010: assign dev_alloc_name() result to variable before check it Sergio Paracuellos
2018-04-12 15:50 ` [PATCH 14/14] staging: ks7010: use linux circular buffer header macros to handle tx and rx queues Sergio Paracuellos
2018-04-13 12:14   ` Dan Carpenter
2018-04-13 13:19     ` Sergio Paracuellos
2018-04-13 13:27       ` Dan Carpenter
2018-04-13 13:32         ` Sergio Paracuellos

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.