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

[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: <stable@vger.kernel.org> # 4.9+
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
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 == 0xef0000) {
+		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: [v6] wlcore: add missing nvs file name info for wilink8

[Tony Lindgren]

* Reizer, Eyal <eyalr@ti.com> [170809 00:55]:
> --- 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 == 0xef0000) {
> +		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);

I just gave this a quick try on omap3-evm with wl1271, now I get
mac address of 00:00:00:00:00:01 :) So looks like at least wl1271 needs
to use the random mac address here.

Note that we should now have struct wilink_family_data available
so maybe that can be used to check if the hardware mac address
exists?

Regards,

Tony

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

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [170809 10:26]:
> * Reizer, Eyal <eyalr@ti.com> [170809 00:55]:
> > --- 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 == 0xef0000) {
> > +		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);
> 
> I just gave this a quick try on omap3-evm with wl1271, now I get
> mac address of 00:00:00:00:00:01 :) So looks like at least wl1271 needs
> to use the random mac address here.
> 
> Note that we should now have struct wilink_family_data available
> so maybe that can be used to check if the hardware mac address
> exists?

Or actually, in the case where no hardawre mac address exists,
the mac is all zeroes at this point it seems. So maybe that
can be used to determine if a random mac address should be used
here.

Regards,

Tony

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

[Tony Lindgren]

* Reizer, Eyal <eyalr@ti.com> [170809 10:40]:
> Hi Tony,
> 
> 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.

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.

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

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

Regards,

Tony

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

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

[Kalle Valo]

"Reizer, Eyal" <eyalr@ti.com> writes:

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

-- 
Kalle Valo

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

[Julian Calaby]

Hi Eyal,

On Thu, Aug 10, 2017 at 4:35 PM, Reizer, Eyal <eyalr@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.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

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

[Reizer, Eyal]

> 
> On Thu, Aug 10, 2017 at 4:35 PM, Reizer, Eyal <eyalr@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]

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

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

[Tony Lindgren]

* Reizer, Eyal <eyalr@ti.com> [170810 07:24]:
> > >
> > 
> > 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.

Regards,

Tony

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