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@xxxxxxxx> --- 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@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel