Message ID | 1395360324-20433-1-git-send-email-greearb@candelatech.com (mailing list archive) |
---|---|
State | Not Applicable, archived |
Headers | show |
On 21 March 2014 01:05, <greearb@candelatech.com> wrote: > From: Ben Greear <greearb@candelatech.com> > > Make the request command object the right size so that > firmware will not just throw it away. > Tested customized and upstream firmware. Did you test 636 as well? 636 doesn't seem to support more than just `stats_id`. > Signed-off-by: Ben Greear <greearb@candelatech.com> > --- > drivers/net/wireless/ath/ath10k/wmi.h | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h > index fa1b9e0..4946471 100644 > --- a/drivers/net/wireless/ath/ath10k/wmi.h > +++ b/drivers/net/wireless/ath/ath10k/wmi.h > @@ -2766,6 +2766,11 @@ enum wmi_stats_id { > WMI_REQUEST_AP_STAT = 0x02 > }; > > +struct wlan_inst_rssi_args { > + __le16 cfg_retry_count; > + __le16 retry_count; > +}; > + > struct wmi_request_stats_cmd { > __le32 stats_id; > > @@ -2773,6 +2778,12 @@ struct wmi_request_stats_cmd { > * Space to add parameters like > * peer mac addr > */ You can probably remove the comment now :-) > + __le32 vdev_id; > + /* peer MAC address */ > + struct wmi_mac_addr peer_macaddr; > + > + /* Instantaneous RSSI arguments */ > + struct wlan_inst_rssi_args inst_rssi_args; > } __packed; Micha? -- 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
> > Did you test 636 as well? 636 doesn't seem to support more than just `stats_id`. > Nope. Tested with 636 not working. ---- Chun-Yeow -- 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
On 03/20/2014 11:33 PM, Michal Kazior wrote: > On 21 March 2014 01:05, <greearb@candelatech.com> wrote: >> From: Ben Greear <greearb@candelatech.com> >> >> Make the request command object the right size so that >> firmware will not just throw it away. >> Tested customized and upstream firmware. > > Did you test 636 as well? 636 doesn't seem to support more than just `stats_id`. The firmware source I have checks in such a way that sending too large of a cmd should be fine, only too small of a command packet causes it to be ignored. I do not have the 636 firmware to test against. >> Signed-off-by: Ben Greear <greearb@candelatech.com> >> --- >> drivers/net/wireless/ath/ath10k/wmi.h | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h >> index fa1b9e0..4946471 100644 >> --- a/drivers/net/wireless/ath/ath10k/wmi.h >> +++ b/drivers/net/wireless/ath/ath10k/wmi.h >> @@ -2766,6 +2766,11 @@ enum wmi_stats_id { >> WMI_REQUEST_AP_STAT = 0x02 >> }; >> >> +struct wlan_inst_rssi_args { >> + __le16 cfg_retry_count; >> + __le16 retry_count; >> +}; >> + >> struct wmi_request_stats_cmd { >> __le32 stats_id; >> >> @@ -2773,6 +2778,12 @@ struct wmi_request_stats_cmd { >> * Space to add parameters like >> * peer mac addr >> */ > > You can probably remove the comment now :-) Ok, I can do that. Thanks, Ben
On 03/20/2014 11:41 PM, Yeoh Chun-Yeow wrote: >> >> Did you test 636 as well? 636 doesn't seem to support more than just `stats_id`. >> > Nope. Tested with 636 not working. Does the existing code work with 636? If so, we can add two different cmd structs and use the smaller one with 636, and the one I modified with 10.x firmware? Thanks, Ben > > ---- > Chun-Yeow >
Ben Greear <greearb@candelatech.com> writes: > On 03/20/2014 11:33 PM, Michal Kazior wrote: >> On 21 March 2014 01:05, <greearb@candelatech.com> wrote: >>> From: Ben Greear <greearb@candelatech.com> >>> >>> Make the request command object the right size so that >>> firmware will not just throw it away. >>> Tested customized and upstream firmware. >> >> Did you test 636 as well? 636 doesn't seem to support more than just `stats_id`. > > The firmware source I have checks in such a way that sending too > large of a cmd should be fine, only too small of a command packet > causes it to be ignored. > > I do not have the 636 firmware to test against. The firmare image is here: https://github.com/kvalo/ath10k-firmware/tree/master/main
Ben Greear <greearb@candelatech.com> writes: > On 03/20/2014 11:41 PM, Yeoh Chun-Yeow wrote: >>> >>> Did you test 636 as well? 636 doesn't seem to support more than just `stats_id`. >>> >> Nope. Tested with 636 not working. > > Does the existing code work with 636? If so, we can add two different cmd > structs and use the smaller one with 636, and the one I modified with > 10.x firmware? For this issue that would be the best approach. See Marek P's patch "ath10k: update regulatory domain settings for 10.x firmware" (not yet applied, still in ath-next-test branch) as an example how this can be done.
Hi, Ben I did a quick check again on STA mode. With or without your patch, the printed peer stats are both identical but both wrong. See below: ath10k PEER stats ================= Peer MAC address 00:00:00:00:00:00 Peer RSSI 256 Peer TX rate 0 Peer MAC address 00:00:00:00:04:f0 Peer RSSI 17317 Peer TX rate 73 So I think that your patch makes no difference but do indeed providing "correct" peer stats on latest AP firmware. After applying the "ath10k: add the Rx rate in FW stats", I am able to get the Tx and Rx Rate for all connected STAs to my AP. --- Chun-Yeow On Sat, Mar 22, 2014 at 12:12 AM, Kalle Valo <kvalo@qca.qualcomm.com> wrote: > Ben Greear <greearb@candelatech.com> writes: > >> On 03/20/2014 11:41 PM, Yeoh Chun-Yeow wrote: >>>> >>>> Did you test 636 as well? 636 doesn't seem to support more than just `stats_id`. >>>> >>> Nope. Tested with 636 not working. >> >> Does the existing code work with 636? If so, we can add two different cmd >> structs and use the smaller one with 636, and the one I modified with >> 10.x firmware? > > For this issue that would be the best approach. See Marek P's patch > "ath10k: update regulatory domain settings for 10.x firmware" (not yet > applied, still in ath-next-test branch) as an example how this can be > done. > > -- > Kalle Valo -- 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
BTW, is anyone ever tested the get stats for firmware 636 before your patch and getting positive results? --- Chun-Yeow On Sat, Mar 22, 2014 at 12:42 AM, Yeoh Chun-Yeow <yeohchunyeow@gmail.com> wrote: > Hi, Ben > > I did a quick check again on STA mode. With or without your patch, the > printed peer stats are both identical but both wrong. See below: > > ath10k PEER stats > ================= > > Peer MAC address 00:00:00:00:00:00 > Peer RSSI 256 > Peer TX rate 0 > > Peer MAC address 00:00:00:00:04:f0 > Peer RSSI 17317 > Peer TX rate 73 > > So I think that your patch makes no difference but do indeed providing > "correct" peer stats on latest AP firmware. > > After applying the "ath10k: add the Rx rate in FW stats", I am able to > get the Tx and Rx Rate for all connected STAs to my AP. > > --- > Chun-Yeow > > On Sat, Mar 22, 2014 at 12:12 AM, Kalle Valo <kvalo@qca.qualcomm.com> wrote: >> Ben Greear <greearb@candelatech.com> writes: >> >>> On 03/20/2014 11:41 PM, Yeoh Chun-Yeow wrote: >>>>> >>>>> Did you test 636 as well? 636 doesn't seem to support more than just `stats_id`. >>>>> >>>> Nope. Tested with 636 not working. >>> >>> Does the existing code work with 636? If so, we can add two different cmd >>> structs and use the smaller one with 636, and the one I modified with >>> 10.x firmware? >> >> For this issue that would be the best approach. See Marek P's patch >> "ath10k: update regulatory domain settings for 10.x firmware" (not yet >> applied, still in ath-next-test branch) as an example how this can be >> done. >> >> -- >> Kalle Valo -- 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
On 03/21/2014 09:42 AM, Yeoh Chun-Yeow wrote: > Hi, Ben > > I did a quick check again on STA mode. With or without your patch, the > printed peer stats are both identical but both wrong. See below: Ok, it's unlikely my patch would have changed it one way or another. I'll leave my patch mostly as it was (ie, no special handling for older v/s newer firmware). I am just starting to look at stats, and since 10.x was completely broken and has been for some time, probably no one else has paid much attention to it yet either. Thanks for testing. Ben
On 03/21/2014 09:48 AM, Yeoh Chun-Yeow wrote: >> I did a quick check again on STA mode. With or without your patch, the >> printed peer stats are both identical but both wrong. See below: >> >> ath10k PEER stats >> ================= >> >> Peer MAC address 00:00:00:00:00:00 >> Peer RSSI 256 >> Peer TX rate 0 >> >> Peer MAC address 00:00:00:00:04:f0 >> Peer RSSI 17317 >> Peer TX rate 73 >> >> So I think that your patch makes no difference but do indeed providing >> "correct" peer stats on latest AP firmware. It looks like the peer stats are broken on my 10.x firmware as well. Other pdev tx/rx stats look like they are at least possibly correct. Since ath10k supports more than one vdev, we also will want to extend it to get peer for the rest of the vdevs some day. Thanks, Ben
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h index fa1b9e0..4946471 100644 --- a/drivers/net/wireless/ath/ath10k/wmi.h +++ b/drivers/net/wireless/ath/ath10k/wmi.h @@ -2766,6 +2766,11 @@ enum wmi_stats_id { WMI_REQUEST_AP_STAT = 0x02 }; +struct wlan_inst_rssi_args { + __le16 cfg_retry_count; + __le16 retry_count; +}; + struct wmi_request_stats_cmd { __le32 stats_id; @@ -2773,6 +2778,12 @@ struct wmi_request_stats_cmd { * Space to add parameters like * peer mac addr */ + __le32 vdev_id; + /* peer MAC address */ + struct wmi_mac_addr peer_macaddr; + + /* Instantaneous RSSI arguments */ + struct wlan_inst_rssi_args inst_rssi_args; } __packed; /* Suspend option */