All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] wlcore: time sync : add support for 64 bit clock
@ 2016-06-23 11:12 Yaniv Machani
  2016-06-23 11:18 ` Johannes Berg
  0 siblings, 1 reply; 6+ messages in thread
From: Yaniv Machani @ 2016-06-23 11:12 UTC (permalink / raw)
  To: linux-kernel
  Cc: Yaniv Machani, Kalle Valo, Guy Mishol, Eliad Peller,
	Johannes Berg, linux-wireless, netdev

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 <yanivma@ti.com>
---
 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] wlcore: time sync : add support for 64 bit clock
  2016-06-23 11:12 [PATCH] wlcore: time sync : add support for 64 bit clock Yaniv Machani
@ 2016-06-23 11:18 ` Johannes Berg
  2016-06-23 11:32     ` Machani, Yaniv
  0 siblings, 1 reply; 6+ messages in thread
From: Johannes Berg @ 2016-06-23 11:18 UTC (permalink / raw)
  To: Yaniv Machani, linux-kernel
  Cc: Kalle Valo, Guy Mishol, Eliad Peller, linux-wireless, netdev

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] wlcore: time sync : add support for 64 bit clock
  2016-06-23 11:18 ` Johannes Berg
@ 2016-06-23 11:32     ` Machani, Yaniv
  0 siblings, 0 replies; 6+ messages in thread
From: Machani, Yaniv @ 2016-06-23 11:32 UTC (permalink / raw)
  To: Johannes Berg, linux-kernel@vger.kernel.org
  Cc: Kalle Valo, Mishol, Guy, Eliad Peller,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org

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=

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] wlcore: time sync : add support for 64 bit clock
@ 2016-06-23 11:32     ` Machani, Yaniv
  0 siblings, 0 replies; 6+ messages in thread
From: Machani, Yaniv @ 2016-06-23 11:32 UTC (permalink / raw)
  To: Johannes Berg, linux-kernel@vger.kernel.org
  Cc: Kalle Valo, Mishol, Guy, Eliad Peller,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org

On Thu, Jun 23, 2016 at 14:18:00, Johannes Berg wrote:
> linux-wireless@vger.kernel.org; netdev@vger.kernel.org
> 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.

Agree, should be changed.

> 
> 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.
Of course that for actually using the 64bit information, you will have to upgrade the firmware.

Yaniv

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] wlcore: time sync : add support for 64 bit clock
@ 2016-06-23 11:35       ` Johannes Berg
  0 siblings, 0 replies; 6+ messages in thread
From: Johannes Berg @ 2016-06-23 11:35 UTC (permalink / raw)
  To: Machani, Yaniv, linux-kernel@vger.kernel.org
  Cc: Kalle Valo, Mishol, Guy, Eliad Peller,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] wlcore: time sync : add support for 64 bit clock
@ 2016-06-23 11:35       ` Johannes Berg
  0 siblings, 0 replies; 6+ messages in thread
From: Johannes Berg @ 2016-06-23 11:35 UTC (permalink / raw)
  To: Machani, Yaniv,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
  Cc: Kalle Valo, Mishol, Guy, Eliad Peller,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org


> > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-06-23 11:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-23 11:12 [PATCH] wlcore: time sync : add support for 64 bit clock Yaniv Machani
2016-06-23 11:18 ` Johannes Berg
2016-06-23 11:32   ` Machani, Yaniv
2016-06-23 11:32     ` Machani, Yaniv
2016-06-23 11:35     ` Johannes Berg
2016-06-23 11:35       ` Johannes Berg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.