All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Tobin C. Harding" <me@tobin.cc>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Wolfram Sang <wsa@the-dreams.de>, driverdev-devel@linuxdriverproject.org
Subject: [PATCH 07/15] staging: ks7010: move hw info into dev private data
Date: Tue, 18 Apr 2017 10:35:35 +1000	[thread overview]
Message-ID: <1492475743-25189-8-git-send-email-me@tobin.cc> (raw)
In-Reply-To: <1492475743-25189-1-git-send-email-me@tobin.cc>

Currently driver uses a hardware information struct description to
group some SDIO related functionality (work, work queue, sdio private
data pointer). This structure is then embedded in the device private
data structure. Having nested structures described in different header
files means that to view the device private data programmers must open
two header files. This structure could be embedded anonymously in the
device private data and achieve the same result (grouping of function
specific to SDIO) without the need to open multiple headers. However,
the SDIO private data structure already has various different data and
pointers, adding the embedded structure adds little extra meaning and
lengthens all the dereferences throughout the driver, often meaning
addition line breaks and braces. We can increase readability and
reduce code complexity by moving the hardware information data and
pointers to directly be within the device private data structure
description.

While preparing for this refactoring it was noted that the identifier
currently used for the delayed work is 'rw_wq', this is confusing
since the 'wq' suffix typically means 'work queue'. This identifier
would be more meaningful if it used the suffix 'dwork' as does the
declaration of queue_delayed_work() (include/linux/workqueue.h).

The identifier for the work queue is currently 'ks7010sdio_wq'. This
identifier can be shortened without loss of meaning because there is
only one work queue within the driver. Identifier 'wq' is typical
within in-tree driver code and aptly describes the pointer.

Current pointer to the SDIO private data is identified by 'sdio_card',
this is sufficiently meaningful from within the hw_info structure but
once the hw_info_t structure is removed the pointer would be better to
have a prefix appended to it to retain the prior level of meaning.

Move members from struct hw_info_t to struct ks_wlan_private.

Rename identifiers;
struct delayed_work pointer 'rw_wq' to 'rw_dwork'.
struct workqueue_struct pointer 'ks7010sdio_wq' to 'wq'.
struct ks_sdio_card  pointer 'sdio_card' to 'ks_sdio_card'.

Remove structure description hw_info_t. Fix init/destroy calls. Fix
all call sites, SDIO private data access calls, and queuing calls.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 drivers/staging/ks7010/ks7010_sdio.c | 70 +++++++++++++++---------------------
 drivers/staging/ks7010/ks7010_sdio.h |  6 ----
 drivers/staging/ks7010/ks_hostif.c   |  6 ++--
 drivers/staging/ks7010/ks_wlan.h     |  5 ++-
 4 files changed, 34 insertions(+), 53 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 2d73ebb..e3e2989 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -54,7 +54,7 @@ static int ks7010_sdio_read(struct ks_wlan_private *priv, unsigned int address,
 	struct ks_sdio_card *card;
 	int ret;
 
-	card = priv->ks_wlan_hw.sdio_card;
+	card = priv->ks_sdio_card;
 
 	if (length == 1)	/* CMD52 */
 		*buffer = sdio_readb(card->func, address, &ret);
@@ -75,7 +75,7 @@ static int ks7010_sdio_write(struct ks_wlan_private *priv, unsigned int address,
 	struct ks_sdio_card *card;
 	int ret;
 
-	card = priv->ks_wlan_hw.sdio_card;
+	card = priv->ks_sdio_card;
 
 	if (length == 1)	/* CMD52 */
 		sdio_writeb(card->func, *buffer, address, &ret);
@@ -198,8 +198,7 @@ static void _ks_wlan_hw_power_save(struct ks_wlan_private *priv)
 	if (atomic_read(&priv->psstatus.confirm_wait) ||
 	    atomic_read(&priv->psstatus.snooze_guard) ||
 	    cnt_txqbody(priv)) {
-		queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
-				   &priv->ks_wlan_hw.rw_wq, 0);
+		queue_delayed_work(priv->wq, &priv->rw_dwork, 0);
 		return;
 	}
 
@@ -224,14 +223,12 @@ static void _ks_wlan_hw_power_save(struct ks_wlan_private *priv)
 	return;
 
 queue_delayed_work:
-	queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
-			   &priv->ks_wlan_hw.rw_wq, 1);
+	queue_delayed_work(priv->wq, &priv->rw_dwork, 1);
 }
 
 int ks_wlan_hw_power_save(struct ks_wlan_private *priv)
 {
-	queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
-			   &priv->ks_wlan_hw.rw_wq, 1);
+	queue_delayed_work(priv->wq, &priv->rw_dwork, 1);
 	return 0;
 }
 
