| Message ID | 8665E2433BC68541A24DFFCA87B70F5B36420E5A@DFRE01.ent.ti.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
* 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
* 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
* 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
> > > > 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
"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.
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,
> > 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
> > >> 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
> > > > 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
* 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
> > > > > > > > > > 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
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 {
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(+)