Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1678473 > unrolled thread
| Started by | Mark Rogers <mail@mark-rogers.org> |
|---|---|
| First post | 2017-06-30 07:00 +0200 |
| Last post | 2017-06-30 20:40 +0200 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH] staging: ks7010: fix styling WARNINGs Mark Rogers <mail@mark-rogers.org> - 2017-06-30 07:00 +0200
Re: [PATCH] staging: ks7010: fix styling WARNINGs Frans Klaver <fransklaver@gmail.com> - 2017-06-30 08:00 +0200
Re: [PATCH] staging: ks7010: fix styling WARNINGs Mark Rogers <mail@mark-rogers.org> - 2017-06-30 20:40 +0200
| From | Mark Rogers <mail@mark-rogers.org> |
|---|---|
| Date | 2017-06-30 07:00 +0200 |
| Subject | [PATCH] staging: ks7010: fix styling WARNINGs |
| Message-ID | <tXWc9-KG-15@gated-at.bofh.it> |
Trivial style changes. There are still 3 "line over 80 characters"
checkpatch.pl warnings, but I think they are best left alone as
breaking the first two warning lines could hurt readability. The third
warning is a message that should not be broken for the sake of grep.
All but one of the changes fix lines that exceed 80 characters. An
embedded function name was replaced by __func__ as well.
Signed-off-by: Mark Rogers <mail@mark-rogers.org>
---
drivers/staging/ks7010/ks7010_sdio.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index c325f48..6c0c6b2 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -548,7 +548,8 @@ 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->wq, &priv->rw_dwork, 1);
+ queue_delayed_work(priv->wq,
+ &priv->rw_dwork, 1);
return;
}
} else {
@@ -693,15 +694,18 @@ static int ks7010_upload_firmware(struct ks_sdio_card *card)
memcpy(rom_buf, fw_entry->data + n, size);
offset = n;
- ret = ks7010_sdio_update_index(priv, KS7010_IRAM_ADDRESS + offset);
+ ret = ks7010_sdio_update_index(priv,
+ KS7010_IRAM_ADDRESS + offset);
if (ret)
goto release_firmware;
- ret = ks7010_sdio_write(priv, DATA_WINDOW, rom_buf, size);
+ ret = ks7010_sdio_write(priv,
+ DATA_WINDOW, rom_buf, size);
if (ret)
goto release_firmware;
- ret = ks7010_sdio_data_compare(priv, DATA_WINDOW, rom_buf, size);
+ ret = ks7010_sdio_data_compare(priv,
+ DATA_WINDOW, rom_buf, size);
if (ret)
goto release_firmware;
@@ -889,7 +893,7 @@ static int ks7010_sdio_probe(struct sdio_func *func,
priv = netdev_priv(netdev);
card->priv = priv;
- SET_NETDEV_DEV(netdev, &card->func->dev); /* for create sysfs symlinks */
+ SET_NETDEV_DEV(netdev, &card->func->dev);/* for create sysfs symlinks */
/* private memory initialize */
priv->ks_sdio_card = card;
@@ -923,7 +927,7 @@ static int ks7010_sdio_probe(struct sdio_func *func,
}
/* interrupt setting */
- /* clear Interrupt status write (ARMtoSD_InterruptPending FN1:00_0024) */
+ /* clear Interrupt status write (ARMtoSD_InterruptPending FN1:00_0024)*/
sdio_claim_host(func);
ret = ks7010_sdio_writeb(priv, INT_PENDING, 0xff);
sdio_release_host(func);
@@ -1006,7 +1010,7 @@ static void ks7010_sdio_remove(struct sdio_func *func)
struct ks_sdio_card *card;
struct ks_wlan_private *priv;
- DPRINTK(1, "ks7010_sdio_remove()\n");
+ DPRINTK(1, "%s()\n", __func__);
card = sdio_get_drvdata(func);
--
2.7.4
[toc] | [next] | [standalone]
| From | Frans Klaver <fransklaver@gmail.com> |
|---|---|
| Date | 2017-06-30 08:00 +0200 |
| Message-ID | <tXX8e-1pk-5@gated-at.bofh.it> |
| In reply to | #1678473 |
On Fri, Jun 30, 2017 at 6:52 AM, Mark Rogers <mail@mark-rogers.org> wrote:
> Trivial style changes. There are still 3 "line over 80 characters"
> checkpatch.pl warnings, but I think they are best left alone as
> breaking the first two warning lines could hurt readability. The third
> warning is a message that should not be broken for the sake of grep.
>
> All but one of the changes fix lines that exceed 80 characters. An
> embedded function name was replaced by __func__ as well.
>
> Signed-off-by: Mark Rogers <mail@mark-rogers.org>
> ---
> drivers/staging/ks7010/ks7010_sdio.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
> index c325f48..6c0c6b2 100644
> --- a/drivers/staging/ks7010/ks7010_sdio.c
> +++ b/drivers/staging/ks7010/ks7010_sdio.c
> @@ -548,7 +548,8 @@ 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->wq, &priv->rw_dwork, 1);
> + queue_delayed_work(priv->wq,
> + &priv->rw_dwork, 1);
> return;
> }
> } else {
> @@ -693,15 +694,18 @@ static int ks7010_upload_firmware(struct ks_sdio_card *card)
> memcpy(rom_buf, fw_entry->data + n, size);
>
> offset = n;
> - ret = ks7010_sdio_update_index(priv, KS7010_IRAM_ADDRESS + offset);
> + ret = ks7010_sdio_update_index(priv,
> + KS7010_IRAM_ADDRESS + offset);
> if (ret)
> goto release_firmware;
>
> - ret = ks7010_sdio_write(priv, DATA_WINDOW, rom_buf, size);
> + ret = ks7010_sdio_write(priv,
> + DATA_WINDOW, rom_buf, size);
> if (ret)
> goto release_firmware;
>
> - ret = ks7010_sdio_data_compare(priv, DATA_WINDOW, rom_buf, size);
> + ret = ks7010_sdio_data_compare(priv,
> + DATA_WINDOW, rom_buf, size);
> if (ret)
> goto release_firmware;
>
> @@ -889,7 +893,7 @@ static int ks7010_sdio_probe(struct sdio_func *func,
> priv = netdev_priv(netdev);
>
> card->priv = priv;
> - SET_NETDEV_DEV(netdev, &card->func->dev); /* for create sysfs symlinks */
> + SET_NETDEV_DEV(netdev, &card->func->dev);/* for create sysfs symlinks */
I don't think this is much of an improvement for readability. Should
we move the comment about a bit?
>
> /* private memory initialize */
> priv->ks_sdio_card = card;
> @@ -923,7 +927,7 @@ static int ks7010_sdio_probe(struct sdio_func *func,
> }
>
> /* interrupt setting */
> - /* clear Interrupt status write (ARMtoSD_InterruptPending FN1:00_0024) */
> + /* clear Interrupt status write (ARMtoSD_InterruptPending FN1:00_0024)*/
This is a bit of a pointless change, isn't it? It also makes the comment uglier.
> sdio_claim_host(func);
> ret = ks7010_sdio_writeb(priv, INT_PENDING, 0xff);
> sdio_release_host(func);
> @@ -1006,7 +1010,7 @@ static void ks7010_sdio_remove(struct sdio_func *func)
> struct ks_sdio_card *card;
> struct ks_wlan_private *priv;
>
> - DPRINTK(1, "ks7010_sdio_remove()\n");
> + DPRINTK(1, "%s()\n", __func__);
You might get a "one thing per patch please" for this. You wouldn't
have had to change this line if you'd strictly stuck to that.
Frans
[toc] | [prev] | [next] | [standalone]
| From | Mark Rogers <mail@mark-rogers.org> |
|---|---|
| Date | 2017-06-30 20:40 +0200 |
| Message-ID | <tY8ZH-w7-7@gated-at.bofh.it> |
| In reply to | #1678496 |
Thank you for your feedback. I guess when making this patch I had the preferred coding style in mind, but didn't ask myself if making the code conform to it would truly improve readability. I agree with all of your comments. Do you think the best course of action is to create a new patch with this change alone and forget the rest? - DPRINTK(1, "ks7010_sdio_remove()\n"); + DPRINTK(1, "%s()\n", __func__); Sorry about the newbie questions and bad patch, I will do better with the next one. Thanks again for your time and feedback, I really appreciate it. Mark
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web