@@ -320,8 +317,7 @@ static void tx_device_task(struct ks_wlan_private *priv)
 		ret = write_to_device(priv, sp->sendp, sp->size);
 		if (ret) {
 			DPRINTK(1, "write_to_device error !!(%d)\n", ret);
-			queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
-					   &priv->ks_wlan_hw.rw_wq, 1);
+			queue_delayed_work(priv->wq, &priv->rw_dwork, 1);
 			return;
 		}
 	}
@@ -330,10 +326,8 @@ static void tx_device_task(struct ks_wlan_private *priv)
 		(*sp->complete_handler)(priv, sp->skb);
 	inc_txqhead(priv);
 
-	if (cnt_txqbody(priv) > 0) {
-		queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
-				   &priv->ks_wlan_hw.rw_wq, 0);
-	}
+	if (cnt_txqbody(priv) > 0)
+		queue_delayed_work(priv->wq, &priv->rw_dwork, 0);
 }
 
 int ks_wlan_hw_tx(struct ks_wlan_private *priv, void *p, unsigned long size,
@@ -360,10 +354,9 @@ 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) {
-		queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
-				   &priv->ks_wlan_hw.rw_wq, 0);
-	}
+	if (cnt_txqbody(priv) > 0)
+		queue_delayed_work(priv->wq, &priv->rw_dwork, 0);
+
 	return result;
 }
 
@@ -452,42 +445,38 @@ static void ks_wlan_hw_rx(struct ks_wlan_private *priv, uint16_t size)
 
 static void ks7010_rw_function(struct work_struct *work)
 {
-	struct hw_info_t *hw;
 	struct ks_wlan_private *priv;
 	unsigned char rw_data;
 	int ret;
 
-	hw = container_of(work, struct hw_info_t, rw_wq.work);
-	priv = container_of(hw, struct ks_wlan_private, ks_wlan_hw);
+	priv = container_of(work, struct ks_wlan_private, rw_dwork.work);
 
 	DPRINTK(4, "\n");
 
 	/* wiat after DOZE */
 	if (time_after(priv->last_doze + ((30 * HZ) / 1000), jiffies)) {
 		DPRINTK(4, "wait after DOZE\n");
-		queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
-				   &priv->ks_wlan_hw.rw_wq, 1);
+		queue_delayed_work(priv->wq, &priv->rw_dwork, 1);
 		return;
 	}
 
 	/* wiat after WAKEUP */
 	while (time_after(priv->last_wakeup + ((30 * HZ) / 1000), jiffies)) {
 		DPRINTK(4, "wait after WAKEUP\n");
-		dev_info(&priv->ks_wlan_hw.sdio_card->func->dev,
+		dev_info(&priv->ks_sdio_card->func->dev,
 			 "wake: %lu %lu\n",
 			 priv->last_wakeup + (30 * HZ) / 1000,
 				jiffies);
 		msleep(30);
 	}
 
-	sdio_claim_host(priv->ks_wlan_hw.sdio_card->func);
+	sdio_claim_host(priv->ks_sdio_card->func);
 
 	/* power save wakeup */
 	if (atomic_read(&priv->psstatus.status) == PS_SNOOZE) {
 		if (cnt_txqbody(priv) > 0) {
 			ks_wlan_hw_wakeup_request(priv);
-			queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
-					   &priv->ks_wlan_hw.rw_wq, 1);
+			queue_delayed_work(priv->wq, &priv->rw_dwork, 1);
 		}
 		goto err_release_host;
 	}
@@ -521,7 +510,7 @@ static void ks7010_rw_function(struct work_struct *work)
 	_ks_wlan_hw_power_save(priv);
 
 err_release_host:
-	sdio_release_host(priv->ks_wlan_hw.sdio_card->func);
+	sdio_release_host(priv->ks_sdio_card->func);
 }
 
 static void ks_sdio_interrupt(struct sdio_func *func)
@@ -584,8 +573,7 @@ static void ks_sdio_interrupt(struct sdio_func *func)
 			if (atomic_read(&priv->psstatus.status) == PS_SNOOZE) {
 				if (cnt_txqbody(priv)) {
 					ks_wlan_hw_wakeup_request(priv);
-					queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
-							   &priv->ks_wlan_hw.rw_wq, 1);
+					queue_delayed_work(priv->wq, &priv->rw_dwork, 1);
 					return;
 				}
 			} else {
@@ -595,8 +583,7 @@ static void ks_sdio_interrupt(struct sdio_func *func)
 	} while (rsize);
 
 queue_delayed_work:
-	queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
-			   &priv->ks_wlan_hw.rw_wq, 0);
+	queue_delayed_work(priv->wq, &priv->rw_dwork, 0);
 }
 
 static int trx_device_init(struct ks_wlan_private *priv)
@@ -714,7 +701,7 @@ static int ks7010_upload_firmware(struct ks_sdio_card *card)
 	}
 
 	ret = request_firmware(&fw_entry, ROM_FILE,
-			       &priv->ks_wlan_hw.sdio_card->func->dev);
+			       &priv->ks_sdio_card->func->dev);
 	if (ret)
 		goto release_host_and_free;
 
@@ -949,7 +936,7 @@ static int ks7010_sdio_probe(struct sdio_func *func,
 	SET_NETDEV_DEV(netdev, &card->func->dev);	/* for create sysfs symlinks */
 
 	/* private memory initialize */
-	priv->ks_wlan_hw.sdio_card = card;
+	priv->ks_sdio_card = card;
 
 	priv->dev_state = DEVICE_STATE_PREBOOT;
 	priv->net_dev = netdev;
@@ -1001,13 +988,13 @@ static int ks7010_sdio_probe(struct sdio_func *func,
 	DPRINTK(4, " enable Interrupt : INT_ENABLE=%02X\n", rw_data);
 	priv->dev_state = DEVICE_STATE_BOOT;
 
-	priv->ks_wlan_hw.ks7010sdio_wq = create_workqueue("ks7010sdio_wq");
-	if (!priv->ks_wlan_hw.ks7010sdio_wq) {
+	priv->wq = create_workqueue("wq");
+	if (!priv->wq) {
 		DPRINTK(1, "create_workqueue failed !!\n");
 		goto err_free_netdev;
 	}
 
-	INIT_DELAYED_WORK(&priv->ks_wlan_hw.rw_wq, ks7010_rw_function);
+	INIT_DELAYED_WORK(&priv->rw_dwork, ks7010_rw_function);
 	ks7010_card_init(priv);
 
 	ret = register_netdev(priv->net_dev);
@@ -1095,12 +1082,11 @@ static void ks7010_sdio_remove(struct sdio_func *func)
 
 		DPRINTK(1, "STOP Req\n");
 
-		if (priv->ks_wlan_hw.ks7010sdio_wq) {
-			flush_workqueue(priv->ks_wlan_hw.ks7010sdio_wq);
-			destroy_workqueue(priv->ks_wlan_hw.ks7010sdio_wq);
+		if (priv->wq) {
+			flush_workqueue(priv->wq);
+			destroy_workqueue(priv->wq);
 		}
-		DPRINTK(1,
-			"destroy_workqueue(priv->ks_wlan_hw.ks7010sdio_wq);\n");
+		DPRINTK(1, "destroy_workqueue(priv->wq);\n");
 
 		hostif_exit(priv);
 		DPRINTK(1, "hostif_exit\n");
diff --git a/drivers/staging/ks7010/ks7010_sdio.h b/drivers/staging/ks7010/ks7010_sdio.h
index 76c6b102..22c7ba7 100644
--- a/drivers/staging/ks7010/ks7010_sdio.h
+++ b/drivers/staging/ks7010/ks7010_sdio.h
@@ -85,12 +85,6 @@ enum gen_com_reg_b {
 
 #define KS7010_IRAM_ADDRESS	0x06000000
 
-struct hw_info_t {
-	struct ks_sdio_card *sdio_card;
-	struct workqueue_struct *ks7010sdio_wq;
-	struct delayed_work rw_wq;
-};
-
 struct ks_sdio_card {
 	struct sdio_func *func;
 	struct ks_wlan_private *priv;
diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c
index 2a54b4c..7151f16 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -743,8 +743,7 @@ void hostif_sleep_confirm(struct ks_wlan_private *priv)
 	DPRINTK(3, "\n");
 
 	atomic_set(&priv->sleepstatus.doze_request, 1);
-	queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
-			   &priv->ks_wlan_hw.rw_wq, 1);
+	queue_delayed_work(priv->wq, &priv->rw_dwork, 1);
 }
 
 static
@@ -1745,8 +1744,7 @@ void hostif_sleep_request(struct ks_wlan_private *priv, unsigned long mode)
 			      NULL);
 	} else if (mode == SLP_ACTIVE) {
 		atomic_set(&priv->sleepstatus.wakeup_request, 1);
-		queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,
-				   &priv->ks_wlan_hw.rw_wq, 1);
+		queue_delayed_work(priv->wq, &priv->rw_dwork, 1);
 	} else {
 		DPRINTK(3, "invalid mode %ld\n", mode);
 		return;
diff --git a/drivers/staging/ks7010/ks_wlan.h b/drivers/staging/ks7010/ks_wlan.h
index b0bc7a8..eb15db9 100644
--- a/drivers/staging/ks7010/ks_wlan.h
+++ b/drivers/staging/ks7010/ks_wlan.h
@@ -413,7 +413,10 @@ struct wps_status_t {
 #endif /* WPS */
 
 struct ks_wlan_private {
-	struct hw_info_t ks_wlan_hw;	/* hardware information */
+	/* hardware information */
+	struct ks_sdio_card *ks_sdio_card;
+	struct workqueue_struct *wq;
+	struct delayed_work rw_dwork;
 	struct tasklet_struct rx_bh_task;
 
 	struct net_device *net_dev;
-- 
2.7.4

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

  parent reply	other threads:[~2017-04-18  0:36 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-18  0:35 [PATCH 00/15] staging: ks7010: refactor SDIO code Tobin C. Harding
2017-04-18  0:35 ` [PATCH 01/15] staging: ks7010: create reg_status_type enum type Tobin C. Harding
2017-04-18  0:35 ` [PATCH 02/15] staging: ks7010: replace defines with enum types Tobin C. Harding
2017-04-18  0:35 ` [PATCH 03/15] staging: ks7010: fix complete_handler Tobin C. Harding
2017-04-18  0:35 ` [PATCH 04/15] staging: ks7010: clean up SDIO header comments Tobin C. Harding
2017-04-18  0:35 ` [PATCH 05/15] staging: ks7010: rename wakeup work struct Tobin C. Harding
2017-04-18  0:35 ` [PATCH 06/15] staging: ks7010: move tasklet_struct to ks_wlan_private Tobin C. Harding
2017-04-18  0:35 ` Tobin C. Harding [this message]
2017-04-18  0:35 ` [PATCH 08/15] staging: ks7010: add struct comment to ks_sdio_card Tobin C. Harding
2017-04-18  0:35 ` [PATCH 09/15] staging: ks7010: clean up SDIO source comments Tobin C. Harding
2017-04-18  0:35 ` [PATCH 10/15] staging: ks7010: remove err_ from non-error path label Tobin C. Harding
2017-04-18  0:35 ` [PATCH 11/15] staging: ks7010: fix checkpatch SPACE_BEFORE_TAB Tobin C. Harding
2017-04-18  0:35 ` [PATCH 12/15] staging: ks7010: fix checkpatch LINE_SPACING Tobin C. Harding
2017-04-18  0:35 ` [PATCH 13/15] staging: ks7010: refactor SDIO read/write helpers Tobin C. Harding
2017-04-18  0:35 ` [PATCH 14/15] staging: ks7010: fix checkpatch SPLIT_STRING Tobin C. Harding
2017-04-18  0:35 ` [PATCH 15/15] staging: ks7010: rename SDIO files Tobin C. Harding
2017-04-18 11:47   ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1492475743-25189-8-git-send-email-me@tobin.cc \
    --to=me@tobin.cc \
    --cc=driverdev-devel@linuxdriverproject.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=wsa@the-dreams.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.