[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/t=
i/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 =3D wl->fuse_nic_addr + 1;
 	}
=20
+	if (oui_addr =3D=3D 0xdeadbe && nic_addr =3D=3D 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 =3D wl->fuse_oui_addr;
+		/* fuse has the BD_ADDR, the WLAN addresses are the next two */
+		nic_addr =3D wl->fuse_nic_addr + 1;
+	}
+
 	wl12xx_derive_mac_addresses(wl, oui_addr, nic_addr);
=20
 	ret =3D ieee80211_register_hw(wl->hw);
diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/t=
i/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 =3D =
{
 static const struct wilink_family_data wl18xx_data =3D {
 	.name =3D "wl18xx",
 	.cfg_name =3D "ti-connectivity/wl18xx-conf.bin",
+	.nvs_name =3D "ti-connectivity/wl1271-nvs.bin",
 };
=20
 static const struct of_device_id wlcore_sdio_of_match_table[] =3D {
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 =3D {
 static const struct wilink_family_data wl18xx_data =3D {
 	.name =3D "wl18xx",
 	.cfg_name =3D "ti-connectivity/wl18xx-conf.bin",
+	.nvs_name =3D "ti-connectivity/wl1271-nvs.bin",
 };
=20
 struct wl12xx_spi_glue {
--=20
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 batche=
s?
> 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.
>=20
> Well clearly at least this one does not have any valid hardware
> mac address, the hardware mac address is broken with all zeroes.
>=20

Looks like it is not really all zeros but rather 00:00:00:00:00:01.=20
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.
>=20

Of course I can add a check for this but need to make sure it=20
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=
=20
Sure it is really a common issue before adding additional checks to the ker=
nel.

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

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.
>=20
> Sounds like there are multiple variants of the wl12xx
> available then.
>=20
I am trying to find out internally if there is a possibility that there wer=
e devices
Produced in the past where the internal fuses were not programmed with a va=
lid=20
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

[Reizer, Eyal]

>=20
> >> 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 an=
d
> >> 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.
>=20
> 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.
>=20
> 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.
>=20

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

Best Regards,
Eyal

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]

PiANCj4gT24gVGh1LCBBdWcgMTAsIDIwMTcgYXQgNDozNSBQTSwgUmVpemVyLCBFeWFsIDxleWFs
ckB0aS5jb20+IHdyb3RlOg0KPiA+PiA+DQo+ID4+ID4gU29ycnkgZm9yIHRvcCBwb3N0aW5nICht
b2JpbGUuLi4pDQo+ID4+ID4gSSBoYXZlIHZlcmlmaWVkIHdpdGggc3lzdGVtIGRlc2lnbiBhbmQg
dGhlIGRhdGEgc2hlZXQgdGhhdCBldmVyeSB3aWxpbmsNCj4gNi83DQo+ID4+IGNoaXAgaGFzIGEg
bWFjIGFkZHJlc3MgaW4gZnVzZSBzbyBwcm9iYWJseSB0aGUgYm9hcmQgeW91IGhhdmUgKHByZXR0
eSBvbGQsDQo+ID4+IHJpZ2h0PykgaGFzIHRoaXMgbWFjIGFkZHJlc3MgaW4gZnVzZS4gTWF5YmUg
aXQgd2FzIGZyb20gdmVyeSBlYXJseQ0KPiBiYXRjaGVzPw0KPiA+PiBBbnl3YXkgSSBzZWUgbm8g
cmVhc29uIHRvIGNoYW5nZSBpdC4NCj4gPj4gPiBBbnl3YXkgdGhlIGNhbGlicmF0b3IgY2FuIGJl
IHVzZWQgdG8gc3RvcmUgYSBkaWZmZXJlbnQgb25lIGludG8gdGhlIG52cw0KPiBmaWxlDQo+ID4+
IHRoYXQgd2lsbCBvdmVyaWRlIGl0Lg0KPiA+Pg0KPiA+PiBXZWxsIGNsZWFybHkgYXQgbGVhc3Qg
dGhpcyBvbmUgZG9lcyBub3QgaGF2ZSBhbnkgdmFsaWQgaGFyZHdhcmUNCj4gPj4gbWFjIGFkZHJl
c3MsIHRoZSBoYXJkd2FyZSBtYWMgYWRkcmVzcyBpcyBicm9rZW4gd2l0aCBhbGwgemVyb2VzLg0K
PiA+Pg0KPiA+DQo+ID4gTG9va3MgbGlrZSBpdCBpcyBub3QgcmVhbGx5IGFsbCB6ZXJvcyBidXQg
cmF0aGVyIDAwOjAwOjAwOjAwOjAwOjAxLg0KPiA+IEkgd29uZGVyIGlmIGl0IGlzIGp1c3QgYSBv
bmUgYm9hcmQgaXNzdWUgb3Igbm90Li4uDQo+IA0KPiBJdCdzIDAwOjAwOjAwOjAwOjAwOjAxIGJl
Y2F1c2UgeW91ciBjb2RlIGFkZHMgMSB0byBpdC4NCj4gDQpZb3UgYXJlIHJpZ2h0IG9mIGNvdXJz
ZSENCkkgc2F3IGl0IGFzIHdlbGwgYWZ0ZXIgc2VuZGluZyBteSBwcmV2aW91cyByZXBseS4NCg0K
QmVzdCBSZWdhcmRzLA0KRXlhbA0K

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

[Reizer, Eyal]

> >
>=20
> Sure, just want to make sure we are not trying to add work around just fo=
r
> A couple of faulty devices.
>=20
> > > 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 w=
ere
> devices
> Produced in the past where the internal fuses were not programmed with a
> valid
> Address before being assembled into the modules.
>=20

Seems like MAC address for wilink6/7 was added to devices that were produce=
d  around Q3 of 2010=20
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 jus=
t
> 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 the=
re 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?
>=20
> Sure, my omap3-evm has hw_pg_ver set to 3.
>=20
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