Break lines with length over 80 characters to
conform to the linux coding style and refactor
wherever necessary.
Signed-off-by: Suraj Upadhyay <[email protected]>
---
Changes in v4:
- Added a space after declaration in wfx_get_hw_rate().
- A checkpatch warning for this commit is retained at line 75,
to maintain uniformity in function declarations. (Reviewer
jerome suggested).
Changes in v3:
- Changed the temporary variable name for the memzcmp statement
to is_used. (as suggested).
- Added a temporary ieee80211_supported_band variable to address
the problem in wfx_get_hw_rate() more efficiently. (not
suggested, but still).
Changes in v2:
- Introduced a temporary variable for the memzcmp statement.
- Addressed the checkpatch problem with wfx_get_hw_rate().
- Restored the function definition of wfx_tx_get_tx_parms
as suggested by the reviewer.
- Added suggested changes for req->packet_id statement.
drivers/staging/wfx/data_tx.c | 40 +++++++++++++++++++++++------------
1 file changed, 26 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 9c1a91207dd8..2f8a254fa453 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -20,6 +20,8 @@
static int wfx_get_hw_rate(struct wfx_dev *wdev,
const struct ieee80211_tx_rate *rate)
{
+ struct ieee80211_supported_band *band;
+
if (rate->idx < 0)
return -1;
if (rate->flags & IEEE80211_TX_RC_MCS) {
@@ -31,7 +33,8 @@ static int wfx_get_hw_rate(struct wfx_dev *wdev,
}
// WFx only support 2GHz, else band information should be retrieved
// from ieee80211_tx_info
- return wdev->hw->wiphy->bands[NL80211_BAND_2GHZ]->bitrates[rate->idx].hw_value;
+ band = wdev->hw->wiphy->bands[NL80211_BAND_2GHZ];
+ return band->bitrates[rate->idx].hw_value;
}
/* TX policy cache implementation */
@@ -159,14 +162,16 @@ static int wfx_tx_policy_upload(struct wfx_vif *wvif)
{
struct tx_policy *policies = wvif->tx_policy_cache.cache;
u8 tmp_rates[12];
- int i;
+ int i, is_used;
do {
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)))
+ for (i = 0; i < HIF_TX_RETRY_POLICY_MAX; ++i) {
+ is_used = memzcmp(policies[i].rates,
+ sizeof(policies[i].rates));
+ if (!policies[i].uploaded && is_used)
break;
+ }
if (i < HIF_TX_RETRY_POLICY_MAX) {
policies[i].uploaded = true;
memcpy(tmp_rates, policies[i].rates, sizeof(tmp_rates));
@@ -290,7 +295,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 +324,8 @@ 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 +388,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;
@@ -392,9 +400,10 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta,
// packet_id just need to be unique on device. 32bits are more than
// necessary for that task, so we tae advantage of it to add some extra
// 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);
+ 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;
+
req->data_flags.fc_offset = offset;
if (tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM)
req->data_flags.after_dtim = 1;
@@ -517,7 +526,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))
@@ -554,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);
@@ -588,7 +599,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
On Saturday 25 April 2020 13:32:34 CEST Suraj Upadhyay wrote:
> Break lines with length over 80 characters to
> conform to the linux coding style and refactor
> wherever necessary.
>
> Signed-off-by: Suraj Upadhyay <[email protected]>
> ---
>
> Changes in v4:
> - Added a space after declaration in wfx_get_hw_rate().
> - A checkpatch warning for this commit is retained at line 75,
> to maintain uniformity in function declarations. (Reviewer
> jerome suggested).
>
> Changes in v3:
> - Changed the temporary variable name for the memzcmp statement
> to is_used. (as suggested).
> - Added a temporary ieee80211_supported_band variable to address
> the problem in wfx_get_hw_rate() more efficiently. (not
> suggested, but still).
>
> Changes in v2:
> - Introduced a temporary variable for the memzcmp statement.
> - Addressed the checkpatch problem with wfx_get_hw_rate().
> - Restored the function definition of wfx_tx_get_tx_parms
> as suggested by the reviewer.
> - Added suggested changes for req->packet_id statement.
>
> drivers/staging/wfx/data_tx.c | 40 +++++++++++++++++++++++------------
> 1 file changed, 26 insertions(+), 14 deletions(-)
This patch does not contain the suggestions from Dan. However, it is
sufficient from my personal point of view.
Reviewed-by: J?r?me Pouiller <[email protected]>
--
J?r?me Pouiller
On Mon, Apr 27, 2020 at 11:50:23AM +0000, Jerome Pouiller wrote:
> On Saturday 25 April 2020 13:32:34 CEST Suraj Upadhyay wrote:
> > Break lines with length over 80 characters to
> > conform to the linux coding style and refactor
> > wherever necessary.
> >
> > Signed-off-by: Suraj Upadhyay <[email protected]>
> > ---
> >
> > Changes in v4:
> > - Added a space after declaration in wfx_get_hw_rate().
> > - A checkpatch warning for this commit is retained at line 75,
> > to maintain uniformity in function declarations. (Reviewer
> > jerome suggested).
> >
> > Changes in v3:
> > - Changed the temporary variable name for the memzcmp statement
> > to is_used. (as suggested).
> > - Added a temporary ieee80211_supported_band variable to address
> > the problem in wfx_get_hw_rate() more efficiently. (not
> > suggested, but still).
> >
> > Changes in v2:
> > - Introduced a temporary variable for the memzcmp statement.
> > - Addressed the checkpatch problem with wfx_get_hw_rate().
> > - Restored the function definition of wfx_tx_get_tx_parms
> > as suggested by the reviewer.
> > - Added suggested changes for req->packet_id statement.
> >
> > drivers/staging/wfx/data_tx.c | 40 +++++++++++++++++++++++------------
> > 1 file changed, 26 insertions(+), 14 deletions(-)
>
> This patch does not contain the suggestions from Dan. However, it is
> sufficient from my personal point of view.
Yes, I considered them but thought it would be bad to introduce a new
variable at every iteration of the for-loop.
> Reviewed-by: J?r?me Pouiller <[email protected]>
Thanks Jerome, This was my first patch to the linux kernel.
>
> --
> J?r?me Pouiller
>
Regards,
Suraj Upadhyay.
On Mon, Apr 27, 2020 at 11:50:23AM +0000, Jerome Pouiller wrote:
> On Saturday 25 April 2020 13:32:34 CEST Suraj Upadhyay wrote:
> > Break lines with length over 80 characters to
> > conform to the linux coding style and refactor
> > wherever necessary.
> >
> > Signed-off-by: Suraj Upadhyay <[email protected]>
> > ---
> >
> > Changes in v4:
> > - Added a space after declaration in wfx_get_hw_rate().
> > - A checkpatch warning for this commit is retained at line 75,
> > to maintain uniformity in function declarations. (Reviewer
> > jerome suggested).
> >
> > Changes in v3:
> > - Changed the temporary variable name for the memzcmp statement
> > to is_used. (as suggested).
> > - Added a temporary ieee80211_supported_band variable to address
> > the problem in wfx_get_hw_rate() more efficiently. (not
> > suggested, but still).
> >
> > Changes in v2:
> > - Introduced a temporary variable for the memzcmp statement.
> > - Addressed the checkpatch problem with wfx_get_hw_rate().
> > - Restored the function definition of wfx_tx_get_tx_parms
> > as suggested by the reviewer.
> > - Added suggested changes for req->packet_id statement.
> >
> > drivers/staging/wfx/data_tx.c | 40 +++++++++++++++++++++++------------
> > 1 file changed, 26 insertions(+), 14 deletions(-)
>
> This patch does not contain the suggestions from Dan. However, it is
> sufficient from my personal point of view.
Yeah. It's fine. It wasn't 100% cut and paste from my suggestions but
it did address the stuff I cared about. Thanks!
Reviewed-by: Dan Carpenter <[email protected]>
regards,
dan carpenter
On Mon, Apr 27, 2020 at 05:28:27PM +0530, Suraj Upadhyay wrote:
> On Mon, Apr 27, 2020 at 11:50:23AM +0000, Jerome Pouiller wrote:
> > On Saturday 25 April 2020 13:32:34 CEST Suraj Upadhyay wrote:
> > > Break lines with length over 80 characters to
> > > conform to the linux coding style and refactor
> > > wherever necessary.
> > >
> > > Signed-off-by: Suraj Upadhyay <[email protected]>
> > > ---
> > >
> > > Changes in v4:
> > > - Added a space after declaration in wfx_get_hw_rate().
> > > - A checkpatch warning for this commit is retained at line 75,
> > > to maintain uniformity in function declarations. (Reviewer
> > > jerome suggested).
> > >
> > > Changes in v3:
> > > - Changed the temporary variable name for the memzcmp statement
> > > to is_used. (as suggested).
> > > - Added a temporary ieee80211_supported_band variable to address
> > > the problem in wfx_get_hw_rate() more efficiently. (not
> > > suggested, but still).
> > >
> > > Changes in v2:
> > > - Introduced a temporary variable for the memzcmp statement.
> > > - Addressed the checkpatch problem with wfx_get_hw_rate().
> > > - Restored the function definition of wfx_tx_get_tx_parms
> > > as suggested by the reviewer.
> > > - Added suggested changes for req->packet_id statement.
> > >
> > > drivers/staging/wfx/data_tx.c | 40 +++++++++++++++++++++++------------
> > > 1 file changed, 26 insertions(+), 14 deletions(-)
> >
> > This patch does not contain the suggestions from Dan. However, it is
> > sufficient from my personal point of view.
>
> Yes, I considered them but thought it would be bad to introduce a new
> variable at every iteration of the for-loop.
>
It's not bad at all. I don't know why someone would think that.
regards,
dan carpenter