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 <usura...@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
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
