RE: [EXTERNAL] Re: [RFT 3/6] wlcore: Add support for runtime PM

[Reizer, Eyal]

> >This happens on runtime_suspend() where we are already in PLT
> >and then that error gets stored and then next pm_runtime_get()
> >returns -EINVAL. The patch below should fix it. I'll fold it
> i>nto the runtime PM related patch assuming it works for you.
> >
> >Regards,
> >
> >Tony
> >
> >8< ---
> >diff --git a/drivers/net/wireless/ti/wlcore/main.c
> b/drivers/net/wireless/ti/wlcore/main.c
> >--- a/drivers/net/wireless/ti/wlcore/main.c
> >+++ b/drivers/net/wireless/ti/wlcore/main.c
> >@@ -6677,7 +6677,7 @@ static int __maybe_unused
> wlcore_runtime_suspend(struct device *dev)
> >
> > /* We do not enter elp sleep in PLT mode */
> > if (wl->plt)
> >-return -EINVAL;
> >+return 0;
> >
> > /* Nothing to do if no ELP mode requested */
> > if (wl->sleep_auth != WL1271_PSM_ELP)
> 
> Even with this change I still see issues with a wl1281 module plugged in.
> It take a few seconds for the crash to happen once you turn plt on.
> Log below.
> Do you see this on your wl12xx based platform?
> 
> sh-4.4#
> sh-4.4# calibrator wlan0 plt power_mode on
> [  231.105877] wlcore: power up
> [  231.667604] wlcore: firmware booted in PLT mode PLT_ON (PLT
> 7.3.10.2.142)
> sh-4.4#
> sh-4.4#
> sh-4.4# [  236.900817] [ cut here ]
> [  236.906012] WARNING: CPU: 0 PID: 520 at
> drivers/net/wireless/ti/wlcore/main.c:806

Hold on, I might have edited the wrong version of the file (main.c). 
Sorry about that. Working on too many branches/boards... :(
Applied this change again and things look good now with the wl1281 based module.

Best Regards,
Eyal

RE: [EXTERNAL] Re: [RFT 3/6] wlcore: Add support for runtime PM

[Reizer, Eyal]

>> However, trying a wl1281 module and enabling PLT mode it boots ok but after 
>> a couple of seconds the below crash is seen.
>> Seems like a similar crash to the one I have seen before,right?
>
>Sorry for the delay, only today had enough time to figure
>this one out, see below.
>
>> sh-4.4# calibrator wlan0 plt power_mode on
>> [   57.198492] wlcore: power up
>> [   57.757871] wlcore: firmware booted in PLT mode PLT_ON (PLT 7.3.10.2.142)
>> sh-4.4#
>> sh-4.4#
>> sh-4.4# ca[   86.485020] [ cut here ]
>> [   86.490334] WARNING: CPU: 0 PID: 502 at 
>> drivers/net/wireless/ti/wlcore/main.c:806
>
>This happens on runtime_suspend() where we are already in PLT
>and then that error gets stored and then next pm_runtime_get()
>returns -EINVAL. The patch below should fix it. I'll fold it
i>nto the runtime PM related patch assuming it works for you.
>
>Regards,
>
>Tony
>
>8< ---
>diff --git a/drivers/net/wireless/ti/wlcore/main.c 
>b/drivers/net/wireless/ti/wlcore/main.c
>--- a/drivers/net/wireless/ti/wlcore/main.c
>+++ b/drivers/net/wireless/ti/wlcore/main.c
>@@ -6677,7 +6677,7 @@ static int __maybe_unused wlcore_runtime_suspend(struct 
>device *dev)
> 
>   /* We do not enter elp sleep in PLT mode */
>   if (wl->plt)
>-  return -EINVAL;
>+  return 0;
> 
>   /* Nothing to do if no ELP mode requested */
>   if (wl->sleep_auth != WL1271_PSM_ELP)

Even with this change I still see issues with a wl1281 module plugged in.
It take a few seconds for the crash to happen once you turn plt on.
Log below.
Do you see this on your wl12xx based platform?

sh-4.4#
sh-4.4# calibrator wlan0 plt power_mode on
[  231.105877] wlcore: power up
[  231.667604] wlcore: firmware booted in PLT mode PLT_ON (PLT 7.3.10.2.142)
sh-4.4#
sh-4.4#
sh-4.4# [  236.900817] [ cut here ]
[  236.906012] WARNING: CPU: 0 PID: 520 at 
drivers/net/wireless/ti/wlcore/main.c:806 wl12xx_queue_recovery_work+0x64/0x6c 
[wlcore]
[  236.917783] Modules linked in: ctr aes_arm_bs crypto_simd cryptd ccm arc4 
pru_rproc pruss_intc wl12xx wlcore mac80211 cfg80211 pruss musb_dsps musb_hdrc 
udc_core usbcore phy_am335x phy_generic usb_common phy_am335x_control 
ti_am335x_adc ti_am335x_tsc snd_soc_simple_card snd_soc_simple_card_utils 
pm33xx wkup_m3_ipc wkup_m3_rproc remoteproc omap_aes_driver crypto_engine 
omap_crypto omap_sham ti_emif_sram pruss_soc_bus wlcore_sdio 
snd_soc_tlv320aic3x rtc_omap musb_am335x omap_wdt ti_am335x_tscadc 
matrix_keypad matrix_keymap sch_fq_codel
[  236.965780] CPU: 0 PID: 520 Comm: irq/71-wl12xx Not tainted 
4.14.40-01415-g47241db-dirty #123
[  236.974439] Hardware name: Generic AM33XX (Flattened Device Tree)
[  236.980666] Backtrace:
[  236.983250] [] (dump_backtrace) from [] 
(show_stack+0x18/0x1c)
[  236.990967]  r6: r5:bf2e25fc r4: r3:
[  236.996799] [] (show_stack) from [] 
(dump_stack+0x20/0x28)
[  237.004145] [] (dump_stack) from [] (__warn+0xdc/0x104)
[  237.011339] [] (__warn) from [] 
(warn_slowpath_null+0x28/0x30)
[  237.019099]  r10:c0d4ea79 r8:c0169590 r7:db1bee58 r6:dc726ec8 r5:dc726d38 
r4:dc726d00
[  237.027285] [] (warn_slowpath_null) from [] 
(wl12xx_queue_recovery_work+0x64/0x6c [wlcore])
[  237.037892] [] (wl12xx_queue_recovery_work [wlcore]) from 
[] (wlcore_irq+0x21c/0x234 [wlcore])
[  237.048412]  r4:dc726d00 r3:db2d3810
[  237.052260] [] (wlcore_irq [wlcore]) from [] 
(irq_thread_fn+0x24/0x3c)
[  237.060688]  r7:db1bee58 r6:db1bee00 r5:0001 r4:db1af3c0
[  237.066481] [] (irq_thread_fn) from [] 
(irq_thread+0x10c/0x1ec)
[  237.074242]  r6:db1bee00 r5:0001 r4:db1af3c0 r3:dc73aa00
[  237.080069] [] (irq_thread) from [] (kthread+0x11c/0x154)
[  237.087350]  r10:c0169154 r8:db1af3c0 r7:db1af918 r6:db1af340 r5: 
r4:db1af900
[  237.095305] [] (kthread) from [] 
(ret_from_fork+0x14/0x2c)
[  237.102673]  r10: r9: r8: r7: r6: 
r5:c0145150
[  237.110646]  r4:db1af340 r3:
[  237.114292] ---[ end trace 2c3213a5c8df86b0 ]---
[  237.856438] sched: RT throttling activated
[  238.886549] omap_i2c 44e0b000.i2c: controller timed out
[  242.846592] omap_i2c 44e0b000.i2c: controller timed out

BR,
Eyal

RE: [EXTERNAL] Re: [RFT 3/6] wlcore: Add support for runtime PM

[Reizer, Eyal]

> > * Tony Lindgren  [180605 04:22]:
> > > * Reizer, Eyal  [180603 06:07]:
> > > > I have noticed the following recovery a couple of times on my setup
> > when the board was
> > > > just sitting for a long time with just pings
> > > > It starts with a firmware recovery started from the interrupt handler
> but
> > the recovery fails
> > >
> > > Sounds like the recovery needs some more work :)
> > >
> > > > leaving the sdio stuck.
> > > > At this stage the only way to get out of it is unload/load of the driver
> > modules.
> > > > Have you seen this on your side as well?
> > >
> > > Hmm I don't think I've seen this one yet.
> > >
> > > > 64 bytes from 192.168.100.1: seq=32772 ttl=64 time=9.644 ms
> > > > 64 bytes from 192.168.100.1: seq=32773 ttl=64 time=9.572 ms
> > > > 64 bytes from 192.168.100.1: seq=32774 ttl=64 time=10.974 ms
> > > > 64 bytes from 192.168.100.1: seq=32775 ttl=64 time=9.618 ms
> > > > [127899.040526] wlcore: ERROR SW watchdog interrupt received!
> starting
> > recovery.
> > >
> > > Do you know what does the SW watchdog means here? Does it mean the
> > > interrupt did not get delivered to wlcore? Or a spurious IRQ to wlcore?
> > > Or a timeout waiting for the ELP wake interrupt?
> >
> > Also, can you check if patch "[RFT 7/6] wlcore: Make sure firmware is
> > initialized
> > in wl1271_op_add_interface()" already fixes this issue if you did not have 
> > it
> > already applied?
> >
> Sure will do. The firmware that showed this issue has some extra debugging
> info
> Enabled in it and this may have been the cause of the recovery (Infernal
> logging error).
> The fact that it didn't recover was the issue itself. I am now running with 
> the
> latest's
> firmware 8.9.0.0.78 and will see if I can still replicate it.
> If I see it, I will try applying patch 7 and see if it helps. Right now I 
> don't yet
> have
> It applied.
> 

Latest wl18xx firmware was running fine for two days without the crash, so it 
was indeed just a logging issue in this case.
However, trying a wl1281 module and enabling PLT mode it boots ok but after a 
couple of seconds the below crash is seen.
Seems like a similar crash to the one I have seen before,right?
I do have patch 7/6 applied now.

sh-4.4# calibrator wlan0 plt power_mode on
[   57.198492] wlcore: power up
[   57.757871] wlcore: firmware booted in PLT mode PLT_ON (PLT 7.3.10.2.142)
sh-4.4#
sh-4.4#
sh-4.4# ca[   86.485020] [ cut here ]
[   86.490334] WARNING: CPU: 0 PID: 502 at 
drivers/net/wireless/ti/wlcore/main.c:806 wl12xx_queue_recovery_work+0x64/0x6c 
[wlcore]
[   86.502047] Modules linked in: arc4 pru_rproc wl12xx pruss_intc wlcore 
mac80211 cfg80211 pruss ti_am335x_tsc ti_am335x_adc musb_dsps musb_hdrc 
udc_core usbcore phy_am335x phy_generic phy_am335x_control usb_common pm33xx 
wkup_m3_ipc snd_soc_simple_card snd_soc_simple_card_utils wkup_m3_rproc 
remoteproc omap_aes_driver crypto_engine omap_sham omap_crypto ti_emif_sram 
pruss_soc_bus snd_soc_tlv320aic3x wlcore_sdio matrix_keypad rtc_omap 
ti_am335x_tscadc omap_wdt matrix_keymap musb_am335x sch_fq_codel
[   86.546686] CPU: 0 PID: 502 Comm: irq/71-wl12xx Not tainted 
4.14.40-01415-g47241db-dirty #119
[   86.555312] Hardware name: Generic AM33XX (Flattened Device Tree)
[   86.561540] Backtrace:
[   86.564119] [] (dump_backtrace) from [] 
(show_stack+0x18/0x1c)
[   86.571838]  r6: r5:bf30a5fc r4: r3:
[   86.577661] [] (show_stack) from [] 
(dump_stack+0x20/0x28)
[   86.584994] [] (dump_stack) from [] (__warn+0xdc/0x104)
[   86.592121] [] (__warn) from [] 
(warn_slowpath_null+0x28/0x30)
[   86.599838]  r10:c0d4ea79 r8:c0169590 r7:db0f1558 r6:dc716ec8 r5:dc716d38 
r4:dc716d00
[   86.608019] [] (warn_slowpath_null) from [] 
(wl12xx_queue_recovery_work+0x64/0x6c [wlcore])
[   86.618630] [] (wl12xx_queue_recovery_work [wlcore]) from 
[] (wlcore_irq+0x21c/0x234 [wlcore])
[   86.629157]  r4:dc716d00 r3:db164010
[   86.633009] [] (wlcore_irq [wlcore]) from [] 
(irq_thread_fn+0x24/0x3c)
[   86.641441]  r7:db0f1558 r6:db0f1500 r5:0001 r4:db1b8680
[   86.647236] [] (irq_thread_fn) from [] 
(irq_thread+0x10c/0x1ec)
[   86.655044]  r6:db0f1500 r5:0001 r4:db1b8680 r3:db623600
[   86.660875] [] (irq_thread) from [] (kthread+0x11c/0x154)
[   86.668148]  r10:c0169154 r8:db1b8680 r7:db1b8958 r6:db1b8600 r5: 
r4:db1b8940
[   86.676104] [] (kthread) from [] 
(ret_from_fork+0x14/0x2c)
[   86.683479]  r10: r9: r8: r7: r6: 
r5:c0145150
[   86.691451]  r4:db1b8600 r3:
[   86.695099] ---[ end trace dea7def5777109c9 ]---

BR,
Eyal

RE: [EXTERNAL] Re: [RFT 3/6] wlcore: Add support for runtime PM

[Reizer, Eyal]

> 
> * Tony Lindgren  [180605 04:22]:
> > * Reizer, Eyal  [180603 06:07]:
> > > I have noticed the following recovery a couple of times on my setup
> when the board was
> > > just sitting for a long time with just pings
> > > It starts with a firmware recovery started from the interrupt handler but
> the recovery fails
> >
> > Sounds like the recovery needs some more work :)
> >
> > > leaving the sdio stuck.
> > > At this stage the only way to get out of it is unload/load of the driver
> modules.
> > > Have you seen this on your side as well?
> >
> > Hmm I don't think I've seen this one yet.
> >
> > > 64 bytes from 192.168.100.1: seq=32772 ttl=64 time=9.644 ms
> > > 64 bytes from 192.168.100.1: seq=32773 ttl=64 time=9.572 ms
> > > 64 bytes from 192.168.100.1: seq=32774 ttl=64 time=10.974 ms
> > > 64 bytes from 192.168.100.1: seq=32775 ttl=64 time=9.618 ms
> > > [127899.040526] wlcore: ERROR SW watchdog interrupt received! starting
> recovery.
> >
> > Do you know what does the SW watchdog means here? Does it mean the
> > interrupt did not get delivered to wlcore? Or a spurious IRQ to wlcore?
> > Or a timeout waiting for the ELP wake interrupt?
> 
> Also, can you check if patch "[RFT 7/6] wlcore: Make sure firmware is
> initialized
> in wl1271_op_add_interface()" already fixes this issue if you did not have it
> already applied?
> 
Sure will do. The firmware that showed this issue has some extra debugging info
Enabled in it and this may have been the cause of the recovery (Infernal 
logging error).
The fact that it didn't recover was the issue itself. I am now running with the 
latest's 
firmware 8.9.0.0.78 and will see if I can still replicate it.
If I see it, I will try applying patch 7 and see if it helps. Right now I don't 
yet have
It applied.

Best Regard,
Eyal

RE: [EXTERNAL] Re: [RFT 3/6] wlcore: Add support for runtime PM

[Reizer, Eyal]

