2016-06-23 11:11:52

by Yaniv Machani

[permalink] [raw]
Subject: [PATCH] wlcore: time sync : add support for 64 bit clock

Changed the configuration to support 64bit instead of 32bit
this in order to offload the driver from handling a wraparound.

Signed-off-by: Yaniv Machani <[email protected]>
---
drivers/net/wireless/ti/wl18xx/event.c | 26 +++++++++++++++++---------
drivers/net/wireless/ti/wl18xx/event.h | 19 +++++++++++++------
2 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wireless/ti/wl18xx/event.c b/drivers/net/wireless/ti/wl18xx/event.c
index ef81184..2c5df43 100644
--- a/drivers/net/wireless/ti/wl18xx/event.c
+++ b/drivers/net/wireless/ti/wl18xx/event.c
@@ -112,12 +112,18 @@ static int wlcore_smart_config_decode_event(struct wl1271 *wl,
return 0;
}

-static void wlcore_event_time_sync(struct wl1271 *wl, u16 tsf_msb, u16 tsf_lsb)
+static void wlcore_event_time_sync(struct wl1271 *wl,
+ u16 tsf_high_msb, u16 tsf_high_lsb,
+ u16 tsf_low_msb, u16 tsf_low_lsb)
{
- u32 clock;
- /* convert the MSB+LSB to a u32 TSF value */
- clock = (tsf_msb << 16) | tsf_lsb;
- wl1271_info("TIME_SYNC_EVENT_ID: clock %u", clock);
+ u32 clock_low;
+ u32 clock_high;
+
+ clock_high = (tsf_high_msb << 16) | tsf_high_lsb;
+ clock_low = (tsf_low_msb << 16) | tsf_low_lsb;
+
+ wl1271_info("TIME_SYNC_EVENT_ID: clock_high %u, clock low %u",
+ clock_high, clock_low);
}

int wl18xx_process_mailbox_events(struct wl1271 *wl)
@@ -138,8 +144,10 @@ int wl18xx_process_mailbox_events(struct wl1271 *wl)

if (vector & TIME_SYNC_EVENT_ID)
wlcore_event_time_sync(wl,
- mbox->time_sync_tsf_msb,
- mbox->time_sync_tsf_lsb);
+ mbox->time_sync_tsf_high_msb,
+ mbox->time_sync_tsf_high_lsb,
+ mbox->time_sync_tsf_low_msb,
+ mbox->time_sync_tsf_low_lsb);

if (vector & RADAR_DETECTED_EVENT_ID) {
wl1271_info("radar event: channel %d type %s",
@@ -187,11 +195,11 @@ int wl18xx_process_mailbox_events(struct wl1271 *wl)
*/
if (vector & MAX_TX_FAILURE_EVENT_ID)
wlcore_event_max_tx_failure(wl,
- le32_to_cpu(mbox->tx_retry_exceeded_bitmap));
+ le16_to_cpu(mbox->tx_retry_exceeded_bitmap));

if (vector & INACTIVE_STA_EVENT_ID)
wlcore_event_inactive_sta(wl,
- le32_to_cpu(mbox->inactive_sta_bitmap));
+ le16_to_cpu(mbox->inactive_sta_bitmap));

if (vector & REMAIN_ON_CHANNEL_COMPLETE_EVENT_ID)
wlcore_event_roc_complete(wl);
diff --git a/drivers/net/wireless/ti/wl18xx/event.h b/drivers/net/wireless/ti/wl18xx/event.h
index 070de12..b436bf9 100644
--- a/drivers/net/wireless/ti/wl18xx/event.h
+++ b/drivers/net/wireless/ti/wl18xx/event.h
@@ -74,10 +74,16 @@ struct wl18xx_event_mailbox {
__le16 bss_loss_bitmap;

/* bitmap of stations (by HLID) which exceeded max tx retries */
- __le32 tx_retry_exceeded_bitmap;
+ __le16 tx_retry_exceeded_bitmap;
+
+ /* time sync high msb*/
+ u16 time_sync_tsf_high_msb;

/* bitmap of inactive stations (by HLID) */
- __le32 inactive_sta_bitmap;
+ __le16 inactive_sta_bitmap;
+
+ /* time sync high lsb*/
+ u16 time_sync_tsf_high_lsb;

/* rx BA win size indicated by RX_BA_WIN_SIZE_CHANGE_EVENT_ID */
u8 rx_ba_role_id;
@@ -98,14 +104,15 @@ struct wl18xx_event_mailbox {
u8 sc_sync_channel;
u8 sc_sync_band;

- /* time sync msb*/
- u16 time_sync_tsf_msb;
+ /* time sync low msb*/
+ u16 time_sync_tsf_low_msb;
+
/* radar detect */
u8 radar_channel;
u8 radar_type;

- /* time sync lsb*/
- u16 time_sync_tsf_lsb;
+ /* time sync low lsb*/
+ u16 time_sync_tsf_low_lsb;

} __packed;

--
2.9.0



2016-06-23 11:18:09

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] wlcore: time sync : add support for 64 bit clock

On Thu, 2016-06-23 at 14:12 +0300, Yaniv Machani wrote:
> Changed the configuration to support 64bit instead of 32bit
> this in order to offload the driver from handling a wraparound.

[...]

Since you Cc'ed me, and presumably want me to review it, I'll say that
this looks like a terrible idea:

