diff mbox

[08/31] iwlwifi: mvm: rs: print single stream params via debugfs

Message ID 1425238304-498-8-git-send-email-emmanuel.grumbach@intel.com
State Not Applicable
Delegated to: Johannes Berg
Headers show

Commit Message

Emmanuel Grumbach March 1, 2015, 7:31 p.m. UTC
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>
---
 drivers/net/wireless/iwlwifi/mvm/rs.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Joe Perches March 1, 2015, 7:42 p.m. UTC | #1
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
Julian Calaby March 1, 2015, 10:20 p.m. UTC | #2
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,
Eyal Shapira March 2, 2015, 8:46 a.m. UTC | #3
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 mbox

Patch

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],