| Message ID | 1425238304-498-8-git-send-email-emmanuel.grumbach@intel.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Johannes Berg |
| Headers | show |
On Sun, 2015-03-01 at 21:31 +0200, Emmanuel Grumbach wrote: > From: Eyal Shapira <eyal@wizery.com> > > Add this to the info printed when reading rate_scale_table. > Useful for debugging. [] > diff --git a/drivers/net/wireless/iwlwifi/mvm/rs.c b/drivers/net/wireless/iwlwifi/mvm/rs.c [] > @@ -3369,6 +3370,16 @@ static ssize_t rs_sta_dbgfs_scale_table_read(struct file *file, > lq_sta->lq.agg_frame_cnt_limit); > > desc += sprintf(buff+desc, "reduced tpc=%d\n", lq_sta->lq.reduced_tpc); > + ss_params = le32_to_cpu(lq_sta->lq.ss_params); > + desc += sprintf(buff+desc, "single stream params: %s%s%s%s\n", > + (ss_params & LQ_SS_PARAMS_VALID) ? > + "VALID," : "INVALID", > + (ss_params & LQ_SS_BFER_ALLOWED) ? > + "BFER," : "", > + (ss_params & LQ_SS_STBC_1SS_ALLOWED) ? > + "STBC," : "", > + (ss_params & LQ_SS_FORCE) ? > + "FORCE" : ""); Are all things exclusive? If no, the output is not easily readable. It would probably be better to use: 12345678901234567890123456789012345678901234567890123456789012345678901234567890 desc += sprintf(buff+desc, "single stream params: %s%s%s%s\n", (ss_params & LQ_SS_PARAMS_VALID) ? "VALID" : "INVALID", (ss_params & LQ_SS_BFER_ALLOWED) ? ", BFER" : "", (ss_params & LQ_SS_STBC_1SS_ALLOWED) ? ", STBC" : "", (ss_params & LQ_SS_FORCE) ? ", FORCE" : ""); -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Emmanuel, On Mon, Mar 2, 2015 at 6:31 AM, Emmanuel Grumbach <emmanuel.grumbach@intel.com> wrote: > From: Eyal Shapira <eyal@wizery.com> > > Add this to the info printed when reading rate_scale_table. > Useful for debugging. > > Signed-off-by: Eyal Shapira <eyalx.shapira@intel.com> > Reviewed-by: Johannes Berg <johannes.berg@intel.com> > Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com> This signoff chain doesn't look right to me (however other people might not care): Shouldn't Eyal have used their intel.com email address in the author field? Thanks,
On Sun, Mar 1, 2015 at 9:42 PM, Joe Perches <joe@perches.com> wrote: > On Sun, 2015-03-01 at 21:31 +0200, Emmanuel Grumbach wrote: >> From: Eyal Shapira <eyal@wizery.com> >> >> Add this to the info printed when reading rate_scale_table. >> Useful for debugging. > [] >> diff --git a/drivers/net/wireless/iwlwifi/mvm/rs.c b/drivers/net/wireless/iwlwifi/mvm/rs.c > [] >> @@ -3369,6 +3370,16 @@ static ssize_t rs_sta_dbgfs_scale_table_read(struct file *file, >> lq_sta->lq.agg_frame_cnt_limit); >> >> desc += sprintf(buff+desc, "reduced tpc=%d\n", lq_sta->lq.reduced_tpc); >> + ss_params = le32_to_cpu(lq_sta->lq.ss_params); >> + desc += sprintf(buff+desc, "single stream params: %s%s%s%s\n", >> + (ss_params & LQ_SS_PARAMS_VALID) ? >> + "VALID," : "INVALID", >> + (ss_params & LQ_SS_BFER_ALLOWED) ? >> + "BFER," : "", >> + (ss_params & LQ_SS_STBC_1SS_ALLOWED) ? >> + "STBC," : "", >> + (ss_params & LQ_SS_FORCE) ? >> + "FORCE" : ""); > > Are all things exclusive? INVALID is exclusive to the other options so there's no real readability issue. I like your version better though as it avoids the additional comma in the end. Submitted to Emmanuel's internal tree. Thanks > If no, the output is not easily readable. > > It would probably be better to use: > 12345678901234567890123456789012345678901234567890123456789012345678901234567890 > desc += sprintf(buff+desc, "single stream params: %s%s%s%s\n", > (ss_params & LQ_SS_PARAMS_VALID) ? > "VALID" : "INVALID", > (ss_params & LQ_SS_BFER_ALLOWED) ? > ", BFER" : "", > (ss_params & LQ_SS_STBC_1SS_ALLOWED) ? > ", STBC" : "", > (ss_params & LQ_SS_FORCE) ? > ", FORCE" : ""); > -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/wireless/iwlwifi/mvm/rs.c b/drivers/net/wireless/iwlwifi/mvm/rs.c index 43f807f..6578498 100644 --- a/drivers/net/wireless/iwlwifi/mvm/rs.c +++ b/drivers/net/wireless/iwlwifi/mvm/rs.c @@ -3323,6 +3323,7 @@ static ssize_t rs_sta_dbgfs_scale_table_read(struct file *file, struct iwl_mvm *mvm; struct iwl_scale_tbl_info *tbl = &(lq_sta->lq_info[lq_sta->active_tbl]); struct rs_rate *rate = &tbl->rate; + u32 ss_params; mvm = lq_sta->pers.drv; buff = kmalloc(2048, GFP_KERNEL); if (!buff) @@ -3369,6 +3370,16 @@ static ssize_t rs_sta_dbgfs_scale_table_read(struct file *file, lq_sta->lq.agg_frame_cnt_limit); desc += sprintf(buff+desc, "reduced tpc=%d\n", lq_sta->lq.reduced_tpc); + ss_params = le32_to_cpu(lq_sta->lq.ss_params); + desc += sprintf(buff+desc, "single stream params: %s%s%s%s\n", + (ss_params & LQ_SS_PARAMS_VALID) ? + "VALID," : "INVALID", + (ss_params & LQ_SS_BFER_ALLOWED) ? + "BFER," : "", + (ss_params & LQ_SS_STBC_1SS_ALLOWED) ? + "STBC," : "", + (ss_params & LQ_SS_FORCE) ? + "FORCE" : ""); desc += sprintf(buff+desc, "Start idx [0]=0x%x [1]=0x%x [2]=0x%x [3]=0x%x\n", lq_sta->lq.initial_rate_index[0],