> @@ -74,10 +74,16 @@ struct wl18xx_event_mailbox {

This struct is evidently used for firmware/host communication.

>   __le16 bss_loss_bitmap;
>  
>   /* bitmap of stations (by HLID) which exceeded max tx
> retries */
> - __le32 tx_retry_exceeded_bitmap;
> + __le16 tx_retry_exceeded_bitmap;
> +
> + /* time sync high msb*/
> + u16 time_sync_tsf_high_msb;

So first of all, just using u16 instead of __le16 seems wrong.

Additionally, this looks like it changes the firmware API, so that
older firmware images will no longer work?

johannes

2016-06-23 11:32:44

by Yaniv Machani

[permalink] [raw]
Subject: RE: [PATCH] wlcore: time sync : add support for 64 bit clock

T24gVGh1LCBKdW4gMjMsIDIwMTYgYXQgMTQ6MTg6MDAsIEpvaGFubmVzIEJlcmcgd3JvdGU6DQo+
IGxpbnV4LXdpcmVsZXNzQHZnZXIua2VybmVsLm9yZzsgbmV0ZGV2QHZnZXIua2VybmVsLm9yZw0K
PiBTdWJqZWN0OiBSZTogW1BBVENIXSB3bGNvcmU6IHRpbWUgc3luYyA6IGFkZCBzdXBwb3J0IGZv
ciA2NCBiaXQgY2xvY2sNCj4gDQo+IE9uIFRodSwgMjAxNi0wNi0yMyBhdCAxNDoxMiArMDMwMCwg
WWFuaXYgTWFjaGFuaSB3cm90ZToNCj4gPiBDaGFuZ2VkIHRoZSBjb25maWd1cmF0aW9uIHRvIHN1
cHBvcnQgNjRiaXQgaW5zdGVhZCBvZiAzMmJpdCB0aGlzIGluIA0KPiA+IG9yZGVyIHRvIG9mZmxv
YWQgdGhlIGRyaXZlciBmcm9tIGhhbmRsaW5nIGEgd3JhcGFyb3VuZC4NCj4gDQo+IFsuLi5dDQo+
IA0KPiBTaW5jZSB5b3UgQ2MnZWQgbWUsIGFuZCBwcmVzdW1hYmx5IHdhbnQgbWUgdG8gcmV2aWV3
IGl0LCBJJ2xsIHNheSB0aGF0IA0KPiB0aGlzIGxvb2tzIGxpa2UgYSB0ZXJyaWJsZSBpZGVhOg0K
PiANCj4gPiBAQCAtNzQsMTAgKzc0LDE2IEBAIHN0cnVjdCB3bDE4eHhfZXZlbnRfbWFpbGJveCB7
DQo+IA0KPiBUaGlzIHN0cnVjdCBpcyBldmlkZW50bHkgdXNlZCBmb3IgZmlybXdhcmUvaG9zdCBj
b21tdW5pY2F0aW9uLg0KPiANCj4gPiDCoAlfX2xlMTYgYnNzX2xvc3NfYml0bWFwOw0KPiA+DQo+
ID4gwqAJLyogYml0bWFwIG9mIHN0YXRpb25zIChieSBITElEKSB3aGljaCBleGNlZWRlZCBtYXgg
dHggcmV0cmllcyAqLw0KPiA+IC0JX19sZTMyIHR4X3JldHJ5X2V4Y2VlZGVkX2JpdG1hcDsNCj4g
PiArCV9fbGUxNiB0eF9yZXRyeV9leGNlZWRlZF9iaXRtYXA7DQo+ID4gKw0KPiA+ICsJLyogdGlt
ZSBzeW5jIGhpZ2ggbXNiKi8NCj4gPiArCXUxNiB0aW1lX3N5bmNfdHNmX2hpZ2hfbXNiOw0KPiAN
Cj4gU28gZmlyc3Qgb2YgYWxsLCBqdXN0IHVzaW5nIHUxNiBpbnN0ZWFkIG9mIF9fbGUxNiBzZWVt
cyB3cm9uZy4NCg0KQWdyZWUsIHNob3VsZCBiZSBjaGFuZ2VkLg0KDQo+IA0KPiBBZGRpdGlvbmFs
bHksIHRoaXMgbG9va3MgbGlrZSBpdCBjaGFuZ2VzIHRoZSBmaXJtd2FyZSBBUEksIHNvIHRoYXQg
DQo+IG9sZGVyIGZpcm13YXJlIGltYWdlcyB3aWxsIG5vIGxvbmdlciB3b3JrPw0KDQpJdCBpcyBi
YWNrd2FyZHMgY29tcGF0aWJsZSwgDQphbHRob3VnaCBpdCBjaGFuZ2VzIGEgQVBJIHN0cnVjdHVy
ZSwgb2xkZXIgZmlybXdhcmUgYXJlIHVzaW5nIG9ubHkgdTE2IGZvciB0aGUgZmllbGQgc28gdGhl
cmUgaXMgbm8gaW1wYWN0IG9uIHRoYXQuDQpPZiBjb3Vyc2UgdGhhdCBmb3IgYWN0dWFsbHkgdXNp
bmcgdGhlIDY0Yml0IGluZm9ybWF0aW9uLCB5b3Ugd2lsbCBoYXZlIHRvIHVwZ3JhZGUgdGhlIGZp
cm13YXJlLg0KDQpZYW5pdg0KDQo=

2016-06-23 11:35:45

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] wlcore: time sync : add support for 64 bit clock


> > Additionally, this looks like it changes the firmware API, so that 
> > older firmware images will no longer work?
>
> It is backwards compatible, 
> although it changes a API structure, older firmware are using only
> u16 for the field so there is no impact on that.
>

Oh, ok. I had also thought that the size changed, but missed that you
replaced a u32 with two u16. Thanks for checking :)

johannes