> 
> * Tony Lindgren  [180529 18:09]:
> > --- a/drivers/net/wireless/ti/wlcore/main.c
> > +++ b/drivers/net/wireless/ti/wlcore/main.c
> ...
> > +static int __maybe_unused wlcore_runtime_resume(struct device *dev)
> > +{
> > +   struct wl1271 *wl = dev_get_drvdata(dev);
> > +   DECLARE_COMPLETION_ONSTACK(compl);
> > +   unsigned long flags;
> > +   int ret;
> > +   unsigned long start_time = jiffies;
> > +   bool pending = false;
> > +
> > +   /* Nothing to do if no ELP mode requested */
> > +   if (!test_bit(WL1271_FLAG_IN_ELP, >flags))
> > +   return 0;
> > +
> > +   wl1271_debug(DEBUG_PSM, "waking up chip from elp");
> > +
> > +   spin_lock_irqsave(>wl_lock, flags);
> > +   if (test_bit(WL1271_FLAG_IRQ_RUNNING, >flags))
> > +   pending = true;
> > +   else
> > +   wl->elp_compl = 
> > +   spin_unlock_irqrestore(>wl_lock, flags);
> > +
> > +   ret = wlcore_raw_write32(wl, HW_ACCESS_ELP_CTRL_REG,
> ELPCTRL_WAKE_UP);
> > +   if (ret < 0) {
> > +   wl12xx_queue_recovery_work(wl);
> > +   goto err;
> > +   }
> > +
> > +   if (!pending) {
> > +   ret = wait_for_completion_timeout(,
> > +   msecs_to_jiffies(WL1271_WAKEUP_TIMEOUT));
> > +   if (ret == 0) {
> > +   wl1271_error("ELP wakeup timeout!");
> > +   wl12xx_queue_recovery_work(wl);
> > +
> > +   /* Return no error for runtime PM for recovery */
> > +   return 0;
> 
> I noticed returning here and not clearing WL1271_FLAG_IN_ELP can
> produce errors. But I think it's unsafe to clear WL1271_FLAG_IN_ELP
> if wlcore did not wake. So I suggest we return early here to see the
> errors so we can fix the issues. I'll post one more patch that fixes
> the init time "ELP wakeup timeout!" issues for me.
> 

I have noticed the following recovery a couple of times on my setup when the 
board was 
just sitting for a long time with just pings
It starts with a firmware recovery started from the interrupt handler but the 
recovery fails 
leaving the sdio stuck.
At this stage the only way to get out of it is unload/load of the driver 
modules.
Have you seen this on your side as well?

64 bytes from 192.168.100.1: seq=32772 ttl=64 time=9.644 ms
64 bytes from 192.168.100.1: seq=32773 ttl=64 time=9.572 ms
64 bytes from 192.168.100.1: seq=32774 ttl=64 time=10.974 ms
64 bytes from 192.168.100.1: seq=32775 ttl=64 time=9.618 ms
[127899.040526] wlcore: ERROR SW watchdog interrupt received! starting recovery.
[127899.047922] [ cut here ]
[127899.053009] WARNING: CPU: 0 PID: 20135 at 
drivers/net/wireless/ti/wlcore/main.c:806 wl12xx_queue_recovery_work+0x64/0x6c 
[wlcore]
[127899.064899] Modules linked in: wlcore_sdio wl18xx wlcore mac80211 cfg80211 
ctr aes_arm_bs crypto_simd cryptd ccm usb_f_acm u_serial arc4 pru_rproc 
pruss_intc usb_f_ecm pruss musb_dsps musb_hdrc phy_am335x usbcore phy_generic 
phy_am335x_control xfrm_user xfrm4_tunnel ipcomp xfrm_ipcomp esp4 ah4 g_multi 
usb_f_mass_storage usb_f_rndis af_key u_ether xfrm_algo libcomposite udc_core 
usb_common bluetooth ecdh_generic snd_soc_simple_card snd_soc_simple_card_utils 
pm33xx wkup_m3_rproc wkup_m3_ipc remoteproc omap_aes_driver crypto_engine 
omap_crypto omap_sham ti_emif_sram pruss_soc_bus musb_am335x rtc_omap omap_wdt 
sch_fq_codel [last unloaded: cfg80211]
[127899.122874] CPU: 0 PID: 20135 Comm: irq/65-wl18xx Tainted: GW   
4.14.40-01413-g9541ff6-dirty #107
[127899.132988] Hardware name: Generic AM33XX (Flattened Device Tree)
[127899.139247] Backtrace:
[127899.141866] [] (dump_backtrace) from [] 
(show_stack+0x18/0x1c)
[127899.149636]  r6: r5:bf4e25fc r4: r3:
[127899.155493] [] (show_stack) from [] 
(dump_stack+0x20/0x28)
[127899.162876] [] (dump_stack) from [] (__warn+0xdc/0x104)
[127899.170032] [] (__warn) from [] 
(warn_slowpath_null+0x28/0x30)
[127899.177795]  r10:c0d4ea79 r8:c0169590 r7:db276658 r6:d6abaec8 r5:d6abad38 
r4:d6abad00
[127899.185989] [] (warn_slowpath_null) from [] 
(wl12xx_queue_recovery_work+0x64/0x6c [wlcore])
[127899.196605] [] (wl12xx_queue_recovery_work [wlcore]) from 
[] (wlcore_irq+0x21c/0x234 [wlcore])
[127899.207161]  r4:d6abad00 r3:0001
[127899.211036] [] (wlcore_irq [wlcore]) from [] 
(irq_thread_fn+0x24/0x3c)
[127899.219498]  r7:db276658 r6:db276600 r5:0001 r4:d6818ec0
[127899.225341] [] (irq_thread_fn) from [] 
(irq_thread+0x10c/0x1ec)
[127899.233143]  r6:db276600 r5:0001 r4:d6818ec0 r3:db361200
[127899.238997] [] (irq_thread) from [] 
(kthread+0x11c/0x154)
[127899.246324]  r10:c0169154 r8:d6818ec0 r7:d6818bd8 r6:d6818b80 r5: 
r4:d6818bc0
[127899.254313] [] (kthread) from [] 
(ret_from_fork+0x14/0x2c)
[127899.261724]  r10: r9: r8: r7: r6: 
r5:c0145150
[127899.269728]  r4:d6818b80 r3:
[127899.273422] ---[ end trace 5ef4c2e6abb6b669 ]---
[127899.285612] wlcore: Hardware recovery in progress. FW ver: Rev 8.9.0.6.76

Re: [RFT 6/6] wlcore: Use generic runtime pm calls for wowlan elp configuration

[Reizer, Eyal]

> > >
> > > With runtime PM enabled, we can now use generic calls to
> > > pm_generic_runtime_suspend and pm_generic_runtime_resume for
> enabling elp
> > > during suspend when wowlan is enabled and waking the chip from elp
> > > on resume.
> >
> > Sry, but not sure you can :(
> >
> > These functions are not used by drivers directly because system suspend
> > are not synchronized with PM runtime, so if you call
> pm_generic_runtime_suspend()
> > and, at the same time, there is pm_runtime_get_() in progress --> race ...
> >
> > The pm_runtime_force_() APIs have to be used, or
> > PM runtime drivers functions can be called directly, but only if it 
> > possible to
> be
> > sure no other PM runtime calls active which is usually true at
> suspend_noirq stage.
> 
> Oh right, those are subsystem calls. Seems like
> pm_runtime_force_suspend/resume() should work here,
> Eyal?
> 

Oh, nice, wasn't aware of the pm_runtime_force_() calls.
For some reason they are not documented in:
https://www.kernel.org/doc/Documentation/power/runtime_pm.txt
Perhaps they should be?

Anyway I have tried them instead of pm_generic_runtime_() and they seem
To work fine on my platform.
Will test some more and submit a v2.

Best Regards,
Eyal

RE: [EXTERNAL] Re: [PATCH] wlcore: sdio: check for valid platform device data before suspend

[Reizer, Eyal]

> > the wl pointer can be null In case only wlcore_sdio is probed while
> > no WiLink module is successfully probed, as in the case of mounting a
> > wl12xx module while using a device tree file configured with wl18xx
> > related settings.
> > In this case the system was crashing in wl1271_suspend() as platform
> > device data is not set.
> > Make sure wl the pointer is valid before using it.
> 
> Just checking.. It seems like this fix is separate from the PM
> runtime conversion, right?

Correct. This is a separate patch/
> 
> > Signed-off-by: Eyal Reizer mamai
> 
> Looks like you have some extra characters ^ here.
> 
Thanks!. Will fix and submit v2

Best Regards,
Eyal

RE: [EXTERNAL] Re: [PATCH] wlcore: use generic runtime pm calls for wowlan elp configuration

[Reizer, Eyal]

> 
> > With runtime PM enabled, we can now use generic calls to
> > pm_generic_runtime_suspend and pm_generic_runtime_resume for
> enabling elp
> > during suspend when wowlan is enabled and waking the chip from elp
> > on resume.
> > remove the custom API that was used to ensure that the command
> > that is used to allow ELP during suspend is completed before the system
> > suspend.
> >
> > Signed-off-by: Eyal Reizer 
> > ---
> > Dependent on the Runtime PM support for wlcore patch set that was
> submitted
> > by Tony Lindgren 
> 
> Ok, I'll drop this from my queue. Please resubmit once Tony's patches
> are ready to apply (or Tony can include this patch in his patchset).
> 
> BTW, for me it would be a lot easier if you could mark patches like this
> as RFT (Request For Test) or RFC (Request For Comments). That way I can
> automatically drop the patch without sending any emails.
> 

Understood. Thanks!

Best Regards,
Eyal

RE: [PATCHv2 0/5] Runtime PM support for wlcore

[Reizer, Eyal]

> 
> >
> > Here's a modified version of your patch, does that put wlcore to
> > idle with wowlan during suspend for you?
> >
> 
> Still no joy.
> It suspends/resumes ok but leaves the firmware disabled from entering ELP.

Spent some time on it today and looks like adding calls to 
pm_generic_runtime_suspend ()
And pm_generic_runtime_resume is helping and all seems to work well.
See the modified version of your patch below.
Let me know what you think.

Best Regards,
Eyal

8< --
diff --git a/drivers/net/wireless/ti/wlcore/main.c
b/drivers/net/wireless/ti/wlcore/main.c
index 4c297aa..9859e5a 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -1001,24 +1001,6 @@ static int wlcore_fw_wakeup(struct wl1271 *wl)
 return wlcore_raw_write32(wl, HW_ACCESS_ELP_CTRL_REG, ELPCTRL_WAKE_UP);
 }

-static int wlcore_fw_sleep(struct wl1271 *wl)
-{
-int ret;
-
-mutex_lock(>mutex);
-ret = wlcore_raw_write32(wl, HW_ACCESS_ELP_CTRL_REG, ELPCTRL_SLEEP);
-if (ret < 0) {
-wl12xx_queue_recovery_work(wl);
-goto out;
-}
-set_bit(WL1271_FLAG_IN_ELP, >flags);
-out:
-mutex_unlock(>mutex);
-mdelay(WL1271_SUSPEND_SLEEP);
-
-return 0;
-}
-
 static int wl1271_setup(struct wl1271 *wl)
 {
 wl->raw_fw_status = kzalloc(wl->fw_status_len, GFP_KERNEL);
@@ -1742,6 +1724,7 @@ static int wl1271_op_suspend(struct ieee80211_hw *hw,
 {
 struct wl1271 *wl = hw->priv;
 struct wl12xx_vif *wlvif;
+unsigned long flags;
 int ret;

 wl1271_debug(DEBUG_MAC80211, "mac80211 suspend wow=%d", !!wow);
@@ -1800,19 +1783,6 @@ static int wl1271_op_suspend(struct ieee80211_hw *hw,
 /* flush any remaining work */
 wl1271_debug(DEBUG_MAC80211, "flushing remaining works");

-/*
- * disable and re-enable interrupts in order to flush
- * the threaded_irq
- */
-wlcore_disable_interrupts(wl);
-
-/*
- * set suspended flag to avoid triggering a new threaded_irq
- * work. no need for spinlock as interrupts are disabled.
- */
-set_bit(WL1271_FLAG_SUSPENDED, >flags);
-
-wlcore_enable_interrupts(wl);
 flush_work(>tx_work);

 /*
@@ -1822,13 +1792,14 @@ static int wl1271_op_suspend(struct ieee80211_hw *hw,
 cancel_delayed_work(>tx_watchdog_work);

 /*
- * Use an immediate call for allowing the firmware to go into power
- * save during suspend.
- * Using a workque for this last write was only hapenning on resume
- * leaving the firmware with power save disabled during suspend,
- * while consuming full power during wowlan suspend.
+ * set suspended flag to avoid triggering a new threaded_irq
+ * work.
  */
-wlcore_fw_sleep(wl);
+spin_lock_irqsave(>wl_lock, flags);
+set_bit(WL1271_FLAG_SUSPENDED, >flags);
+spin_unlock_irqrestore(>wl_lock, flags);
+
+pm_generic_runtime_suspend(wl->dev);

 return 0;
 }
@@ -1845,6 +1816,8 @@ static int wl1271_op_resume(struct ieee80211_hw *hw)
  wl->wow_enabled);
 WARN_ON(!wl->wow_enabled);

+pm_generic_runtime_resume(wl->dev);
+
 /*
  * re-enable irq_work enqueuing, and call irq_work directly if
  * there is a pending work.

RE: [EXTERNAL] [PATCHv2 0/5] Runtime PM support for wlcore

[Reizer, Eyal]

> 
> Here's a modified version of your patch, does that put wlcore to
> idle with wowlan during suspend for you?
> 

Still no joy.
It suspends/resumes ok but leaves the firmware disabled from entering ELP.
You can see the log below with some prints added to wlcore_runtime_suspend()
And wlcore_runtime_resume().
What you can see is that normally after each transaction, such as scan below,
It ends where the firmware is allowed to enter ELP based on its internal logic.
You can see the print "chip allowed to entered elp" below.

root@am335x-evm:~#
root@am335x-evm:~#
root@am335x-evm:~# iw wlan0 scan | grep SSID
[  106.010879] disabling the FW from enering ELP
[  106.026780] wlcore_runtime_suspend -> enter
[  106.01] allowing chip to entered elp
 [  106.037823] chip allowed to entered elp
...
...
 [  110.110140] disabling the FW from enering ELP
[  110.224902] wlcore_runtime_suspend -> enter
[  110.229208] allowing chip to entered elp
SSID: IOTLP_521
SSID: Reizer
SSID: LinksysADSL
SSID: RT2880_AP
SSID: net4guest
[  110.252460] chip allowed to entered elp
SSID: halekoa75
SSID: externalhotspot84
SSID: cpn84
SSID: WPS_AP_5G
SSID: MarvellAP95
[  110.266707] disabling the FW from enering ELP
[  110.279297] wlcore_runtime_suspend -> enter
SSID:
SSID:
[  110.292041] allowing chip to entered elp
SSID: net4guest
[  110.303485] chip allowed to entered elp
SSID: halekoa75
SSID: externalhotspot84
SSID: cpn84
SSID: net4guest
SSID: halekoa75
SSID: externalhotspot84
SSID: cpn84
root@am335x-evm:~#
root@am335x-evm:~#
root@am335x-evm:~#

This is not the case when suspending.
You can see below that the message " PM: Successfully put all powerdomains to 
target state"
Comes before the call to pm_runtime_suspend() was executed and the firmware
Remained in full active state consuming full power during the whole time the 
system was 
suspended.
The call to pm_runtime_suspend is only seen on resume:

[  124.153960] Restarting tasks ... 
[  124.154702] wlcore_runtime_suspend -> enter

I have also verified that this is not just a print issue by using a firmware 
logger that 
Shows the internal state of the firmware and can see that the call to allow ELP
Actually comes only after resume.
This is what I am trying to chase now. Something is not right here with 
pm_runtime.
Any ideas here?

root@am335x-evm:~#
root@am335x-evm:~# echo mem > /sys/power/state
[  123.72] PM: suspend entry (deep)
[  123.448119] PM: Syncing filesystems ... done.
[  123.467382] Freezing user space processes ... (elapsed 0.002 seconds) done.
[  123.477144] OOM killer disabled.
[  123.480424] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) 
done.
[  123.489880] Suspending console(s) (use no_console_suspend to debug)
[  123.505821] disabling the FW from enering ELP
[  123.861590] pm33xx pm33xx: PM: Successfully put all powerdomains to target 
state
[  123.861590] PM: Wakeup source UART
[  123.886091] net eth0: initializing cpsw version 1.12 (0)
[  123.984353] SMSC LAN8710/LAN8720 4a101000.mdio:00: attached PHY driver [SMSC 
LAN8710/LAN8720] (mii_bus:phy_addr=4a101000.mdio:00, irq=POLL)
[  124.150623] OOM killer enabled.
[  124.153960] Restarting tasks ...
[  124.154702] wlcore_runtime_suspend -> enter
[  124.171414] done.
[  124.190085] allowing chip to entered elp
[  124.199877] chip allowed to entered elp
[  124.208633] PM: suspend exit
root@am335x-evm:~#

Best Regards,
Eyal

RE: [EXTERNAL] [PATCHv2 0/5] Runtime PM support for wlcore

[Reizer, Eyal]

> > >
> > > OK try replacing the pm_runtime_put_noidle() above with just
> > > pm_runtime_put_sync(). The reason why I put noidle there was the
> > > wlcore_fw_sleep() call, with that gone put_sync should do the trick.
> > >
> >
> > I have tried that already. Same problem. The last call to:
> > ret = wlcore_raw_write32(wl, HW_ACCESS_ELP_CTRL_REG, ELPCTRL_SLEEP)
> >
> > which allows the firmware to get into ELP state during wowlan suspend is
> > only completing after system resume for some unknown reason...
> 
> Hmm maybe try also adding wl1271_power_off(wl) after put_sync()?
> 

No, we don't want to power off the chip in wowlan mode.
We power it of only during standard suspend.

The trick is that it stays on during suspend and can be used 
As a wakeup source to the host on specific packets received by
The firmware over the air.

BR,
Eyal

RE: [EXTERNAL] [PATCHv2 0/5] Runtime PM support for wlcore

[Reizer, Eyal]

> 
> * Reizer, Eyal <ey...@ti.com> [180522 13:28]:
> > Actually the below patch removing the call to wlcore_fw_sleep() avoids this
> error.
> > The downside is that the wl8 firmware remains fully active during supend, so
> we
> > Would need to find the root cause why the last call allowing the wilink8
> firmware
> > To go into ELP mode during suspend is only completing on resume and not
> during
> > Suspend enter.
> >
> > diff --git a/drivers/net/wireless/ti/wlcore/main.c
> > b/drivers/net/wireless/ti/wlcore/main.c
> > index 4c297aa..8df1ae6 100644
> > --- a/drivers/net/wireless/ti/wlcore/main.c
> > +++ b/drivers/net/wireless/ti/wlcore/main.c
> > @@ -1789,7 +1789,6 @@ static int wl1271_op_suspend(struct
> ieee80211_hw *hw,
> > goto out_sleep;
> >
> >  out_sleep:
> > -   pm_runtime_put_noidle(wl->dev);
> > mutex_unlock(>mutex);
> >
> > if (ret < 0) {
> > @@ -1821,15 +1820,7 @@ static int wl1271_op_suspend(struct
> ieee80211_hw *hw,
> >  */
> > cancel_delayed_work(>tx_watchdog_work);
> >
> > -   /*
> > -* Use an immediate call for allowing the firmware to go into power
> > -* save during suspend.
> > -* Using a workque for this last write was only hapenning on resume
> > -* leaving the firmware with power save disabled during suspend,
> > -* while consuming full power during wowlan suspend.
> > -*/
> > -   wlcore_fw_sleep(wl);
> > -
> > +   pm_runtime_put_noidle(wl->dev);
> > return 0;
> >  }
> 
> OK try replacing the pm_runtime_put_noidle() above with just
> pm_runtime_put_sync(). The reason why I put noidle there was the
> wlcore_fw_sleep() call, with that gone put_sync should do the trick.
> 

I have tried that already. Same problem. The last call to:
ret = wlcore_raw_write32(wl, HW_ACCESS_ELP_CTRL_REG, ELPCTRL_SLEEP)

which allows the firmware to get into ELP state during wowlan suspend is 
only completing after system resume for some unknown reason...

Best Regards,
Eyal

RE: [EXTERNAL] [PATCHv2 0/5] Runtime PM support for wlcore

[Reizer, Eyal]

> >
> > 8< 
> > diff --git a/drivers/net/wireless/ti/wlcore/main.c
> > b/drivers/net/wireless/ti/wlcore/main.c
> > --- a/drivers/net/wireless/ti/wlcore/main.c
> > +++ b/drivers/net/wireless/ti/wlcore/main.c
> > @@ -1867,8 +1867,6 @@ static int __maybe_unused
> > wl1271_op_resume(struct ieee80211_hw *hw)
> > if (ret)
> > wl12xx_queue_recovery_work(wl);
> > }
> > -
> > -   wlcore_enable_interrupts(wl);
> > }
> >
> > if (pending_recovery) {
> > @@ -1877,6 +1875,8 @@ static int __maybe_unused
> > wl1271_op_resume(struct ieee80211_hw *hw)
> > goto out_sleep;
> > }
> >
> > +   wlcore_enable_interrupts(wl);
> > +
> > ret = pm_runtime_get_sync(wl->dev);
> > if (ret < 0) {
> > pm_runtime_put_noidle(wl->dev);
> 
> It still crash.
> The crash is different now.
> It also complains about:
> [   60.544224] Unbalanced enable for IRQ 65
> Need down/up of the interface to recover after it.
> Log below:
> 

Actually the below patch removing the call to wlcore_fw_sleep() avoids this 
error.
The downside is that the wl8 firmware remains fully active during supend, so we
Would need to find the root cause why the last call allowing the wilink8 
firmware 
To go into ELP mode during suspend is only completing on resume and not during
Suspend enter.

diff --git a/drivers/net/wireless/ti/wlcore/main.c
b/drivers/net/wireless/ti/wlcore/main.c
index 4c297aa..8df1ae6 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -1789,7 +1789,6 @@ static int wl1271_op_suspend(struct ieee80211_hw *hw,
goto out_sleep;

 out_sleep:
-   pm_runtime_put_noidle(wl->dev);
mutex_unlock(>mutex);

if (ret < 0) {
@@ -1821,15 +1820,7 @@ static int wl1271_op_suspend(struct ieee80211_hw *hw,
 */
cancel_delayed_work(>tx_watchdog_work);

-   /*
-* Use an immediate call for allowing the firmware to go into power
-* save during suspend.
-* Using a workque for this last write was only hapenning on resume
-* leaving the firmware with power save disabled during suspend,
-* while consuming full power during wowlan suspend.
-*/
-   wlcore_fw_sleep(wl);
-
+   pm_runtime_put_noidle(wl->dev);
return 0;
 }

With this wowlan seems to work ok and suspend/resume is not crashing when 
enabling wowlan. See below:

root@am335x-evm:/usr/share/wl18xx#
root@am335x-evm:/usr/share/wl18xx#
root@am335x-evm:/usr/share/wl18xx#
root@am335x-evm:/usr/share/wl18xx# iw phy0 wowlan enable any
root@am335x-evm:/usr/share/wl18xx#
root@am335x-evm:/usr/share/wl18xx# echo mem > /sys/power/state
[   63.794805] PM: suspend entry (deep)
[   63.798455] PM: Syncing filesystems ... done.
[   65.779673] Freezing user space processes ... (elapsed 0.001 seconds) done.
[   65.788878] OOM killer disabled.
[   65.792117] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) 
done.
[   65.801196] Suspending console(s) (use no_console_suspend to debug)
[   65.952459] pm33xx pm33xx: PM: Successfully put all powerdomains to target 
state
[   65.952459] PM: Wakeup source GPIO0
[   65.977028] net eth0: initializing cpsw version 1.12 (0)
[   66.074419] SMSC LAN8710/LAN8720 4a101000.mdio:00: attached PHY driver [SMSC 
LAN8710/LAN8720] (mii_bus:phy_addr=4a101000.mdio:00, irq=POLL)
[   66.236312] OOM killer enabled.
[   66.239604] Restarting tasks ... done.
[   66.282501] PM: suspend exit
root@am335x-evm:/usr/share/wl18xx#

BR,
Eyal

RE: [EXTERNAL] [PATCHv2 0/5] Runtime PM support for wlcore

[Reizer, Eyal]

> >
> > This warning is because wlcore is wlcore is still in ELP. This is
> > somehow possible even though we call pm_runtime_get_sync() in
> > wl1271_op_resume(). Anyways, I'll try to reproduce it here.
> 
> Sorry I can't somehow get my beagleboard to wake-up from suspend,
> I'm almost certain that worked the last time I tried.
> 
> Anyways, maybe the following patch fixes this if you care to
> test again.
> 
> Regards,
> 
> Tony
> 
> 8< 
> diff --git a/drivers/net/wireless/ti/wlcore/main.c
> b/drivers/net/wireless/ti/wlcore/main.c
> --- a/drivers/net/wireless/ti/wlcore/main.c
> +++ b/drivers/net/wireless/ti/wlcore/main.c
> @@ -1867,8 +1867,6 @@ static int __maybe_unused
> wl1271_op_resume(struct ieee80211_hw *hw)
>   if (ret)
>   wl12xx_queue_recovery_work(wl);
>   }
> -
> - wlcore_enable_interrupts(wl);
>   }
> 
>   if (pending_recovery) {
> @@ -1877,6 +1875,8 @@ static int __maybe_unused
> wl1271_op_resume(struct ieee80211_hw *hw)
>   goto out_sleep;
>   }
> 
> + wlcore_enable_interrupts(wl);
> +
>   ret = pm_runtime_get_sync(wl->dev);
>   if (ret < 0) {
>   pm_runtime_put_noidle(wl->dev);

It still crash.
The crash is different now.
It also complains about:
[   60.544224] Unbalanced enable for IRQ 65
Need down/up of the interface to recover after it.
Log below:

root@am335x-evm:~#
root@am335x-evm:~#
root@am335x-evm:~# echo mem > /sys/power/state
[   42.994534] PM: suspend entry (deep)
[   42.998182] PM: Syncing filesystems ... done.
[   43.946134] Freezing user space processes ... (elapsed 0.002 seconds) done.
[   43.957020] OOM killer disabled.
[   43.960449] Freezing remaining freezable tasks ... (elapsed 0.002 seconds) 
done.
[   43.971227] Suspending console(s) (use no_console_suspend to debug)
[   44.093784] wlcore: down
[   44.181597] pm33xx pm33xx: PM: Successfully put all powerdomains to target 
state
[   44.181597] PM: Wakeup source UART
[   44.206334] net eth0: initializing cpsw version 1.12 (0)
[   44.304417] SMSC LAN8710/LAN8720 4a101000.mdio:00: attached PHY driver [SMSC 
LAN8710/LAN8720] (mii_bus:phy_addr=4a101000.mdio:00, irq=POLL)
[   44.650784] wlcore: PHY firmware version: Rev 8.2.0.0.242
[   44.746182] wlcore: firmware booted (Rev 8.9.0.0.78)
[   44.929226] OOM killer enabled.
[   44.932602] Restarting tasks ... done.
[   44.961136] PM: suspend exit
root@am335x-evm:~# iw phy0 wowlan enable any
root@am335x-evm:~#
root@am335x-evm:~#
root@am335x-evm:~#
root@am335x-evm:~#
root@am335x-evm:~# echo mem > /sys/power/state
[   60.114495] PM: suspend entry (deep)
[   60.118146] PM: Syncing filesystems ... done.
[   60.139243] Freezing user space processes ... (elapsed 0.001 seconds) done.
[   60.148483] OOM killer disabled.
[   60.151723] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) 
done.
[   60.160874] Suspending console(s) (use no_console_suspend to debug)
[   60.411089] pm33xx pm33xx: PM: Successfully put all powerdomains to target 
state
[   60.411089] PM: Wakeup source UART
[   60.435069] net eth0: initializing cpsw version 1.12 (0)
[   60.534303] SMSC LAN8710/LAN8720 4a101000.mdio:00: attached PHY driver [SMSC 
LAN8710/LAN8720] (mii_bus:phy_addr=4a101000.mdio:00, irq=POLL)
[   60.544143] [ cut here ]
[   60.544213] WARNING: CPU: 0 PID: 730 at kernel/irq/manage.c:525 
__enable_irq+0x4c/0x6c
[   60.544224] Unbalanced enable for IRQ 65
[   60.544232] Modules linked in: usb_f_acm u_serial arc4 pru_rproc pruss_intc 
wl18xx usb_f_ecm wlcore musb_dsps mac80211 musb_hdrc cfg80211 pruss phy_am335x 
usbcore phy_generic phy_am335x_control xfrm_user xfrm4_tunnel ipcomp 
xfrm_ipcomp esp4 ah4 af_key xfrm_algo g_multi usb_f_mass_storage usb_f_rndis 
u_ether libcomposite udc_core usb_common bluetooth ecdh_generic 
snd_soc_simple_card snd_soc_simple_card_utils wkup_m3_rproc pm33xx wkup_m3_ipc 
remoteproc omap_aes_driver crypto_engine omap_crypto omap_sham ti_emif_sram 
pruss_soc_bus wlcore_sdio rtc_omap musb_am335x omap_wdt sch_fq_codel
[   60.544574] CPU: 0 PID: 730 Comm: kworker/u2:11 Not tainted 
4.14.40-01413-g36a61bea-dirty #105
[   60.544584] Hardware name: Generic AM33XX (Flattened Device Tree)
[   60.544624] Workqueue: events_unbound async_run_entry_fn
[   60.544639] Backtrace:
[   60.544698] [] (dump_backtrace) from [] 
(show_stack+0x18/0x1c)
[   60.544722]  r6: r5:c0a95d64 r4:db363d78 r3:c0d53158
[   60.544758] [] (show_stack) from [] 
(dump_stack+0x20/0x28)
[   60.544794] [] (dump_stack) from [] (__warn+0xdc/0x104)
[   60.544821] [] (__warn) from [] 
(warn_slowpath_fmt+0x40/0x48)
[   60.544848]  r10:c0d5310c r8: r7:db5a6d38 r6: r5:0041 
r4:
[   60.544874] [] (warn_slowpath_fmt) from [] 
(__enable_irq+0x4c/0x6c)
[   60.544889]  r3:0041 r2:c0a95ec4
[   60.544898]  r4:db278000
[   60.544922] [] (__enable_irq) from [] 
(enable_irq+0x3c/0x74)
[   60.545265] [] (enable_irq) 

RE: [EXTERNAL] [PATCHv2 0/5] Runtime PM support for wlcore

[Reizer, Eyal]

Hi Tony,

> 
> Hi all,
> 
> Here's a series of patches to add runtime PM support for wlcore. It does not
> yet implement autosuspend support, but let's get this tested first as the
> autosuspend can mask enable/disable issues easily.
> 
> Regards,
> 
> Tony
> 
> Changes since v1:
> 
> - Fix issues reported by Eyal for recovery
> 

Testing on BBB+WL1837 cape, scan, recovery, down/up and basic traffic seems ok 
now.
Of course we need to test some more.
Standard suspend/resume seems to work ok as well.
Ennabling wowlan and suspending is  crashing on resume. See below.

root@am335x-evm:/usr/share/wl18xx# iw phy0 wowlan enable any 
root@am335x-evm:/usr/share/wl18xx#
root@am335x-evm:/usr/share/wl18xx# echo mem > /sys/power/state
[  541.567039] PM: suspend entry (deep)
[  541.570688] PM: Syncing filesystems ... done.
[  541.594277] Freezing user space processes ... (elapsed 0.001 seconds) done.
[  541.603160] OOM killer disabled.
[  541.606738] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) 
done.
[  541.615984] Suspending console(s) (use no_console_suspend to debug)
[  542.895091] pm33xx pm33xx: PM: Successfully put all powerdomains to target 
state
[  542.895091] PM: Wakeup source UART
[  542.919791] net eth0: initializing cpsw version 1.12 (0)
[  543.017880] SMSC LAN8710/LAN8720 4a101000.mdio:00: attached PHY driver [SMSC 
LAN8710/LAN8720] (mii_bus:phy_addr=4a101000.mdio:00, irq=POLL)
[  543.027646] [ cut here ]
[  543.028023] WARNING: CPU: 0 PID: 932 at 
drivers/net/wireless/ti/wlcore/cmd.c:76 wlcore_cmd_send_failsafe+0x498/0x4f8 
[wlcore]
[  543.028033] Modules linked in: ctr aes_arm_bs crypto_simd cryptd ccm arc4 
pru_rproc pruss_intc wl18xx usb_f_acm u_serial wlcore mac80211 cfg80211 pruss 
usb_f_ecm musb_dsps musb_hdrc usbcore phy_am335x phy_generic phy_am335x_control 
xfrm_user xfrm4_tunnel ipcomp xfrm_ipcomp esp4 ah4 af_key xfrm_algo g_multi 
usb_f_mass_storage usb_f_rndis u_ether libcomposite udc_core usb_common 
bluetooth ecdh_generic snd_soc_simple_card snd_soc_simple_card_utils pm33xx 
wkup_m3_ipc wkup_m3_rproc remoteproc omap_aes_driver crypto_engine omap_crypto 
omap_sham ti_emif_sram pruss_soc_bus wlcore_sdio rtc_omap musb_am335x omap_wdt 
sch_fq_codel
[  543.028408] CPU: 0 PID: 932 Comm: kworker/u2:9 Not tainted 
4.14.40-01413-g36a61bea-dirty #104
[  543.028418] Hardware name: Generic AM33XX (Flattened Device Tree)
[  543.028466] Workqueue: events_unbound async_run_entry_fn
[  543.028480] Backtrace:
[  543.028541] [] (dump_backtrace) from [] 
(show_stack+0x18/0x1c)
[  543.028565]  r6: r5:bf3a759c r4: r3:c0d53158
[  543.028602] [] (show_stack) from [] 
(dump_stack+0x20/0x28)
[  543.028637] [] (dump_stack) from [] (__warn+0xdc/0x104)
[  543.028665] [] (__warn) from [] 
(warn_slowpath_null+0x28/0x30)
[  543.028691]  r10:c0d5310c r8: r7: r6:d6be5080 r5:000c 
r4:db59ed00
[  543.028896] [] (warn_slowpath_null) from [] 
(wlcore_cmd_send_failsafe+0x498/0x4f8 [wlcore])
[  543.029232] [] (wlcore_cmd_send_failsafe [wlcore]) from 
[] (wlcore_cmd_configure_failsafe+0x60/0xd8 [wlcore])
[  543.029259]  r10:c0d5310c r9:db59e258 r8: r7: r6:0024 
r5:db59ed00
[  543.029269]  r4:d6be5080
[  543.029597] [] (wlcore_cmd_configure_failsafe [wlcore]) from 
[] (wl1271_cmd_configure+0x1c/0x28 [wlcore])
[  543.029614]  r6:db59ed00 r5:0001 r4:d6be5080
[  543.029943] [] (wl1271_cmd_configure [wlcore]) from [] 
(wl1271_acx_default_rx_filter_enable+0x60/0xac [wlcore])
[  543.030272] [] (wl1271_acx_default_rx_filter_enable [wlcore]) from 
[] (wl1271_configure_wowlan+0x148/0x3c4 [wlcore])
[  543.030292]  r7:db59ed38 r6:db59ee20 r5: r4:dc705bc8
[  543.030616] [] (wl1271_configure_wowlan [wlcore]) from 
[] (wl1271_op_resume+0x310/0x344 [wlcore])
[  543.030643]  r10:c0d5310c r9:db59e258 r8: r7:db59ed38 r6:db59ee20 
r5:db59ed00
[  543.030652]  r4:dc705bc8
[  543.031484] [] (wl1271_op_resume [wlcore]) from [] 
(ieee80211_reconfig+0x418/0xc0c [mac80211])
[  543.031509]  r8: r7:db59e28c r6:bf2c75e8 r5: r4:db59e420
[  543.032290] [] (ieee80211_reconfig [mac80211]) from [] 
(ieee80211_resume+0x58/0x70 [mac80211])
[  543.032318]  r10:c0d5310c r9:db59e258 r8: r7:db59e28c r6:bf2c75e8 
r5:
[  543.032327]  r4:db59e420
[  543.033150] [] (ieee80211_resume [mac80211]) from [] 
(wiphy_resume+0x54/0x64 [cfg80211])
[  543.033165]  r4:db59e258 r3:bf32fd44
[  543.033448] [] (wiphy_resume [cfg80211]) from [] 
(dpm_run_callback+0x40/0xcc)
[  543.033462]  r4: r3:
[  543.033489] [] (dpm_run_callback) from [] 
(device_resume+0xbc/0x234)
[  543.033515]  r10: r9:dc005000 r8: r6:0010 r5:0001 
r4:db59e258
[  543.033542] [] (device_resume) from [] 
(async_resume+0x20/0x4c)
[  543.033567]  r8: r7:dc004100 r6:c0d4fde0 r5:c0d89b30 r4:db59e258 
r3:
[  543.033600] [] (async_resume) from [] 
(async_run_entry_fn+0x44/0x140)
[  543.033614]  r5:d6be5480 r4:d6be5490
[  

RE: [EXTERNAL] Re: [PATCH v2] wlcore: sdio: allow pm to handle sdio power

[Reizer, Eyal]

> 
> >>
> >> > pm_runtime handles sdio power on and power off transitions.
> >> > An old workaround for trying to control the power explicitly from the
> >> > driver was in fact causing failures on suspend/resume as the mmc layer
> >> > already power the module on resume.
> >> >
> >> > In case of resume pm_runtime_get sync returns a positive device's
> usage
> >> > count causing the driver to try an re-initialize an already initialized
> >> > device. This was causing sdio bus failure on resume.
> >> >
> >> > Remove this manual power on/off sequence as it is in-fact not needed.
> >> >
> >> > Signed-off-by: Eyal Reizer 
> >> > Acked-by: Tony Lindgren 
> >>
> >> No changelog.
> >>
> >>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingp
> >> atches#changelog_missing
> >>
> >> No need to resubmit because of this, I guess you just fixed the title
> >> and added Tony's Acked-by?
> >
> > Yes, this is correct. No change in the actual patch hence there was no
> change
> > Log.
> 
> _Always_ include a change log, even if you didn't actually change
> anything. Otherwise the maintainer has no clue what has changed and why
> a new version was submitted.
> 
Understood. Thanks!

BR,
Eyal

RE: [EXTERNAL] Re: [PATCH v2] wlcore: sdio: allow pm to handle sdio power

[Reizer, Eyal]

> 
> > pm_runtime handles sdio power on and power off transitions.
> > An old workaround for trying to control the power explicitly from the
> > driver was in fact causing failures on suspend/resume as the mmc layer
> > already power the module on resume.
> >
> > In case of resume pm_runtime_get sync returns a positive device's usage
> > count causing the driver to try an re-initialize an already initialized
> > device. This was causing sdio bus failure on resume.
> >
> > Remove this manual power on/off sequence as it is in-fact not needed.
> >
> > Signed-off-by: Eyal Reizer 
> > Acked-by: Tony Lindgren 
> 
> No changelog.
> 
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingp
> atches#changelog_missing
> 
> No need to resubmit because of this, I guess you just fixed the title
> and added Tony's Acked-by?

Yes, this is correct. No change in the actual patch hence there was no change 
Log.

Best Regards,
Eyal

[PATCH] wlcore: allow elp during wowlan suspend

[Reizer, Eyal]

when enabling wowlan and entering suspend the last write to the firmware
allowing it to go into elp mode was not completing before suspend, leaving
the firmware running in full active mode consuming high power.
Use an immediate call instead of a work queue for this last access
allowing the firmware to go into power save during wowlan uspend.

Signed-off-by: Eyal Reizer 
---
 drivers/net/wireless/ti/wlcore/main.c | 29 -
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ti/wlcore/main.c 
b/drivers/net/wireless/ti/wlcore/main.c
index 792cb91..1c31555 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -42,6 +42,7 @@
 #include "sysfs.h"
 
 #define WL1271_BOOT_RETRIES 3
+#define WL1271_SUSPEND_SLEEP 100
 
 static char *fwlog_param;
 static int fwlog_mem_blocks = -1;
@@ -979,6 +980,24 @@ static int wlcore_fw_wakeup(struct wl1271 *wl)
return wlcore_raw_write32(wl, HW_ACCESS_ELP_CTRL_REG, ELPCTRL_WAKE_UP);
 }
 
+static int wlcore_fw_sleep(struct wl1271 *wl)
+{
+   int ret;
+
+   mutex_lock(>mutex);
+   ret = wlcore_raw_write32(wl, HW_ACCESS_ELP_CTRL_REG, ELPCTRL_SLEEP);
+   if (ret < 0) {
+   wl12xx_queue_recovery_work(wl);
+   goto out;
+   }
+   set_bit(WL1271_FLAG_IN_ELP, >flags);
+out:
+   mutex_unlock(>mutex);
+   mdelay(WL1271_SUSPEND_SLEEP);
+
+   return 0;
+}
+
 static int wl1271_setup(struct wl1271 *wl)
 {
wl->raw_fw_status = kzalloc(wl->fw_status_len, GFP_KERNEL);
@@ -1750,7 +1769,6 @@ static int wl1271_op_suspend(struct ieee80211_hw *hw,
goto out_sleep;
 
 out_sleep:
-   wl1271_ps_elp_sleep(wl);
mutex_unlock(>mutex);
 
if (ret < 0) {
@@ -1783,6 +1801,15 @@ static int wl1271_op_suspend(struct ieee80211_hw *hw,
 */
cancel_delayed_work(>tx_watchdog_work);
 
+   /*
+* Use an immediate call for allowing the firmware to go into power
+* save during suspend.
+* Using a workque for this last write was only hapenning on resume
+* leaving the firmware with power save disabled during suspend,
+* while consuming full power during wowlan suspend.
+*/
+   wlcore_fw_sleep(wl);
+
return 0;
 }
 
-- 
2.7.4

RE: wl1837: poor performance?

[Reizer, Eyal]

Hi Russel,

> -Original Message-
> From: Russell King - ARM Linux [mailto:li...@armlinux.org.uk]
> Sent: Saturday, October 14, 2017 2:30 PM
> To: Reizer, Eyal
> Cc: linux-wireless@vger.kernel.org; Altshul, Maxim; Narang, Saurabh
> Subject: Re: wl1837: poor performance?
> 
> On Fri, Oct 13, 2017 at 07:43:37AM +, Reizer, Eyal wrote:
> > Sorry for top posting.
> > Signal level on wlan0 seems low (-79 dbm) are you sure there are antennas
> > connected?
> 
> Thanks for the reply.
> 
> In development environments, it's common to have the AP and station
> nearby, which will give good signal.  Out of the development
> environment, the AP and station may be separated by several 10s of ft
> and objects in the way.  That's the case here, so about -80dBm is to
> be expected.
> 
> As I mentioned, there's two stations each with different chipsets, both
> with the same signal level being reported, both at a similar distance
> from the AP, but one performs way better than the other.  I've found
> the reason for this (see below.)
> 
> I should also mention that this is installed remotely, and I'm debugging
> it over the 'net, involving this very Wi-Fi connection.
> 
> The radio environment is not excessively populated - both systems are
> within a tower with >2ft thick stone and lime mortar walls which radio
> has difficulty penetrating, which is itself in a park with very few
> other buildings around.  I'm using the 2.4G channel 8, I've seen some
> other APs on channel 1 when outside but almost never inside the tower.
> 
> nmcli (used to?) occasionally reports that the wl1837 can see another
> AP ("CARWIFI") on channel 1, but that's rare - I suspect it requires
> someone to park their car in direct line of literal sight through a
> tower window from the machine.  I suspect NM noticed that before I
> configured the BSSID of the associated AP as I haven't recently
> noticed NM reporting any other networks.  Could also be that non-wifi
> enabled cars are parked in those spaces!
> 
> > In addition what type of module is used here?
> 
> It's a WL1837 integrated onto SolidRun's iMX6 microsom.  I'm not sure
> what other information in terms of "module" you're asking for.

Do you know what type of wilink8 module is installed with this SOM board?
Is it a TI module similar to this one?:

http://www.ti.com/lit/ds/symlink/wl1837mod.pdf

> 
> > Di you use the configure_device.sh scripts as documented in the
> > "wlconf" manual?
> 
> No, because quite honestly the wilink tooling is something of a mess
> in terms of trying to find the right tooling.  This is what I ended
> up doing:

Wlconf should be taken from:
https://git.ti.com/wilink8-wlan/18xx-ti-utils/trees/R8.7_SP2

I think you have used something a bit outdated.
You can see the following wiki for building all latest components:
http://processors.wiki.ti.com/index.php/WL18xx_System_Build_Scripts

You can use the build script for building just the "utils" part if you just 
need wlconf. You can also use the git above directly.
Once elconf is build and installed you will see the following script in your 
rootfs:
https://git.ti.com/wilink8-wlan/18xx-ti-utils/blobs/R8.7_SP2/wlconf/configure-device.sh

That you just need to run once, answering the relevant questions and it will 
create wl18xx-conf.bin for you.
There is no need  for manually editing any files.
See the following document as well:
http://www.ti.com/lit/an/swra489/swra489.pdf
You can follow section 2 of this document.


> 
> 1. cloning git://github.com/TI-OpenLink/18xx-ti-utils
> 2. taking the wl18xx driver default configuration (from debugfs).
> 3. taking the ti wilink driver conf.h files.
> 4. massaging the conf.h files into a form that wlconf can read
> 5. generating a new struct.bin
> 6. changing:
>wl18xx.phy.number_of_assembled_ant2_4 = 0x01
>wl18xx.phy.number_of_assembled_ant5 = 0x00
> 
> since this variant of the microsom I have only populates one antenna.
> (The layout allows for the design to be extended to a second antenna.)
> 
> I've since found the _right_ repository for the tooling
> (git://git.ti.com/wilink8-wlan/18xx-ti-utils.git), and compared the
> resulting files, and confirmed that my new struct.bin is identical to
> the one in the right repository.
> 
> There are some differences between the two files (- = mine,
> + = configure_device.sh generated):
> 
> -core.sg.params = 0x, 0x, 0x, 0x,
> 0x, 0x, 0x, 0x, 0x, 0x,
> 0x, 0x, 0x, 0x, 0x, 0x,
> 0x, 0x, 0x, 0x, 0x, 0x,
> 0x, 0x, 0x, 0x00

[v9] wlcore: add missing nvs file name info for wilink8

[Reizer, Eyal]

The following commits:
commit c815fdebef44 ("wlcore: spi: Populate config firmware data")
commit d776fc86b82f ("wlcore: sdio: Populate config firmware data")

Populated the nvs entry for wilink6 and wilink7 only while it is
still needed for wilink8 as well.
This broke user space backward compatibility when upgrading from older
kernels, as the alternate mac address would not be read from the nvs that
is present in the file system (lib/firmware/ti-connectivity/wl1271-nvs.bin)
causing mac address change of the wlan interface.

This patch fix this and update the structure field with the same default
nvs file name that has been used before.

In addition, some distros hold a default wl1271-nvs.bin in the file
system with a bogus mac address (deadbeef...) that overrides the mac
address that is stored inside the device.
Warn users about this bogus mac address and use the internal mac address

Fixes: c815fdebef44 ("wlcore: spi: Populate config firmware data")
Fixes: d776fc86b82f ("wlcore: sdio: Populate config firmware data")
Signed-off-by: Eyal Reizer 
Reviewed-by: Sebastian Reichel 
Tested-by: Tony Lindgren 
---
v2->v3: add a check for default deadbeef... mac address and warn about it
v3->v4: use a random TI mac address instead of the bogus one
v4->v5: add constant definition for TI oui address
v5->v6: after also verifying on wilink6/7 Use internal mac address
instead of a random one
v6->v7: use random mac in case internal mac is zero
v7->v8: adjust warning message based on wilink family (wl12xx/wl18xx)
v8->v9: change to detecting wl18xx family for correct warning string
when bogus nvs is used
---
 drivers/net/wireless/ti/wlcore/main.c   | 23 +++
 drivers/net/wireless/ti/wlcore/sdio.c   |  1 +
 drivers/net/wireless/ti/wlcore/spi.c|  1 +
 drivers/net/wireless/ti/wlcore/wlcore.h |  3 +++
 4 files changed, 28 insertions(+)

diff --git a/drivers/net/wireless/ti/wlcore/main.c 
b/drivers/net/wireless/ti/wlcore/main.c
index 60aaa85..c346c02 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -6016,6 +6016,8 @@ static int wl1271_register_hw(struct wl1271 *wl)
 {
int ret;
u32 oui_addr = 0, nic_addr = 0;
+   struct platform_device *pdev = wl->pdev;
+   struct wlcore_platdev_data *pdev_data = dev_get_platdata(>dev);
 
if (wl->mac80211_registered)
return 0;
@@ -6040,6 +6042,27 @@ static int wl1271_register_hw(struct wl1271 *wl)
nic_addr = wl->fuse_nic_addr + 1;
}
 
+   if (oui_addr == 0xdeadbe && nic_addr == 0xef) {
+   wl1271_warning("Detected unconfigured mac address in nvs, 
derive from fuse instead.\n");
+   if (!strcmp(pdev_data->family->name, "wl18xx")) {
+   wl1271_warning("This default nvs file can be removed 
from the file system\n");
+   } else {
+   wl1271_warning("Your device performance is not 
optimized.\n");
+   wl1271_warning("Please use the calibrator tool to 
configure your device.\n");
+   }
+
+   if (wl->fuse_oui_addr == 0 && wl->fuse_nic_addr == 0) {
+   wl1271_warning("Fuse mac address is zero. using random 
mac\n");
+   /* Use TI oui and a random nic */
+   oui_addr = WLCORE_TI_OUI_ADDRESS;
+   nic_addr = get_random_int();
+   } else {
+   oui_addr = wl->fuse_oui_addr;
+   /* fuse has the BD_ADDR, the WLAN addresses are the 
next two */
+   nic_addr = wl->fuse_nic_addr + 1;
+   }
+   }
+
wl12xx_derive_mac_addresses(wl, oui_addr, nic_addr);
 
ret = ieee80211_register_hw(wl->hw);
diff --git a/drivers/net/wireless/ti/wlcore/sdio.c 
b/drivers/net/wireless/ti/wlcore/sdio.c
index 2fb3871..f8a1fea 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -230,6 +230,7 @@ static const struct wilink_family_data wl128x_data = {
 static const struct wilink_family_data wl18xx_data = {
.name = "wl18xx",
.cfg_name = "ti-connectivity/wl18xx-conf.bin",
+   .nvs_name = "ti-connectivity/wl1271-nvs.bin",
 };
 
 static const struct of_device_id wlcore_sdio_of_match_table[] = {
diff --git a/drivers/net/wireless/ti/wlcore/spi.c 
b/drivers/net/wireless/ti/wlcore/spi.c
index fdabb92..62ce54a 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -92,6 +92,7 @@ static const struct wilink_family_data wl128x_data = {
 static const struct wilink_family_data wl18xx_data = {
.name = "wl18xx",
.cfg_name = "ti-connectivity/wl18xx-conf.bin",
+   .nvs_name = "ti-connectivity/wl1271-nvs.bin",
 };
 
 struct wl12xx_spi_glue {
diff --git a/drivers/net/wireless/ti/wlcore/wlcore.h 

[v8] wlcore: add missing nvs file name info for wilink8

[Reizer, Eyal]

The following commits:
commit c815fdebef44 ("wlcore: spi: Populate config firmware data")
commit d776fc86b82f ("wlcore: sdio: Populate config firmware data")

Populated the nvs entry for wilink6 and wilink7 only while it is
still needed for wilink8 as well.
This broke user space backward compatibility when upgrading from older
kernels, as the alternate mac address would not be read from the nvs that
is present in the file system (lib/firmware/ti-connectivity/wl1271-nvs.bin)
causing mac address change of the wlan interface.

This patch fix this and update the structure field with the same default
nvs file name that has been used before.

In addition, some distros hold a default wl1271-nvs.bin in the file
system with a bogus mac address (deadbeef...) that overrides the mac
address that is stored inside the device.
Warn users about this bogus mac address and use the internal mac address

Fixes: c815fdebef44 ("wlcore: spi: Populate config firmware data")
Fixes: d776fc86b82f ("wlcore: sdio: Populate config firmware data")
Signed-off-by: Eyal Reizer 
Reviewed-by: Sebastian Reichel 
Tested-by: Tony Lindgren 
---
v2->v3: add a check for default deadbeef... mac address and warn about it
v3->v4: use a random TI mac address instead of the bogus one
v4->v5: add constant definition for TI oui address
v5->v6: after also verifying on wilink6/7 Use internal mac address
instead of a random one
v6->v7: use random mac in case internal mac is zero
v7->v8: adjust warning message based on wilink family (wl12xx/wl18xx)
---
 drivers/net/wireless/ti/wlcore/main.c   | 23 +++
 drivers/net/wireless/ti/wlcore/sdio.c   |  1 +
 drivers/net/wireless/ti/wlcore/spi.c|  1 +
 drivers/net/wireless/ti/wlcore/wlcore.h |  3 +++
 4 files changed, 28 insertions(+)

diff --git a/drivers/net/wireless/ti/wlcore/main.c 
b/drivers/net/wireless/ti/wlcore/main.c
index 60aaa85..2c80cea 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -6016,6 +6016,8 @@ static int wl1271_register_hw(struct wl1271 *wl)
 {
int ret;
u32 oui_addr = 0, nic_addr = 0;
+   struct platform_device *pdev = wl->pdev;
+   struct wlcore_platdev_data *pdev_data = dev_get_platdata(>dev);
 
if (wl->mac80211_registered)
return 0;
@@ -6040,6 +6042,27 @@ static int wl1271_register_hw(struct wl1271 *wl)
nic_addr = wl->fuse_nic_addr + 1;
}
 
+   if (oui_addr == 0xdeadbe && nic_addr == 0xef) {
+   wl1271_warning("Detected unconfigured mac address in nvs, 
derive from fuse instead.\n");
+   if (!strcmp(pdev_data->family->name, "wl12xx")) {
+   wl1271_warning("Your device performance is not 
optimized.\n");
+   wl1271_warning("Please use the calibrator tool to 
configure your device.\n");
+   } else {
+   wl1271_warning("This default nvs file can be removed 
from the file system\n");
+   }
+
+   if (wl->fuse_oui_addr == 0 && wl->fuse_nic_addr == 0) {
+   wl1271_warning("Fuse mac address is zero. using random 
mac\n");
+   /* Use TI oui and a random nic */
+   oui_addr = WLCORE_TI_OUI_ADDRESS;
+   nic_addr = get_random_int();
+   } else {
+   oui_addr = wl->fuse_oui_addr;
+   /* fuse has the BD_ADDR, the WLAN addresses are the 
next two */
+   nic_addr = wl->fuse_nic_addr + 1;
+   }
+   }
+
wl12xx_derive_mac_addresses(wl, oui_addr, nic_addr);
 
ret = ieee80211_register_hw(wl->hw);
diff --git a/drivers/net/wireless/ti/wlcore/sdio.c 
b/drivers/net/wireless/ti/wlcore/sdio.c
index 2fb3871..f8a1fea 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -230,6 +230,7 @@ static const struct wilink_family_data wl128x_data = {
 static const struct wilink_family_data wl18xx_data = {
.name = "wl18xx",
.cfg_name = "ti-connectivity/wl18xx-conf.bin",
+   .nvs_name = "ti-connectivity/wl1271-nvs.bin",
 };
 
 static const struct of_device_id wlcore_sdio_of_match_table[] = {
diff --git a/drivers/net/wireless/ti/wlcore/spi.c 
b/drivers/net/wireless/ti/wlcore/spi.c
index fdabb92..62ce54a 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -92,6 +92,7 @@ static const struct wilink_family_data wl128x_data = {
 static const struct wilink_family_data wl18xx_data = {
.name = "wl18xx",
.cfg_name = "ti-connectivity/wl18xx-conf.bin",
+   .nvs_name = "ti-connectivity/wl1271-nvs.bin",
 };
 
 struct wl12xx_spi_glue {
diff --git a/drivers/net/wireless/ti/wlcore/wlcore.h 
b/drivers/net/wireless/ti/wlcore/wlcore.h
index 1827546..95fbedc 100644
--- a/drivers/net/wireless/ti/wlcore/wlcore.h
+++ 

RE: [v7 wlcore: add missing nvs file name info for wilink8

[Reizer, Eyal]

> >
> > +   if (oui_addr == 0xdeadbe && nic_addr == 0xef) {
> > +   wl1271_warning("Detected unconfigured mac address in
> nvs.\n"
> > +   "derive from fuse instead.\n"
> > +   "in case of using a wl12xx device, your "
> > +   "device performance may not be
> optimized.\n"
> > +   "Please use the calibrator tool to configure "
> > +   "your device.\n"
> > +   "When using a wl18xx device this default nvs
> "
> > +   "file can be removed from the file
> system\n");
> 
> Usually we do not break multiline messages to make it easier to grep
> for them. Also it feels weird to say "if your device is ..., then
> ...", when we actually know which device it is. I suggest the
> following:
> 
> wl1271_warning("Detected unconfigured mac address in nvs, derive from
> fuse instead.\n");
> if (device_is_wl12xx) {
>   wl1271_warning("Your device performance is not optimized.\n");
>   wl1271_warning("Please use the calibrator tool to configure your
> device.\n");
> } else {
>   wl1271_warning("This default nvs file can be removed from the file
> system\n");
> }
OK, will try that out.

> 
> > +   if (wl->fuse_oui_addr == 0 && wl->fuse_nic_addr == 0) {
> > +   wl1271_warning("Fuse mac address is zero. using "
> > +   "random mac\n");
> 
> This one should also go into one line.

This will still exceed 80 characters. Is this still ok?

> 
> > +   /* Use TI oui and a random nic */
> > +   oui_addr = WLCORE_TI_OUI_ADDRESS;
> > +   nic_addr = get_random_int();
> > +   } else {
> > +   oui_addr = wl->fuse_oui_addr;
> > +   /* fuse has the BD_ADDR, the WLAN addresses are
> the next two */
> > +   nic_addr = wl->fuse_nic_addr + 1;
> > +   }
> > +   }
> > +
> > wl12xx_derive_mac_addresses(wl, oui_addr, nic_addr);
> >
> > ret = ieee80211_register_hw(wl->hw);
> > diff --git a/drivers/net/wireless/ti/wlcore/sdio.c
> b/drivers/net/wireless/ti/wlcore/sdio.c
> > index 2fb3871..f8a1fea 100644
> > --- a/drivers/net/wireless/ti/wlcore/sdio.c
> > +++ b/drivers/net/wireless/ti/wlcore/sdio.c
> > @@ -230,6 +230,7 @@ static const struct wilink_family_data wl128x_data =
> {
> >  static const struct wilink_family_data wl18xx_data = {
> > .name = "wl18xx",
> > .cfg_name = "ti-connectivity/wl18xx-conf.bin",
> > +   .nvs_name = "ti-connectivity/wl1271-nvs.bin",
> >  };
> >
> >  static const struct of_device_id wlcore_sdio_of_match_table[] = {
> > diff --git a/drivers/net/wireless/ti/wlcore/spi.c
> b/drivers/net/wireless/ti/wlcore/spi.c
> > index fdabb92..62ce54a 100644
> > --- a/drivers/net/wireless/ti/wlcore/spi.c
> > +++ b/drivers/net/wireless/ti/wlcore/spi.c
> > @@ -92,6 +92,7 @@ static const struct wilink_family_data wl128x_data = {
> >  static const struct wilink_family_data wl18xx_data = {
> > .name = "wl18xx",
> > .cfg_name = "ti-connectivity/wl18xx-conf.bin",
> > +   .nvs_name = "ti-connectivity/wl1271-nvs.bin",
> >  };
> >
> >  struct wl12xx_spi_glue {
> > diff --git a/drivers/net/wireless/ti/wlcore/wlcore.h
> b/drivers/net/wireless/ti/wlcore/wlcore.h
> > index 1827546..95fbedc 100644
> > --- a/drivers/net/wireless/ti/wlcore/wlcore.h
> > +++ b/drivers/net/wireless/ti/wlcore/wlcore.h
> > @@ -40,6 +40,9 @@
> >  /* wl12xx/wl18xx maximum transmission power (in dBm) */
> >  #define WLCORE_MAX_TXPWR25
> >
> > +/* Texas Instruments pre assigned OUI */
> > +#define WLCORE_TI_OUI_ADDRESS 0x080028
> > +
> >  /* forward declaration */
> >  struct wl1271_tx_hw_descr;
> >  enum wl_rx_buf_align;
> 
> Otherwise:
> 
> Reviewed-by: Sebastian Reichel 
> 
Thank you. Will submit v8 with  your comments about the strings. 
Just waiting for clarification on the 80 characters question above

Best Regards,
Eyal

[v7 wlcore: add missing nvs file name info for wilink8

[Reizer, Eyal]

The following commits:
commit c815fdebef44 ("wlcore: spi: Populate config firmware data")
commit d776fc86b82f ("wlcore: sdio: Populate config firmware data")

Populated the nvs entry for wilink6 and wilink7 only while it is
still needed for wilink8 as well.
This broke user space backward compatibility when upgrading from older
kernels, as the alternate mac address would not be read from the nvs that
is present in the file system (lib/firmware/ti-connectivity/wl1271-nvs.bin)
causing mac address change of the wlan interface.

This patch fix this and update the structure field with the same default
nvs file name that has been used before.

In addition, some distros hold a default wl1271-nvs.bin in the file
system with a bogus mac address (deadbeef...) that overrides the mac
address that is stored inside the device.
Warn users about this bogus mac address and use the internal mac address

Fixes: c815fdebef44 ("wlcore: spi: Populate config firmware data")
Fixes: d776fc86b82f ("wlcore: sdio: Populate config firmware data")
Signed-off-by: Eyal Reizer 
---
v2->v3: add a check for default deadbeef... mac address and warn about it
v3->v4: use a random TI mac address instead of the bogus one
v4->v5: add constant definition for TI oui address
v5->v6: after also verifying on wilink6/7 Use mac internal mac address
instead of a random one
v6->v7: use random mac in case internal mac is zero
---
 drivers/net/wireless/ti/wlcore/main.c   | 23 +++
 drivers/net/wireless/ti/wlcore/sdio.c   |  1 +
 drivers/net/wireless/ti/wlcore/spi.c|  1 +
 drivers/net/wireless/ti/wlcore/wlcore.h |  3 +++
 4 files changed, 28 insertions(+)

diff --git a/drivers/net/wireless/ti/wlcore/main.c 
b/drivers/net/wireless/ti/wlcore/main.c
index 60aaa85..40692ac 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -6040,6 +6040,29 @@ static int wl1271_register_hw(struct wl1271 *wl)
nic_addr = wl->fuse_nic_addr + 1;
}
 
+   if (oui_addr == 0xdeadbe && nic_addr == 0xef) {
+   wl1271_warning("Detected unconfigured mac address in nvs.\n"
+   "derive from fuse instead.\n"
+   "in case of using a wl12xx device, your "
+   "device performance may not be optimized.\n"
+   "Please use the calibrator tool to configure "
+   "your device.\n"
+   "When using a wl18xx device this default nvs "
+   "file can be removed from the file system\n");
+
+   if (wl->fuse_oui_addr == 0 && wl->fuse_nic_addr == 0) {
+   wl1271_warning("Fuse mac address is zero. using "
+   "random mac\n");
+   /* Use TI oui and a random nic */
+   oui_addr = WLCORE_TI_OUI_ADDRESS;
+   nic_addr = get_random_int();
+   } else {
+   oui_addr = wl->fuse_oui_addr;
+   /* fuse has the BD_ADDR, the WLAN addresses are the 
next two */
+   nic_addr = wl->fuse_nic_addr + 1;
+   }
+   }
+
wl12xx_derive_mac_addresses(wl, oui_addr, nic_addr);
 
ret = ieee80211_register_hw(wl->hw);
diff --git a/drivers/net/wireless/ti/wlcore/sdio.c 
b/drivers/net/wireless/ti/wlcore/sdio.c
index 2fb3871..f8a1fea 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -230,6 +230,7 @@ static const struct wilink_family_data wl128x_data = {
 static const struct wilink_family_data wl18xx_data = {
.name = "wl18xx",
.cfg_name = "ti-connectivity/wl18xx-conf.bin",
+   .nvs_name = "ti-connectivity/wl1271-nvs.bin",
 };
 
 static const struct of_device_id wlcore_sdio_of_match_table[] = {
diff --git a/drivers/net/wireless/ti/wlcore/spi.c 
b/drivers/net/wireless/ti/wlcore/spi.c
index fdabb92..62ce54a 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -92,6 +92,7 @@ static const struct wilink_family_data wl128x_data = {
 static const struct wilink_family_data wl18xx_data = {
.name = "wl18xx",
.cfg_name = "ti-connectivity/wl18xx-conf.bin",
+   .nvs_name = "ti-connectivity/wl1271-nvs.bin",
 };
 
 struct wl12xx_spi_glue {
diff --git a/drivers/net/wireless/ti/wlcore/wlcore.h 
b/drivers/net/wireless/ti/wlcore/wlcore.h
index 1827546..95fbedc 100644
--- a/drivers/net/wireless/ti/wlcore/wlcore.h
+++ b/drivers/net/wireless/ti/wlcore/wlcore.h
@@ -40,6 +40,9 @@
 /* wl12xx/wl18xx maximum transmission power (in dBm) */
 #define WLCORE_MAX_TXPWR25
 
+/* Texas Instruments pre assigned OUI */
+#define WLCORE_TI_OUI_ADDRESS 0x080028
+
 /* forward declaration */
 struct wl1271_tx_hw_descr;
 enum wl_rx_buf_align;
-- 
2.7.4

RE: [v6] wlcore: add missing nvs file name info for wilink8

[Reizer, Eyal]

> > > >
> > >
> > > Sure, just want to make sure we are not trying to add work around just
> for
> > > A couple of faulty devices.
> > >
> > > > > I have verified using a couple of com6 modules with an am335x-evm
> and
> > > > they had mac addresses read ok.
> > > >
> > > > Sounds like there are multiple variants of the wl12xx
> > > > available then.
> > > >
> > > I am trying to find out internally if there is a possibility that there 
> > > were
> > > devices
> > > Produced in the past where the internal fuses were not programmed
> with a
> > > valid
> > > Address before being assembled into the modules.
> > >
> >
> > Seems like MAC address for wilink6/7 was added to devices that were
> produced  around Q3 of 2010
> > In case you still have this omap3-evm up and can just do a quick check for
> me as I don't have this board, can you try using the following command:
> >
> > cat /sys/bus/platform/drivers/wl12xx_driver/wl12xx.0.auto/hw_pg_ver
> >
> > and let me know what the output you get is?
> 
> Sure, my omap3-evm has hw_pg_ver set to 3.
> 
Ok, you have a PG3.0 device. Mac address was added in PG3.1 so it explains
Why the mac address remains zero and not read from fuse.
Will use random mac in case it is read a zero.

Best Regards,
Eyal

RE: [v6] wlcore: add missing nvs file name info for wilink8

[Reizer, Eyal]

> >
> 
> Sure, just want to make sure we are not trying to add work around just for
> A couple of faulty devices.
> 
> > > I have verified using a couple of com6 modules with an am335x-evm and
> > they had mac addresses read ok.
> >
> > Sounds like there are multiple variants of the wl12xx
> > available then.
> >
> I am trying to find out internally if there is a possibility that there were
> devices
> Produced in the past where the internal fuses were not programmed with a
> valid
> Address before being assembled into the modules.
> 

Seems like MAC address for wilink6/7 was added to devices that were produced  
around Q3 of 2010 
In case you still have this omap3-evm up and can just do a quick check for me 
as I don't have this board, can you try using the following command:

cat /sys/bus/platform/drivers/wl12xx_driver/wl12xx.0.auto/hw_pg_ver

and let me know what the output you get is?

Best Regards,
Eyal

RE: [v6] wlcore: add missing nvs file name info for wilink8

[Reizer, Eyal]

> 
> >> The fact that is old does not change a thing, we still need to
> >> support it no matter what the data sheet and your system design
> >> says. A fix that breaks other things is not really a fix :)
> >>
> >
> > Sure, just want to make sure we are not trying to add work around just for
> > A couple of faulty devices.
> >
> >> > I have verified using a couple of com6 modules with an am335x-evm and
> >> they had mac addresses read ok.
> >>
> >> Sounds like there are multiple variants of the wl12xx
> >> available then.
> >>
> > I am trying to find out internally if there is a possibility that there were
> devices
> > Produced in the past where the internal fuses were not programmed with
> a valid
> > Address before being assembled into the modules.
> 
> Just a general remark, based on my past experience, you can't really
> know what hardware is out there, no matter how someone in the company
> claims otherwise. Uncalibrated devices, prototypes and calibration data
> broken are all possible and better be preparared for that in the driver.
> 
> It's a good idea at least to detect and print a proper error message if
> the calibration data is broken. But if the data on the device only
> consists of MAC address and nothing else, then I guess using a random
> address is fine.
> 

Understood. I will handle the zero mac address case and use random mac instaed.
Just trying to find out how common it is as it seems strange devices like that 
found their way to boards that we shipped to customers.

Best Regards,
Eyal

RE: [v6] wlcore: add missing nvs file name info for wilink8

[Reizer, Eyal]

> 
> On Thu, Aug 10, 2017 at 4:35 PM, Reizer, Eyal <ey...@ti.com> wrote:
> >> >
> >> > Sorry for top posting (mobile...)
> >> > I have verified with system design and the data sheet that every wilink
> 6/7
> >> chip has a mac address in fuse so probably the board you have (pretty old,
> >> right?) has this mac address in fuse. Maybe it was from very early
> batches?
> >> Anyway I see no reason to change it.
> >> > Anyway the calibrator can be used to store a different one into the nvs
> file
> >> that will overide it.
> >>
> >> Well clearly at least this one does not have any valid hardware
> >> mac address, the hardware mac address is broken with all zeroes.
> >>
> >
> > Looks like it is not really all zeros but rather 00:00:00:00:00:01.
> > I wonder if it is just a one board issue or not...
> 
> It's 00:00:00:00:00:01 because your code adds 1 to it.
> 
You are right of course!
I saw it as well after sending my previous reply.

Best Regards,
Eyal

RE: [v6] wlcore: add missing nvs file name info for wilink8

[Reizer, Eyal]

> >
> > Sorry for top posting (mobile...)
> > I have verified with system design and the data sheet that every wilink 6/7
> chip has a mac address in fuse so probably the board you have (pretty old,
> right?) has this mac address in fuse. Maybe it was from very early batches?
> Anyway I see no reason to change it.
> > Anyway the calibrator can be used to store a different one into the nvs file
> that will overide it.
> 
> Well clearly at least this one does not have any valid hardware
> mac address, the hardware mac address is broken with all zeroes.
> 

Looks like it is not really all zeros but rather 00:00:00:00:00:01. 
I wonder if it is just a one board issue or not...

> It seems that you can easily add a check for empty mac address, no?
> And you already showed a version that falls back to a random mac
> address.
> 

Of course I can add a check for this but need to make sure it 
is not just one private case.
Do you happen to have another omap3-evm and can check if this is a typical case
For the wilink modules that were assembled on this EVM?
I have not seen another module here that showed this issue and want to make 
Sure it is really a common issue before adding additional checks to the kernel.

> The fact that is old does not change a thing, we still need to
> support it no matter what the data sheet and your system design
> says. A fix that breaks other things is not really a fix :)
> 

Sure, just want to make sure we are not trying to add work around just for
A couple of faulty devices.

> > I have verified using a couple of com6 modules with an am335x-evm and
> they had mac addresses read ok.
> 
> Sounds like there are multiple variants of the wl12xx
> available then.
> 
I am trying to find out internally if there is a possibility that there were 
devices
Produced in the past where the internal fuses were not programmed with a valid 
Address before being assembled into the modules.

Best Regards,
Eyal

[v6] wlcore: add missing nvs file name info for wilink8

[Reizer, Eyal]

The following commits:
commit c815fdebef44 ("wlcore: spi: Populate config firmware data")
commit d776fc86b82f ("wlcore: sdio: Populate config firmware data")

Populated the nvs entry for wilink6 and wilink7 only while it is
still needed for wilink8 as well for specifying an alternate mac address.
This broke user space backward compatibility when upgrading from older
kernels, as the alternate mac address would not be read from the nvs that
is present in the file system (lib/firmware/ti-connectivity/wl1271-nvs.bin)
causing mac address change of the wlan interface.

This patch fix this and update the structure field with the same default
nvs file name that has been used before.

In addition, some distros hold a default wl1271-nvs.bin in the file
system with a bogus mac address (deadbeef...) that overrides the mac
address that is stored inside the device.
Warn users about this bogus mac address and use the internal mac address

Fixes: c815fdebef44 ("wlcore: spi: Populate config firmware data")
Fixes: d776fc86b82f ("wlcore: sdio: Populate config firmware data")
Cc:  # 4.9+
Signed-off-by: Eyal Reizer 
---
v2->v3: add a check for default deadbeef... mac address and warn about it
v3->v4: use a random TI mac address instead of the bogus one
v4->v5: add constant definition for TI oui address
v5->v6: after also verifying on wilink6/7 Use mac internal mac address
instead of a random one
---
 drivers/net/wireless/ti/wlcore/main.c | 15 +++
 drivers/net/wireless/ti/wlcore/sdio.c |  1 +
 drivers/net/wireless/ti/wlcore/spi.c  |  1 +
 3 files changed, 17 insertions(+)

diff --git a/drivers/net/wireless/ti/wlcore/main.c 
b/drivers/net/wireless/ti/wlcore/main.c
index 60aaa85..dd1ac48 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -6040,6 +6040,21 @@ static int wl1271_register_hw(struct wl1271 *wl)
nic_addr = wl->fuse_nic_addr + 1;
}
 
+   if (oui_addr == 0xdeadbe && nic_addr == 0xef) {
+   wl1271_warning("Detected unconfigured mac address in nvs.\n"
+   "derive from fuse instead.\n"
+   "in case of using a wl12xx device, your "
+   "device performance may not be optimized.\n"
+   "Please use the calibrator tool to configure "
+   "your device.\n"
+   "When using a wl18xx device this default nvs "
+   "file can be removed from the file system\n");
+
+   oui_addr = wl->fuse_oui_addr;
+   /* fuse has the BD_ADDR, the WLAN addresses are the next two */
+   nic_addr = wl->fuse_nic_addr + 1;
+   }
+
wl12xx_derive_mac_addresses(wl, oui_addr, nic_addr);
 
ret = ieee80211_register_hw(wl->hw);
diff --git a/drivers/net/wireless/ti/wlcore/sdio.c 
b/drivers/net/wireless/ti/wlcore/sdio.c
index 2fb3871..f8a1fea 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -230,6 +230,7 @@ static const struct wilink_family_data wl128x_data = {
 static const struct wilink_family_data wl18xx_data = {
.name = "wl18xx",
.cfg_name = "ti-connectivity/wl18xx-conf.bin",
+   .nvs_name = "ti-connectivity/wl1271-nvs.bin",
 };
 
 static const struct of_device_id wlcore_sdio_of_match_table[] = {
diff --git a/drivers/net/wireless/ti/wlcore/spi.c 
b/drivers/net/wireless/ti/wlcore/spi.c
index fdabb92..62ce54a 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -92,6 +92,7 @@ static const struct wilink_family_data wl128x_data = {
 static const struct wilink_family_data wl18xx_data = {
.name = "wl18xx",
.cfg_name = "ti-connectivity/wl18xx-conf.bin",
+   .nvs_name = "ti-connectivity/wl1271-nvs.bin",
 };
 
 struct wl12xx_spi_glue {
-- 
2.7.4

RE: [v5] wlcore: add missing nvs file name info for wilink8

[Reizer, Eyal]

> 
> > Subject: Re: [v5] wlcore: add missing nvs file name info for wilink8
> >
> > * Reizer, Eyal <ey...@ti.com> [170807 00:47]:
> > > Hi Tony,
> > > >
> > > > * Reizer, Eyal <ey...@ti.com> [170807 00:32]:
> > > > > The following commits:
> > > > > c815fde wlcore: spi: Populate config firmware data
> > > > > d776fc8 wlcore: sdio: Populate config firmware data
> > > > >
> > > > > Populated the nvs entry for wilink6 and wilink7 only while it is
> > > > > still needed for wilink8 as well.
> > > > > This broke user space backward compatibility when upgrading from
> > older
> > > > > kernels, as the alternate mac address would not be read from the nvs
> > that
> > > > is
> > > > > present in the file system (lib/firmware/ti-connectivity/wl1271-
> nvs.bin)
> > > > > causing mac address change of the wlan interface.
> > > > >
> > > > > This patch fix this and update the structure field with the same
> default
> > > > > nvs file name that has been used before.
> > > > >
> > > > > In addition, some distros hold a default wl1271-nvs.bin in the file
> > > > > system with a bogus mac address (deadbeef...) that for a wl18xx
> device
> > > > > also overrides the mac address that is stored inside the device.
> > > > > Warn users about this bogus mac address and use a random mac
> > instead
> > > >
> > > > Hmm looks pretty good to me except for one more thing I just noticed.
> > > >
> > > > Why don't you just use the hardware mac address instead of a random
> > > > mac address on wl18xx device when you see a bogus nvs file?
> > > >
> > >
> > > I agree that this would have been better but the problem is that
> hardware
> > > mac address is available for wilink8 onlyWilink6/7 don't have one stored.
> > > The wlcore code responsible for handling mac address is common code
> > > and there is method for detecting between them in this module.
> >
> > Care to clarify a bit.. Are you saying wilink8 will use the hardware
> > mac address in case of bogus nvs file?
> >
> With present implementation it will not. It will use the random one for both
> wilink6/7 as well as wilink8.
> I need to get a hold of a wilink6/7 module and see if reverting to an internal
> mac address is working. Will try to look around as it has been a while since
> I used one.
> 
Managed to test with a wilink6 module and in fact reading hardware mac
Address from fuse is working ok for wilink6/7 as well.
submitting v6 using this mac address instead of a random one when the 
bogus (deadbeef...) mac address is read from default nvs file.

Best Regards,
Eyal

RE: [v5] wlcore: add missing nvs file name info for wilink8

[Reizer, Eyal]

> Subject: Re: [v5] wlcore: add missing nvs file name info for wilink8
> 
> * Reizer, Eyal <ey...@ti.com> [170807 00:47]:
> > Hi Tony,
> > >
> > > * Reizer, Eyal <ey...@ti.com> [170807 00:32]:
> > > > The following commits:
> > > > c815fde wlcore: spi: Populate config firmware data
> > > > d776fc8 wlcore: sdio: Populate config firmware data
> > > >
> > > > Populated the nvs entry for wilink6 and wilink7 only while it is
> > > > still needed for wilink8 as well.
> > > > This broke user space backward compatibility when upgrading from
> older
> > > > kernels, as the alternate mac address would not be read from the nvs
> that
> > > is
> > > > present in the file system (lib/firmware/ti-connectivity/wl1271-nvs.bin)
> > > > causing mac address change of the wlan interface.
> > > >
> > > > This patch fix this and update the structure field with the same default
> > > > nvs file name that has been used before.
> > > >
> > > > In addition, some distros hold a default wl1271-nvs.bin in the file
> > > > system with a bogus mac address (deadbeef...) that for a wl18xx device
> > > > also overrides the mac address that is stored inside the device.
> > > > Warn users about this bogus mac address and use a random mac
> instead
> > >
> > > Hmm looks pretty good to me except for one more thing I just noticed.
> > >
> > > Why don't you just use the hardware mac address instead of a random
> > > mac address on wl18xx device when you see a bogus nvs file?
> > >
> >
> > I agree that this would have been better but the problem is that hardware
> > mac address is available for wilink8 onlyWilink6/7 don't have one stored.
> > The wlcore code responsible for handling mac address is common code
> > and there is method for detecting between them in this module.
> 
> Care to clarify a bit.. Are you saying wilink8 will use the hardware
> mac address in case of bogus nvs file?
> 
With present implementation it will not. It will use the random one for both 
wilink6/7 as well as wilink8.
I need to get a hold of a wilink6/7 module and see if reverting to an internal 
mac address is working. Will try to look around as it has been a while since 
I used one.

Best Regards,
Eyal

RE: [v5] wlcore: add missing nvs file name info for wilink8

[Reizer, Eyal]

Hi Tony,
> 
> * Reizer, Eyal <ey...@ti.com> [170807 00:32]:
> > The following commits:
> > c815fde wlcore: spi: Populate config firmware data
> > d776fc8 wlcore: sdio: Populate config firmware data
> >
> > Populated the nvs entry for wilink6 and wilink7 only while it is
> > still needed for wilink8 as well.
> > This broke user space backward compatibility when upgrading from older
> > kernels, as the alternate mac address would not be read from the nvs that
> is
> > present in the file system (lib/firmware/ti-connectivity/wl1271-nvs.bin)
> > causing mac address change of the wlan interface.
> >
> > This patch fix this and update the structure field with the same default
> > nvs file name that has been used before.
> >
> > In addition, some distros hold a default wl1271-nvs.bin in the file
> > system with a bogus mac address (deadbeef...) that for a wl18xx device
> > also overrides the mac address that is stored inside the device.
> > Warn users about this bogus mac address and use a random mac instead
> 
> Hmm looks pretty good to me except for one more thing I just noticed.
> 
> Why don't you just use the hardware mac address instead of a random
> mac address on wl18xx device when you see a bogus nvs file?
> 

I agree that this would have been better but the problem is that hardware 
mac address is available for wilink8 onlyWilink6/7 don't have one stored.
The wlcore code responsible for handling mac address is common code 
and there is method for detecting between them in this module.

Best Regards,
Eyal

RE: [v4] wlcore: add missing nvs file name info for wilink8

[Reizer, Eyal]

> -Original Message-
> From: Julian Calaby [mailto:julian.cal...@gmail.com]
> Sent: Saturday, August 5, 2017 8:51 AM
> To: Reizer, Eyal
> Cc: Kalle Valo; ,Tony Lindgren; linux-wireless@vger.kernel.org; linux-
> ker...@vger.kernel.org; sebastian.reic...@collabora.co.uk
> Subject: Re: [v4] wlcore: add missing nvs file name info for wilink8
> 
> Hi Eyal,
> 
> On Mon, Jul 31, 2017 at 6:45 PM, Reizer, Eyal <ey...@ti.com> wrote:
> > The following commits:
> > c815fde wlcore: spi: Populate config firmware data
> > d776fc8 wlcore: sdio: Populate config firmware data
> >
> > Populated the nvs entry for wilink6 and wilink7 only while it is
> > still needed for wilink8 as well.
> > This broke user space backward compatibility when upgrading from older
> > kernels, as the alternate mac address would not be read from the nvs that
> > is present in the file system (lib/firmware/ti-connectivity/wl1271-nvs.bin)
> > causing mac address change of the wlan interface.
> >
> > This patch fix this and update the structure field with the same default
> > nvs file name that has been used before.
> >
> > In addition, some distros hold a default wl1271-nvs.bin in the file
> > system with a bogus mac address (deadbeef...) that for a wl18xx device
> > also overrides the mac address that is stored inside the device.
> > Warn users about this bogus mac address and use a random mac instead
> >
> > Cc: stable <sta...@vger.kernel.org>
> > Signed-off-by: Eyal Reizer <ey...@ti.com>
> > ---
> >
> > diff --git a/drivers/net/wireless/ti/wlcore/main.c
> b/drivers/net/wireless/ti/wlcore/main.c
> > index 60aaa85..7ce4221 100644
> > --- a/drivers/net/wireless/ti/wlcore/main.c
> > +++ b/drivers/net/wireless/ti/wlcore/main.c
> > @@ -5961,6 +5961,22 @@ static void wl12xx_derive_mac_addresses(struct
> wl1271 *wl, u32 oui, u32 nic)
> > if (nic + WLCORE_NUM_MAC_ADDRESSES - wl->num_mac_addr >
> 0xff)
> > wl1271_warning("NIC part of the MAC address wraps around!");
> >
> > +   if (oui == 0xdeadbe && nic == 0xef) {
> > +   wl1271_warning("Detected unconfigured mac address in nvs.\n"
> > +   "Using a random TI mac address instead.\n"
> > +   "in case of using a wl12xx device, your "
> > +   "device performance may not be optimized.\n"
> > +   "Please use the calibrator tool to 
> > configure "
> > +   "your device.\n"
> > +   "When using a wl18xx device the nvs file 
> > can "
> > +   "be removed as a default mac address is "
> > +   "stored internally.\n");
> > +
> > +   /* Use TI oui and a random nic */
> > +   oui = 0x080028;
> 
> Is there (or should there be) a constant for this?

Thanks for the comment. Submitting v5 fixing this.

Best Regards,
Eyal

[v4] wlcore: add missing nvs file name info for wilink8

[Reizer, Eyal]

The following commits:
c815fde wlcore: spi: Populate config firmware data
d776fc8 wlcore: sdio: Populate config firmware data

Populated the nvs entry for wilink6 and wilink7 only while it is
still needed for wilink8 as well.
This broke user space backward compatibility when upgrading from older
kernels, as the alternate mac address would not be read from the nvs that
is present in the file system (lib/firmware/ti-connectivity/wl1271-nvs.bin)
causing mac address change of the wlan interface.

This patch fix this and update the structure field with the same default
nvs file name that has been used before.

In addition, some distros hold a default wl1271-nvs.bin in the file
system with a bogus mac address (deadbeef...) that for a wl18xx device
also overrides the mac address that is stored inside the device.
Warn users about this bogus mac address and use a random mac instead

Cc: stable 
Signed-off-by: Eyal Reizer 
---
v2->v3: add a check for default deadbeef... mac address and warn about it
v3->v4: use a random TI mac address instead of the bogus one

---
 drivers/net/wireless/ti/wlcore/main.c | 16 
 drivers/net/wireless/ti/wlcore/sdio.c |  1 +
 drivers/net/wireless/ti/wlcore/spi.c  |  1 +
 3 files changed, 18 insertions(+)

diff --git a/drivers/net/wireless/ti/wlcore/main.c 
b/drivers/net/wireless/ti/wlcore/main.c
index 60aaa85..7ce4221 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -5961,6 +5961,22 @@ static void wl12xx_derive_mac_addresses(struct wl1271 
*wl, u32 oui, u32 nic)
if (nic + WLCORE_NUM_MAC_ADDRESSES - wl->num_mac_addr > 0xff)
wl1271_warning("NIC part of the MAC address wraps around!");
 
+   if (oui == 0xdeadbe && nic == 0xef) {
+   wl1271_warning("Detected unconfigured mac address in nvs.\n"
+   "Using a random TI mac address instead.\n"
+   "in case of using a wl12xx device, your "
+   "device performance may not be optimized.\n"
+   "Please use the calibrator tool to configure "
+   "your device.\n"
+   "When using a wl18xx device the nvs file can "
+   "be removed as a default mac address is "
+   "stored internally.\n");
+
+   /* Use TI oui and a random nic */
+   oui = 0x080028;
+   nic = get_random_int();
+   }
+
for (i = 0; i < wl->num_mac_addr; i++) {
wl->addresses[i].addr[0] = (u8)(oui >> 16);
wl->addresses[i].addr[1] = (u8)(oui >> 8);
diff --git a/drivers/net/wireless/ti/wlcore/sdio.c 
b/drivers/net/wireless/ti/wlcore/sdio.c
index 2fb3871..f8a1fea 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -230,6 +230,7 @@ static const struct wilink_family_data wl128x_data = {
 static const struct wilink_family_data wl18xx_data = {
.name = "wl18xx",
.cfg_name = "ti-connectivity/wl18xx-conf.bin",
+   .nvs_name = "ti-connectivity/wl1271-nvs.bin",
 };
 
 static const struct of_device_id wlcore_sdio_of_match_table[] = {
diff --git a/drivers/net/wireless/ti/wlcore/spi.c 
b/drivers/net/wireless/ti/wlcore/spi.c
index fdabb92..62ce54a 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -92,6 +92,7 @@ static const struct wilink_family_data wl128x_data = {
 static const struct wilink_family_data wl18xx_data = {
.name = "wl18xx",
.cfg_name = "ti-connectivity/wl18xx-conf.bin",
+   .nvs_name = "ti-connectivity/wl1271-nvs.bin",
 };
 
 struct wl12xx_spi_glue {
-- 
2.7.4

RE: [v3] wl18xx: add missing nvs file name info for wilink8

[Reizer, Eyal]

Please ignore this patch. Wrong header.
Submitted the correct one instead:
https://patchwork.kernel.org/patch/9854809/


> -Original Message-
> From: Reizer, Eyal
> Sent: Thursday, July 20, 2017 3:13 PM
> To: 'Kalle Valo'; ',Tony Lindgren'; 'linux-wireless@vger.kernel.org'; 'linux-
> ker...@vger.kernel.org'
> Cc: 'sebastian.reic...@collabora.co.uk'
> Subject: [v3] wl18xx: add missing nvs file name info for wilink8
> 
> The following commits:
> c815fde wlcore: spi: Populate config firmware data
> d776fc8 wlcore: sdio: Populate config firmware data
> 
> Populated the nvs entry for wilink6 and wilink7 only while it is
> still needed for wilink8 as well.
> This broke user space backward compatibility when upgrading from older
> kernels, as the alternate mac address would not be read from the nvs that is
> already present in the file system 
> (lib/firmware/ti-connectivity/wl1271-nvs.bin)
> causing mac address change of the wlan interface.
> 
> This patch fix this and update the structure field with the same default nvs 
> file
> name that has been used before.
> 
> In addition, some distros hold a default wl1271-nvs.bin in the file
> system with a bogus mac address (deadbeef...) that for a wl18xx device
> also overrides the mac address that is stored inside the device.
> Warn users about this bogus mac address.
> 
> Cc: stable <sta...@vger.kernel.org>
> Signed-off-by: Eyal Reizer <ey...@ti.com>
> ---
> v2->v3: add a check for default deadbeef... mac address and warn about it
> 
>  drivers/net/wireless/ti/wlcore/main.c | 10 ++
>  drivers/net/wireless/ti/wlcore/sdio.c |  1 +
>  drivers/net/wireless/ti/wlcore/spi.c  |  1 +
>  3 files changed, 12 insertions(+)
> 
> diff --git a/drivers/net/wireless/ti/wlcore/main.c
> b/drivers/net/wireless/ti/wlcore/main.c
> index 60aaa85..37c35aa 100644
> --- a/drivers/net/wireless/ti/wlcore/main.c
> +++ b/drivers/net/wireless/ti/wlcore/main.c
> @@ -5961,6 +5961,16 @@ static void wl12xx_derive_mac_addresses(struct
> wl1271 *wl, u32 oui, u32 nic)
>   if (nic + WLCORE_NUM_MAC_ADDRESSES - wl->num_mac_addr >
> 0xff)
>   wl1271_warning("NIC part of the MAC address wraps
> around!");
> 
> + if (oui == 0xdeadbe && nic == 0xef)
> + wl1271_warning("Detected unconfigured mac address in
> nvs.\n"
> + "in case of using a wl12xx device, your "
> + "device performance may not be optimized.\n"
> + "Please use the calibrator tool to configure "
> + "your device.\n"
> + "When using a wl18xx device the nvs file can "
> + "be removed as a default mac address is "
> + "stored internally.\n");
> +
>   for (i = 0; i < wl->num_mac_addr; i++) {
>   wl->addresses[i].addr[0] = (u8)(oui >> 16);
>   wl->addresses[i].addr[1] = (u8)(oui >> 8);
> diff --git a/drivers/net/wireless/ti/wlcore/sdio.c
> b/drivers/net/wireless/ti/wlcore/sdio.c
> index 2fb3871..f8a1fea 100644
> --- a/drivers/net/wireless/ti/wlcore/sdio.c
> +++ b/drivers/net/wireless/ti/wlcore/sdio.c
> @@ -230,6 +230,7 @@ static const struct wilink_family_data wl128x_data = {
>  static const struct wilink_family_data wl18xx_data = {
>   .name = "wl18xx",
>   .cfg_name = "ti-connectivity/wl18xx-conf.bin",
> + .nvs_name = "ti-connectivity/wl1271-nvs.bin",
>  };
> 
>  static const struct of_device_id wlcore_sdio_of_match_table[] = {
> diff --git a/drivers/net/wireless/ti/wlcore/spi.c
> b/drivers/net/wireless/ti/wlcore/spi.c
> index fdabb92..62ce54a 100644
> --- a/drivers/net/wireless/ti/wlcore/spi.c
> +++ b/drivers/net/wireless/ti/wlcore/spi.c
> @@ -92,6 +92,7 @@ static const struct wilink_family_data wl128x_data = {
>  static const struct wilink_family_data wl18xx_data = {
>   .name = "wl18xx",
>   .cfg_name = "ti-connectivity/wl18xx-conf.bin",
> + .nvs_name = "ti-connectivity/wl1271-nvs.bin",
>  };
> 
>  struct wl12xx_spi_glue {
> --
> 2.7.4

Best Regards,
Eyal

[v3] wlcore: add missing nvs file name info for wilink8

[Reizer, Eyal]

The following commits:
c815fde wlcore: spi: Populate config firmware data
d776fc8 wlcore: sdio: Populate config firmware data

Populated the nvs entry for wilink6 and wilink7 only while it is 
still needed for wilink8 as well. 
This broke user space backward compatibility when upgrading from older 
kernels, as the alternate mac address would not be read from the nvs that is 
already present in the file system 
(lib/firmware/ti-connectivity/wl1271-nvs.bin) 
causing mac address change of the wlan interface.

This patch fix this and update the structure field with the same default nvs 
file 
name that has been used before.

In addition, some distros hold a default wl1271-nvs.bin in the file
system with a bogus mac address (deadbeef...) that for a wl18xx device
also overrides the mac address that is stored inside the device.
Warn users about this bogus mac address.

Cc: stable 
Signed-off-by: Eyal Reizer 
---
v2->v3: add a check for default deadbeef... mac address and warn about it

 drivers/net/wireless/ti/wlcore/main.c | 10 ++
 drivers/net/wireless/ti/wlcore/sdio.c |  1 +
 drivers/net/wireless/ti/wlcore/spi.c  |  1 +
 3 files changed, 12 insertions(+)

diff --git a/drivers/net/wireless/ti/wlcore/main.c 
b/drivers/net/wireless/ti/wlcore/main.c
index 60aaa85..37c35aa 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -5961,6 +5961,16 @@ static void wl12xx_derive_mac_addresses(struct wl1271 
*wl, u32 oui, u32 nic)
if (nic + WLCORE_NUM_MAC_ADDRESSES - wl->num_mac_addr > 0xff)
wl1271_warning("NIC part of the MAC address wraps around!");
 
+   if (oui == 0xdeadbe && nic == 0xef)
+   wl1271_warning("Detected unconfigured mac address in nvs.\n"
+   "in case of using a wl12xx device, your "
+   "device performance may not be optimized.\n"
+   "Please use the calibrator tool to configure "
+   "your device.\n"
+   "When using a wl18xx device the nvs file can "
+   "be removed as a default mac address is "
+   "stored internally.\n");
+
for (i = 0; i < wl->num_mac_addr; i++) {
wl->addresses[i].addr[0] = (u8)(oui >> 16);
wl->addresses[i].addr[1] = (u8)(oui >> 8);
diff --git a/drivers/net/wireless/ti/wlcore/sdio.c 
b/drivers/net/wireless/ti/wlcore/sdio.c
index 2fb3871..f8a1fea 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -230,6 +230,7 @@ static const struct wilink_family_data wl128x_data = {
 static const struct wilink_family_data wl18xx_data = {
.name = "wl18xx",
.cfg_name = "ti-connectivity/wl18xx-conf.bin",
+   .nvs_name = "ti-connectivity/wl1271-nvs.bin",
 };
 
 static const struct of_device_id wlcore_sdio_of_match_table[] = {
diff --git a/drivers/net/wireless/ti/wlcore/spi.c 
b/drivers/net/wireless/ti/wlcore/spi.c
index fdabb92..62ce54a 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -92,6 +92,7 @@ static const struct wilink_family_data wl128x_data = {
 static const struct wilink_family_data wl18xx_data = {
.name = "wl18xx",
.cfg_name = "ti-connectivity/wl18xx-conf.bin",
+   .nvs_name = "ti-connectivity/wl1271-nvs.bin",
 };
 
 struct wl12xx_spi_glue {
-- 
2.7.4

RE: [v3] wl18xx: add missing nvs file name info for wilink8

[Reizer, Eyal]

> The following commits:
> c815fde wlcore: spi: Populate config firmware data
> d776fc8 wlcore: sdio: Populate config firmware data
> 
> Populated the nvs entry for wilink6 and wilink7 only while it is
> still needed for wilink8 as well.
> This broke user space backward compatibility when upgrading from older
> kernels, as the alternate mac address would not be read from the nvs that is
> already present in the file system 
> (lib/firmware/ti-connectivity/wl1271-nvs.bin)
> causing mac address change of the wlan interface.
> 
> This patch fix this and update the structure field with the same default nvs 
> file
> name that has been used before.
> 
> In addition, some distros hold a default wl1271-nvs.bin in the file
> system with a bogus mac address (deadbeef...) that for a wl18xx device
> also overrides the mac address that is stored inside the device.
> Warn users about this bogus mac address.
> 
> Cc: stable 
> Signed-off-by: Eyal Reizer 
> ---
> v2->v3: add a check for default deadbeef... mac address and warn about it
> 
>  drivers/net/wireless/ti/wlcore/main.c | 10 ++
>  drivers/net/wireless/ti/wlcore/sdio.c |  1 +
>  drivers/net/wireless/ti/wlcore/spi.c  |  1 +
>  3 files changed, 12 insertions(+)
> 
> diff --git a/drivers/net/wireless/ti/wlcore/main.c
> b/drivers/net/wireless/ti/wlcore/main.c
> index 60aaa85..37c35aa 100644
> --- a/drivers/net/wireless/ti/wlcore/main.c
> +++ b/drivers/net/wireless/ti/wlcore/main.c
> @@ -5961,6 +5961,16 @@ static void wl12xx_derive_mac_addresses(struct
> wl1271 *wl, u32 oui, u32 nic)
>   if (nic + WLCORE_NUM_MAC_ADDRESSES - wl->num_mac_addr >
> 0xff)
>   wl1271_warning("NIC part of the MAC address wraps
> around!");
> 
> + if (oui == 0xdeadbe && nic == 0xef)
> + wl1271_warning("Detected unconfigured mac address in
> nvs.\n"
> + "in case of using a wl12xx device, your "
> + "device performance may not be optimized.\n"
> + "Please use the calibrator tool to configure "
> + "your device.\n"
> + "When using a wl18xx device the nvs file can "
> + "be removed as a default mac address is "
> + "stored internally.\n");
> +
>   for (i = 0; i < wl->num_mac_addr; i++) {
>   wl->addresses[i].addr[0] = (u8)(oui >> 16);
>   wl->addresses[i].addr[1] = (u8)(oui >> 8);
> diff --git a/drivers/net/wireless/ti/wlcore/sdio.c
> b/drivers/net/wireless/ti/wlcore/sdio.c
> index 2fb3871..f8a1fea 100644
> --- a/drivers/net/wireless/ti/wlcore/sdio.c
> +++ b/drivers/net/wireless/ti/wlcore/sdio.c
> @@ -230,6 +230,7 @@ static const struct wilink_family_data wl128x_data = {
>  static const struct wilink_family_data wl18xx_data = {
>   .name = "wl18xx",
>   .cfg_name = "ti-connectivity/wl18xx-conf.bin",
> + .nvs_name = "ti-connectivity/wl1271-nvs.bin",
>  };
> 
>  static const struct of_device_id wlcore_sdio_of_match_table[] = {
> diff --git a/drivers/net/wireless/ti/wlcore/spi.c
> b/drivers/net/wireless/ti/wlcore/spi.c
> index fdabb92..62ce54a 100644
> --- a/drivers/net/wireless/ti/wlcore/spi.c
> +++ b/drivers/net/wireless/ti/wlcore/spi.c
> @@ -92,6 +92,7 @@ static const struct wilink_family_data wl128x_data = {
>  static const struct wilink_family_data wl18xx_data = {
>   .name = "wl18xx",
>   .cfg_name = "ti-connectivity/wl18xx-conf.bin",
> + .nvs_name = "ti-connectivity/wl1271-nvs.bin",
>  };
> 
>  struct wl12xx_spi_glue {
> --
> 2.7.4

Please ignore. Wrong patch name.

Best Regards,
Eyal Reizer

[v3] wl18xx: add missing nvs file name info for wilink8

[Reizer, Eyal]

The following commits:
c815fde wlcore: spi: Populate config firmware data
d776fc8 wlcore: sdio: Populate config firmware data

Populated the nvs entry for wilink6 and wilink7 only while it is 
still needed for wilink8 as well. 
This broke user space backward compatibility when upgrading from older 
kernels, as the alternate mac address would not be read from the nvs that is 
already present in the file system 
(lib/firmware/ti-connectivity/wl1271-nvs.bin) 
causing mac address change of the wlan interface.

This patch fix this and update the structure field with the same default nvs 
file 
name that has been used before.

In addition, some distros hold a default wl1271-nvs.bin in the file
system with a bogus mac address (deadbeef...) that for a wl18xx device
also overrides the mac address that is stored inside the device.
Warn users about this bogus mac address.

Cc: stable 
Signed-off-by: Eyal Reizer 
---
v2->v3: add a check for default deadbeef... mac address and warn about it

 drivers/net/wireless/ti/wlcore/main.c | 10 ++
 drivers/net/wireless/ti/wlcore/sdio.c |  1 +
 drivers/net/wireless/ti/wlcore/spi.c  |  1 +
 3 files changed, 12 insertions(+)

diff --git a/drivers/net/wireless/ti/wlcore/main.c 
b/drivers/net/wireless/ti/wlcore/main.c
index 60aaa85..37c35aa 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -5961,6 +5961,16 @@ static void wl12xx_derive_mac_addresses(struct wl1271 
*wl, u32 oui, u32 nic)
if (nic + WLCORE_NUM_MAC_ADDRESSES - wl->num_mac_addr > 0xff)
wl1271_warning("NIC part of the MAC address wraps around!");
 
+   if (oui == 0xdeadbe && nic == 0xef)
+   wl1271_warning("Detected unconfigured mac address in nvs.\n"
+   "in case of using a wl12xx device, your "
+   "device performance may not be optimized.\n"
+   "Please use the calibrator tool to configure "
+   "your device.\n"
+   "When using a wl18xx device the nvs file can "
+   "be removed as a default mac address is "
+   "stored internally.\n");
+
for (i = 0; i < wl->num_mac_addr; i++) {
wl->addresses[i].addr[0] = (u8)(oui >> 16);
wl->addresses[i].addr[1] = (u8)(oui >> 8);
diff --git a/drivers/net/wireless/ti/wlcore/sdio.c 
b/drivers/net/wireless/ti/wlcore/sdio.c
index 2fb3871..f8a1fea 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -230,6 +230,7 @@ static const struct wilink_family_data wl128x_data = {
 static const struct wilink_family_data wl18xx_data = {
.name = "wl18xx",
.cfg_name = "ti-connectivity/wl18xx-conf.bin",
+   .nvs_name = "ti-connectivity/wl1271-nvs.bin",
 };
 
 static const struct of_device_id wlcore_sdio_of_match_table[] = {
diff --git a/drivers/net/wireless/ti/wlcore/spi.c 
b/drivers/net/wireless/ti/wlcore/spi.c
index fdabb92..62ce54a 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -92,6 +92,7 @@ static const struct wilink_family_data wl128x_data = {
 static const struct wilink_family_data wl18xx_data = {
.name = "wl18xx",
.cfg_name = "ti-connectivity/wl18xx-conf.bin",
+   .nvs_name = "ti-connectivity/wl1271-nvs.bin",
 };
 
 struct wl12xx_spi_glue {
-- 
2.7.4

RE: [PATCH] wlcore: add missing nvs file name info for wilink8

[Reizer, Eyal]

> >
> > On Wed, Jul 05, 2017 at 01:06:54AM -0700, Tony Lindgren wrote:
> > > > > Not sure if this really is a regression as we've always had a bogus
> > > > > wl1271-nvs.bin in linux-firmware.git. Sure would be nice to fix it,
> > > > > but going back to using a generic wl1271-nvs.bin sure does not seem
> > > > > like a good long term fix to me :)
> > > > A lot of legacy here...
> > > > Wl1271-nvs has been used mainly with wilink6/7 and indeed was device
> specific
> > > > Holding calibration info etc.
> > > > When they started with wilink8 the device specific configuration that 
> > > > was
> > > > Part of it has switched to wl18xx-conf.bin and using the wlaconf tool 
> > > > for
> setting it.
> > > > Also there is no calibration specific info per device with wilink8 so 
> > > > the
> wl18xx-conf.bin
> > > > The only thing left in wl1271-nvs.bin for wilink8 was the mac address
> override.
> > >
> > > And the default wl1271-nvs.bin sets the mac address to a bogus deadbeef
> address,
> > > so it's wrong to use and totally broken for distros :(
> >
> > So use something like the following pseudo-code?
> >
> > if (fw->mac_address == deadbeef) {
> > fw->mac_address = get_random_mac();

Deadbeef is a valid mac address, so I suggest detecting it and warning
the user of the bogus (default) nvs but don't attempt to get a random mac
address for him as it has to many rules (registered oui addresses etc.)

> > dev_warn(dev, "Detected unconfigured wl1271-nvs.bin.\n"
> > "Your device will run with limited performance.\n"
> > "Please use ti-utils to configure your device.\n");
> > }
> 
> Yeah something like that should do the trick  :)
> 

Seems logical, will submit a v3 using wl1271-nvs.bin for wl18xx while adding 
this
Check/warn.

Best Regards,
Eyal

RE: [v2] wlcore: add missing nvs file name info for wilink8

[Reizer, Eyal]

Hi Tony,

> > When working with wl18xx the nvs file is used for defining an alternate
> > mac address and override the default mac address that is stored inside
> > the wl18xx chip.
> >
> > The following commits:
> > c815fde wlcore: spi: Populate config firmware data
> > d776fc8 wlcore: sdio: Populate config firmware data
> >
> > Populated the nvs entry for wilink6 and wilink7 only while it is
> > still needed for wilink8 as well.
> > This broke user space backward compatibility when upgrading from older
> > kernels, as the alternate mac address would not be read from the nvs that is
> > already present in the file system (lib/firmware/ti-connectivity/wl1271-
> nvs.bin)
> > causing mac address change of the wlan interface.
> >
> > This patch fix this and update the structure field with the same default nvs
> file
> > name that has been used before.
> 
> I think more checks on the nvs file being used are needed to avoid other
> nasty issues, see the comments I just made in the earlier version of this
> patch.
> 
Just replied on your comments for v1

Best Regards,
Eyal

RE: [PATCH] wlcore: add missing nvs file name info for wilink8

[Reizer, Eyal]

Hi Tony,

> 
> * Kalle Valo <kv...@codeaurora.org> [170703 04:30]:
> > "Reizer, Eyal" <ey...@ti.com> writes:
> >
> > > When working with wl18xx the nvs file is used for defining an alternate
> > > mac address and override the default mac address that is stored inside
> > > the wl18xx chip.
> > > update the structure field with the same default nvs file name that has
> > > been used in the past, otherwise userspace backward compatibility is
> > > broken when upgrading from older kernels, as the alternate mac address
> > > would not be read from the nvs that is already present in the file system
> > > (lib/firmware/ti-connectivity/wl1271-nvs.bin) causing mac address change
> > > of the wlan interface.
> > >
> > > Signed-off-by: Eyal Reizer <ey...@ti.com>
> >
> > Should we also cc stable? And a Fixes line would be nice.
> 
> Argh this mess again. I think there are few things to consider here. What
> about booting the same rootfs on multiple devices for example with NFSroot?
> 
> Not sure if this really is a regression as we've always had a bogus
> wl1271-nvs.bin in linux-firmware.git. Sure would be nice to fix it,
> but going back to using a generic wl1271-nvs.bin sure does not seem
> like a good long term fix to me :)
A lot of legacy here...
Wl1271-nvs has been used mainly with wilink6/7 and indeed was device specific
Holding calibration info etc.
When they started with wilink8 the device specific configuration that was 
Part of it has switched to wl18xx-conf.bin and using the wlaconf tool for 
setting it.
Also there is no calibration specific info per device with wilink8 so the 
wl18xx-conf.bin
The only thing left in wl1271-nvs.bin for wilink8 was the mac address override.
There are wilink8 customer using this feature which is the only reason for 
keeping 
This file for wilink8.
So for wilink8 there is really not issue with having the same wl1271-nvs.bin 
On NFSroot. 

> 
> Isn't the nvs file supposed to be device specific? If so, we should not
> provide it in linux-firmware.git at all because of the issues it causes..
> 
> And since it's provided, how are people supposed to know to remove it
> from their file system and replace it with something board specific?

I think the wl1271-nvs.bin should be removed from linux-firmware.git
anyway.
For wilink6/7 it is really device specific and for wilink8 if a customer
Want an alternate mac address he can create this file on his file system.

> 
> Maybe add a check to first try to find wl18xx-nvs.bin if it exists (and
> not add it to linux-firmware.git), and if not found, then fall back to
> wl1271-nvs.bin, but only if it's not the bogus generic file.. Produce
> a warning if the bogux linux-firmware.git wl1271-nvs.bin is being used..
> 
Removing it from linux-firmware.git may be better instead of adding an
additional check?
People are already familiar with the wl1271-nvs.bin file as a mean for
Mac address override for wilink8:
http://processors.wiki.ti.com/index.php/WL18xx_Writing_MAC_address

so I am not sure that a name change
Here is really necessary and may only create confusion especially for 
Customers that already have products in the field and only updating
Their kernel.

Best Regards,
Eyal

[v2] wlcore: add missing nvs file name info for wilink8

[Reizer, Eyal]

When working with wl18xx the nvs file is used for defining an alternate
mac address and override the default mac address that is stored inside
the wl18xx chip.

The following commits:
c815fde wlcore: spi: Populate config firmware data
d776fc8 wlcore: sdio: Populate config firmware data

Populated the nvs entry for wilink6 and wilink7 only while it is 
still needed for wilink8 as well. 
This broke user space backward compatibility when upgrading from older 
kernels, as the alternate mac address would not be read from the nvs that is 
already present in the file system 
(lib/firmware/ti-connectivity/wl1271-nvs.bin) 
causing mac address change of the wlan interface.

This patch fix this and update the structure field with the same default nvs 
file 
name that has been used before.

Cc: stable 
Signed-off-by: Eyal Reizer 
---
 drivers/net/wireless/ti/wlcore/sdio.c | 1 +
 drivers/net/wireless/ti/wlcore/spi.c  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/net/wireless/ti/wlcore/sdio.c 
b/drivers/net/wireless/ti/wlcore/sdio.c
index 2fb3871..f8a1fea 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -230,6 +230,7 @@ static const struct wilink_family_data wl128x_data = {
 static const struct wilink_family_data wl18xx_data = {
.name = "wl18xx",
.cfg_name = "ti-connectivity/wl18xx-conf.bin",
+   .nvs_name = "ti-connectivity/wl1271-nvs.bin",
 };
 
 static const struct of_device_id wlcore_sdio_of_match_table[] = {
diff --git a/drivers/net/wireless/ti/wlcore/spi.c 
b/drivers/net/wireless/ti/wlcore/spi.c
index fdabb92..62ce54a 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -92,6 +92,7 @@ static const struct wilink_family_data wl128x_data = {
 static const struct wilink_family_data wl18xx_data = {
.name = "wl18xx",
.cfg_name = "ti-connectivity/wl18xx-conf.bin",
+   .nvs_name = "ti-connectivity/wl1271-nvs.bin",
 };
 
 struct wl12xx_spi_glue {
-- 
2.7.4

[PATCH] wlcore: add missing nvs file name info for wilink8

[Reizer, Eyal]

When working with wl18xx the nvs file is used for defining an alternate
mac address and override the default mac address that is stored inside
the wl18xx chip.
update the structure field with the same default nvs file name that has
been used in the past, otherwise userspace backward compatibility is
broken when upgrading from older kernels, as the alternate mac address
would not be read from the nvs that is already present in the file system
(lib/firmware/ti-connectivity/wl1271-nvs.bin) causing mac address change
of the wlan interface.

Signed-off-by: Eyal Reizer 
---
 drivers/net/wireless/ti/wlcore/sdio.c | 1 +
 drivers/net/wireless/ti/wlcore/spi.c  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/net/wireless/ti/wlcore/sdio.c 
b/drivers/net/wireless/ti/wlcore/sdio.c
index 2fb3871..f8a1fea 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -230,6 +230,7 @@ static const struct wilink_family_data wl128x_data = {
 static const struct wilink_family_data wl18xx_data = {
.name = "wl18xx",
.cfg_name = "ti-connectivity/wl18xx-conf.bin",
+   .nvs_name = "ti-connectivity/wl1271-nvs.bin",
 };
 
 static const struct of_device_id wlcore_sdio_of_match_table[] = {
diff --git a/drivers/net/wireless/ti/wlcore/spi.c 
b/drivers/net/wireless/ti/wlcore/spi.c
index fdabb92..62ce54a 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -92,6 +92,7 @@ static const struct wilink_family_data wl128x_data = {
 static const struct wilink_family_data wl18xx_data = {
.name = "wl18xx",
.cfg_name = "ti-connectivity/wl18xx-conf.bin",
+   .nvs_name = "ti-connectivity/wl1271-nvs.bin",
 };
 
 struct wl12xx_spi_glue {
-- 
2.7.4

[PATCH] wlcore: spi: fix build warning caused by redundant variable

[Reizer, Eyal]

The ret variable is unused in wlcore_probe_of()
Remove it for fixing build warning.

Fixes: 01efe65aba65 ("wlcore: spi: add wl18xx support")
Signed-off-by: Eyal Reizer 
---
 drivers/net/wireless/ti/wlcore/spi.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/wireless/ti/wlcore/spi.c 
b/drivers/net/wireless/ti/wlcore/spi.c
index 73fbcf1..6d24040 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -454,7 +454,6 @@ static int wlcore_probe_of(struct spi_device *spi, struct 
wl12xx_spi_glue *glue,
   struct wlcore_platdev_data *pdev_data)
 {
struct device_node *dt_node = spi->dev.of_node;
-   int ret;
const struct of_device_id *of_id;
 
of_id = of_match_node(wlcore_spi_of_match_table, dt_node);
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCHv8] wlcore: spi: add wl18xx support

[Reizer, Eyal]

> 
> > Add support for using with both wl12xx and wl18xx.
> >
> > - all wilink family needs special init command for entering wspi mode.
> >   extra clock cycles should be sent after the spi init command while the
> >   cs pin is high.
> > - Use inverted chip select for sending a dummy 4 bytes command that
> >   completes the init stage.
> >
> > Signed-off-by: Eyal Reizer 
> > Acked-by: Rob Herring 
> > ---
> > v1->v2:update device tree bindings configuration
> > v2->v3:revert from manual gpio manipulation. use inverted chip select
> instead
> > for sending the extra init cycle which, achieves the same hardware purpose.
> > update device tree bindings docucmentation accordingly
> > v3->v4: Remove redundant data form binding documentation and fix chip
> select
> > number mismatch in wl1271 example
> > v4->v5: Rebase on top of head of wireless-drivers-next
> > v5->v6: Add ACKs
> > v6->v7: Mail format issues
> > v7->v8: Remove redundant varaible from wlcore_probe_of
> 
> I have already applied this patch, it's too late to send a new version.
> Now you need to send a new patch, on top of wireless-drivers-next, which
> removes the redundant variable.
> 
Understood. Will submit shortly

Best Regards,
Eyal
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv8] wlcore: spi: add wl18xx support

[Reizer, Eyal]

Add support for using with both wl12xx and wl18xx.

- all wilink family needs special init command for entering wspi mode.
  extra clock cycles should be sent after the spi init command while the
  cs pin is high.
- Use inverted chip select for sending a dummy 4 bytes command that
  completes the init stage.

Signed-off-by: Eyal Reizer 
Acked-by: Rob Herring 
---
v1->v2:update device tree bindings configuration
v2->v3:revert from manual gpio manipulation. use inverted chip select instead
for sending the extra init cycle which, achieves the same hardware purpose.
update device tree bindings docucmentation accordingly
v3->v4: Remove redundant data form binding documentation and fix chip select
number mismatch in wl1271 example
v4->v5: Rebase on top of head of wireless-drivers-next
v5->v6: Add ACKs
v6->v7: Mail format issues
v7->v8: Remove redundant varaible from wlcore_probe_of
 .../bindings/net/wireless/ti,wlcore,spi.txt|  41 +--
 drivers/net/wireless/ti/wlcore/spi.c   | 124 +
 2 files changed, 137 insertions(+), 28 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt 
b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
index 9180724..8f9ced0 100644
--- a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
+++ b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
@@ -1,19 +1,30 @@
-* Texas Instruments wl1271 wireless lan controller
+* Texas Instruments wl12xx/wl18xx wireless lan controller
 
-The wl1271 chip can be connected via SPI or via SDIO. This
+The wl12xx/wl18xx chips can be connected via SPI or via SDIO. This
 document describes the binding for the SPI connected chip.
 
 Required properties:
-- compatible :  Should be "ti,wl1271"
+- compatible :  Should be one of the following:
+* "ti,wl1271"
+* "ti,wl1273"
+* "ti,wl1281"
+* "ti,wl1283"
+* "ti,wl1801"
+* "ti,wl1805"
+* "ti,wl1807"
+* "ti,wl1831"
+* "ti,wl1835"
+* "ti,wl1837"
 - reg : Chip select address of device
 - spi-max-frequency :   Maximum SPI clocking speed of device in Hz
-- ref-clock-frequency : Reference clock frequency
 - interrupt-parent, interrupts :
 Should contain parameters for 1 interrupt line.
 Interrupt parameters: parent, line number, type.
-- vwlan-supply :Point the node of the regulator that powers/enable the 
wl1271 chip
+- vwlan-supply :Point the node of the regulator that powers/enable the
+wl12xx/wl18xx chip
 
 Optional properties:
+- ref-clock-frequency : Reference clock frequency (should be set for wl12xx)
 - clock-xtal :  boolean, clock is generated from XTAL
 
 - Please consult Documentation/devicetree/bindings/spi/spi-bus.txt
@@ -21,16 +32,28 @@ Optional properties:
 
 Examples:
 
+For wl12xx family:
  {
-   wl1271@1 {
+   wlcore: wlcore@1 {
compatible = "ti,wl1271";
-
reg = <1>;
spi-max-frequency = <4800>;
-   clock-xtal;
-   ref-clock-frequency = <3840>;
interrupt-parent = <>;
interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
vwlan-supply = <_fixed>;
+   clock-xtal;
+   ref-clock-frequency = <3840>;
+   };
+};
+
+For wl18xx family:
+ {
+   wlcore: wlcore@0 {
+   compatible = "ti,wl1835";
+   reg = <0>;
+   spi-max-frequency = <4800>;
+   interrupt-parent = <>;
+   interrupts = <27 IRQ_TYPE_EDGE_RISING>;
+   vwlan-supply = <_fixed>;
};
 };
diff --git a/drivers/net/wireless/ti/wlcore/spi.c 
b/drivers/net/wireless/ti/wlcore/spi.c
index cea9443..6d24040 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -70,16 +70,30 @@
 #define WSPI_MAX_CHUNK_SIZE4092
 
 /*
- * only support SPI for 12xx - this code should be reworked when 18xx
- * support is introduced
+ * wl18xx driver aggregation buffer size is (13 * PAGE_SIZE) compared to
+ * (4 * PAGE_SIZE) for wl12xx, so use the larger buffer needed for wl18xx
  */
-#define SPI_AGGR_BUFFER_SIZE (4 * PAGE_SIZE)
+#define SPI_AGGR_BUFFER_SIZE (13 * PAGE_SIZE)
 
 /* Maximum number of SPI write chunks */
 #define WSPI_MAX_NUM_OF_CHUNKS \
((SPI_AGGR_BUFFER_SIZE / WSPI_MAX_CHUNK_SIZE) + 1)
 
 
+struct wilink_familiy_data {
+   char name[8];
+};
+
+const struct wilink_familiy_data *wilink_data;
+
+static const struct wilink_familiy_data wl18xx_data = {
+   .name = "wl18xx",
+};
+
+static const struct wilink_familiy_data wl12xx_data = {
+   .name = "wl12xx",
+};
+
 struct wl12xx_spi_glue {
struct device *dev;
struct platform_device *core;
@@ -119,6 +133,7 @@ static void wl12xx_spi_init(struct device *child)
struct wl12xx_spi_glue *glue = 

RE: linux-next: build warning after merge of the wireless-drivers-next tree

[Reizer, Eyal]

Hi Stephen,

> 
> After merging the wireless-drivers-next tree, today's linux-next build
> (x86_64 allmodconfig) produced this warning:
> 
> drivers/net/wireless/ti/wlcore/spi.c: In function 'wlcore_probe_of':
> drivers/net/wireless/ti/wlcore/spi.c:457:6: warning: unused variable 'ret' [-
> Wunused-variable]
>   int ret;
>   ^
> 
> Introduced by commit
> 
>   01efe65aba65 ("wlcore: spi: add wl18xx support")
> 
Of course you are right. It is indeed redundant now.
Don't know how I have missed it. Don't remember seeing a warning from the tool 
chain I have used.
Anyway ,will submit an amended patch soon.

Best Regards,
Eyal
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v7] wlcore: spi: add wl18xx support

[Reizer, Eyal]

> > From: Eyal Reizer 
> >
> > Add support for using with both wl12xx and wl18xx.
> >
> > - all wilink family needs special init command for entering wspi mode.
> >   extra clock cycles should be sent after the spi init command while the
> >   cs pin is high.
> > - Use inverted chip select for sending a dummy 4 bytes command that
> >   completes the init stage.
> >
> > Signed-off-by: Eyal Reizer 
> > Acked-by: Rob Herring 
> 
> This looks ok in patchwork:
> 
> https://patchwork.kernel.org/patch/9235983/
> 
> Because you used ti.com in S-o-b I assume From should also use ti.com. I can
> fix that before I apply but please confirm that's really the case?
> 

Yes, S-o-b is ey...@ti.com. 
Thank you!

--
Eyal Reizer
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v6] wlcore: spi: add wl18xx support

[Reizer, Eyal]

Add support for using with both wl12xx and wl18xx.

- all wilink family needs special init command for entering wspi mode.
  extra clock cycles should be sent after the spi init command while the
  cs pin is high.
- Use inverted chip select for sending a dummy 4 bytes command that
  completes the init stage.

Signed-off-by: Eyal Reizer 
Acked-by: Rob Herring 
---
v1->v2:update device tree bindings configuration
v2->v3:revert from manual gpio manipulation. use inverted chip select 
v2->instead
for sending the extra init cycle which, achieves the same hardware purpose.
update device tree bindings docucmentation accordingly
v3->v4: Remove redundant data form binding documentation and fix chip 
v3->select
number mismatch in wl1271 example
v4->v5: Rebase on top of head of wireless-drivers-next
v5->v6: Add ACKs
 .../bindings/net/wireless/ti,wlcore,spi.txt|  41 +--
 drivers/net/wireless/ti/wlcore/spi.c   | 123 ++---
 2 files changed, 137 insertions(+), 27 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt 
b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
index 9180724..8f9ced0 100644
--- a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
+++ b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
@@ -1,19 +1,30 @@
-* Texas Instruments wl1271 wireless lan controller
+* Texas Instruments wl12xx/wl18xx wireless lan controller
 
-The wl1271 chip can be connected via SPI or via SDIO. This
+The wl12xx/wl18xx chips can be connected via SPI or via SDIO. This
 document describes the binding for the SPI connected chip.
 
 Required properties:
-- compatible :  Should be "ti,wl1271"
+- compatible :  Should be one of the following:
+* "ti,wl1271"
+* "ti,wl1273"
+* "ti,wl1281"
+* "ti,wl1283"
+* "ti,wl1801"
+* "ti,wl1805"
+* "ti,wl1807"
+* "ti,wl1831"
+* "ti,wl1835"
+* "ti,wl1837"
 - reg : Chip select address of device
 - spi-max-frequency :   Maximum SPI clocking speed of device in Hz
-- ref-clock-frequency : Reference clock frequency
 - interrupt-parent, interrupts :
 Should contain parameters for 1 interrupt line.
 Interrupt parameters: parent, line number, type.
-- vwlan-supply :Point the node of the regulator that powers/enable the 
wl1271 chip
+- vwlan-supply :Point the node of the regulator that powers/enable the
+wl12xx/wl18xx chip
 
 Optional properties:
+- ref-clock-frequency : Reference clock frequency (should be set for 
+wl12xx)
 - clock-xtal :  boolean, clock is generated from XTAL
 
 - Please consult Documentation/devicetree/bindings/spi/spi-bus.txt
@@ -21,16 +32,28 @@ Optional properties:
 
 Examples:
 
+For wl12xx family:
  {
-   wl1271@1 {
+   wlcore: wlcore@1 {
compatible = "ti,wl1271";
-
reg = <1>;
spi-max-frequency = <4800>;
-   clock-xtal;
-   ref-clock-frequency = <3840>;
interrupt-parent = <>;
interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
vwlan-supply = <_fixed>;
+   clock-xtal;
+   ref-clock-frequency = <3840>;
+   };
+};
+
+For wl18xx family:
+ {
+   wlcore: wlcore@0 {
+   compatible = "ti,wl1835";
+   reg = <0>;
+   spi-max-frequency = <4800>;
+   interrupt-parent = <>;
+   interrupts = <27 IRQ_TYPE_EDGE_RISING>;
+   vwlan-supply = <_fixed>;
};
 };
diff --git a/drivers/net/wireless/ti/wlcore/spi.c 
b/drivers/net/wireless/ti/wlcore/spi.c
index cea9443..73fbcf1 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -70,16 +70,30 @@
 #define WSPI_MAX_CHUNK_SIZE4092
 
 /*
- * only support SPI for 12xx - this code should be reworked when 18xx
- * support is introduced
+ * wl18xx driver aggregation buffer size is (13 * PAGE_SIZE) compared 
+ to
+ * (4 * PAGE_SIZE) for wl12xx, so use the larger buffer needed for 
+ wl18xx
  */
-#define SPI_AGGR_BUFFER_SIZE (4 * PAGE_SIZE)
+#define SPI_AGGR_BUFFER_SIZE (13 * PAGE_SIZE)
 
 /* Maximum number of SPI write chunks */  #define WSPI_MAX_NUM_OF_CHUNKS \
((SPI_AGGR_BUFFER_SIZE / WSPI_MAX_CHUNK_SIZE) + 1)
 
 
+struct wilink_familiy_data {
+   char name[8];
+};
+
+const struct wilink_familiy_data *wilink_data;
+
+static const struct wilink_familiy_data wl18xx_data = {
+   .name = "wl18xx",
+};
+
+static const struct wilink_familiy_data wl12xx_data = {
+   .name = "wl12xx",
+};
+
 struct wl12xx_spi_glue {
struct device *dev;
struct platform_device *core;
@@ -119,6 +133,7 @@ static void wl12xx_spi_init(struct device *child)
struct wl12xx_spi_glue *glue = dev_get_drvdata(child->parent);
struct spi_transfer t;
  

[PATCH v5] wlcore: spi: add wl18xx support

[Reizer, Eyal]

Add support for using with both wl12xx and wl18xx.

- all wilink family needs special init command for entering wspi mode.
  extra clock cycles should be sent after the spi init command while the
  cs pin is high.
- Use inverted chip select for sending a dummy 4 bytes command that
  completes the init stage.

Signed-off-by: Eyal Reizer 
Acked-by: Rob Herring 
---
v1->v2:update device tree bindings configuration
v2->v3:revert from manual gpio manipulation. use inverted chip select 
instead for sending the extra init cycle which, achieves the same hardware 
purpose.
update device tree bindings docucmentation accordingly
v3->v4: Remove redundant data form binding documentation and fix chip 
select number mismatch in wl1271 example
v4->v5: Rebase on top of head of wireless-drivers-next

 .../bindings/net/wireless/ti,wlcore,spi.txt|  41 +--
 drivers/net/wireless/ti/wlcore/spi.c   | 123 ++---
 2 files changed, 137 insertions(+), 27 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt 
b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
index 9180724..8f9ced0 100644
--- a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
+++ b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
@@ -1,19 +1,30 @@
-* Texas Instruments wl1271 wireless lan controller
+* Texas Instruments wl12xx/wl18xx wireless lan controller

-The wl1271 chip can be connected via SPI or via SDIO. This
+The wl12xx/wl18xx chips can be connected via SPI or via SDIO. This
 document describes the binding for the SPI connected chip.

 Required properties:
-- compatible :  Should be "ti,wl1271"
+- compatible :  Should be one of the following:
+* "ti,wl1271"
+* "ti,wl1273"
+* "ti,wl1281"
+* "ti,wl1283"
+* "ti,wl1801"
+* "ti,wl1805"
+* "ti,wl1807"
+* "ti,wl1831"
+* "ti,wl1835"
+* "ti,wl1837"
 - reg : Chip select address of device
 - spi-max-frequency :   Maximum SPI clocking speed of device in Hz
-- ref-clock-frequency : Reference clock frequency
 - interrupt-parent, interrupts :
 Should contain parameters for 1 interrupt line.
 Interrupt parameters: parent, line number, type.
-- vwlan-supply :Point the node of the regulator that powers/enable the 
wl1271 chip
+- vwlan-supply :Point the node of the regulator that powers/enable the
+wl12xx/wl18xx chip

 Optional properties:
+- ref-clock-frequency : Reference clock frequency (should be set for 
+wl12xx)
 - clock-xtal :  boolean, clock is generated from XTAL

 - Please consult Documentation/devicetree/bindings/spi/spi-bus.txt
@@ -21,16 +32,28 @@ Optional properties:

 Examples:

+For wl12xx family:
  {
-   wl1271@1 {
+   wlcore: wlcore@1 {
compatible = "ti,wl1271";
-
reg = <1>;
spi-max-frequency = <4800>;
-   clock-xtal;
-   ref-clock-frequency = <3840>;
interrupt-parent = <>;
interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
vwlan-supply = <_fixed>;
+   clock-xtal;
+   ref-clock-frequency = <3840>;
+   };
+};
+
+For wl18xx family:
+ {
+   wlcore: wlcore@0 {
+   compatible = "ti,wl1835";
+   reg = <0>;
+   spi-max-frequency = <4800>;
+   interrupt-parent = <>;
+   interrupts = <27 IRQ_TYPE_EDGE_RISING>;
+   vwlan-supply = <_fixed>;
};
 };
diff --git a/drivers/net/wireless/ti/wlcore/spi.c 
b/drivers/net/wireless/ti/wlcore/spi.c
index cea9443..73fbcf1 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -70,16 +70,30 @@
 #define WSPI_MAX_CHUNK_SIZE4092

 /*
- * only support SPI for 12xx - this code should be reworked when 18xx
- * support is introduced
+ * wl18xx driver aggregation buffer size is (13 * PAGE_SIZE) compared 
+ to
+ * (4 * PAGE_SIZE) for wl12xx, so use the larger buffer needed for 
+ wl18xx
  */
-#define SPI_AGGR_BUFFER_SIZE (4 * PAGE_SIZE)
+#define SPI_AGGR_BUFFER_SIZE (13 * PAGE_SIZE)

 /* Maximum number of SPI write chunks */  #define WSPI_MAX_NUM_OF_CHUNKS \
((SPI_AGGR_BUFFER_SIZE / WSPI_MAX_CHUNK_SIZE) + 1)


+struct wilink_familiy_data {
+   char name[8];
+};
+
+const struct wilink_familiy_data *wilink_data;
+
+static const struct wilink_familiy_data wl18xx_data = {
+   .name = "wl18xx",
+};
+
+static const struct wilink_familiy_data wl12xx_data = {
+   .name = "wl12xx",
+};
+
 struct wl12xx_spi_glue {
struct device *dev;
struct platform_device *core;
@@ -119,6 +133,7 @@ static void wl12xx_spi_init(struct device *child)
struct wl12xx_spi_glue *glue = dev_get_drvdata(child->parent);
struct spi_transfer t;
struct spi_message m;
+   

[PATCH v5] wlcore: spi: add wl18xx support

[Reizer, Eyal]

Add support for using with both wl12xx and wl18xx.

- all wilink family needs special init command for entering wspi mode.
  extra clock cycles should be sent after the spi init command while the
  cs pin is high.
- Use inverted chip select for sending a dummy 4 bytes command that
  completes the init stage.

Signed-off-by: Eyal Reizer 
---
v1->v2:update device tree bindings configuration
v2->v3:revert from manual gpio manipulation. use inverted chip select instead
for sending the extra init cycle which, achieves the same hardware purpose.
update device tree bindings docucmentation accordingly
v3->v4: Remove redundant data form binding documentation and fix chip select
number mismatch in wl1271 example
v4->v5: Rebase on top of head of wireless-drivers-next

 .../bindings/net/wireless/ti,wlcore,spi.txt|  41 +--
 drivers/net/wireless/ti/wlcore/spi.c   | 123 ++---
 2 files changed, 137 insertions(+), 27 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt 
b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
index 9180724..8f9ced0 100644
--- a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
+++ b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
@@ -1,19 +1,30 @@
-* Texas Instruments wl1271 wireless lan controller
+* Texas Instruments wl12xx/wl18xx wireless lan controller

-The wl1271 chip can be connected via SPI or via SDIO. This
+The wl12xx/wl18xx chips can be connected via SPI or via SDIO. This
 document describes the binding for the SPI connected chip.

 Required properties:
-- compatible :  Should be "ti,wl1271"
+- compatible :  Should be one of the following:
+* "ti,wl1271"
+* "ti,wl1273"
+* "ti,wl1281"
+* "ti,wl1283"
+* "ti,wl1801"
+* "ti,wl1805"
+* "ti,wl1807"
+* "ti,wl1831"
+* "ti,wl1835"
+* "ti,wl1837"
 - reg : Chip select address of device
 - spi-max-frequency :   Maximum SPI clocking speed of device in Hz
-- ref-clock-frequency : Reference clock frequency
 - interrupt-parent, interrupts :
 Should contain parameters for 1 interrupt line.
 Interrupt parameters: parent, line number, type.
-- vwlan-supply :Point the node of the regulator that powers/enable the 
wl1271 chip
+- vwlan-supply :Point the node of the regulator that powers/enable the
+wl12xx/wl18xx chip

 Optional properties:
+- ref-clock-frequency : Reference clock frequency (should be set for wl12xx)
 - clock-xtal :  boolean, clock is generated from XTAL

 - Please consult Documentation/devicetree/bindings/spi/spi-bus.txt
@@ -21,16 +32,28 @@ Optional properties:

 Examples:

+For wl12xx family:
  {
-   wl1271@1 {
+   wlcore: wlcore@1 {
compatible = "ti,wl1271";
-
reg = <1>;
spi-max-frequency = <4800>;
-   clock-xtal;
-   ref-clock-frequency = <3840>;
interrupt-parent = <>;
interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
vwlan-supply = <_fixed>;
+   clock-xtal;
+   ref-clock-frequency = <3840>;
+   };
+};
+
+For wl18xx family:
+ {
+   wlcore: wlcore@0 {
+   compatible = "ti,wl1835";
+   reg = <0>;
+   spi-max-frequency = <4800>;
+   interrupt-parent = <>;
+   interrupts = <27 IRQ_TYPE_EDGE_RISING>;
+   vwlan-supply = <_fixed>;
};
 };
diff --git a/drivers/net/wireless/ti/wlcore/spi.c 
b/drivers/net/wireless/ti/wlcore/spi.c
index cea9443..73fbcf1 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -70,16 +70,30 @@
 #define WSPI_MAX_CHUNK_SIZE4092

 /*
- * only support SPI for 12xx - this code should be reworked when 18xx
- * support is introduced
+ * wl18xx driver aggregation buffer size is (13 * PAGE_SIZE) compared to
+ * (4 * PAGE_SIZE) for wl12xx, so use the larger buffer needed for wl18xx
  */
-#define SPI_AGGR_BUFFER_SIZE (4 * PAGE_SIZE)
+#define SPI_AGGR_BUFFER_SIZE (13 * PAGE_SIZE)

 /* Maximum number of SPI write chunks */
 #define WSPI_MAX_NUM_OF_CHUNKS \
((SPI_AGGR_BUFFER_SIZE / WSPI_MAX_CHUNK_SIZE) + 1)


+struct wilink_familiy_data {
+   char name[8];
+};
+
+const struct wilink_familiy_data *wilink_data;
+
+static const struct wilink_familiy_data wl18xx_data = {
+   .name = "wl18xx",
+};
+
+static const struct wilink_familiy_data wl12xx_data = {
+   .name = "wl12xx",
+};
+
 struct wl12xx_spi_glue {
struct device *dev;
struct platform_device *core;
@@ -119,6 +133,7 @@ static void wl12xx_spi_init(struct device *child)
struct wl12xx_spi_glue *glue = dev_get_drvdata(child->parent);
struct spi_transfer t;
struct spi_message m;
+   struct spi_device *spi = to_spi_device(glue->dev);
 

[PATCH v4] wlcore: spi: add wl18xx support

[Reizer, Eyal]

Add support for using with both wl12xx and wl18xx.

- all wilink family needs special init command for entering wspi mode.
  extra clock cycles should be sent after the spi init command while the
  cs pin is high.
- Use inverted chip select for sending a dummy 4 bytes command that
  completes the init stage.

Signed-off-by: Eyal Reizer 
---
v1->v2:update device tree bindings configuration
v2->v3:revert from manual gpio manipulation. use inverted chip select instead
for sending the extra init cycle which, achieves the same hardware purpose.
update device tree bindings docucmentation accordingly
v3->v4: Remove redundant data form binding documentation and fix chip select
number mismatch in wl1271 example

 .../bindings/net/wireless/ti,wlcore,spi.txt|  41 +--
 drivers/net/wireless/ti/wlcore/spi.c   | 124 +
 2 files changed, 137 insertions(+), 28 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt 
b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
index 9180724..8f9ced0 100644
--- a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
+++ b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
@@ -1,19 +1,30 @@
-* Texas Instruments wl1271 wireless lan controller
+* Texas Instruments wl12xx/wl18xx wireless lan controller
 
-The wl1271 chip can be connected via SPI or via SDIO. This
+The wl12xx/wl18xx chips can be connected via SPI or via SDIO. This
 document describes the binding for the SPI connected chip.
 
 Required properties:
-- compatible :  Should be "ti,wl1271"
+- compatible :  Should be one of the following:
+* "ti,wl1271"
+* "ti,wl1273"
+* "ti,wl1281"
+* "ti,wl1283"
+* "ti,wl1801"
+* "ti,wl1805"
+* "ti,wl1807"
+* "ti,wl1831"
+* "ti,wl1835"
+* "ti,wl1837"
 - reg : Chip select address of device
 - spi-max-frequency :   Maximum SPI clocking speed of device in Hz
-- ref-clock-frequency : Reference clock frequency
 - interrupt-parent, interrupts :
 Should contain parameters for 1 interrupt line.
 Interrupt parameters: parent, line number, type.
-- vwlan-supply :Point the node of the regulator that powers/enable the 
wl1271 chip
+- vwlan-supply :Point the node of the regulator that powers/enable the
+wl12xx/wl18xx chip
 
 Optional properties:
+- ref-clock-frequency : Reference clock frequency (should be set for wl12xx)
 - clock-xtal :  boolean, clock is generated from XTAL
 
 - Please consult Documentation/devicetree/bindings/spi/spi-bus.txt
@@ -21,16 +32,28 @@ Optional properties:
 
 Examples:
 
+For wl12xx family:
  {
-   wl1271@1 {
+   wlcore: wlcore@1 {
compatible = "ti,wl1271";
-
reg = <1>;
spi-max-frequency = <4800>;
-   clock-xtal;
-   ref-clock-frequency = <3840>;
interrupt-parent = <>;
interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
vwlan-supply = <_fixed>;
+   clock-xtal;
+   ref-clock-frequency = <3840>;
+   };
+};
+
+For wl18xx family:
+ {
+   wlcore: wlcore@0 {
+   compatible = "ti,wl1835";
+   reg = <0>;
+   spi-max-frequency = <4800>;
+   interrupt-parent = <>;
+   interrupts = <27 IRQ_TYPE_EDGE_RISING>;
+   vwlan-supply = <_fixed>;
};
 };
diff --git a/drivers/net/wireless/ti/wlcore/spi.c 
b/drivers/net/wireless/ti/wlcore/spi.c
index 020ac1a..bd1253d 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -70,16 +70,30 @@
 #define WSPI_MAX_CHUNK_SIZE4092
 
 /*
- * only support SPI for 12xx - this code should be reworked when 18xx
- * support is introduced
+ * wl18xx driver aggregation buffer size is (13 * PAGE_SIZE) compared to
+ * (4 * PAGE_SIZE) for wl12xx, so use the larger buffer needed for wl18xx
  */
-#define SPI_AGGR_BUFFER_SIZE (4 * PAGE_SIZE)
+#define SPI_AGGR_BUFFER_SIZE (13 * PAGE_SIZE)
 
 /* Maximum number of SPI write chunks */
 #define WSPI_MAX_NUM_OF_CHUNKS \
((SPI_AGGR_BUFFER_SIZE / WSPI_MAX_CHUNK_SIZE) + 1)
 
 
+struct wilink_familiy_data {
+   char name[8];
+};
+
+const struct wilink_familiy_data *wilink_data;
+
+static const struct wilink_familiy_data wl18xx_data = {
+   .name = "wl18xx",
+};
+
+static const struct wilink_familiy_data wl12xx_data = {
+   .name = "wl12xx",
+};
+
 struct wl12xx_spi_glue {
struct device *dev;
struct platform_device *core;
@@ -119,6 +133,7 @@ static void wl12xx_spi_init(struct device *child)
struct wl12xx_spi_glue *glue = dev_get_drvdata(child->parent);
struct spi_transfer t;
struct spi_message m;
+   struct spi_device *spi = to_spi_device(glue->dev);
u8 *cmd = kzalloc(WSPI_INIT_CMD_LEN, 

RE: [PATCHv3] wlcore: spi: add wl18xx support

[Reizer, Eyal]

> >
> > - all wilink family needs special init command for entering wspi mode.
> >   extra clock cycles should be sent after the spi init command while the
> >   cs pin is high.
> > - Use inverted chip select for sending a dummy 4 bytes command that
> >   completes the init stage and puts the wilink chip into wspi mode.
> >
> > Signed-off-by: Eyal Reizer 
> > ---
> > v1->v2:update device tree bindings configuration
> > v2->v3:revert from manual gpio manipulation. use inverted chip select
> instead
> > for sending the extra init cycle, which achieves the same hardware purpose.
> > update device tree bindings docucmentation accordingly
> >
> >  .../bindings/net/wireless/ti,wlcore,spi.txt|  47 ++--
> >  drivers/net/wireless/ti/wlcore/spi.c   | 124 
> > +
> >  2 files changed, 145 insertions(+), 26 deletions(-)
> >
> > diff --git
> a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
> b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
> > index 9180724..35467cf 100644
> > --- a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
> > +++ b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
> > @@ -1,19 +1,30 @@
> > -* Texas Instruments wl1271 wireless lan controller
> > +* Texas Instruments wl12xx/wl18xx wireless lan controller
> >
> > -The wl1271 chip can be connected via SPI or via SDIO. This
> > +The wl12xx/wl18xx chips can be connected via SPI or via SDIO. This
> >  document describes the binding for the SPI connected chip.
> >
> >  Required properties:
> > -- compatible :  Should be "ti,wl1271"
> > +- compatible :  Should be one of the following:
> > +* "ti,wl1271"
> > +* "ti,wl1273"
> > +* "ti,wl1281"
> > +* "ti,wl1283"
> > +* "ti,wl1801"
> > +* "ti,wl1805"
> > +* "ti,wl1807"
> > +* "ti,wl1831"
> > +* "ti,wl1835"
> > +* "ti,wl1837"
> >  - reg : Chip select address of device
> >  - spi-max-frequency :   Maximum SPI clocking speed of device in Hz
> > -- ref-clock-frequency : Reference clock frequency
> >  - interrupt-parent, interrupts :
> >  Should contain parameters for 1 interrupt line.
> >  Interrupt parameters: parent, line number, type.
> > -- vwlan-supply :Point the node of the regulator that powers/enable 
> > the
> wl1271 chip
> > +- vwlan-supply :Point the node of the regulator that powers/enable
> the
> > +wl12xx/wl18xx chip
> >
> >  Optional properties:
> > +- ref-clock-frequency : Reference clock frequency (should be set for
> wl12xx)
> >  - clock-xtal :  boolean, clock is generated from XTAL
> >
> >  - Please consult Documentation/devicetree/bindings/spi/spi-bus.txt
> > @@ -21,10 +32,15 @@ Optional properties:
> >
> >  Examples:
> >
> > +For wl12xx family:
> >   {
> > -   wl1271@1 {
> > +   status = "okay";
> > +   pinctrl-names = "default";
> > +   pinctrl-0 = <_pins>;
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> 
> None of this is really relevant to this binding.
> 

Understood. Will remove in v4

> > +   wlcore: wlcore@0 {
> 
> Now your unit-address and reg value don't match.

You are right. Will fix in v4

> 
> > compatible = "ti,wl1271";
> > -
> > reg = <1>;
> > spi-max-frequency = <4800>;
> > clock-xtal;
> > @@ -34,3 +50,20 @@ Examples:
> > vwlan-supply = <_fixed>;
> > };
> >  };
> > +
> > +For wl18xx family:
> > +  {
> > +   status = "okay";
> > +   pinctrl-names = "default";
> > +   pinctrl-0 = <_pins>;
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +   wlcore: wlcore@0 {
> > +   compatible = "ti,wl1835";
> > +   vwlan-supply = <_en_reg>;
> > +   spi-max-frequency = <4800>;
> > +   reg = <0>;
> > +   interrupt-parent = <>;
> > +   interrupts = <27 IRQ_TYPE_EDGE_RISING>;
> > +   };
> > +};
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3] wlcore: spi: add wl18xx support

[Reizer, Eyal]

Add support for using with both wl12xx and wl18xx.

- all wilink family needs special init command for entering wspi mode.
  extra clock cycles should be sent after the spi init command while the
  cs pin is high.
- Use inverted chip select for sending a dummy 4 bytes command that
  completes the init stage and puts the wilink chip into wspi mode.

Signed-off-by: Eyal Reizer 
---
v1->v2:update device tree bindings configuration
v2->v3:revert from manual gpio manipulation. use inverted chip select instead
for sending the extra init cycle, which achieves the same hardware purpose.
update device tree bindings docucmentation accordingly

 .../bindings/net/wireless/ti,wlcore,spi.txt|  47 ++--
 drivers/net/wireless/ti/wlcore/spi.c   | 124 +
 2 files changed, 145 insertions(+), 26 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt 
b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
index 9180724..35467cf 100644
--- a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
+++ b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
@@ -1,19 +1,30 @@
-* Texas Instruments wl1271 wireless lan controller
+* Texas Instruments wl12xx/wl18xx wireless lan controller
 
-The wl1271 chip can be connected via SPI or via SDIO. This
+The wl12xx/wl18xx chips can be connected via SPI or via SDIO. This
 document describes the binding for the SPI connected chip.
 
 Required properties:
-- compatible :  Should be "ti,wl1271"
+- compatible :  Should be one of the following:
+* "ti,wl1271"
+* "ti,wl1273"
+* "ti,wl1281"
+* "ti,wl1283"
+* "ti,wl1801"
+* "ti,wl1805"
+* "ti,wl1807"
+* "ti,wl1831"
+* "ti,wl1835"
+* "ti,wl1837"
 - reg : Chip select address of device
 - spi-max-frequency :   Maximum SPI clocking speed of device in Hz
-- ref-clock-frequency : Reference clock frequency
 - interrupt-parent, interrupts :
 Should contain parameters for 1 interrupt line.
 Interrupt parameters: parent, line number, type.
-- vwlan-supply :Point the node of the regulator that powers/enable the 
wl1271 chip
+- vwlan-supply :Point the node of the regulator that powers/enable the
+wl12xx/wl18xx chip
 
 Optional properties:
+- ref-clock-frequency : Reference clock frequency (should be set for wl12xx)
 - clock-xtal :  boolean, clock is generated from XTAL
 
 - Please consult Documentation/devicetree/bindings/spi/spi-bus.txt
@@ -21,10 +32,15 @@ Optional properties:
 
 Examples:
 
+For wl12xx family:
  {
-   wl1271@1 {
+   status = "okay";
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   wlcore: wlcore@0 {
compatible = "ti,wl1271";
-
reg = <1>;
spi-max-frequency = <4800>;
clock-xtal;
@@ -34,3 +50,20 @@ Examples:
vwlan-supply = <_fixed>;
};
 };
+
+For wl18xx family:
+  {
+   status = "okay";
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   wlcore: wlcore@0 {
+   compatible = "ti,wl1835";
+   vwlan-supply = <_en_reg>;
+   spi-max-frequency = <4800>;
+   reg = <0>;
+   interrupt-parent = <>;
+   interrupts = <27 IRQ_TYPE_EDGE_RISING>;
+   };
+};
diff --git a/drivers/net/wireless/ti/wlcore/spi.c 
b/drivers/net/wireless/ti/wlcore/spi.c
index 020ac1a..bd1253d 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -70,16 +70,30 @@
 #define WSPI_MAX_CHUNK_SIZE4092
 
 /*
- * only support SPI for 12xx - this code should be reworked when 18xx
- * support is introduced
+ * wl18xx driver aggregation buffer size is (13 * PAGE_SIZE) compared to
+ * (4 * PAGE_SIZE) for wl12xx, so use the larger buffer needed for wl18xx
  */
-#define SPI_AGGR_BUFFER_SIZE (4 * PAGE_SIZE)
+#define SPI_AGGR_BUFFER_SIZE (13 * PAGE_SIZE)
 
 /* Maximum number of SPI write chunks */
 #define WSPI_MAX_NUM_OF_CHUNKS \
((SPI_AGGR_BUFFER_SIZE / WSPI_MAX_CHUNK_SIZE) + 1)
 
 
+struct wilink_familiy_data {
+   char name[8];
+};
+
+const struct wilink_familiy_data *wilink_data;
+
+static const struct wilink_familiy_data wl18xx_data = {
+   .name = "wl18xx",
+};
+
+static const struct wilink_familiy_data wl12xx_data = {
+   .name = "wl12xx",
+};
+
 struct wl12xx_spi_glue {
struct device *dev;
struct platform_device *core;
@@ -119,6 +133,7 @@ static void wl12xx_spi_init(struct device *child)
struct wl12xx_spi_glue *glue = dev_get_drvdata(child->parent);
struct spi_transfer t;
struct spi_message m;
+   struct spi_device *spi = to_spi_device(glue->dev);
u8 

RE: [PATCHv2] wlcore: spi: add wl18xx support

[Reizer, Eyal]

> 
> > Thanks! Glad the illustration helped.
> > I will try it out again as if i recall cotrectly, i did try that l, and it 
> > didnt produce
> the correct waveform, but perhaps i didnt understand the usage of
> .cs_change correctly.
> > Will double check anyway.
> 
> It could also be a bug in your controller driver, it's common for them to not
> handle cs_change correctly (at least those that open code the message loop,
> obviously modern ones just factor this out into the core).

Tried looking into using cs_change and not sure I understand how it would do 
what I need.
According to, include/linux/spi/spi.h:

* All SPI transfers start with the relevant chipselect active.  Normally
* it stays selected until after the last transfer in a message.  Drivers
* can affect the chipselect signal using cs_change.
*
* (i) If the transfer isn't the last one in the message, this flag is
* used to make the chipselect briefly go inactive in the middle of the
* message.  Toggling chipselect in this way may be needed to terminate
* a chip command, letting a single spi_message perform all of group of
* chip transactions together.

Now, in my case I need the CS pin to go inactive and stay inactive during the 
transfer 
of the next byte for completing the SPI init correctly (sending the extra clock 
cycles while CS is inactive)
If I read the above correctly, CS will only briefly go inactive but will when 
the next byte 
is sent it will be active again.
What am I missing?

Best Regards,
Eyal

RE: [PATCHv2] wlcore: spi: add wl18xx support

[Reizer, Eyal]

> > > > It is also part of the generic spi.h (include/Linux/spi/spi.h),
> > > > already part of " struct spi_device" So it seemed redundant adding
> > > > another mechanism for implementing the same.
> > > > Platform that interact with a wilink need to use it, and platforms
> > > > that don't have this capability will probably not interact with a
> > > > wilink device
> > > using SPI.
> > >
> > > The cs_gpio field in spi_device belongs to the spi host controller,
> > > no other slave driver uses it.
> > >
> > > I wasn't asking for a duplication of this mechanism, but an
> > > interface to use it properly. Internally, the spi core uses the 
> > > spi_set_cs()
> function to pick a CS.
> > > Find a way to use that rather than reimplementing it incorrectly.
> > >
> >
> > Understood. As this special CS manipulation is unique to wspi (wilink
> > spi)  I think the best option is to move this gpio allocation into
> > wlcore_spi as a new device tree entry used only by this driver.
> > If you agree I will submit a v3.
> 
> I don't think that can work either: aside of not solving the problem of wilink
> devices on spi controllers that don't use gpio, it also doesn't solve the
> problem of what happens when the driver manually triggers the gpio to hold
> the CS signal while another driver talks to a different device using another 
> CS
> on the same controller.
> 
Ok, understood. Will look into it.

Best Regards,
Eyal
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCHv2] wlcore: spi: add wl18xx support

[Reizer, Eyal]

Arnd,

> > > >
> > > > - all wilink family needs special init command for entering wspi mode.
> > > >   extra clock cycles should be sent after the spi init command while the
> > > >   cs pin is high.
> > > > - switch to controling the cs pin from the spi driver for achieveing the
> > > >   above.
> > > > - the selected cs gpio is read from the spi device-tree node using the
> > > >   cs-gpios field and setup as a gpio.
> > > > - See the example below for specifying the cs gpio using the cs-gpios
> entry
> > > >{
> > > > ...
> > > > cs-gpios = < 5 0>;
> > > > ...
> > > > wlcore: wlcore@0 {
> > > > compatible = "ti,wl1835";
> > > > ...
> > > > ...
> > > > };
> > > > };
> > > >
> > > > Signed-off-by: Eyal Reizer 
> > >
> > > I don't think this can work in general: not all SPI hosts uses GPIOs
> > > for controlling CS, so the logic can't work, and it's also a
> > > layering violation for the driver to look at the parent.
> > >
> > > I would suggest fixing this using a new API function from the SPI
> > > core, if we don't already have a generic way to do it.
> > >
> > Originally this is what I have done until I was pointed to the generic
> > cs-gpio mechanism in the SPI core.
> > It is a generic mechanism already in the SPI core driver.
> > See: Documentation/devicetree/bindings/spi/spi-bus.txt
> 
> The cs-gpios property is documented as optional, it defines how you should
> list the gpios if CS is implemented using gpio, but not all hardware does it 
> like
> this.
> 
> > It is also part of the generic spi.h (include/Linux/spi/spi.h),
> > already part of " struct spi_device" So it seemed redundant adding
> > another mechanism for implementing the same.
> > Platform that interact with a wilink need to use it, and platforms
> > that don't have this capability will probably not interact with a wilink 
> > device
> using SPI.
> 
> The cs_gpio field in spi_device belongs to the spi host controller, no other
> slave driver uses it.
> 
> I wasn't asking for a duplication of this mechanism, but an interface to use 
> it
> properly. Internally, the spi core uses the spi_set_cs() function to pick a 
> CS.
> Find a way to use that rather than reimplementing it incorrectly.
> 

Understood. As this special CS manipulation is unique to wspi (wilink spi)  I 
think the 
best option is to move this gpio allocation into wlcore_spi as a new device 
tree entry
used only by this driver.
If you agree I will submit a v3.

Best Regards,
Eyal
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCHv2] wlcore: spi: add wl18xx support

[Reizer, Eyal]

> >
> > - all wilink family needs special init command for entering wspi mode.
> >   extra clock cycles should be sent after the spi init command while the
> >   cs pin is high.
> > - switch to controling the cs pin from the spi driver for achieveing the
> >   above.
> > - the selected cs gpio is read from the spi device-tree node using the
> >   cs-gpios field and setup as a gpio.
> > - See the example below for specifying the cs gpio using the cs-gpios entry
> >{
> > ...
> > cs-gpios = < 5 0>;
> > ...
> > wlcore: wlcore@0 {
> > compatible = "ti,wl1835";
> > ...
> > ...
> > };
> > };
> >
> > Signed-off-by: Eyal Reizer 
> 
> I don't think this can work in general: not all SPI hosts uses GPIOs for
> controlling CS, so the logic can't work, and it's also a layering violation 
> for the
> driver to look at the parent.
> 
> I would suggest fixing this using a new API function from the SPI core, if we
> don't already have a generic way to do it.
>
Originally this is what I have done until I was pointed to the generic cs-gpio 
mechanism 
in the SPI core. 
It is a generic mechanism already in the SPI core driver.
See: Documentation/devicetree/bindings/spi/spi-bus.txt

It is also part of the generic spi.h (include/Linux/spi/spi.h), already part of 
" struct spi_device" So it seemed redundant adding another mechanism for 
implementing the same.
Platform that interact with a wilink need to use it, and platforms that don't  
have this capability will probably not interact with a wilink device using SPI.

Best Regards,
Eyal



--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2] wlcore: spi: add wl18xx support

[Reizer, Eyal]

Add support for using with both wl12xx and wl18xx.

- all wilink family needs special init command for entering wspi mode.
  extra clock cycles should be sent after the spi init command while the
  cs pin is high.
- switch to controling the cs pin from the spi driver for achieveing the
  above.
- the selected cs gpio is read from the spi device-tree node using the
  cs-gpios field and setup as a gpio.
- See the example below for specifying the cs gpio using the cs-gpios entry
   {
...
cs-gpios = < 5 0>;
...
wlcore: wlcore@0 {
compatible = "ti,wl1835";
...
...
};
};

Signed-off-by: Eyal Reizer 
---
v1 -> v2: update device tree bindings documentation

 .../bindings/net/wireless/ti,wlcore,spi.txt|   50 +-
 drivers/net/wireless/ti/wlcore/spi.c   |  176 +---
 2 files changed, 200 insertions(+), 26 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt 
b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
index 9180724..912ab0c 100644
--- a/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
+++ b/Documentation/devicetree/bindings/net/wireless/ti,wlcore,spi.txt
@@ -1,19 +1,31 @@
-* Texas Instruments wl1271 wireless lan controller
+* Texas Instruments wl12xx/wl18xx wireless lan controller
 
-The wl1271 chip can be connected via SPI or via SDIO. This
+The wl12xx/wl18xx chips can be connected via SPI or via SDIO. This
 document describes the binding for the SPI connected chip.
 
 Required properties:
-- compatible :  Should be "ti,wl1271"
+- compatible :  Should be one of the following:
+* "ti,wl1271"
+* "ti,wl1273"
+* "ti,wl1281"
+* "ti,wl1283"
+* "ti,wl1801"
+* "ti,wl1805"
+* "ti,wl1807"
+* "ti,wl1831"
+* "ti,wl1835"
+* "ti,wl1837"
 - reg : Chip select address of device
 - spi-max-frequency :   Maximum SPI clocking speed of device in Hz
-- ref-clock-frequency : Reference clock frequency
 - interrupt-parent, interrupts :
 Should contain parameters for 1 interrupt line.
 Interrupt parameters: parent, line number, type.
-- vwlan-supply :Point the node of the regulator that powers/enable the 
wl1271 chip
+- vwlan-supply :Point the node of the regulator that powers/enable the
+wl12xx/wl18xx chip
+- cs-gpios :GPIO pin used as the spi chip select
 
 Optional properties:
+- ref-clock-frequency : Reference clock frequency (should be set for wl12xx)
 - clock-xtal :  boolean, clock is generated from XTAL
 
 - Please consult Documentation/devicetree/bindings/spi/spi-bus.txt
@@ -21,10 +33,16 @@ Optional properties:
 
 Examples:
 
+For wl12xx family:
  {
-   wl1271@1 {
+   status = "okay";
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins>;
+   cs-gpios = < 5 0>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   wlcore: wlcore@0 {
compatible = "ti,wl1271";
-
reg = <1>;
spi-max-frequency = <4800>;
clock-xtal;
@@ -34,3 +52,21 @@ Examples:
vwlan-supply = <_fixed>;
};
 };
+
+For wl18xx family:
+  {
+   status = "okay";
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins>;
+   cs-gpios = < 5 0>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   wlcore: wlcore@0 {
+   compatible = "ti,wl1835";
+   vwlan-supply = <_en_reg>;
+   spi-max-frequency = <4800>;
+   reg = <0>;
+   interrupt-parent = <>;
+   interrupts = <27 IRQ_TYPE_EDGE_RISING>;
+   };
+};
diff --git a/drivers/net/wireless/ti/wlcore/spi.c 
b/drivers/net/wireless/ti/wlcore/spi.c
index 020ac1a..fb48a0d 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -32,6 +32,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "wlcore.h"
 #include "wl12xx_80211.h"
@@ -70,16 +71,30 @@
 #define WSPI_MAX_CHUNK_SIZE4092
 
 /*
- * only support SPI for 12xx - this code should be reworked when 18xx
- * support is introduced
+ * wl18xx driver aggregation buffer size is (13 * PAGE_SIZE) compared to
+ * (4 * PAGE_SIZE) for wl12xx, so use the larger buffer needed for wl18xx
  */
-#define SPI_AGGR_BUFFER_SIZE (4 * PAGE_SIZE)
+#define SPI_AGGR_BUFFER_SIZE (13 * PAGE_SIZE)
 
 /* Maximum number of SPI write chunks */
 #define WSPI_MAX_NUM_OF_CHUNKS \
((SPI_AGGR_BUFFER_SIZE / WSPI_MAX_CHUNK_SIZE) + 1)
 
 
+struct wilink_familiy_data {
+   char name[8];
+};
+
+const struct wilink_familiy_data *wilink_data;
+
+static const struct wilink_familiy_data wl18xx_data = {
+   .name = "wl18xx",
+};
+
+static const struct wilink_familiy_data wl12xx_data = {
+   .name = "wl12xx",
+};
+
 struct wl12xx_spi_glue {
struct device 

RE: [PATCH] wlcore: spi: add wl18xx support

[Reizer, Eyal]

> -Original Message-
> From: Kalle Valo [mailto:kv...@codeaurora.org]
> Sent: Thursday, April 07, 2016 3:25 PM
> To: Eyal Reizer
> Cc: linux-wireless@vger.kernel.org; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Reizer, Eyal; devicet...@vger.kernel.org
> Subject: Re: [PATCH] wlcore: spi: add wl18xx support
> 
> Eyal Reizer <eyalrei...@gmail.com> writes:
> 
> > Add support for using with both wl12xx and wl18xx.
> >
> > - all wilink family needs special init command for entering wspi mode.
> >   extra clock cycles should be sent after the spi init command while the
> >   cs pin is high.
> > - switch to controling the cs pin from the spi driver for achieveing the
> >   above.
> > - the selected cs gpio is read from the spi device-tree node using the
> >   cs-gpios field and setup as a gpio.
> > - See the example below for specifying the cs gpio using the cs-gpios
> > entry
> >
> >{
> > status = "okay";
> > pinctrl-names = "default";
> > pinctrl-0 = <_pins>;
> > cs-gpios = < 5 0>;
> > #address-cells = <1>;
> > #size-cells = <0>;
> > wlcore: wlcore@0 {
> > compatible = "ti,wl1835";
> > vwlan-supply = <_en_reg>;
> > spi-max-frequency = <4800>;
> > reg = <0>;  /* chip select 0 on spi0, ie spi0.0 */
> > interrupt-parent = <>;
> > interrupts = <27 IRQ_TYPE_EDGE_RISING>;
> > };
> > };
> >
> > Signed-off-by: Eyal Reizer <ey...@ti.com>
> 
> [...]
> 
> >  static const struct of_device_id wlcore_spi_of_match_table[] = {
> > -   { .compatible = "ti,wl1271" },
> > +   { .compatible = "ti,wl1271", .data = _data},
> > +   { .compatible = "ti,wl1273", .data = _data},
> > +   { .compatible = "ti,wl1281", .data = _data},
> > +   { .compatible = "ti,wl1283", .data = _data},
> > +   { .compatible = "ti,wl1801", .data = _data},
> > +   { .compatible = "ti,wl1805", .data = _data},
> > +   { .compatible = "ti,wl1807", .data = _data},
> > +   { .compatible = "ti,wl1831", .data = _data},
> > +   { .compatible = "ti,wl1835", .data = _data},
> > +   { .compatible = "ti,wl1837", .data = _data},
> > { }
> 
> Shouldn't you also update bindings/net/wireless/ti,wlcore,spi.txt? Now it only
> mentions about ti,wl1271 and not anything about the rest.

You are right! 
Will be fixed in v2

> 
> Adding devicetree list for further comments.
> 
> --
> Kalle Valo

Best Regards,
Eyal
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] wlcore: spi: add wl18xx support

[Reizer, Eyal]

Ping on this patch

> -Original Message-
> From: Eyal Reizer [mailto:eyalrei...@gmail.com]
> Sent: Wednesday, March 30, 2016 4:07 PM
> To: kv...@codeaurora.org; linux-wireless@vger.kernel.org;
> net...@vger.kernel.org; linux-ker...@vger.kernel.org
> Cc: Reizer, Eyal
> Subject: [PATCH] wlcore: spi: add wl18xx support
> 
> Add support for using with both wl12xx and wl18xx.
> 
> - all wilink family needs special init command for entering wspi mode.
>   extra clock cycles should be sent after the spi init command while the
>   cs pin is high.
> - switch to controling the cs pin from the spi driver for achieveing the
>   above.
> - the selected cs gpio is read from the spi device-tree node using the
>   cs-gpios field and setup as a gpio.
> - See the example below for specifying the cs gpio using the cs-gpios entry
> 
>  {
>   status = "okay";
>   pinctrl-names = "default";
>   pinctrl-0 = <_pins>;
>   cs-gpios = < 5 0>;
>   #address-cells = <1>;
>   #size-cells = <0>;
>   wlcore: wlcore@0 {
>   compatible = "ti,wl1835";
>   vwlan-supply = <_en_reg>;
>   spi-max-frequency = <4800>;
>   reg = <0>;  /* chip select 0 on spi0, ie spi0.0 */
>   interrupt-parent = <>;
>   interrupts = <27 IRQ_TYPE_EDGE_RISING>;
>   };
> };
> 
> Signed-off-by: Eyal Reizer <ey...@ti.com>
> ---
>  drivers/net/wireless/ti/wlcore/spi.c |  176
> ++
>  1 file changed, 157 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/wireless/ti/wlcore/spi.c
> b/drivers/net/wireless/ti/wlcore/spi.c
> index 96d9c9d..6c5a369 100644
> --- a/drivers/net/wireless/ti/wlcore/spi.c
> +++ b/drivers/net/wireless/ti/wlcore/spi.c
> @@ -32,6 +32,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #include "wlcore.h"
>  #include "wl12xx_80211.h"
> @@ -70,16 +71,30 @@
>  #define WSPI_MAX_CHUNK_SIZE4092
> 
>  /*
> - * only support SPI for 12xx - this code should be reworked when 18xx
> - * support is introduced
> + * wl18xx driver aggregation buffer size is (13 * PAGE_SIZE) compared
> + to
> + * (4 * PAGE_SIZE) for wl12xx, so use the larger buffer needed for
> + wl18xx
>   */
> -#define SPI_AGGR_BUFFER_SIZE (4 * PAGE_SIZE)
> +#define SPI_AGGR_BUFFER_SIZE (13 * PAGE_SIZE)
> 
>  /* Maximum number of SPI write chunks */  #define
> WSPI_MAX_NUM_OF_CHUNKS \
>   ((SPI_AGGR_BUFFER_SIZE / WSPI_MAX_CHUNK_SIZE) + 1)
> 
> 
> +struct wilink_familiy_data {
> + char name[8];
> +};
> +
> +const struct wilink_familiy_data *wilink_data;
> +
> +static const struct wilink_familiy_data wl18xx_data = {
> + .name = "wl18xx",
> +};
> +
> +static const struct wilink_familiy_data wl12xx_data = {
> + .name = "wl12xx",
> +};
> +
>  struct wl12xx_spi_glue {
>   struct device *dev;
>   struct platform_device *core;
> @@ -120,6 +135,8 @@ static void wl12xx_spi_init(struct device *child)
>   struct spi_transfer t;
>   struct spi_message m;
>   u8 *cmd = kzalloc(WSPI_INIT_CMD_LEN, GFP_KERNEL);
> + struct spi_device *spi = (struct spi_device *)glue->dev;
> + struct spi_master *master = spi->master;
> 
>   if (!cmd) {
>   dev_err(child->parent,
> @@ -127,6 +144,15 @@ static void wl12xx_spi_init(struct device *child)
>   return;
>   }
> 
> + if (!master->cs_gpios) {
> + dev_err(child->parent,
> + "spi chip select pin missing in platform data!\n");
> + return;
> + }
> +
> + /* Drive CS line low */
> + gpio_direction_output(master->cs_gpios[0], 0);
> +
>   memset(, 0, sizeof(t));
>   spi_message_init();
> 
> @@ -163,6 +189,26 @@ static void wl12xx_spi_init(struct device *child)
>   spi_message_add_tail(, );
> 
>   spi_sync(to_spi_device(glue->dev), );
> +
> + /* Send extra clocks with CS high. this is required by the wilink
> +  * family in order for successfully enter WSPI mode
> +  */
> + gpio_direction_output(master->cs_gpios[0], 1);
> +
> + memset(, 0, sizeof(m));
> + spi_message_init();
> +
> + cmd[0] = 0xff;
> + cmd[1] = 0xff;
> + cmd[2] = 0xff;
> + cmd[3] = 0xff;
> + swab32s((u32 *)cmd);
> +
> + t.tx_buf = cmd;
> + t.len = 4;
> + spi_message_add_tail(, );
> + spi_sync(to_spi_device(glue->dev), );
> +
>   kfree(cmd);
>