| Message ID | 20220225112405.355599-4-Jerome.Pouiller@silabs.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | staging: wfx: usual maintenance | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Fri, 2022-02-25 at 12:23 +0100, Jerome Pouiller wrote: > From: Jérôme Pouiller <jerome.pouiller@silabs.com> > > A few comments were not yet formatted on 100 columns. IMO, none of these changes are necessary or good changes. 80 columns is preferred. Really comments should most always use 80 columns, and only occasionally should code be more than 80 columns and almost never should code be more than 100 columns. > diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c [] > @@ -117,9 +117,7 @@ static int wfx_tx_policy_get(struct wfx_vif *wvif, struct ieee80211_tx_rate *rat > if (idx >= 0) { > *renew = false; > } else { > - /* If policy is not found create a new one using the oldest > - * entry in "free" list > - */ > + /* If policy is not found create a new one using the oldest entry in "free" list */ > *renew = true; > entry = list_entry(cache->free.prev, struct wfx_tx_policy, link); > memcpy(entry->rates, wanted.rates, sizeof(entry->rates)); > @@ -494,9 +492,7 @@ void wfx_tx_confirm_cb(struct wfx_dev *wdev, const struct wfx_hif_cnf_tx *arg) > wfx_tx_fill_rates(wdev, tx_info, arg); > skb_trim(skb, skb->len - tx_priv->icv_size); > > - /* From now, you can touch to tx_info->status, but do not touch to > - * tx_priv anymore > - */ > + /* From now, you can touch to tx_info->status, but do not touch to tx_priv anymore */ > /* FIXME: use ieee80211_tx_info_clear_status() */ > memset(tx_info->rate_driver_data, 0, sizeof(tx_info->rate_driver_data)); > memset(tx_info->pad, 0, sizeof(tx_info->pad)); > diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c [] > @@ -210,8 +210,8 @@ bool wfx_tx_queues_has_cab(struct wfx_vif *wvif) > if (wvif->vif->type != NL80211_IFTYPE_AP) > return false; > for (i = 0; i < IEEE80211_NUM_ACS; ++i) > - /* Note: since only AP can have mcast frames in queue and only > - * one vif can be AP, all queued frames has same interface id > + /* Note: since only AP can have mcast frames in queue and only one vif can be AP, > + * all queued frames has same interface id > */ > if (!skb_queue_empty_lockless(&wvif->tx_queue[i].cab)) > return true; > @@ -253,9 +253,8 @@ static struct sk_buff *wfx_tx_queues_get_skb(struct wfx_dev *wdev) > skb = skb_dequeue(&queues[i]->cab); > if (!skb) > continue; > - /* Note: since only AP can have mcast frames in queue > - * and only one vif can be AP, all queued frames has > - * same interface id > + /* Note: since only AP can have mcast frames in queue and only one vif can > + * be AP, all queued frames has same interface id > */ > hif = (struct wfx_hif_msg *)skb->data; > WARN_ON(hif->interface != wvif->id);
On 2/28/2022 5:12 PM, Joe Perches wrote: > On Fri, 2022-02-25 at 12:23 +0100, Jerome Pouiller wrote: >> From: Jérôme Pouiller <jerome.pouiller@silabs.com> >> >> A few comments were not yet formatted on 100 columns. > > IMO, none of these changes are necessary or good changes. > > 80 columns is preferred. > > Really comments should most always use 80 columns, and > only occasionally should code be more than 80 columns > and almost never should code be more than 100 columns. That was my reaction as well. Just because we've relaxed rules so that we *can* exceed 80 columns, it doesn't mean we *should*, and definitely doesn't mean we should *strive* to do so.
diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c index d7bcf3bae08a..e07381b2ff4d 100644 --- a/drivers/staging/wfx/data_tx.c +++ b/drivers/staging/wfx/data_tx.c @@ -117,9 +117,7 @@ static int wfx_tx_policy_get(struct wfx_vif *wvif, struct ieee80211_tx_rate *rat if (idx >= 0) { *renew = false; } else { - /* If policy is not found create a new one using the oldest - * entry in "free" list - */ + /* If policy is not found create a new one using the oldest entry in "free" list */ *renew = true; entry = list_entry(cache->free.prev, struct wfx_tx_policy, link); memcpy(entry->rates, wanted.rates, sizeof(entry->rates)); @@ -494,9 +492,7 @@ void wfx_tx_confirm_cb(struct wfx_dev *wdev, const struct wfx_hif_cnf_tx *arg) wfx_tx_fill_rates(wdev, tx_info, arg); skb_trim(skb, skb->len - tx_priv->icv_size); - /* From now, you can touch to tx_info->status, but do not touch to - * tx_priv anymore - */ + /* From now, you can touch to tx_info->status, but do not touch to tx_priv anymore */ /* FIXME: use ieee80211_tx_info_clear_status() */ memset(tx_info->rate_driver_data, 0, sizeof(tx_info->rate_driver_data)); memset(tx_info->pad, 0, sizeof(tx_info->pad)); diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c index 9186726ff07f..729825230db2 100644 --- a/drivers/staging/wfx/queue.c +++ b/drivers/staging/wfx/queue.c @@ -210,8 +210,8 @@ bool wfx_tx_queues_has_cab(struct wfx_vif *wvif) if (wvif->vif->type != NL80211_IFTYPE_AP) return false; for (i = 0; i < IEEE80211_NUM_ACS; ++i) - /* Note: since only AP can have mcast frames in queue and only - * one vif can be AP, all queued frames has same interface id + /* Note: since only AP can have mcast frames in queue and only one vif can be AP, + * all queued frames has same interface id */ if (!skb_queue_empty_lockless(&wvif->tx_queue[i].cab)) return true; @@ -253,9 +253,8 @@ static struct sk_buff *wfx_tx_queues_get_skb(struct wfx_dev *wdev) skb = skb_dequeue(&queues[i]->cab); if (!skb) continue; - /* Note: since only AP can have mcast frames in queue - * and only one vif can be AP, all queued frames has - * same interface id + /* Note: since only AP can have mcast frames in queue and only one vif can + * be AP, all queued frames has same interface id */ hif = (struct wfx_hif_msg *)skb->data; WARN_ON(hif->interface != wvif->id);