Break lines with length over 80 characters to conform
to the linux coding style. Issue found by checkpatch.
Signed-off-by: Suraj Upadhyay <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
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
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 <[email protected]>
>
> 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
>