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 <[email protected]>
---
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
"Reizer, Eyal" <[email protected]> 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 <[email protected]>
Should we also cc stable? And a Fixes line would be nice.
--
Kalle Valo
* Kalle Valo <[email protected]> [170703 04:30]:
> "Reizer, Eyal" <[email protected]> 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 <[email protected]>
>
> 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 :)
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?
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..
Regards,
Tony
Hi Tony,
>
> * Kalle Valo <[email protected]> [170703 04:30]:
> > "Reizer, Eyal" <[email protected]> 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 <[email protected]>
> >
> > 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
* Reizer, Eyal <[email protected]> [170704 01:47]:
> Hi Tony,
>
> >
> > * Kalle Valo <[email protected]> [170703 04:30]:
> > > "Reizer, Eyal" <[email protected]> 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 <[email protected]>
> > >
> > > 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.
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 :(
> 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.
I agree. But distros carry it already, so we need to also add checks to
the device driver to warn about the bogus wl1271-nvs.bin. And the driver
should only attempt to use wl1271-nvs.bin if not the bogus one.
> 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.
Yes makes sense, but only with fixing the driver to ignore the bogus
wl1271-nvs.bin.
> > 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?
I think we need both to avoid totally broken defaults.
> 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
Yes so checking, ignoring and warning about the bogus one should solve
your customer issues. And it will also fix the defaults for distros.
> so I am not sure that a name change
Well that can be done later. The problem of booting the same rootfs
on multiple devices remains where having wl1271-nvs.bin breaks things.
> Here is really necessary and may only create confusion especially for
> Customers that already have products in the field and only updating
> Their kernel.
Yeah using wl1271-nvs.bin should be the last resort but only if properly
configured.
Regards,
Tony
Hi,
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();
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");
}
-- Sebastian
* Sebastian Reichel <[email protected]> [170705 01:29]:
> Hi,
>
> 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();
> 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 :)
Regards,
Tony
> >
> > 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();
Deadbeef0000 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