diff mbox

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

Message ID 8665E2433BC68541A24DFFCA87B70F5B3641E25B@DFRE01.ent.ti.com (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show

Commit Message

Reizer, Eyal Aug. 7, 2017, 7:31 a.m. UTC
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 <stable@vger.kernel.org>
Signed-off-by: Eyal Reizer <eyalr@ti.com>
---
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
---
 drivers/net/wireless/ti/wlcore/main.c   | 16 ++++++++++++++++
 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, 21 insertions(+)

Comments

Tony Lindgren Aug. 7, 2017, 7:38 a.m. UTC | #1
* Reizer, Eyal <eyalr@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?

Regards,

Tony
Reizer, Eyal Aug. 7, 2017, 7:46 a.m. UTC | #2
Hi Tony,
> 
> * Reizer, Eyal <eyalr@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
Kalle Valo Aug. 7, 2017, 12:38 p.m. UTC | #3
"Reizer, Eyal" <eyalr@ti.com> writes:

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

It's recommended to use 12 chars when abbreviating commit ids so the
format should be this:

c815fdebef44 wlcore: spi: Populate config firmware data
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 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 <stable@vger.kernel.org>
> Signed-off-by: Eyal Reizer <eyalr@ti.com>

It's always good to have a fixes line(s) and specify which stable
versions should have the fix. As the commit were first in v4.9-rc1
that's the first version we want this to be included:

Fixes: c815fdebef44 ("wlcore: spi: Populate config firmware data")
Fixes: d776fc86b82f ("wlcore: sdio: Populate config firmware data")
Cc: <stable@vger.kernel.org> # 4.9+

But if you don't submit a new version I can fix the commit log during
commit.
Tony Lindgren Aug. 8, 2017, 4:20 a.m. UTC | #4
* Reizer, Eyal <eyalr@ti.com> [170807 00:47]:
> Hi Tony,
> > 
> > * Reizer, Eyal <eyalr@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?

Regards,

Tony
Reizer, Eyal Aug. 8, 2017, 8:03 a.m. UTC | #5
> Subject: Re: [v5] wlcore: add missing nvs file name info for wilink8
> 
> * Reizer, Eyal <eyalr@ti.com> [170807 00:47]:
> > Hi Tony,
> > >
> > > * Reizer, Eyal <eyalr@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
Reizer, Eyal Aug. 9, 2017, 7:24 a.m. UTC | #6
> 
> > Subject: Re: [v5] wlcore: add missing nvs file name info for wilink8
> >
> > * Reizer, Eyal <eyalr@ti.com> [170807 00:47]:
> > > Hi Tony,
> > > >
> > > > * Reizer, Eyal <eyalr@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
Tony Lindgren Aug. 9, 2017, 4:59 p.m. UTC | #7
* Reizer, Eyal <eyalr@ti.com> [170809 00:26]:
> 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.

Hey great, that should help clear quite a bit of the wl12xx confusion
with distros! Thanks for doing that.

Regards,

Tony
diff mbox

Patch

diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index 60aaa85..257831a 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 > 0xffffff)
 		wl1271_warning("NIC part of the MAC address wraps around!");
 
+	if (oui == 0xdeadbe && nic == 0xef0000) {
+		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 = WLCORE_TI_OUI_ADDRESS;
+		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 {
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_TXPWR        25
 
+/* Texas Instruments pre assigned OUI */
+#define WLCORE_TI_OUI_ADDRESS 0x080028
+
 /* forward declaration */
 struct wl1271_tx_hw_descr;
 enum wl_rx_buf_align;