* [PATCH] staging: wfx: cleanup long lines in data_tx.c
@ 2020-04-22 15:39 Suraj Upadhyay
2020-04-23 9:29 ` Jerome Pouiller
0 siblings, 1 reply; 4+ messages in thread
From: Suraj Upadhyay @ 2020-04-22 15:39 UTC (permalink / raw)
To: jerome.pouiller, gregkh; +Cc: devel, linux-kernel
Break lines with length over 80 characters to conform
to the linux coding style. Issue found by checkpatch.
Signed-off-by: Suraj Upadhyay <usuraj35@gmail.com>
---
drivers/staging/wfx/data_tx.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index c30e4f5b6e2d..85ad94de20e5 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -165,7 +165,8 @@ static int wfx_tx_policy_upload(struct wfx_vif *wvif)
spin_lock_bh(&wvif->tx_policy_cache.lock);
for (i = 0; i < HIF_TX_RETRY_POLICY_MAX; ++i)
if (!policies[i].uploaded &&
- memzcmp(policies[i].rates, sizeof(policies[i].rates)))
+ memzcmp(policies[i].rates,
+ sizeof(policies[i].rates)))
break;
if (i < HIF_TX_RETRY_POLICY_MAX) {
policies[i].uploaded = true;
@@ -290,7 +291,8 @@ static void wfx_tx_fixup_rates(struct ieee80211_tx_rate *rates)
if (rates[i].idx == -1) {
rates[i].idx = 0;
rates[i].count = 8; // == hw->max_rate_tries
- rates[i].flags = rates[i - 1].flags & IEEE80211_TX_RC_MCS;
+ rates[i].flags = rates[i - 1].flags &
+ IEEE80211_TX_RC_MCS;
break;
}
}
@@ -318,7 +320,9 @@ static u8 wfx_tx_get_rate_id(struct wfx_vif *wvif,
return rate_id;
}
-static struct hif_ht_tx_parameters wfx_tx_get_tx_parms(struct wfx_dev *wdev, struct ieee80211_tx_info *tx_info)
+static struct
+hif_ht_tx_parameters wfx_tx_get_tx_parms(struct wfx_dev *wdev,
+ struct ieee80211_tx_info *tx_info)
{
struct ieee80211_tx_rate *rate = &tx_info->driver_rates[0];
struct hif_ht_tx_parameters ret = { };
@@ -381,7 +385,8 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta,
hif_msg->id = HIF_REQ_ID_TX;
hif_msg->interface = wvif->id;
if (skb->len > wvif->wdev->hw_caps.size_inp_ch_buf) {
- dev_warn(wvif->wdev->dev, "requested frame size (%d) is larger than maximum supported (%d)\n",
+ dev_warn(wvif->wdev->dev,
+ "requested frame size (%d) is larger than maximum supported (%d)\n",
skb->len, wvif->wdev->hw_caps.size_inp_ch_buf);
skb_pull(skb, wmsg_len);
return -EIO;
@@ -394,7 +399,8 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta,
// data for debug.
req->packet_id = queue_id << 28 |
IEEE80211_SEQ_TO_SN(le16_to_cpu(hdr->seq_ctrl)) << 16 |
- (atomic_add_return(1, &wvif->wdev->packet_id) & 0xFFFF);
+ (atomic_add_return(1, &wvif->wdev->packet_id) &
+ 0xFFFF);
req->data_flags.fc_offset = offset;
if (tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM)
req->data_flags.after_dtim = 1;
@@ -517,7 +523,8 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
if (tx_count < rate->count &&
arg->status == HIF_STATUS_RETRY_EXCEEDED &&
arg->ack_failures)
- dev_dbg(wvif->wdev->dev, "all retries were not consumed: %d != %d\n",
+ dev_dbg(wvif->wdev->dev,
+ "all retries were not consumed: %d != %d\n",
rate->count, tx_count);
if (tx_count <= rate->count && tx_count &&
arg->txed_rate != wfx_get_hw_rate(wvif->wdev, rate))
@@ -557,7 +564,8 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
else
tx_info->flags |= IEEE80211_TX_STAT_ACK;
} else if (arg->status == HIF_REQUEUE) {
- WARN(!arg->tx_result_flags.requeue, "incoherent status and result_flags");
+ WARN(!arg->tx_result_flags.requeue,
+ "incoherent status and result_flags");
if (tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM) {
wvif->after_dtim_tx_allowed = false; // DTIM period elapsed
schedule_work(&wvif->update_tim_work);
@@ -595,7 +603,8 @@ void wfx_flush(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
if (wait_event_timeout(wdev->tx_dequeue,
wfx_tx_queue_empty(wdev, queue, vif_id),
msecs_to_jiffies(1000)) <= 0)
- dev_warn(wdev->dev, "frames queued while flushing tx queues?");
+ dev_warn(wdev->dev,
+ "frames queued while flushing tx queues?");
}
wfx_tx_flush(wdev);
if (wdev->chip_frozen)
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] staging: wfx: cleanup long lines in data_tx.c
2020-04-22 15:39 [PATCH] staging: wfx: cleanup long lines in data_tx.c Suraj Upadhyay
@ 2020-04-23 9:29 ` Jerome Pouiller
2020-04-23 10:34 ` Dan Carpenter
2020-04-24 12:49 ` Suraj Upadhyay
0 siblings, 2 replies; 4+ messages in thread
From: Jerome Pouiller @ 2020-04-23 9:29 UTC (permalink / raw)
To: Suraj Upadhyay; +Cc: gregkh, devel, linux-kernel
Hello Suraj,
Thank you for your contribution.
On Wednesday 22 April 2020 17:39:00 CEST Suraj Upadhyay wrote:
>
> Break lines with length over 80 characters to conform
> to the linux coding style. Issue found by checkpatch.
>
> Signed-off-by: Suraj Upadhyay <usuraj35@gmail.com>
> ---
> drivers/staging/wfx/data_tx.c | 25 +++++++++++++++++--------
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
> index c30e4f5b6e2d..85ad94de20e5 100644
> --- a/drivers/staging/wfx/data_tx.c
> +++ b/drivers/staging/wfx/data_tx.c
> @@ -165,7 +165,8 @@ static int wfx_tx_policy_upload(struct wfx_vif *wvif)
> spin_lock_bh(&wvif->tx_policy_cache.lock);
> for (i = 0; i < HIF_TX_RETRY_POLICY_MAX; ++i)
> if (!policies[i].uploaded &&
> - memzcmp(policies[i].rates, sizeof(policies[i].rates)))
> + memzcmp(policies[i].rates,
> + sizeof(policies[i].rates)))
hmm... since memzcmp() is called from a if condition, I am not sure that
breaking the line improves the readability. May be you should introduce a
temporary variable instead.
Before that line, I also see a line that exceed 80 columns in
wfx_get_hw_rate() could also address it?
> break;
> if (i < HIF_TX_RETRY_POLICY_MAX) {
> policies[i].uploaded = true;
> @@ -290,7 +291,8 @@ static void wfx_tx_fixup_rates(struct ieee80211_tx_rate *rates)
> if (rates[i].idx == -1) {
> rates[i].idx = 0;
> rates[i].count = 8; // == hw->max_rate_tries
> - rates[i].flags = rates[i - 1].flags & IEEE80211_TX_RC_MCS;
> + rates[i].flags = rates[i - 1].flags &
> + IEEE80211_TX_RC_MCS;
Ack.
> break;
> }
> }
> @@ -318,7 +320,9 @@ static u8 wfx_tx_get_rate_id(struct wfx_vif *wvif,
> return rate_id;
> }
>
> -static struct hif_ht_tx_parameters wfx_tx_get_tx_parms(struct wfx_dev *wdev, struct ieee80211_tx_info *tx_info)
> +static struct
> +hif_ht_tx_parameters wfx_tx_get_tx_parms(struct wfx_dev *wdev,
> + struct ieee80211_tx_info *tx_info)
Please, dont't break between 'struct' and name of the struct. In add, in
the rest of the file, there is no line break between the returned type the
name of the function. Let's try to keep it uniform.
> {
> struct ieee80211_tx_rate *rate = &tx_info->driver_rates[0];
> struct hif_ht_tx_parameters ret = { };
> @@ -381,7 +385,8 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta,
> hif_msg->id = HIF_REQ_ID_TX;
> hif_msg->interface = wvif->id;
> if (skb->len > wvif->wdev->hw_caps.size_inp_ch_buf) {
> - dev_warn(wvif->wdev->dev, "requested frame size (%d) is larger than maximum supported (%d)\n",
> + dev_warn(wvif->wdev->dev,
> + "requested frame size (%d) is larger than maximum supported (%d)\n",
> skb->len, wvif->wdev->hw_caps.size_inp_ch_buf);
> skb_pull(skb, wmsg_len);
> return -EIO;
> @@ -394,7 +399,8 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta,
> // data for debug.
> req->packet_id = queue_id << 28 |
> IEEE80211_SEQ_TO_SN(le16_to_cpu(hdr->seq_ctrl)) << 16 |
> - (atomic_add_return(1, &wvif->wdev->packet_id) & 0xFFFF);
> + (atomic_add_return(1, &wvif->wdev->packet_id) &
> + 0xFFFF);
Since the line is already split on '|', I am not sure that adding a break
here improve the readability. Maybe:
req->packet_id = atomic_add_return(1, &wvif->wdev->packet_id) & 0xFFFF;
req->packet_id |= IEEE80211_SEQ_TO_SN(le16_to_cpu(hdr->seq_ctrl)) << 16;
req->packet_id |= queue_id << 28;
or maybe if you introduce "struct wfx_dev *wdev = wvif->wdev;", you would
simplify the things.
> req->data_flags.fc_offset = offset;
> if (tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM)
> req->data_flags.after_dtim = 1;
> @@ -517,7 +523,8 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
> if (tx_count < rate->count &&
> arg->status == HIF_STATUS_RETRY_EXCEEDED &&
> arg->ack_failures)
> - dev_dbg(wvif->wdev->dev, "all retries were not consumed: %d != %d\n",
> + dev_dbg(wvif->wdev->dev,
> + "all retries were not consumed: %d != %d\n",
Is this reported by checkpatch? The strings can exceed 80 columns.
> rate->count, tx_count);
> if (tx_count <= rate->count && tx_count &&
> arg->txed_rate != wfx_get_hw_rate(wvif->wdev, rate))
> @@ -557,7 +564,8 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
> else
> tx_info->flags |= IEEE80211_TX_STAT_ACK;
> } else if (arg->status == HIF_REQUEUE) {
> - WARN(!arg->tx_result_flags.requeue, "incoherent status and result_flags");
> + WARN(!arg->tx_result_flags.requeue,
> + "incoherent status and result_flags");
Ditto
> if (tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM) {
> wvif->after_dtim_tx_allowed = false; // DTIM period elapsed
> schedule_work(&wvif->update_tim_work);
> @@ -595,7 +603,8 @@ void wfx_flush(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> if (wait_event_timeout(wdev->tx_dequeue,
> wfx_tx_queue_empty(wdev, queue, vif_id),
> msecs_to_jiffies(1000)) <= 0)
> - dev_warn(wdev->dev, "frames queued while flushing tx queues?");
> + dev_warn(wdev->dev,
> + "frames queued while flushing tx queues?");
Ditto
> }
> wfx_tx_flush(wdev);
> if (wdev->chip_frozen)
> --
> 2.17.1
>
>
--
Jérôme Pouiller
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] staging: wfx: cleanup long lines in data_tx.c
2020-04-23 9:29 ` Jerome Pouiller
@ 2020-04-23 10:34 ` Dan Carpenter
2020-04-24 12:49 ` Suraj Upadhyay
1 sibling, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2020-04-23 10:34 UTC (permalink / raw)
To: Jerome Pouiller; +Cc: Suraj Upadhyay, devel, gregkh, linux-kernel
On Thu, Apr 23, 2020 at 09:29:28AM +0000, Jerome Pouiller wrote:
> > req->data_flags.fc_offset = offset;
> > if (tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM)
> > req->data_flags.after_dtim = 1;
> > @@ -517,7 +523,8 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
> > if (tx_count < rate->count &&
> > arg->status == HIF_STATUS_RETRY_EXCEEDED &&
> > arg->ack_failures)
> > - dev_dbg(wvif->wdev->dev, "all retries were not consumed: %d != %d\n",
> > + dev_dbg(wvif->wdev->dev,
> > + "all retries were not consumed: %d != %d\n",
>
> Is this reported by checkpatch? The strings can exceed 80 columns.
>
The string can go over 80 characters, but it's nice to do our best to
avoid it so we do normally break it up like this where the string is
on its own line.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] staging: wfx: cleanup long lines in data_tx.c
2020-04-23 9:29 ` Jerome Pouiller
2020-04-23 10:34 ` Dan Carpenter
@ 2020-04-24 12:49 ` Suraj Upadhyay
1 sibling, 0 replies; 4+ messages in thread
From: Suraj Upadhyay @ 2020-04-24 12:49 UTC (permalink / raw)
To: jerome.pouiller; +Cc: devel, linux-kernel, dan.carpenter
[-- Attachment #1: Type: text/plain, Size: 669 bytes --]
On Thu, Apr 23, 2020 at 09:29:28AM +0000, Jerome Pouiller wrote:
> Hello Suraj,
>
> Thank you for your contribution.
This would be my first patch to linux-kernel :)
> On Wednesday 22 April 2020 17:39:00 CEST Suraj Upadhyay wrote:
> >
> > Break lines with length over 80 characters to conform
> > to the linux coding style. Issue found by checkpatch.
> >
> > Signed-off-by: Suraj Upadhyay <usuraj35@gmail.com>
>
> Is this reported by checkpatch? The strings can exceed 80 columns.
No, it's not. Actually I am using vim with some plugins and it was
showing red their. So, I thought why not make these strings in a new
line.
> Jérôme Pouiller
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-04-24 12:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-22 15:39 [PATCH] staging: wfx: cleanup long lines in data_tx.c Suraj Upadhyay
2020-04-23 9:29 ` Jerome Pouiller
2020-04-23 10:34 ` Dan Carpenter
2020-04-24 12:49 ` Suraj Upadhyay
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).